Improve the output of kubectl get events
#66643
Conversation
k8s-ci-robot
added
release-note
k8s-ci-robot
added
the
cncf-cla: yes
|
@kubernetes/sig-cli-bugs |
k8s-ci-robot
added
sig/cli
|
seems like this fixes an issue i filed long time back #29838 . |
Events have long shown the most data of the core objects in their output, but that data is of varying use to a user. Following the principle that events are intended for the system to communicate information back to the user, and that Message is the primary human readable field, this commit alters the default columns to ensure event is shown with the most width. 1. Events are no longer sorted in the printer (this was a bug and was broken with paging and server side rendering) 2. Only the last seen, type, reason, kind, and message fields are shown by default, which makes the message prominent 3. Source, subobject, count, and first seen are only shown under `-o wide` 4. The duration fields were changed to be the more precise output introduced for job duration (2-3 sig figs)
smarterclayton
force-pushed
the
improve_events
branch
from
a46c483 to
2f275b7
Compare
|
@gmarek for events as well |
| @@ -57,17 +57,29 @@ func HumanDuration(d time.Duration) string { | |||
| } | |||
| minutes := int(d / time.Minute) | |||
| if minutes < 10 { | |||
| return fmt.Sprintf("%dm%ds", minutes, int(d/time.Second)%60) | |||
| s := int(d/time.Second) % 60 | |||
| if s == 0 { | |||
There was a problem hiding this comment.
maybe a small test for these duration changes?
There was a problem hiding this comment.
I added new tests to the _test.go
|
lgtm, one small comment cc @soltysh |
|
|
||
| return []metav1beta1.TableRow{row}, nil | ||
| } | ||
|
|
||
| // Sorts and prints the EventList in a human-friendly format. | ||
| func printEventList(list *api.EventList, options printers.PrintOptions) ([]metav1beta1.TableRow, error) { | ||
| sort.Sort(events.SortableEvents(list.Items)) |
There was a problem hiding this comment.
Not sure where sorting is being done now, are they sorted in the get command now?
There was a problem hiding this comment.
The apiserver returns in natural order. To sort events by timestamp, you have to ask for it - sort doesn't work at this level because chunking would result in the objects being sorted.
We might in the future add a feature that allows a resource to define its default sort on the client side, but we'd need to think more about it.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton, 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:
Approvers can indicate their approval by writing |
|
Remember, columns themselves are not API. We have |
True, we left ourselves the ability to modify them slightly, but changing printers columns you are changing the API, and by moving some to |
|
No, printer columns do not fall under API guarantees. We explicitly say that the content of columns and their display is not an API. The table format is, but not the content of the table. Otherwise we would never be able to improve the experience. The closest analogue is that the We can add that language to the api doc for sure. |
|
Automatic merge from submit-queue (batch tested with PRs 66445, 66643, 60551). If you want to cherry-pick this change to another branch, please follow the instructions here. |
| @@ -1927,7 +1927,7 @@ func TestTranslateTimestampSince(t *testing.T) { | |||
| {"unknown", translateTimestampSince(metav1.Time{}), "<unknown>"}, | |||
| {"30 seconds ago", translateTimestampSince(metav1.Time{Time: time.Now().Add(-3e10)}), "30s"}, | |||
| {"5 minutes ago", translateTimestampSince(metav1.Time{Time: time.Now().Add(-3e11)}), "5m"}, | |||
| {"an hour ago", translateTimestampSince(metav1.Time{Time: time.Now().Add(-6e12)}), "1h"}, | |||
| {"an hour ago", translateTimestampSince(metav1.Time{Time: time.Now().Add(-6e12)}), "100m"}, | |||
There was a problem hiding this comment.
Should this not be "60m" instead of "100m" ? :)
There was a problem hiding this comment.
I think the textual representation gets "truncated" down
Events have long shown the most data of the core objects in their output, but that data is of varying use to a user. Following the principle that events are intended for the system to communicate information back to the user, and that Message is the primary human readable field, this commit alters the default columns to ensure event is shown with the most width given to the message, and all other fields organized by their relevance to the message.
-o wide