Skip to content

Sort and remove duplicate when set "taskrun.status.taskResults" #2467

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 28, 2020
Merged

Sort and remove duplicate when set "taskrun.status.taskResults" #2467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions pkg/pod/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,8 @@ func areStepsComplete(pod *corev1.Pod) bool {
return stepsComplete
}

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

func getFailureMessage(pod *corev1.Pod) string {
sortContainerStatuses(pod)
SortContainerStatuses(pod)
// First, try to surface an error about the actual build step that failed.
for _, status := range pod.Status.ContainerStatuses {
term := status.State.Terminated
Expand Down
2 changes: 1 addition & 1 deletion pkg/pod/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ func TestSortContainerStatuses(t *testing.T) {
},
},
}
sortContainerStatuses(&samplePod)
SortContainerStatuses(&samplePod)
var gotNames []string
for _, status := range samplePod.Status.ContainerStatuses {
gotNames = append(gotNames, status.Name)
Expand Down
24 changes: 21 additions & 3 deletions pkg/reconciler/taskrun/taskrun.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func (c *Reconciler) reconcile(ctx context.Context, tr *v1beta1.TaskRun,
// Convert the Pod's status to the equivalent TaskRun Status.
tr.Status = podconvert.MakeTaskRunStatus(c.Logger, *tr, pod, *taskSpec)

if err := updateTaskRunResourceResult(tr, pod.Status); err != nil {
if err := updateTaskRunResourceResult(tr, *pod); err != nil {
return err
}

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

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

func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodStatus) error {
func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, pod corev1.Pod) error {
podconvert.SortContainerStatuses(&pod)

if taskRun.IsSuccessful() {
for idx, cs := range podStatus.ContainerStatuses {
for idx, cs := range pod.Status.ContainerStatuses {
if cs.State.Terminated != nil {
msg := cs.State.Terminated.Message
r, err := termination.ParseMessage(msg)
Expand All @@ -617,6 +619,7 @@ func updateTaskRunResourceResult(taskRun *v1beta1.TaskRun, podStatus corev1.PodS
taskRun.Status.ResourcesResult = append(taskRun.Status.ResourcesResult, pipelineResourceResults...)
}
}
taskRun.Status.TaskRunResults = removeDuplicateResults(taskRun.Status.TaskRunResults)
}
return nil
}
Expand All @@ -641,6 +644,21 @@ func getResults(results []v1beta1.PipelineResourceResult) ([]v1beta1.TaskRunResu
return taskResults, pipelineResourceResults
}

func removeDuplicateResults(taskRunResult []v1beta1.TaskRunResult) []v1beta1.TaskRunResult {
uniq := make([]v1beta1.TaskRunResult, 0)
latest := make(map[string]v1beta1.TaskRunResult, 0)
for _, res := range taskRunResult {
if _, seen := latest[res.Name]; !seen {
uniq = append(uniq, res)
}
latest[res.Name] = res
}
for i, res := range uniq {
uniq[i] = latest[res.Name]
}
return uniq
}

func isExceededResourceQuotaError(err error) bool {
return err != nil && k8serrors.IsForbidden(err) && strings.Contains(err.Error(), "exceeded quota")
}
Expand Down
108 changes: 62 additions & 46 deletions pkg/reconciler/taskrun/taskrun_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1877,19 +1877,21 @@ func TestReconcileCloudEvents(t *testing.T) {
func TestUpdateTaskRunResourceResult(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
pod corev1.Pod
taskRunStatus *v1beta1.TaskRunStatus
want []resourcev1alpha1.PipelineResourceResult
}{{
desc: "image resource updated",
podStatus: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
pod: corev1.Pod{
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
},
},
},
}},
}},
},
},
want: []resourcev1alpha1.PipelineResourceResult{{
Key: "digest",
Expand All @@ -1904,7 +1906,7 @@ func TestUpdateTaskRunResourceResult(t *testing.T) {
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
})
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
t.Errorf("updateTaskRunResourceResult: %s", err)
}
if d := cmp.Diff(c.want, tr.Status.ResourcesResult); d != "" {
Expand All @@ -1917,20 +1919,22 @@ func TestUpdateTaskRunResourceResult(t *testing.T) {
func TestUpdateTaskRunResult(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
pod corev1.Pod
taskRunStatus *v1beta1.TaskRunStatus
wantResults []v1beta1.TaskRunResult
want []resourcev1alpha1.PipelineResourceResult
}{{
desc: "test result with pipeline result",
podStatus: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`,
pod: corev1.Pod{
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}, "type": "PipelineResourceResult"}]`,
},
},
},
}},
}},
},
},
wantResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Expand All @@ -1950,7 +1954,7 @@ func TestUpdateTaskRunResult(t *testing.T) {
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
})
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
t.Errorf("updateTaskRunResourceResult: %s", err)
}
if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" {
Expand All @@ -1966,20 +1970,22 @@ func TestUpdateTaskRunResult(t *testing.T) {
func TestUpdateTaskRunResult2(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
pod corev1.Pod
taskRunStatus *v1beta1.TaskRunStatus
wantResults []v1beta1.TaskRunResult
want []resourcev1alpha1.PipelineResourceResult
}{{
desc: "test result with pipeline result - no result type",
podStatus: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
pod: corev1.Pod{
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultName","value":"resultValue", "type": "TaskRunResult"}, {"key":"digest","value":"sha256:1234","resourceRef":{"name":"source-image"}}]`,
},
},
},
}},
}},
},
},
wantResults: []v1beta1.TaskRunResult{{
Name: "resultName",
Expand All @@ -1998,7 +2004,7 @@ func TestUpdateTaskRunResult2(t *testing.T) {
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
})
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
t.Errorf("updateTaskRunResourceResult: %s", err)
}
if d := cmp.Diff(c.wantResults, tr.Status.TaskRunResults); d != "" {
Expand All @@ -2014,23 +2020,31 @@ func TestUpdateTaskRunResult2(t *testing.T) {
func TestUpdateTaskRunResultTwoResults(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
pod corev1.Pod
taskRunStatus *v1beta1.TaskRunStatus
want []v1beta1.TaskRunResult
}{{
desc: "two test results",
podStatus: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
pod: corev1.Pod{
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultNameOne","value":"resultValueOne", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
},
},
},
}},
}, {
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `[{"key":"resultNameOne","value":"resultValueThree", "type": "TaskRunResult"},{"key":"resultNameTwo","value":"resultValueTwo", "type": "TaskRunResult"}]`,
},
},
}},
},
},
want: []v1beta1.TaskRunResult{{
Name: "resultNameOne",
Value: "resultValueOne",
Value: "resultValueThree",
}, {
Name: "resultNameTwo",
Value: "resultValueTwo",
Expand All @@ -2043,7 +2057,7 @@ func TestUpdateTaskRunResultTwoResults(t *testing.T) {
Type: apis.ConditionSucceeded,
Status: corev1.ConditionTrue,
})
if err := updateTaskRunResourceResult(tr, c.podStatus); err != nil {
if err := updateTaskRunResourceResult(tr, c.pod); err != nil {
t.Errorf("updateTaskRunResourceResult: %s", err)
}
if d := cmp.Diff(c.want, tr.Status.TaskRunResults); d != "" {
Expand Down Expand Up @@ -2095,19 +2109,21 @@ func TestUpdateTaskRunResultWhenTaskFailed(t *testing.T) {
func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
for _, c := range []struct {
desc string
podStatus corev1.PodStatus
pod corev1.Pod
taskRunStatus *v1beta1.TaskRunStatus
want []resourcev1alpha1.PipelineResourceResult
}{{
desc: "image resource exporter with malformed json output",
podStatus: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
pod: corev1.Pod{
Status: corev1.PodStatus{
ContainerStatuses: []corev1.ContainerStatus{{
State: corev1.ContainerState{
Terminated: &corev1.ContainerStateTerminated{
Message: `MALFORMED JSON{"digest":"sha256:1234"}`,
},
},
},
}},
}},
},
},
taskRunStatus: &v1beta1.TaskRunStatus{
Status: duckv1beta1.Status{Conditions: []apis.Condition{{
Expand All @@ -2119,7 +2135,7 @@ func TestUpdateTaskRunResourceResult_Errors(t *testing.T) {
}} {
t.Run(c.desc, func(t *testing.T) {
names.TestingSeed()
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.podStatus); err == nil {
if err := updateTaskRunResourceResult(&v1beta1.TaskRun{Status: *c.taskRunStatus}, c.pod); err == nil {
t.Error("Expected error, got nil")
}
if d := cmp.Diff(c.want, c.taskRunStatus.ResourcesResult); d != "" {
Expand Down