Surfacing of actual Termination Reason in Step Status

[chitrangpatel]

Current behaviour

Currently, Tekton only reports Completed and Error as Terminated Reasons in Step status after the Step terminates.

The status Error is also applied to the Steps if it "times out" or if the previous Step failed and caused this to be "skipped".
Likewise, the status Completed is applied to the Step if it actually "completed with exit code 0" or if the Step actually errored but the Task set onError: continue for that Step.

The table below summarizes it.

Termination Reason	Meaning
Completed	Finished successfully with exit code 0
Completed	Errored but ignored since onError was set to continue.
Error	Actually failed with a non-zero exit code.
Error	timed out (but only the logs indicate that it timed out)
Error	skipped because the previous Step failed.

This makes getting true visibility into the Steps challenging since the Termination reasons Completed and Error could mean different things.

Proposed behaviour

In order to get better visibility into what's happening in the underlying Steps, here is the proposed changes to the Termination Reason in the Step status.

Termination Reason	Meaning
Completed	Finished successfully with exit code 0
Continued	Errored but ignored since onError was set to continue.
Error	Actually failed with a non-zero exit code.
TimeoutExceeded	timed out (but only the logs indicate that it timed out)
Skipped	skipped because the previous Step failed.
TaskRunCancelled	The task run was cancelled.

/kind bug

[chitrangpatel]

/good-first-issue

[tekton-robot]

@chitrangpatel:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[chengjoey]

Hi @chitrangpatel , Recently a Termination Reason Canceled has been added

pipeline/pkg/entrypoint/entrypointer.go

Lines 195 to 201 in a7d15ce

	output = append(output, result.RunResult{
	Key: "Reason",
	Value: "Cancelled",
	ResultType: result.InternalTektonResultType,
	})
	e.WritePostFile(e.PostFile, ErrContextCanceled)
	e.WriteExitCodeFile(e.StepMetadataDir, syscall.SIGKILL.String())

Termination Reason: Canceled
Meaning: The step was stopped by user

Should this reason also be added to the table?

[chitrangpatel]

Yes, it should @chengjoey!

I aded it to the table. Thanks 🙏

[pritidesai]

@shankarpentyala07 PTAL at the proposed changes to surface actual termination reason, thanks!

[shankarpentyala07]

/assign @shankarpentyala07

[renzodavid9]

Hey @shankarpentyala07! I just wanted to check if you are actively working in this issue. If not I would be happy to help with this. Thanks a lot! 😄

[shankarpentyala07]

Hi @renzodavid9 ,Thanks for checking.
I am yet to start working , I would be happy to work together. Shall we have one meeting and discuss the changes. My email is [email protected] , available any time apart from 9am-11am PST.

[shankarpentyala07]

/unassign @shankarpentyala07

[shankarpentyala07]

Hi @renzodavid9 - I have unassigned , please proceed.

[renzodavid9]

/assign

[renzodavid9]

Got it! Thanks @shankarpentyala07! I just assigned this