From e14bcb0180f95ad0b0345830e4037e5c2479c61d Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Mon, 30 Jan 2023 18:17:03 +0000 Subject: [PATCH] remove results from status when type mismatched and add more logs This commit fixes #6023. Before this commit we only log the result name and needed types, but omit the want types and the invalid results are in status. This commit adds the want types in error message and remove invalid results from status. Signed-off-by: Yongxuan Zhang yongxuanzhang@google.com --- pkg/reconciler/taskrun/taskrun_test.go | 42 ++++++++++++-------- pkg/reconciler/taskrun/validate_resources.go | 34 +++++++++++----- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b73f6f86584..c4a708e400c 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1877,31 +1877,23 @@ spec: taskRef: name: test-results-task status: - taskResults: - - name: aResult - type: string - value: aResultValue conditions: - reason: ToBeRetried status: Unknown type: Succeeded - message: "missmatched Types for these results, map[aResult:[array]]" + message: "mismatched types: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" sideCars: retriesStatus: - conditions: - reason: TaskRunValidationFailed status: "False" type: "Succeeded" - message: "missmatched Types for these results, map[aResult:[array]]" + message: "mismatched types: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" startTime: "2021-12-31T23:59:59Z" completionTime: "2022-01-01T00:00:00Z" podName: "test-taskrun-results-type-mismatched-pod" - taskResults: - - name: aResult - type: string - value: aResultValue `) - reconciliatonError = fmt.Errorf("1 error occurred:\n\t* missmatched Types for these results, map[aResult:[array]]") + reconciliatonError = fmt.Errorf("1 error occurred:\n\t* mismatched types: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"") toBeRetriedTaskRun = parse.MustParseV1beta1TaskRun(t, ` metadata: name: test-taskrun-to-be-retried @@ -4894,7 +4886,7 @@ spec: results: - name: result1 steps: - - script: echo foo >> $(results.result1.path) + - script: echo foo >> $(results.result1.path) image: myimage name: mycontainer status: @@ -4902,7 +4894,7 @@ status: results: - name: result1 steps: - - script: echo foo >> $(results.result1.path) + - script: echo foo >> $(results.result1.path) image: myimage name: mycontainer `) @@ -5084,6 +5076,9 @@ status: - name: aResult type: string value: aResultValue + - name: objectResult + type: string + value: objectResultValue `) taskRunResultsObjectMissKey := parse.MustParseV1beta1TaskRun(t, ` @@ -5098,8 +5093,8 @@ status: - name: aResult type: array value: - - 1 - - 2 + - "1" + - "2" - name: objectResult type: object value: @@ -5125,16 +5120,28 @@ status: taskRun *v1beta1.TaskRun wantFailedReason string expectedError error + expectedResults []v1beta1.TaskRunResult }{{ name: "taskrun results type mismatched", taskRun: taskRunResultsTypeMismatched, wantFailedReason: podconvert.ReasonFailedValidation, - expectedError: fmt.Errorf("1 error occurred:\n\t* missmatched Types for these results, map[aResult:[array]]"), + expectedError: fmt.Errorf("1 error occurred:\n\t* mismatched types: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\", \"objectResult\": task result is expected to be \"object\" type but was initialized to a different type \"string\""), + expectedResults: nil, }, { name: "taskrun results object miss key", taskRun: taskRunResultsObjectMissKey, wantFailedReason: podconvert.ReasonFailedValidation, expectedError: fmt.Errorf("1 error occurred:\n\t* missing keys for these results which are required in TaskResult's properties map[objectResult:[commit]]"), + expectedResults: []v1beta1.TaskRunResult{ + { + Name: "aResult", + Type: "array", + Value: *v1beta1.NewArrayOrString("1", "2"), + }, { + Name: "objectResult", + Type: "object", + Value: *v1beta1.NewObject(map[string]string{"url": "abc"}), + }}, }} { t.Run(tc.name, func(t *testing.T) { testAssets, cancel := getTaskRunController(t, d) @@ -5149,6 +5156,9 @@ status: if err != nil { t.Fatalf("getting updated taskrun: %v", err) } + if d := cmp.Diff(tr.Status.TaskRunResults, tc.expectedResults); d != "" { + t.Errorf("got unexpected results %s", diff.PrintWantGot(d)) + } condition := tr.Status.GetCondition(apis.ConditionSucceeded) if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != tc.wantFailedReason { t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", tc.wantFailedReason, tr.Status.Conditions) diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index a9651ef52fb..31d4ecc2382 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -19,6 +19,8 @@ package taskrun import ( "context" "fmt" + "sort" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -300,7 +302,12 @@ func validateTaskRunResults(tr *v1beta1.TaskRun, resolvedTaskSpec *v1beta1.TaskS // When get the results, check if the type of result is the expected one if missmatchedTypes := mismatchedTypesResults(tr, specResults); len(missmatchedTypes) != 0 { - return fmt.Errorf("missmatched Types for these results, %v", missmatchedTypes) + var s []string + for k, v := range missmatchedTypes { + s = append(s, fmt.Sprintf(" \"%v\": %v", k, v)) + } + sort.Strings(s) + return fmt.Errorf("mismatched types: %v", strings.Join(s, ",")) } // When get the results, for object value need to check if they have missing keys. @@ -311,19 +318,28 @@ func validateTaskRunResults(tr *v1beta1.TaskRun, resolvedTaskSpec *v1beta1.TaskS } // mismatchedTypesResults checks and returns all the mismatched types of emitted results against specified results. -func mismatchedTypesResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskResult) map[string][]string { - neededTypes := make(map[string][]string) - providedTypes := make(map[string][]string) - // collect needed keys for object results +func mismatchedTypesResults(tr *v1beta1.TaskRun, specResults []v1beta1.TaskResult) map[string]string { + neededTypes := make(map[string]string) + mismatchedTypes := make(map[string]string) + var filteredResults []v1beta1.TaskRunResult + // collect needed types for results for _, r := range specResults { - neededTypes[r.Name] = append(neededTypes[r.Name], string(r.Type)) + neededTypes[r.Name] = string(r.Type) } - // collect provided keys for object results + // collect mismatched types for results, and correct results in filteredResults + // TODO(#6097): Validate if the emitted results are defined in taskspec for _, trr := range tr.Status.TaskRunResults { - providedTypes[trr.Name] = append(providedTypes[trr.Name], string(trr.Type)) + needed, ok := neededTypes[trr.Name] + if ok && needed != string(trr.Type) { + mismatchedTypes[trr.Name] = fmt.Sprintf("task result is expected to be \"%v\" type but was initialized to a different type \"%v\"", needed, trr.Type) + } else { + filteredResults = append(filteredResults, trr) + } } - return findMissingKeys(neededTypes, providedTypes) + // remove the mismatched results + tr.Status.TaskRunResults = filteredResults + return mismatchedTypes } // missingKeysofObjectResults checks and returns the missing keys of object results.