Skip to content

Commit 0a93b90

Browse files
committed
fix(pod): order TaskRun status steps to match pod spec containers
The `TaskRun.Status.Steps` list was not guaranteed to reflect the true execution order when StepActions were involved. Steps populated earlier (from StepAction resolution) were left at the front, with inline steps appended later, causing a mismatch with the pod container order and confusing dashboards. This change fixes the issue by creating a temporary slice aligned with the (already sorted) pod step container sequence and then replace `trs.Steps` in one shot. We still preserve existing `Provenance` for matching steps by name.
1 parent 9b49de2 commit 0a93b90

File tree

2 files changed

+151
-17
lines changed

2 files changed

+151
-17
lines changed

pkg/pod/status.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,15 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
268268
}
269269
}
270270

271+
// Build a lookup map for step state provenances.
272+
stepStateProvenances := make(map[string]*v1.Provenance)
273+
for _, ss := range trs.Steps {
274+
stepStateProvenances[ss.Name] = ss.Provenance
275+
}
276+
271277
// Continue with extraction of termination messages
272-
for _, s := range stepStatuses {
278+
orderedStepStates := make([]v1.StepState, len(stepStatuses))
279+
for i, s := range stepStatuses {
273280
// Avoid changing the original value by modifying the pointer value.
274281
state := s.State.DeepCopy()
275282
taskRunStepResults := []v1.TaskRunStepResult{}
@@ -378,18 +385,13 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL
378385
Inputs: sas.Inputs,
379386
Outputs: sas.Outputs,
380387
}
381-
foundStep := false
382-
for i, ss := range trs.Steps {
383-
if ss.Name == stepState.Name {
384-
stepState.Provenance = ss.Provenance
385-
trs.Steps[i] = stepState
386-
foundStep = true
387-
break
388-
}
389-
}
390-
if !foundStep {
391-
trs.Steps = append(trs.Steps, stepState)
388+
if stepStateProvenance, exist := stepStateProvenances[stepState.Name]; exist {
389+
stepState.Provenance = stepStateProvenance
392390
}
391+
orderedStepStates[i] = stepState
392+
}
393+
if len(orderedStepStates) > 0 {
394+
trs.Steps = orderedStepStates
393395
}
394396

395397
return errors.Join(errs...)

pkg/pod/status_test.go

Lines changed: 137 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -814,14 +814,14 @@ func TestMakeTaskRunStatus_StepArtifacts(t *testing.T) {
814814

815815
func TestMakeTaskRunStatus(t *testing.T) {
816816
for _, c := range []struct {
817-
desc string
818-
podStatus corev1.PodStatus
819-
pod corev1.Pod
820-
want v1.TaskRunStatus
817+
desc string
818+
podStatus corev1.PodStatus
819+
pod corev1.Pod
820+
stepStates []v1.StepState
821+
want v1.TaskRunStatus
821822
}{{
822823
desc: "empty",
823824
podStatus: corev1.PodStatus{},
824-
825825
want: v1.TaskRunStatus{
826826
Status: statusRunning(),
827827
TaskRunStatusFields: v1.TaskRunStatusFields{
@@ -1741,6 +1741,137 @@ func TestMakeTaskRunStatus(t *testing.T) {
17411741
CompletionTime: &metav1.Time{Time: time.Now()},
17421742
},
17431743
},
1744+
}, {
1745+
desc: "TaskRun status steps ordering based on pod spec containers",
1746+
pod: corev1.Pod{
1747+
ObjectMeta: metav1.ObjectMeta{
1748+
Name: "pod",
1749+
},
1750+
Spec: corev1.PodSpec{
1751+
Containers: []corev1.Container{{
1752+
Name: "step-first-inline",
1753+
}, {
1754+
Name: "step-second-remote",
1755+
}, {
1756+
Name: "step-third-inline",
1757+
}, {
1758+
Name: "step--inline",
1759+
}, {
1760+
Name: "step-fourth-remote",
1761+
}, {
1762+
Name: "step-fifth-remote",
1763+
}},
1764+
},
1765+
Status: corev1.PodStatus{
1766+
Phase: corev1.PodSucceeded,
1767+
ContainerStatuses: []corev1.ContainerStatus{{
1768+
Name: "step-second-remote",
1769+
State: corev1.ContainerState{
1770+
Terminated: &corev1.ContainerStateTerminated{},
1771+
},
1772+
}, {
1773+
Name: "step-fourth-remote",
1774+
State: corev1.ContainerState{
1775+
Terminated: &corev1.ContainerStateTerminated{},
1776+
},
1777+
}, {
1778+
Name: "step--inline",
1779+
State: corev1.ContainerState{
1780+
Terminated: &corev1.ContainerStateTerminated{},
1781+
},
1782+
}, {
1783+
Name: "step-first-inline",
1784+
State: corev1.ContainerState{
1785+
Terminated: &corev1.ContainerStateTerminated{},
1786+
},
1787+
}, {
1788+
Name: "step-fifth-remote",
1789+
State: corev1.ContainerState{
1790+
Terminated: &corev1.ContainerStateTerminated{},
1791+
},
1792+
}, {
1793+
Name: "step-third-inline",
1794+
State: corev1.ContainerState{
1795+
Terminated: &corev1.ContainerStateTerminated{},
1796+
},
1797+
}},
1798+
},
1799+
},
1800+
stepStates: []v1.StepState{
1801+
{
1802+
Name: "second-remote",
1803+
Provenance: &v1.Provenance{
1804+
RefSource: &v1.RefSource{
1805+
URI: "test-uri",
1806+
Digest: map[string]string{"sha256": "digest"},
1807+
},
1808+
},
1809+
},
1810+
{
1811+
Name: "fourth-remote",
1812+
},
1813+
{
1814+
Name: "fifth-remote",
1815+
Provenance: &v1.Provenance{
1816+
RefSource: nil,
1817+
},
1818+
},
1819+
},
1820+
want: v1.TaskRunStatus{
1821+
Status: statusSuccess(),
1822+
TaskRunStatusFields: v1.TaskRunStatusFields{
1823+
Steps: []v1.StepState{{
1824+
ContainerState: corev1.ContainerState{
1825+
Terminated: &corev1.ContainerStateTerminated{},
1826+
},
1827+
Name: "first-inline",
1828+
Container: "step-first-inline",
1829+
}, {
1830+
ContainerState: corev1.ContainerState{
1831+
Terminated: &corev1.ContainerStateTerminated{},
1832+
},
1833+
Name: "second-remote",
1834+
Container: "step-second-remote",
1835+
Provenance: &v1.Provenance{
1836+
RefSource: &v1.RefSource{
1837+
URI: "test-uri",
1838+
Digest: map[string]string{"sha256": "digest"},
1839+
},
1840+
},
1841+
}, {
1842+
ContainerState: corev1.ContainerState{
1843+
Terminated: &corev1.ContainerStateTerminated{},
1844+
},
1845+
Name: "third-inline",
1846+
Container: "step-third-inline",
1847+
}, {
1848+
ContainerState: corev1.ContainerState{
1849+
Terminated: &corev1.ContainerStateTerminated{},
1850+
},
1851+
Name: "-inline",
1852+
Container: "step--inline",
1853+
}, {
1854+
ContainerState: corev1.ContainerState{
1855+
Terminated: &corev1.ContainerStateTerminated{},
1856+
},
1857+
Name: "fourth-remote",
1858+
Container: "step-fourth-remote",
1859+
}, {
1860+
ContainerState: corev1.ContainerState{
1861+
Terminated: &corev1.ContainerStateTerminated{},
1862+
},
1863+
Name: "fifth-remote",
1864+
Container: "step-fifth-remote",
1865+
Provenance: &v1.Provenance{
1866+
RefSource: nil,
1867+
},
1868+
}},
1869+
Sidecars: []v1.SidecarState{},
1870+
Artifacts: &v1.Artifacts{},
1871+
// We don't actually care about the time, just that it's not nil
1872+
CompletionTime: &metav1.Time{Time: time.Now()},
1873+
},
1874+
},
17441875
}, {
17451876
desc: "include non zero exit code in a container termination message if entrypoint is set to ignore the error",
17461877
pod: corev1.Pod{
@@ -1964,6 +2095,7 @@ func TestMakeTaskRunStatus(t *testing.T) {
19642095
Status: v1.TaskRunStatus{
19652096
TaskRunStatusFields: v1.TaskRunStatusFields{
19662097
StartTime: &metav1.Time{Time: startTime},
2098+
Steps: c.stepStates,
19672099
},
19682100
},
19692101
}

0 commit comments

Comments
 (0)