Improve human-readable output of Deployments and StatefulSets #70466

Pingan2017 · 2018-10-31T06:21:22Z

What type of PR is this?

/kind feature
/sig cli

What this PR does / why we need it:
add Ready column for kubectl get sts command

[root@hpa-vm:/opt/kubernetes/1.4.5]$ kubectl get deploy
NAME               READY   UP-TO-DATE   AVAILABLE   AGE
nginx-deployment   0/1     1            0           10m
[root@hpa-vm:/opt/kubernetes/1.4.5]$ 
[root@hpa-vm:/opt/kubernetes/1.4.5]$ kubectl get sts
NAME   READY   AGE
web    3/3     47m

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

Special notes for your reviewer:
/cc @soltysh
Does this PR introduce a user-facing change?:

add `Ready` column and improve human-readable output of Deployments and StatefulSets

idealhack · 2018-10-31T08:00:06Z

/ok-to-test

Pingan2017 · 2018-10-31T08:31:18Z

/assign @janetkuo

juanvallejo · 2018-11-01T22:33:38Z

@Pingan2017 thanks for addressing this. While you're in this part of the codebase, would you mind updating the overall output to be consistent with what is detailed in #68623 ?

Also, would be great if you'd like to address this for Deployments as well.

Thanks

Pingan2017 · 2018-11-08T02:44:57Z

@juanvallejo PTAL, Thanks!

idealhack

few nits, otherwise lgtm

idealhack · 2018-11-08T03:10:25Z

pkg/printers/internalversion/printers.go

@@ -1042,9 +1040,9 @@ func printStatefulSet(obj *apps.StatefulSet, options printers.PrintOptions) ([]m
 		Object: runtime.RawExtension{Object: obj},
 	}
 	desiredReplicas := obj.Spec.Replicas
-	currentReplicas := obj.Status.Replicas
+	readyReplicas := obj.Status.ReadyReplicas


Since the idea is to merge current/desired to ready, I don't think this renaming is worthy.

Sorry, but could you explain why to choose obj.Status.ReadyReplicas instead of obj.Status.Replicas here?

I think it will make ambiguous sometimes for users.
sometimes , the pod is not Ready, but the current and desired is equal. the Ready (1/1) will make users think the deployment is available.

[root@hpa-vm:/etc/kubernetes]$ kubectl get deploy NAME READY UP-TO-DATE AVAILABLE AGE nginx-deployment 1/1 1 0 45m

OR we can using CURRENT instead of READY?
I dont think Ready is a good idea to show current/desired

Thanks, that's making sense now. I'll defer the choice to @kubernetes/sig-cli-feature-requests

idealhack · 2018-11-08T03:10:46Z

pkg/printers/internalversion/printers.go

 	updatedReplicas := obj.Status.UpdatedReplicas
+	readyReplicas := obj.Status.ReadyReplicas


same here ^

idealhack · 2018-11-08T03:18:24Z

And you need to update tests too

Pingan2017 · 2018-11-09T06:16:53Z

/test pull-kubernetes-e2e-kops-aws

juanvallejo · 2018-11-09T15:38:31Z

/lgtm

cc @soltysh for approval

soltysh

/approve
A few nits that I'd like to see addressed:
/lgtm cancel
and also please update release notes, your change is a user facing one, so you need to have on.

soltysh · 2018-11-09T22:09:48Z

pkg/printers/internalversion/printers.go

@@ -203,8 +203,7 @@ func AddHandlers(h printers.PrintHandler) {

 	statefulSetColumnDefinitions := []metav1beta1.TableColumnDefinition{
 		{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
-		{Name: "Desired", Type: "string", Description: appsv1beta1.StatefulSetSpec{}.SwaggerDoc()["replicas"]},
-		{Name: "Current", Type: "string", Description: appsv1beta1.StatefulSetStatus{}.SwaggerDoc()["replicas"]},
+		{Name: "Ready", Type: "string", Description: "the number of the pod with ready state"},


s/the number/Number

Thanks! fix it.

soltysh · 2018-11-09T22:09:56Z

pkg/printers/internalversion/printers.go

@@ -313,8 +312,7 @@ func AddHandlers(h printers.PrintHandler) {

 	deploymentColumnDefinitions := []metav1beta1.TableColumnDefinition{
 		{Name: "Name", Type: "string", Format: "name", Description: metav1.ObjectMeta{}.SwaggerDoc()["name"]},
-		{Name: "Desired", Type: "string", Description: extensionsv1beta1.DeploymentSpec{}.SwaggerDoc()["replicas"]},
-		{Name: "Current", Type: "string", Description: extensionsv1beta1.DeploymentStatus{}.SwaggerDoc()["replicas"]},
+		{Name: "Ready", Type: "string", Description: "the number of the pod with ready state"},


s/the number/Number

soltysh · 2018-11-09T22:12:06Z

/milestone v1.13

Pingan2017 · 2018-11-10T02:45:11Z

@soltysh Updated. PTAL, Thanks!

soltysh

/lgtm
/approve

k8s-ci-robot · 2018-11-13T21:43:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Pingan2017, soltysh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~pkg/printers/OWNERS~~ [soltysh]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Oct 31, 2018

k8s-ci-robot requested a review from soltysh October 31, 2018 06:21

k8s-ci-robot removed the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 31, 2018

k8s-ci-robot assigned janetkuo Oct 31, 2018

soltysh assigned soltysh and juanvallejo and unassigned janetkuo Nov 1, 2018

Pingan2017 changed the title ~~add 'Ready' column for kubectl get sts command~~ [WIP]add 'Ready' column for kubectl get sts command Nov 2, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 2, 2018

Pingan2017 force-pushed the get-statefulset branch 2 times, most recently from 11cbafa to aeb86d2 Compare November 8, 2018 02:41

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 8, 2018

Pingan2017 changed the title ~~[WIP]add 'Ready' column for kubectl get sts command~~ Improve human-readable output of Deployments and StatefulSets Nov 8, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2018

idealhack reviewed Nov 8, 2018

View reviewed changes

Pingan2017 force-pushed the get-statefulset branch 2 times, most recently from 3de0db6 to 77b0ddc Compare November 9, 2018 03:19

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2018

soltysh approved these changes Nov 9, 2018

View reviewed changes

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2018

k8s-ci-robot added this to the v1.13 milestone Nov 9, 2018

add Ready column for kubectl get sts command

182a66f

Pingan2017 force-pushed the get-statefulset branch from 77b0ddc to 182a66f Compare November 10, 2018 02:41

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 10, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 13, 2018

soltysh approved these changes Nov 13, 2018

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 13, 2018

k8s-ci-robot merged commit a88f297 into kubernetes:master Nov 13, 2018

Pingan2017 deleted the get-statefulset branch November 21, 2018 01:19

juanvallejo mentioned this pull request Feb 1, 2019

UPSTREAM: 70466: add Ready column for kubectl get sts command openshift/origin#21949

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve human-readable output of Deployments and StatefulSets #70466

Improve human-readable output of Deployments and StatefulSets #70466

Pingan2017 commented Oct 31, 2018 •

edited

Loading

idealhack commented Oct 31, 2018

Pingan2017 commented Oct 31, 2018

juanvallejo commented Nov 1, 2018

Pingan2017 commented Nov 8, 2018

idealhack left a comment

idealhack Nov 8, 2018

idealhack Nov 8, 2018

Pingan2017 Nov 8, 2018

idealhack Nov 8, 2018

idealhack Nov 8, 2018

idealhack commented Nov 8, 2018

Pingan2017 commented Nov 9, 2018

juanvallejo commented Nov 9, 2018

soltysh left a comment

soltysh Nov 9, 2018

Pingan2017 Nov 10, 2018

soltysh Nov 9, 2018

soltysh commented Nov 9, 2018

Pingan2017 commented Nov 10, 2018

soltysh left a comment

k8s-ci-robot commented Nov 13, 2018

		updatedReplicas := obj.Status.UpdatedReplicas
		readyReplicas := obj.Status.ReadyReplicas

Improve human-readable output of Deployments and StatefulSets #70466

Improve human-readable output of Deployments and StatefulSets #70466

Conversation

Pingan2017 commented Oct 31, 2018 • edited Loading

idealhack commented Oct 31, 2018

Pingan2017 commented Oct 31, 2018

juanvallejo commented Nov 1, 2018

Pingan2017 commented Nov 8, 2018

idealhack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

idealhack commented Nov 8, 2018

Pingan2017 commented Nov 9, 2018

juanvallejo commented Nov 9, 2018

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soltysh commented Nov 9, 2018

Pingan2017 commented Nov 10, 2018

soltysh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 13, 2018

Pingan2017 commented Oct 31, 2018 •

edited

Loading