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

kubectl rollout status waits for available pods #31499

Merged
merged 1 commit into from
Sep 8, 2016
Merged

kubectl rollout status waits for available pods #31499

merged 1 commit into from
Sep 8, 2016

Conversation

areed
Copy link
Contributor

@areed areed commented Aug 26, 2016 •
edited by janetkuo
Loading

What this PR does / why we need it:
This changes kubectl rollout status to wait until all updated replicas are available before finishing.

Which issue this PR fixes (optional, in fixes #<issue number>(, #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #31130

Special notes for your reviewer:

Release note:

Changes 'kubectl rollout status' to wait until all updated replicas are available before finishing.

Currently kubectl rollout status finishes when Deployment.Spec.Replicas == Deployment.Status.UpdatedReplicas, but it's less surprising to the user for kubectl rollout status to wait until Deployment.Status.UpdatedReplicas == Deployment.Status.Replics == Deployment.Status.AvailableReplicas

This change is Reviewable

@areed
kubectl rollout status waits for available pods
cb5c556 
It's less surprising to the user for kubectl rollout status to wait until
DeploymentStatus.UpdatedReplicas == DeploymentStatus.Replics == DeploymentStatus.AvailableReplicas
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

6 similar comments
@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@k8s-bot
Copy link

k8s-bot commented Aug 26, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@areed
Copy link
Contributor Author

areed commented Aug 26, 2016

I signed it

@googlebot
Copy link

CLAs look good, thanks!

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Aug 26, 2016
@j3ffml
Copy link
Contributor

j3ffml commented Aug 26, 2016

@k8s-bot ok to test

@j3ffml
Copy link
Contributor

j3ffml commented Aug 26, 2016

looks reasonable to me. @janetkuo for second pair of eyes.

@janetkuo janetkuo added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Aug 26, 2016
@janetkuo janetkuo added this to the v1.4 milestone Aug 26, 2016
@janetkuo
Copy link
Member

LGTM 👍 Thanks!

@janetkuo janetkuo modified the milestones: v1.5, v1.4 Aug 26, 2016
@janetkuo
Copy link
Member

Moving this to v1.5, since we don't want to slow down v1.4. Note that if the rollout is scaled, the printed status may not be correct.

@areed
Copy link
Contributor Author

areed commented Aug 27, 2016

Which line would be incorrect? Is that something I can fix?

@janetkuo
Copy link
Member

For example, if the deployment is scaled down during rollout, deployment.Status.UpdatedReplicas may be larger than deployment.Spec.Replicas.

@janetkuo
Copy link
Member

Handling this well in kubectl might be challenging, though.

@k8s-bot
Copy link

k8s-bot commented Aug 30, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@areed
Copy link
Contributor Author

areed commented Aug 31, 2016

Would it be better to print out the data in a table and let the user interpret? It might look like this (in an ASCII table) for these scenarios:

Scale up deployment from 10 to 15 pods.

current target
replicas running latest pod spec 15 15
replicas running old pod spec 0 0
available replicas 10 15

Scale down deployment from 10 to 5 pods.

current target
replicas running latest pod spec 10 5
replicas running old pod spec 0 0
available replicas 10 5

Deployment caused by change to pod spec, 10 replicas in old and new.

current target
replicas running latest pod spec 5 10
replicas running old pod spec 5 0
available replicas 9 10

@k8s-bot
Copy link

k8s-bot commented Sep 8, 2016

GCE e2e build/test passed for commit cb5c556.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 8d9d32d into kubernetes:master Sep 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@j3ffml j3ffml

@janetkuo janetkuo

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

kubectl rollout status does not print actual deployment status
6 participants
@areed @googlebot @k8s-bot @j3ffml @janetkuo @k8s-github-robot