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.

Sign up for GitHub

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

Jump to bottom

Move MinReadySeconds out of RollingUpdateDeployment #20389

Merged
merged 1 commit into from
Feb 4, 2016
Merged

Move MinReadySeconds out of RollingUpdateDeployment #20389

merged 1 commit into from
Feb 4, 2016

Conversation

mqliang
Copy link
Contributor

@mqliang mqliang commented Jan 31, 2016

@k8s-bot
Copy link

k8s-bot commented Jan 31, 2016

GCE e2e build/test failed for commit d96a056771504e202665168ba022df96be5ce8ce.

@0xmichalis
Copy link
Contributor

cc: @bgrant0607 @nikhiljindal

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 31, 2016
@k8s-bot
Copy link

k8s-bot commented Jan 31, 2016

GCE e2e build/test failed for commit 28f4d3a4179ef0d943e01c02e3eab3f12f82517d.

@mqliang mqliang force-pushed the deployment-MinReadySeconds branch 2 times, most recently from 73fcfc3 to fbab974 Compare January 31, 2016 14:39
@k8s-bot
Copy link

k8s-bot commented Jan 31, 2016

GCE e2e test build/test passed for commit 73fcfc30e7e4150bb8c4bc164aa9c949e8ddad88.

@k8s-bot
Copy link

k8s-bot commented Jan 31, 2016

GCE e2e test build/test passed for commit fbab9740c1fb909bfc682f3d81e2acce087f627a.

@k8s-github-robot
Copy link

The author of this PR is not in the whitelist for merge, can one of the admins add the 'ok-to-merge' label?

@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 31, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 1, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 1, 2016

GCE e2e test build/test passed for commit 8b379257a5d6fd9e8c50e079e37d0077fd62d047.

@k8s-github-robot
Copy link

PR needs rebase

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 2, 2016
bgrant0607
bgrant0607 reviewed Feb 2, 2016
View reviewed changes
pkg/apis/extensions/v1beta1/types.go
@@ -242,6 +242,11 @@ type DeploymentSpec struct {
// No label is added if this is set to empty string.
UniqueLabelKey *string `json:"uniqueLabelKey,omitempty"`

// Minimum number of seconds for which a newly created pod should be ready
Copy link
Member

Choose a reason for hiding this comment

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

Could you please put this field above RevisionHistoryLimit? I think it will be more frequently used, so I'd like to keep it closer to the top.

@bgrant0607
Copy link
Member

Sorry for the delay. Please ping me and @janetkuo once you move the field and rebase.

@bgrant0607 bgrant0607 mentioned this pull request Feb 2, 2016
Merged
@janetkuo janetkuo added this to the v1.2 milestone Feb 2, 2016
@janetkuo
Copy link
Member

janetkuo commented Feb 3, 2016

@mqliang Can you also update https://github.com/kubernetes/kubernetes/blob/master/docs/user-guide/deployments.md#min-ready-seconds?

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 3, 2016

GCE e2e test build/test passed for commit 652f987801f6369a5d500c3dd763f54dc6d989ac.

@k8s-bot
Copy link

k8s-bot commented Feb 3, 2016

GCE e2e test build/test passed for commit c4a1ab4.

@mqliang
Copy link
Contributor Author

mqliang commented Feb 3, 2016

@janetkuo @bgrant0607 updated. PTAL

@bgrant0607
Copy link
Member

LGTM

Thanks!

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2016
@mqliang
Copy link
Contributor Author

mqliang commented Feb 4, 2016

@janetkuo @bgrant0607 All tests passed, could this be manually merged? This will conflict with #20368. I'd like finish this PR and #20368 today(I am unavailable tomorrow). Once this get merge, I could rebase(and rework according to your suggestion) #20368.

@bgrant0607
Copy link
Member

@saad-ali Could you please manually merge this PR?

saad-ali added a commit that referenced this pull request Feb 4, 2016
@saad-ali
Merge pull request #20389 from mqliang/deployment-MinReadySeconds
0952dcd 
Move MinReadySeconds out of RollingUpdateDeployment
@saad-ali saad-ali merged commit 0952dcd into kubernetes:master Feb 4, 2016
@mqliang mqliang deleted the deployment-MinReadySeconds branch February 4, 2016 06:54
@bgrant0607 bgrant0607 mentioned this pull request Feb 4, 2016
Closed
@david-mcmahon david-mcmahon added the release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. label Apr 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bgrant0607 bgrant0607

Labels
area/app-lifecycle kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@mqliang @k8s-bot @0xmichalis @k8s-github-robot @bgrant0607 @janetkuo @k8s-teamcity-mesosphere @googlebot @saad-ali @david-mcmahon