Skip to content

Commit a3b3e0d

Browse files
vincent-plivincent-pli
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 3554a30 commit a3b3e0d

File tree

4 files changed

+80
-51
lines changed

4 files changed

+80
-51
lines changed

‎pkg/pod/status.go‎

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

269-
func sortContainerStatuses(podInstance *corev1.Pod) {
269+
//SortContainerStatuses sort ContainerStatuses based on "FinishedAt"
270+
func SortContainerStatuses(podInstance *corev1.Pod) {
270271
sort.Slice(podInstance.Status.ContainerStatuses, func(i, j int) bool {
271272
var ifinish, jfinish time.Time
272273
if term := podInstance.Status.ContainerStatuses[i].State.Terminated; term != nil {
@@ -281,7 +282,7 @@ func sortContainerStatuses(podInstance *corev1.Pod) {
281282
}
282283

283284
func getFailureMessage(pod *corev1.Pod) string {
284-
sortContainerStatuses(pod)
285+
SortContainerStatuses(pod)
285286
// First, try to surface an error about the actual build step that failed.
286287
for _, status := range pod.Status.ContainerStatuses {
287288
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
@@ -398,7 +398,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun,
398398
// Convert the Pod's status to the equivalent TaskRun Status.
399399
tr.Status = podconvert.MakeTaskRunStatus(c.Logger, *tr, pod, *taskSpec)
400400

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

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

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

606-
func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodStatus) error {
606+
func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, pod corev1.Pod) error {
607+
podconvert.SortContainerStatuses(&pod)
608+
607609
if taskRun.IsSuccessful() {
608-
for idx, cs := range podStatus.ContainerStatuses {
610+
for idx, cs := range pod.Status.ContainerStatuses {
609611
if cs.State.Terminated != nil {
610612
msg := cs.State.Terminated.Message
611613
r, err := termination.ParseMessage(msg)
@@ -617,6 +619,7 @@ func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodS
617619
taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, pipelineResourceResults...)
618620
}
619621
}
622+
taskRun.Status.TaskRunResults = removeDuplicateResults(taskRun.Status.TaskRunResults)
620623
}
621624
return nil
622625
}
@@ -641,6 +644,21 @@ func getResults(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResu
641644
return taskResults, pipelineResourceResults
642645
}
643646

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

‎pkg/reconciler/taskrun/taskrun_test.go‎

Lines changed: 55 additions & 45 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,19 +2020,21 @@ 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+
},
20302038
},
20312039
want: []v1beta1.TaskRunResult{{
20322040
Name: "resultNameOne",
@@ -2043,7 +2051,7 @@ func TestUpdateTaskRunResultTwoResults(t *testing.T) {
20432051
Type: apis.ConditionSucceeded,
20442052
Status: corev1.ConditionTrue,
20452053
})
2046-
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
2054+
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
20472055
t.Errorf("updateTaskRunResourceResult: %s", err)
20482056
}
20492057
if d := cmp.Diff(c.want, tr.Status.TaskRunResults); d != "" {
@@ -2095,19 +2103,21 @@ func TestUpdateTaskRunResultWhenTaskFailed(t *testing.T) {
20952103
func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
20962104
for _, c := range []struct {
20972105
desc string
2098-
podStatus corev1.PodStatus
2106+
pod corev1.Pod
20992107
taskRunStatus *v1beta1.TaskRunStatus
21002108
want []resourcev1alpha1.PipelineResourceResult
21012109
}{{
21022110
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"}`,
2111+
pod: corev1.Pod{
2112+
Status: corev1.PodStatus{
2113+
ContainerStatuses: []corev1.ContainerStatus{{
2114+
State: corev1.ContainerState{
2115+
Terminated: &corev1.ContainerStateTerminated{
2116+
Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
2117+
},
21082118
},
2109-
},
2110-
}},
2119+
}},
2120+
},
21112121
},
21122122
taskRunStatus: &v1beta1.TaskRunStatus{
21132123
Status: duckv1beta1.Status{Conditions: []apis.Condition{{
@@ -2119,7 +2129,7 @@ func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
21192129
}} {
21202130
t.Run(c.desc, func(t *testing.T) {
21212131
names.TestingSeed()
2122-
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.podStatus); err == nil {
2132+
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.pod); err == nil {
21232133
t.Error("Expected error, got nil")
21242134
}
21252135
if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" {

0 commit comments

Comments
 (0)