Copying of labels/annotations from Task to TaskRun is unreliable

[stuartwdouglas]

Expected Behavior

The Task labels should be copied to the TaskRun

Actual Behavior

If there is a conflict (which is not uncommon when there are multiple controllers such as tekton-results, tekton-chains etc all reconciling on TaskRuns) Tekton does not retry, it just logs a warning and the labels are missing. This breaks multi-platform builds in RHTAP which use labels to identify multi platform tasks.

Steps to Reproduce the Problem

It's hard to reproduce consistently, but the code is here: https://github.com/tektoncd/pipeline/blob/7069889e796b0b12e12a3d1095a5f1a3d98c2739/pkg/reconciler/taskrun/taskrun.go#L304

You can see that is just logs a warning instead of returning an error to retry. Note that I am not familiar with this code so I am not 100% sure if the obvious solution of just returning the error is the correct one.

[vdemeester]

@stuartwdouglas it does return an error afterward, few lines later https://github.com/tektoncd/pipeline/blob/7069889e796b0b12e12a3d1095a5f1a3d98c2739/pkg/reconciler/taskrun/taskrun.go#L308.

The issue is usually different than this (but same results). Any controller that watches and mutate the labels of a TaskRun or PipelineRun can overwrite what another did. This is a very similar issue than https://github.com/tektoncd/pipeline/issues/5297 (that was fixed by https://github.com/tektoncd/pipeline/pull/5597/files that makes the race less likely but still possible).

[stuartwdouglas]

Ah, I missed that the error is returned, but I still think that it is unlikely to be a coincidence that the labels failed to copy on a task that had that log message. Looking through the log that message is not very common, so it seems very likely to be related.

That said if the copying is going away then I am going to need to come up with another solution anyway, so I am happy for this to be closed.