PipelineResourceResult Renamed and Moving off resource_types

[JeromeJu]

In v1beta1 PipelineResourceResult is both a struct and a result type that was used to be in the resource related codebase. Now that we are removing pipelineResources, either renaming or a new design is required to be moved to V1. It is only used now internally as a result-like struct, the usage could be found below.

There are 3 resultTypes of PipelineResourceResult:

• pipelineResourceResultType will be removed
• InternalTektonResultType
• TaskRunResultType

This would need to be brought to v1 in some way:

The usage of PipelineResourceResult with respective types:

• Git-init image usage of Termination package for pod termination message via termination.WriteMessage - possibly blocker for TEP074 pipelineresources deprecation
• Pod package status

  pipeline/pkg/pod/status.go

  Line 258 in 5b702f9

  func filterResultsAndResources(results []v1beta1.PipelineResourceResult, specResults []v1beta1.TaskResult) ([]v1beta1.TaskRunResult, []v1beta1.PipelineResourceResult, []v1beta1.PipelineResourceResult) {

  used as TaskRunResult.
• For the spire package spire mock test etc as v1beta1.TaskRunResultType, learned from @jagathprakash that this could be removed
• Entrypointer package https://github.com/tektoncd/pipeline/blob/f732985ec30817703e6eeaa6914cc77a2b8adf3c/pkg/entrypoint/entrypointer.go as in InternalTektonResultType
• Termination package

---

Goal of this issue is:

• Decouple pipelineResourceResult
• Remove in v1beta1 along with pipelineResouces
• Introduce new struct in v1 along with resourceResult
  • the resourceResult is in v1beta1 but not moved to v1 so we have to introduce a new field

---

PRs for review (chained in sequence:)
Stop populating resourceName in git-init image
Migrate git-init image off PipelineResourceResult
[TEP074] Tombstone ResourceResult field with the removal of PipelineResources

[JeromeJu]

One question regarding pkg/pod/status: when will we move it to v1? I am assuming by the time when v1 is swapped as storage version?

[JeromeJu]

How PipelineResourceResult has been introduced and removed, this is tracked to find the actual purpose of this struct:

• First PR introducing PipelineResourceResult

3fe2584#diff-5589d22b1568e49f5351ae729c0c0214014a7d88bd82c8b7ae31eb7ea9d41cb8:
So it was first introduced only fo add a field in the taskRun status to include the image name and
digest -> resourcesResult

Then its modification to add more generic functions
e01fe60#diff-5589d22b1568e49f5351ae729c0c0214014a7d88bd82c8b7ae31eb7ea9d41cb8

Then made more generic to include other results
#2011

[JeromeJu]

/kind misc

[JeromeJu]

Here are a couple of questions to be answered before moving forward after the offline conversation with @dibyom
@lbernick , @pritidesai and @Yongxuanzhang 🙏 :

• In which package shall we move the PipelineResourceResult Struct to since resource_types is going to be remoed?
  • to not to introduce a new package, so to pkg/status
  • to introduce a new package, eg. pkg/result
    • cons: not sure if this is necessary to be in a separate package.
  • to move into pkg/apis/v1beta1/result_types since it is related to results
    • cons: result_types are mainly API-related, would have to migrate out of v1beta1 later
• Shall we keep this Struct or refactor it to be more generic?
  • whether we could introduce a generics for result to be refactored for func WriteMessage and func ParseMessage
  • Can we remove the resourceName and resourceType field?
• what shall we rename the Struct to?
  • Result
  • PipelineStatusResult

[dibyom]

In which package shall we move the PipelineResourceResult Struct to since resource_types is going to be remoed?

no strong feelings - if the main use for this struct is writing to termination message maybe pkg/termination?

Shall we keep this Struct or refactor it to be more generic?

not sure what a more generic solution would be so a struct sounds find

what shall we rename the Struct to?

I like TektonTerminationMessage since this struct encapsulates the different kinds of things we write to the termination message

[Yongxuanzhang]

In which package shall we move the PipelineResourceResult

I think if we still use this struct in v1beta1.taskrun, we may need to keep it in the api?

[pritidesai]

I think the ResourceName can be safely deleted. I am not finding any usage left after we delete the pipeline resources related code.

The ResultType field of PipelineResourceResult is mainly used to identify whether a particular object is a taskResult (TaskRunResultType) or some other metadata (InternalTektonResultType) from the termination message, such as, startedAt, exitCode, etc.

pipeline/pkg/pod/status.go

Line 268 in 90f33a6

case v1beta1.TaskRunResultType:

pipeline/pkg/pod/status.go

Line 290 in 90f33a6

case v1beta1.InternalTektonResultType:

One question regarding pkg/pod/status: when will we move it to v1? I am assuming by the time when v1 is swapped as storage version?

Yes, pkg/pod/status.go will use v1.* instead of v1beta1.* eventually 🙃

I think if we still use this struct in v1beta1.taskrun, we may need to keep it in the api?

Yes, I am thinking the same, it will go under api.

The PipelineResourceResult can be stripped down to:

type PipelineResourceResult struct {
	Key          string     `json:"key"`
	Value        string     `json:"value"`
	ResultType   ResultType `json:"type,omitempty"`
}

There is a possibility of deleting ResultType from this struct if we set the Key to its actual value StartedAt and ExitCode either while parsing the termination message ParseMessage rather than wait until setting it in:

pipeline/pkg/pod/status.go

Line 323 in 90f33a6

func extractStartedAtTimeFromResults(results []v1beta1.PipelineResourceResult) (*metav1.Time, error) {

or while reading from a termination message in extractStartedAtTimeFromResults and extractExitCodeFromResults, once the Key is set to startedAt or ExitCode, instead of checking ResultType, check the Key.

whether we could introduce a generics for result to be refactored for func WriteMessage and func ParseMessage

Please elaborate on using generics. I couldn't envision using generics here but will be great if we could.

[JeromeJu]

Thanks all for the inputs and guidance on this @afrittoli @pritidesai @dibyom @lbernick @Yongxuanzhang ,

There are two concerns related with the migration of PipelineResourceResult and taskrun.status.resourcesresult:

• git-init image used to write PipelineResourceResult to taskrun.status.resourcesresult, so we need to first migrate it off from using PipelineResourceResult and then tombstoning the resourcesresult field
• taskrun.status.resourcesresult filed has not been explicitly declared to be deprecated previously for users even though it would not function much without pipelineResources

Therefore, the following steps are planned to move forward targeting a cleanup for easier storage swap:

• Migrating git-init image off from populating resourcesname (thanks to @lbernick
  at Stop populating resourceName in git-init image #6310)
• Create a new Struct Result renamed from the PipelineResourceResult struct that is used by termination package; whilst we keep the old PipelineResourceResult struct for backward compatibility.
• Refactor the usage of PipeilneResourceResult to be on actual TaskRunResult for git-init image and for pod status etc.

[JeromeJu]

This comment tracks the refactor of the PipelineResourceResult struct:

PipelineResourceResult is used as a means to read the container status and make it available to the taskRun status, for example, startedAt, exitCode, taskResults, failure message, etc.

This struct introduced ResultType to support identifying internal usage v/s pipeline resources.

Now, since pipeline resources are being deleted, please evaluate to see if ResourceName and ResultType are needed or not. Thanks!

• note for refactoring
  Stop populating resourceName in git-init image #6310 (comment)

mark fields/ name deprecated

[JeromeJu]

A followup PR for #6434 will move the renamed struct to a proper space since resource_types is no longer used after the removal of all PipelineResources.

[JeromeJu]

So far we have renamed and going to tombstone ResourcesResult + move the renamed RunResult to a different package.
One question remaining is that whether we are going to move this struct to v1 or a un-versioned package otherwise this would be left in v1beta1. 🤔