Skip to content

Commit bb99bf6

Browse files
vincent-plitekton-robot
vincent-pli
authored and
tekton-robot
committed
Sort and remove duplicate when set "taskrun.status.taskResults"
Fix issue: #2466 1. Sort `pod.containerStatuses` before parse 2. Overwrite older items
1 parent f60cff9 commit bb99bf6

File tree

4 files changed

+87
-52
lines changed

4 files changed

+87
-52
lines changed

‎pkg/pod/status.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ func areStepsComplete(pod *corev1.Pod) bool {
269269
return stepsComplete
270270
}
271271

272-
func sortContainerStatuses(podInstance *corev1.Pod) {
272+
//SortContainerStatuses sort ContainerStatuses based on "FinishedAt"
273+
func SortContainerStatuses(podInstance *corev1.Pod) {
273274
sort.Slice(podInstance.Status.ContainerStatuses, func(i, j int) bool {
274275
var ifinish, istart, jfinish, jstart time.Time
275276
if term := podInstance.Status.ContainerStatuses[i].State.Terminated; term != nil {
@@ -290,7 +291,7 @@ func sortContainerStatuses(podInstance *corev1.Pod) {
290291
}
291292

292293
func getFailureMessage(pod *corev1.Pod) string {
293-
sortContainerStatuses(pod)
294+
SortContainerStatuses(pod)
294295
// First, try to surface an error about the actual build step that failed.
295296
for _, status := range pod.Status.ContainerStatuses {
296297
term := status.State.Terminated

‎pkg/pod/status_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -910,7 +910,7 @@ func TestSortContainerStatuses(t *testing.T) {
910910
},
911911
},
912912
}
913-
sortContainerStatuses(&samplePod)
913+
SortContainerStatuses(&samplePod)
914914
var gotNames []string
915915
for _, status := range samplePod.Status.ContainerStatuses {
916916
gotNames = append(gotNames, status.Name)

‎pkg/reconciler/taskrun/taskrun.go‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun,
399399
// Convert the Pod's status to the equivalent TaskRun Status.
400400
tr.Status = podconvert.MakeTaskRunStatus(c.Logger, *tr, pod, *taskSpec)
401401

402-
if err := updateTaskRunResourceResult(tr, pod.Status); err != nil {
402+
if err := updateTaskRunResourceResult(tr, *pod); err != nil {
403403
return err
404404
}
405405

@@ -604,9 +604,11 @@ func (c *Reconciler) createPod(tr *v1beta1.TaskRun, rtr *resources.ResolvedTaskR
604604

605605
type DeletePod func(podName string, options *metav1.DeleteOptions) error
606606

607-
func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodStatus) error {
607+
func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, pod corev1.Pod) error {
608+
podconvert.SortContainerStatuses(&pod)
609+
608610
if taskRun.IsSuccessful() {
609-
for idx, cs := range podStatus.ContainerStatuses {
611+
for idx, cs := range pod.Status.ContainerStatuses {
610612
if cs.State.Terminated != nil {
611613
msg := cs.State.Terminated.Message
612614
r, err := termination.ParseMessage(msg)
@@ -618,6 +620,7 @@ func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodS
618620
taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, pipelineResourceResults...)
619621
}
620622
}
623+
taskRun.Status.TaskRunResults = removeDuplicateResults(taskRun.Status.TaskRunResults)
621624
}
622625
return nil
623626
}
@@ -642,6 +645,21 @@ func getResults(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResu
642645
return taskResults, pipelineResourceResults
643646
}
644647

648+
func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult {
649+
uniq := make([]v1beta1.TaskRunResult, 0)
650+
latest := make(map[string]v1beta1.TaskRunResult, 0)
651+
for _, res := range taskRunResult {
652+
if _, seen := latest[res.Name]; !seen {
653+
uniq = append(uniq, res)
654+
}
655+
latest[res.Name] = res
656+
}
657+
for i, res := range uniq {
658+
uniq[i] = latest[res.Name]
659+
}
660+
return uniq
661+
}
662+
645663
func isExceededResourceQuotaError(err error) bool {
646664
return err != nil && k8serrors.IsForbidden(err) && strings.Contains(err.Error(), "exceeded quota")
647665
}

‎pkg/reconciler/taskrun/taskrun_test.go‎

Lines changed: 62 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1877,19 +1877,21 @@ func TestReconcileCloudEvents(t *testing.T) {
18771877
func TestUpdateTaskRunResourceResult(t *testing.T) {
18781878
for _, c := range []struct {
18791879
desc string
1880-
podStatus corev1.PodStatus
1880+
pod corev1.Pod
18811881
taskRunStatus *v1beta1.TaskRunStatus
18821882
want []resourcev1alpha1.PipelineResourceResult
18831883
}{{
18841884
desc: "image resource updated",
1885-
podStatus: corev1.PodStatus{
1886-
ContainerStatuses: []corev1.ContainerStatus{{
1887-
State: corev1.ContainerState{
1888-
Terminated: &corev1.ContainerStateTerminated{
1889-
Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
1885+
pod: corev1.Pod{
1886+
Status: corev1.PodStatus{
1887+
ContainerStatuses: []corev1.ContainerStatus{{
1888+
State: corev1.ContainerState{
1889+
Terminated: &corev1.ContainerStateTerminated{
1890+
Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
1891+
},
18901892
},
1891-
},
1892-
}},
1893+
}},
1894+
},
18931895
},
18941896
want: []resourcev1alpha1.PipelineResourceResult{{
18951897
Key: "digest",
@@ -1904,7 +1906,7 @@ func TestUpdateTaskRunResourceResult(t *testing.T) {
19041906
Type: apis.ConditionSucceeded,
19051907
Status: corev1.ConditionTrue,
19061908
})
1907-
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
1909+
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
19081910
t.Errorf("updateTaskRunResourceResult: %s", err)
19091911
}
19101912
if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" {
@@ -1917,20 +1919,22 @@ func TestUpdateTaskRunResourceResult(t *testing.T) {
19171919
func TestUpdateTaskRunResult(t *testing.T) {
19181920
for _, c := range []struct {
19191921
desc string
1920-
podStatus corev1.PodStatus
1922+
pod corev1.Pod
19211923
taskRunStatus *v1beta1.TaskRunStatus
19221924
wantResults []v1beta1.TaskRunResult
19231925
want []resourcev1alpha1.PipelineResourceResult
19241926
}{{
19251927
desc: "test result with pipeline result",
1926-
podStatus: corev1.PodStatus{
1927-
ContainerStatuses: []corev1.ContainerStatus{{
1928-
State: corev1.ContainerState{
1929-
Terminated: &corev1.ContainerStateTerminated{
1930-
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`,
1928+
pod: corev1.Pod{
1929+
Status: corev1.PodStatus{
1930+
ContainerStatuses: []corev1.ContainerStatus{{
1931+
State: corev1.ContainerState{
1932+
Terminated: &corev1.ContainerStateTerminated{
1933+
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`,
1934+
},
19311935
},
1932-
},
1933-
}},
1936+
}},
1937+
},
19341938
},
19351939
wantResults: []v1beta1.TaskRunResult{{
19361940
Name: "resultName",
@@ -1950,7 +1954,7 @@ func TestUpdateTaskRunResult(t *testing.T) {
19501954
Type: apis.ConditionSucceeded,
19511955
Status: corev1.ConditionTrue,
19521956
})
1953-
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
1957+
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
19541958
t.Errorf("updateTaskRunResourceResult: %s", err)
19551959
}
19561960
if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" {
@@ -1966,20 +1970,22 @@ func TestUpdateTaskRunResult(t *testing.T) {
19661970
func TestUpdateTaskRunResult2(t *testing.T) {
19671971
for _, c := range []struct {
19681972
desc string
1969-
podStatus corev1.PodStatus
1973+
pod corev1.Pod
19701974
taskRunStatus *v1beta1.TaskRunStatus
19711975
wantResults []v1beta1.TaskRunResult
19721976
want []resourcev1alpha1.PipelineResourceResult
19731977
}{{
19741978
desc: "test result with pipeline result - no result type",
1975-
podStatus: corev1.PodStatus{
1976-
ContainerStatuses: []corev1.ContainerStatus{{
1977-
State: corev1.ContainerState{
1978-
Terminated: &corev1.ContainerStateTerminated{
1979-
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
1979+
pod: corev1.Pod{
1980+
Status: corev1.PodStatus{
1981+
ContainerStatuses: []corev1.ContainerStatus{{
1982+
State: corev1.ContainerState{
1983+
Terminated: &corev1.ContainerStateTerminated{
1984+
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
1985+
},
19801986
},
1981-
},
1982-
}},
1987+
}},
1988+
},
19831989
},
19841990
wantResults: []v1beta1.TaskRunResult{{
19851991
Name: "resultName",
@@ -1998,7 +2004,7 @@ func TestUpdateTaskRunResult2(t *testing.T) {
19982004
Type: apis.ConditionSucceeded,
19992005
Status: corev1.ConditionTrue,
20002006
})
2001-
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
2007+
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
20022008
t.Errorf("updateTaskRunResourceResult: %s", err)
20032009
}
20042010
if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" {
@@ -2014,23 +2020,31 @@ func TestUpdateTaskRunResult2(t *testing.T) {
20142020
func TestUpdateTaskRunResultTwoResults(t *testing.T) {
20152021
for _, c := range []struct {
20162022
desc string
2017-
podStatus corev1.PodStatus
2023+
pod corev1.Pod
20182024
taskRunStatus *v1beta1.TaskRunStatus
20192025
want []v1beta1.TaskRunResult
20202026
}{{
20212027
desc: "two test results",
2022-
podStatus: corev1.PodStatus{
2023-
ContainerStatuses: []corev1.ContainerStatus{{
2024-
State: corev1.ContainerState{
2025-
Terminated: &corev1.ContainerStateTerminated{
2026-
Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
2028+
pod: corev1.Pod{
2029+
Status: corev1.PodStatus{
2030+
ContainerStatuses: []corev1.ContainerStatus{{
2031+
State: corev1.ContainerState{
2032+
Terminated: &corev1.ContainerStateTerminated{
2033+
Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
2034+
},
20272035
},
2028-
},
2029-
}},
2036+
}, {
2037+
State: corev1.ContainerState{
2038+
Terminated: &corev1.ContainerStateTerminated{
2039+
Message: `[{"key":"resultNameOne","value":"resultValueThree", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
2040+
},
2041+
},
2042+
}},
2043+
},
20302044
},
20312045
want: []v1beta1.TaskRunResult{{
20322046
Name: "resultNameOne",
2033-
Value: "resultValueOne",
2047+
Value: "resultValueThree",
20342048
}, {
20352049
Name: "resultNameTwo",
20362050
Value: "resultValueTwo",
@@ -2043,7 +2057,7 @@ func TestUpdateTaskRunResultTwoResults(t *testing.T) {
20432057
Type: apis.ConditionSucceeded,
20442058
Status: corev1.ConditionTrue,
20452059
})
2046-
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
2060+
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
20472061
t.Errorf("updateTaskRunResourceResult: %s", err)
20482062
}
20492063
if d := cmp.Diff(c.want, tr.Status.TaskRunResults); d != "" {
@@ -2095,19 +2109,21 @@ func TestUpdateTaskRunResultWhenTaskFailed(t *testing.T) {
20952109
func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
20962110
for _, c := range []struct {
20972111
desc string
2098-
podStatus corev1.PodStatus
2112+
pod corev1.Pod
20992113
taskRunStatus *v1beta1.TaskRunStatus
21002114
want []resourcev1alpha1.PipelineResourceResult
21012115
}{{
21022116
desc: "image resource exporter with malformed json output",
2103-
podStatus: corev1.PodStatus{
2104-
ContainerStatuses: []corev1.ContainerStatus{{
2105-
State: corev1.ContainerState{
2106-
Terminated: &corev1.ContainerStateTerminated{
2107-
Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
2117+
pod: corev1.Pod{
2118+
Status: corev1.PodStatus{
2119+
ContainerStatuses: []corev1.ContainerStatus{{
2120+
State: corev1.ContainerState{
2121+
Terminated: &corev1.ContainerStateTerminated{
2122+
Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
2123+
},
21082124
},
2109-
},
2110-
}},
2125+
}},
2126+
},
21112127
},
21122128
taskRunStatus: &v1beta1.TaskRunStatus{
21132129
Status: duckv1beta1.Status{Conditions: []apis.Condition{{
@@ -2119,7 +2135,7 @@ func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
21192135
}} {
21202136
t.Run(c.desc, func(t *testing.T) {
21212137
names.TestingSeed()
2122-
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.podStatus); err == nil {
2138+
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.pod); err == nil {
21232139
t.Error("Expected error, got nil")
21242140
}
21252141
if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" {

0 commit comments

Comments
 (0)