Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support old restart policy in compose v3 #950

Merged
merged 1 commit into from
Mar 7, 2018

Conversation

hangyan
Copy link
Contributor

@hangyan hangyan commented Mar 3, 2018

related to #876

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 3, 2018
@surajnarwade
Copy link
Contributor

LGTM

restart: "unless-stopped"
environment:
GITHUB: surajssd

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove blank line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@surajnarwade updated! thanks!

@hangyan hangyan force-pushed the support-old-restart-policy branch from 0dd7126 to ca53676 Compare March 6, 2018 09:15
@surajnarwade
Copy link
Contributor

@hangyan one last thing, can you please add documentation stating that we are converting unless-stopped restart policy to always

@hangyan hangyan force-pushed the support-old-restart-policy branch from ca53676 to acfab6b Compare March 6, 2018 10:02
@hangyan
Copy link
Contributor Author

hangyan commented Mar 6, 2018

@surajnarwade thanks, I will remeber this in the feature PRs.

Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one small change, otherwise LGTM!

if composeServiceConfig.Deploy.RestartPolicy != nil {
serviceConfig.Restart = composeServiceConfig.Deploy.RestartPolicy.Condition
}
if serviceConfig.Restart == "unless-stopped" {
log.Warnf("Restart policy 'unless-stopped in service %s is not supported, convert it to 'always'", name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing a ' around unless-stopped

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated!

@hangyan hangyan force-pushed the support-old-restart-policy branch from acfab6b to 723bd0f Compare March 7, 2018 05:35
@cdrage
Copy link
Member

cdrage commented Mar 7, 2018

Awesome! Tests pass and this looks great to me 👍 merging!

@cdrage cdrage merged commit a5a113d into kubernetes:master Mar 7, 2018
@hangyan hangyan deleted the support-old-restart-policy branch March 31, 2018 04:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants