Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEP074] Tombstone ResourceResult field with the removal of PipelineResources #6301

Merged
merged 1 commit into from
Apr 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,6 @@ See [TEP-0074](https://github.com/tektoncd/community/blob/main/teps/0074-depreca

- The artifacts bucket/pvc setup by the `pkg/artifacts` package related with Storage PipelineResources

- The generic pipelineResources functions including inputs and outputs resources and the `from` type
- The generic pipelineResources functions including inputs and outputs resources and the `from` type

- [TaskRun.Status.ResourcesResult is deprecated and tombstoned #6301](https://github.com/tektoncd/pipeline/issues/6325)
5 changes: 3 additions & 2 deletions docs/pipeline-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -13973,8 +13973,9 @@ All TaskRunStatus stored in RetriesStatus will have no date within the RetriesSt
</td>
<td>
<em>(Optional)</em>
<p>Results from Resources built during the TaskRun. currently includes
the digest of build container images</p>
<p>Results from Resources built during the TaskRun.
This is tomb-stoned along with the removal of pipelineResources
Deprecated: this field is not populated and is preserved only for backwards compatibility</p>
</td>
</tr>
<tr>
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1beta1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,6 @@ var (
ImageID: "image-id",
ContainerName: "sidecar-error",
}},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
},
},
WhenExpressions: v1beta1.WhenExpressions{{
Expand Down
4 changes: 2 additions & 2 deletions pkg/apis/pipeline/v1beta1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -2849,7 +2849,7 @@
"$ref": "#/definitions/v1beta1.Provenance"
},
"resourcesResult": {
"description": "Results from Resources built during the TaskRun. currently includes the digest of build container images",
"description": "Results from Resources built during the TaskRun. This is tomb-stoned along with the removal of pipelineResources Deprecated: this field is not populated and is preserved only for backwards compatibility",
"type": "array",
"items": {
"default": {},
Expand Down Expand Up @@ -2941,7 +2941,7 @@
"$ref": "#/definitions/v1beta1.Provenance"
},
"resourcesResult": {
"description": "Results from Resources built during the TaskRun. currently includes the digest of build container images",
"description": "Results from Resources built during the TaskRun. This is tomb-stoned along with the removal of pipelineResources Deprecated: this field is not populated and is preserved only for backwards compatibility",
"type": "array",
"items": {
"default": {},
Expand Down
25 changes: 0 additions & 25 deletions pkg/apis/pipeline/v1beta1/taskrun_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ func (tr *TaskRun) ConvertTo(ctx context.Context, to apis.Convertible) error {
if err := serializeTaskRunCloudEvents(&sink.ObjectMeta, &tr.Status); err != nil {
return err
}
if err := serializeTaskRunResourcesResult(&sink.ObjectMeta, &tr.Status); err != nil {
return err
}
if err := tr.Status.ConvertTo(ctx, &sink.Status); err != nil {
return err
}
Expand Down Expand Up @@ -118,9 +115,6 @@ func (tr *TaskRun) ConvertFrom(ctx context.Context, from apis.Convertible) error
if err := deserializeTaskRunCloudEvents(&tr.ObjectMeta, &tr.Status); err != nil {
return err
}
if err := deserializeTaskRunResourcesResult(&tr.ObjectMeta, &tr.Status); err != nil {
return err
}
if err := tr.Status.ConvertFrom(ctx, source.Status); err != nil {
return err
}
Expand Down Expand Up @@ -372,22 +366,3 @@ func deserializeTaskRunCloudEvents(meta *metav1.ObjectMeta, status *TaskRunStatu
}
return nil
}

func serializeTaskRunResourcesResult(meta *metav1.ObjectMeta, status *TaskRunStatus) error {
if status.ResourcesResult == nil {
return nil
}
return version.SerializeToMetadata(meta, status.ResourcesResult, resourcesResultAnnotationKey)
}

func deserializeTaskRunResourcesResult(meta *metav1.ObjectMeta, status *TaskRunStatus) error {
resourcesResult := []RunResult{}
err := version.DeserializeFromMetadata(meta, &resourcesResult, resourcesResultAnnotationKey)
if err != nil {
return err
}
if len(resourcesResult) != 0 {
status.ResourcesResult = resourcesResult
}
return nil
}
50 changes: 0 additions & 50 deletions pkg/apis/pipeline/v1beta1/taskrun_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,56 +377,6 @@ func TestTaskRunConversionFromDeprecated(t *testing.T) {
},
},
},
}, {
name: "resourcesResult",
in: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "test-resources-result",
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}, {
Key: "digest-11",
Value: "sha256:1234",
ResourceName: "source-image",
}},
},
},
},
want: &v1beta1.TaskRun{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.TaskRunSpec{
TaskRef: &v1beta1.TaskRef{
Name: "test-resources-result",
},
},
Status: v1beta1.TaskRunStatus{
TaskRunStatusFields: v1beta1.TaskRunStatusFields{
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}, {
Key: "digest-11",
Value: "sha256:1234",
ResourceName: "source-image",
}},
},
},
},
}}
for _, test := range tests {
versions := []apis.Convertible{&v1.TaskRun{}}
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1beta1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,9 @@ type TaskRunStatusFields struct {
// +listType=atomic
RetriesStatus []TaskRunStatus `json:"retriesStatus,omitempty"`

// Results from Resources built during the TaskRun. currently includes
// the digest of build container images
// Results from Resources built during the TaskRun.
// This is tomb-stoned along with the removal of pipelineResources
// Deprecated: this field is not populated and is preserved only for backwards compatibility
// +optional
// +listType=atomic
ResourcesResult []RunResult `json:"resourcesResult,omitempty"`
Expand Down
15 changes: 7 additions & 8 deletions pkg/pod/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,9 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
}

// populate task run CRD with results from sidecar logs
taskResults, RunResults, _ := filterResultsAndResources(sidecarLogResults, specResults)
taskResults, _ := filterResults(sidecarLogResults, specResults)
if tr.IsSuccessful() {
trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
trs.ResourcesResult = append(trs.ResourcesResult, RunResults...)
}
}
// Continue with extraction of termination messages
Expand All @@ -208,10 +207,9 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
merr = multierror.Append(merr, err)
}

taskResults, RunResults, filteredResults := filterResultsAndResources(results, specResults)
taskResults, filteredResults := filterResults(results, specResults)
if tr.IsSuccessful() {
trs.TaskRunResults = append(trs.TaskRunResults, taskResults...)
trs.ResourcesResult = append(trs.ResourcesResult, RunResults...)
}
msg, err = createMessageFromResults(filteredResults)
if err != nil {
Expand Down Expand Up @@ -261,9 +259,11 @@ func createMessageFromResults(results []v1beta1.RunResult) (string, error) {
return string(bytes), nil
}

func filterResultsAndResources(results []v1beta1.RunResult, specResults []v1beta1.TaskResult) ([]v1beta1.TaskRunResult, []v1beta1.RunResult, []v1beta1.RunResult) {
// filterResults filters the RunResults and TaskResults based on the results declared in the task spec.
// It returns a slice of any of the input results that are defined in the task spec, converted to TaskRunResults,
// and a slice of any of the RunResults that don't represent internal values (i.e. those that should not be displayed in the TaskRun status.
func filterResults(results []v1beta1.RunResult, specResults []v1beta1.TaskResult) ([]v1beta1.TaskRunResult, []v1beta1.RunResult) {
var taskResults []v1beta1.TaskRunResult
var RunResults []v1beta1.RunResult
var filteredResults []v1beta1.RunResult
neededTypes := make(map[string]v1beta1.ResultsType)
for _, r := range specResults {
Expand Down Expand Up @@ -297,12 +297,11 @@ func filterResultsAndResources(results []v1beta1.RunResult, specResults []v1beta
// Internal messages are ignored because they're not used as external result
continue
default:
RunResults = append(RunResults, r)
filteredResults = append(filteredResults, r)
}
}

return taskResults, RunResults, filteredResults
return taskResults, filteredResults
lbernick marked this conversation as resolved.
Show resolved Hide resolved
}

func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult {
Expand Down
37 changes: 1 addition & 36 deletions pkg/pod/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,7 @@ func TestMakeTaskRunStatus(t *testing.T) {
},
},
}, {
desc: "image resource updated",
JeromeJu marked this conversation as resolved.
Show resolved Hide resolved
desc: "image resource that should not populate resourcesResult",
podStatus: corev1.PodStatus{
Phase: corev1.PodSucceeded,
ContainerStatuses: []corev1.ContainerStatus{{
Expand All @@ -671,11 +671,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
ContainerName: "step-foo",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:12345",
ResourceName: "source-image",
}},
// We don't actually care about the time, just that it's not nil
CompletionTime: &metav1.Time{Time: time.Now()},
},
Expand Down Expand Up @@ -705,11 +700,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
ContainerName: "step-bar",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeString,
Expand Down Expand Up @@ -744,11 +734,6 @@ func TestMakeTaskRunStatus(t *testing.T) {
ContainerName: "step-banana",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeString,
Expand Down Expand Up @@ -1297,11 +1282,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
ContainerName: "step-bar",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeString,
Expand Down Expand Up @@ -1344,11 +1324,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
ContainerName: "step-bar",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeString,
Expand Down Expand Up @@ -1391,11 +1366,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
ContainerName: "step-bar",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeArray,
Expand Down Expand Up @@ -1438,11 +1408,6 @@ func TestMakeTaskRunStatusAlpha(t *testing.T) {
ContainerName: "step-bar",
}},
Sidecars: []v1beta1.SidecarState{},
ResourcesResult: []v1beta1.RunResult{{
Key: "digest",
Value: "sha256:1234",
ResourceName: "source-image",
}},
TaskRunResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Type: v1beta1.ResultsTypeObject,
Expand Down