[kueuectl] Add EXEC TIME column on list workload command.

[mbobrovskyi]

What type of PR is this?

/kind feature

What this PR does / why we need it:

Add EXEC TIME column on kueuectl list workload command.

Which issue(s) this PR fixes:

Fixes #2972

Special notes for your reviewer:

Does this PR introduce a user-facing change?

CLI: Added EXEC TIME column on kueuectl list workload command.

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	82c50ef
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66d87ec69e9dc200086e24aa

[mimowo]

Add a test cases / or modify the exiting ones to cover the two scenarios:

1. the workload is re-admitted
2. the workload finished in the past (last transition time < now)

[mbobrovskyi]

the workload finished in the past (last transition time < now)

We have a few tests with this condition. For example:

kueue/cmd/kueuectl/app/list/list_workload_test.go

Lines 223 to 227 in 94319f9

	wantOut: `NAME JOB TYPE JOB NAME LOCALQUEUE CLUSTERQUEUE STATUS POSITION IN QUEUE DURATION AGE
	wl1 j1 lq1 cq1 PENDING 60m
	wl2 j2 lq2 cq2 ADMITTED 60m 120m
	wl3 j3 lq3 cq3 FINISHED 60m 3h
	`,

[mimowo]

Awesome, i missed that

[mbobrovskyi]

the workload is re-admitted

I'm not clear on the difference between admitted and re-admitted, especially in the context of conditions. Are you asking if we should test with Admitted=false?

[mimowo]

Yeah, I think we don't need to test re-admission actually. My intention was to just test when lastTransitionTime for Admitted > creation time. Do we have that covered already too?

[mbobrovskyi]

Are you asking if we should test with Admitted=false?

This test cases I missed. Thanks!

[mbobrovskyi]

My intention was to just test when lastTransitionTime for Admitted > creation time.

I thought that it can't be possible... But I think we can add this case.
What should we need to print on this way? WDYT? Just empty line?

[mimowo]

I just want to make sure the very basic scenario is covered, when Admitted=True, and the lastTransitionTime is later than creation time of the workload. This would be in practice almost all the time, but since we mock the timer, I'm not clear if this is tested.

[mimowo]

/cc @trasc

[mimowo]

I'm still thinking about a potentially better name for the column. Alternative names that come to my mind:

• EXECUTION TIME
• RUNTIME

I would be leaning towards EXECUTION TIME as more descriptive of the semantics, but the downside is that it is longer. Any preferences @alculquicondor @mwielgus @tenzen-y ?

EDIT: or maybe EXEC TIME?

[mwielgus]

I'm for EXEC TIME. RUNTIME is confusing (should it be "docker") and EXECUTION TIME unnecessarily long.

[alculquicondor]

+1 on EXEC TIME

[alculquicondor]

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 26dec89eb48fc8e540c86b2bb819c8bc3588389d

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, mbobrovskyi

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:

• cmd/OWNERS [alculquicondor]
• test/OWNERS [alculquicondor]

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

[tenzen-y]

I would be leaning towards EXECUTION TIME as more descriptive of the semantics, but the downside is that it is longer. Any preferences @alculquicondor @mwielgus @tenzen-y ?

EDIT: or maybe EXEC TIME?

SGTM