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

Improve human-readable output of Deployments and StatefulSets #70466

Merged
merged 1 commit into from
Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 6 additions & 8 deletions pkg/printers/internalversion/printers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "Number of the pod with ready state"},
{Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]},
{Name: "Containers", Type: "string", Priority: 1, Description: "Names of each container in the template."},
{Name: "Images", Type: "string", Priority: 1, Description: "Images referenced by each container in the template."},
Expand Down Expand Up @@ -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: "Number of the pod with ready state"},
{Name: "Up-to-date", Type: "string", Description: extensionsv1beta1.DeploymentStatus{}.SwaggerDoc()["updatedReplicas"]},
{Name: "Available", Type: "string", Description: extensionsv1beta1.DeploymentStatus{}.SwaggerDoc()["availableReplicas"]},
{Name: "Age", Type: "string", Description: metav1.ObjectMeta{}.SwaggerDoc()["creationTimestamp"]},
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

createTime := translateTimestampSince(obj.CreationTimestamp)
row.Cells = append(row.Cells, obj.Name, int64(desiredReplicas), int64(currentReplicas), createTime)
row.Cells = append(row.Cells, obj.Name, fmt.Sprintf("%d/%d", int64(readyReplicas), int64(desiredReplicas)), createTime)
if options.Wide {
names, images := layoutContainerCells(obj.Spec.Template.Spec.Containers)
row.Cells = append(row.Cells, names, images)
Expand Down Expand Up @@ -1561,8 +1559,8 @@ func printDeployment(obj *apps.Deployment, options printers.PrintOptions) ([]met
Object: runtime.RawExtension{Object: obj},
}
desiredReplicas := obj.Spec.Replicas
currentReplicas := obj.Status.Replicas
updatedReplicas := obj.Status.UpdatedReplicas
readyReplicas := obj.Status.ReadyReplicas
Copy link
Member

Choose a reason for hiding this comment

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

same here ^

availableReplicas := obj.Status.AvailableReplicas
age := translateTimestampSince(obj.CreationTimestamp)
containers := obj.Spec.Template.Spec.Containers
Expand All @@ -1571,7 +1569,7 @@ func printDeployment(obj *apps.Deployment, options printers.PrintOptions) ([]met
// this shouldn't happen if LabelSelector passed validation
return nil, err
}
row.Cells = append(row.Cells, obj.Name, int64(desiredReplicas), int64(currentReplicas), int64(updatedReplicas), int64(availableReplicas), age)
row.Cells = append(row.Cells, obj.Name, fmt.Sprintf("%d/%d", int64(readyReplicas), int64(desiredReplicas)), int64(updatedReplicas), int64(availableReplicas), age)
if options.Wide {
containers, images := layoutContainerCells(containers)
row.Cells = append(row.Cells, containers, images, selector.String())
Expand Down
4 changes: 2 additions & 2 deletions pkg/printers/internalversion/printers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2006,8 +2006,8 @@ func TestPrintDeployment(t *testing.T) {
UnavailableReplicas: 4,
},
},
"test1\t5\t10\t2\t1\t0s\n",
"test1\t5\t10\t2\t1\t0s\tfake-container1,fake-container2\tfake-image1,fake-image2\tfoo=bar\n",
"test1\t0/5\t2\t1\t0s\n",
"test1\t0/5\t2\t1\t0s\tfake-container1,fake-container2\tfake-image1,fake-image2\tfoo=bar\n",
},
}

Expand Down