kubectl get priorityclass prints value column

[Huang-Wei]

What this PR does / why we need it:

Prints out value column when running kubectl get priorityclass.

$ kubectl get priorityclass
NAME                      VALUE        AGE
system-cluster-critical   2000000000   6m33s
system-node-critical      2000001000   6m33s

Before:

$ kubectl get priorityclass
NAME                      CREATED AT
system-cluster-critical   2018-10-04T06:18:27Z
system-node-critical      2018-10-04T06:18:27Z

Special notes for your reviewer:

• If necessary, other columns can be added as well, or to -o wide columns.
• Optionally we can follow storageclass way to append (global default) to the name if it's a global default priorityclass.

Release note:

`kubectl get priorityclass` now prints value column by default.

/kind feature
/sig cli
/sig scheduling

[Huang-Wei]

@kubernetes/sig-cli-pr-reviews

[Huang-Wei]

@bsalamat @liggitt could you take a chance to review this? Thanks.

[dixudx]

/lgtm

[nikhita]

/assign @deads2k
for approval

[liggitt]

should we include the GlobalDefault field value? that seems more useful than the creation time

[dixudx]

should we include the GlobalDefault field value? that seems more useful than the creation time

Yeah. Correct. @liggitt 👍

[Huang-Wei]

@liggitt I think so. Basically there are two options:

• add it as part of Name column (sort of like how StorageClass prints)

  NAME                          VALUE        AGE
  priority1 (global default)    2000000000   6m33s

• or put it in a separated column

  NAME        GLOBAL_DEFAULT    VALUE        AGE
  priority1   true              2000000000   6m33s

BTW: as PriorityClass is mostly used and designed by @bsalamat in preemption logic. I'd like to hear about Bobby's comments.

[bsalamat]

Thanks Wei for this improvement. I would prefer the following output:

NAME         VALUE        GLOBAL_DEFAULT      AGE
priority1    2000000000   true                6m33s

This is very similar to what you wrote. It just puts "value" before "global default". Value is more important in my opinion. Hence, it is show before global default.
This is just my personal preference though. It has nothing to do with the fact that I have designed pod priority. In other words, there is no technical wisdom behind my suggestion! :-)

[Huang-Wei]

Added GLOBAL-DEFAULT column. @bsalamat @liggitt @dixudx PTAL.

BTW: according to kubectl dev guide, it said:

Column titles and values should not contain spaces in order to facilitate commands that break lines into fields: cut, awk, etc. Instead, use - as the word separator.

So I used - over _:

NAME                      VALUE        GLOBAL-DEFAULT   AGE
system-cluster-critical   2000000000   false            38s
system-node-critical      2000001000   false            38s

And I do see other API objects follow this convention:

NAME         TYPE        CLUSTER-IP   EXTERNAL-IP   PORT(S)   AGE
kubernetes   ClusterIP   10.0.0.1     <none>        443/TCP   3m11s

[Huang-Wei]

/retest

[bsalamat]

/lgtm

Thanks, @Huang-Wei! We should add "Description" to "-o wide" as well.

[Huang-Wei]

@bsalamat hmm, I'm a little hesitant on adding Description column, as:

1. there is no API object has this kind of column
2. it will breaks Column titles and values should not contain spaces, and maybe has string truncation issue.

So maybe better for users to use -o json/yaml/... to get "description" info.

[bsalamat]

@Huang-Wei I agree. In fact, since kubectl describe priorityClass shows description of priority classes, users have a way to see the description and we don't need to have it here in get.

[Huang-Wei]

@bsalamat yeap, that makes sense.

/cc @liggitt @deads2k for approval.

[liggitt]

/lgtm

[liggitt]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Huang-Wei, liggitt

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:

• hack/OWNERS [liggitt]
• pkg/printers/OWNERS [liggitt]
• pkg/registry/OWNERS [liggitt]

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