Skip to content

Commit

Permalink
Prior to this PR, the propagatePipelineNameLabelToPipelineRun
Browse files Browse the repository at this point in the history
function will continuously set the pipeline name label during
each `reconcile` process. This may override the results by the
`storePipelineSpecAndMergeMeta` function. This may cause some
remote resource names to not be set correctly.

This commit, when the pipeline name label has been set, the next
`reconcile` will not be reset again.
  • Loading branch information
cugykw authored and tekton-robot committed May 14, 2024
1 parent 5c40712 commit 8a938fc
Show file tree
Hide file tree
Showing 2 changed files with 134 additions and 2 deletions.
9 changes: 8 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -1308,6 +1308,11 @@ func propagatePipelineNameLabelToPipelineRun(pr *v1.PipelineRun) error {
if pr.ObjectMeta.Labels == nil {
pr.ObjectMeta.Labels = make(map[string]string)
}

if _, ok := pr.ObjectMeta.Labels[pipeline.PipelineLabelKey]; ok {
return nil
}

switch {
case pr.Spec.PipelineRef != nil && pr.Spec.PipelineRef.Name != "":
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = pr.Spec.PipelineRef.Name
Expand Down Expand Up @@ -1426,7 +1431,9 @@ func storePipelineSpecAndMergeMeta(ctx context.Context, pr *v1.PipelineRun, ps *

// Propagate labels from Pipeline to PipelineRun. PipelineRun labels take precedences over Pipeline.
pr.ObjectMeta.Labels = kmap.Union(meta.Labels, pr.ObjectMeta.Labels)
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = meta.Name
if len(meta.Name) > 0 {
pr.ObjectMeta.Labels[pipeline.PipelineLabelKey] = meta.Name
}

// Propagate annotations from Pipeline to PipelineRun. PipelineRun annotations take precedences over Pipeline.
pr.ObjectMeta.Annotations = kmap.Union(kmap.ExcludeKeys(meta.Annotations, tknreconciler.KubectlLastAppliedAnnotationKey), pr.ObjectMeta.Annotations)
Expand Down
127 changes: 126 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3689,7 +3689,6 @@ metadata:
PipelineRunAnnotation: PipelineRunValue
labels:
PipelineRunLabel: PipelineRunValue
tekton.dev/pipeline: WillNotBeUsed
name: test-pipeline-run-with-labels
namespace: foo
spec:
Expand Down Expand Up @@ -8485,6 +8484,132 @@ spec:
}
}

func TestReconcile_RemotePipeline_PipelineNameLabel(t *testing.T) {
names.TestingSeed()

namespace := "foo"
prName := "test-pipeline-run-success"
trName := "test-pipeline-run-success-unit-test-1"

prs := []*v1.PipelineRun{parse.MustParseV1PipelineRun(t, `
metadata:
name: test-pipeline-run-success
namespace: foo
spec:
pipelineRef:
resolver: bar
taskRunTemplate:
serviceAccountName: test-sa
timeout: 1h0m0s
`)}
ps := parse.MustParseV1Pipeline(t, `
metadata:
name: test-pipeline
namespace: foo
spec:
tasks:
- name: unit-test-1
taskRef:
resolver: bar
`)
notNamePipeline := parse.MustParseV1Pipeline(t, `
metadata:
namespace: foo
spec:
tasks:
- name: unit-test-1
taskRef:
resolver: bar
`)

remoteTask := parse.MustParseV1Task(t, `
metadata:
name: unit-test-task
namespace: foo
`)

pipelineBytes, err := yaml.Marshal(ps)
if err != nil {
t.Fatal("fail to marshal pipeline", err)
}
notNamePipelineBytes, err := yaml.Marshal(notNamePipeline)
if err != nil {
t.Fatal("fail to marshal pipeline", err)
}

taskBytes, err := yaml.Marshal(remoteTask)
if err != nil {
t.Fatal("fail to marshal task", err)
}

pipelineReq := getResolvedResolutionRequest(t, "bar", pipelineBytes, "foo", prName)
notNamePipelineReq := getResolvedResolutionRequest(t, "bar", notNamePipelineBytes, "foo", prName)
taskReq := getResolvedResolutionRequest(t, "bar", taskBytes, "foo", trName)

tcs := []struct {
name string
wantPipelineName string
pipelineReq resolutionv1beta1.ResolutionRequest
taskReq resolutionv1beta1.ResolutionRequest
}{{
name: "remote pipeline contains name",
// Use the name from the remote pipeline
wantPipelineName: ps.Name,
pipelineReq: pipelineReq,
taskReq: taskReq,
}, {
name: "remote pipeline without name",
wantPipelineName: prs[0].Name,
pipelineReq: notNamePipelineReq,
taskReq: taskReq,
}}

for _, tc := range tcs {
// Unlike the tests above, we do *not* locally define our pipeline or unit-test task.
d := test.Data{
PipelineRuns: prs,
ServiceAccounts: []*corev1.ServiceAccount{{
ObjectMeta: metav1.ObjectMeta{Name: prs[0].Spec.TaskRunTemplate.ServiceAccountName, Namespace: namespace},
}},
ConfigMaps: []*corev1.ConfigMap{
{
ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()},
Data: map[string]string{
"enable-api-fields": "beta",
},
},
},
ResolutionRequests: []*resolutionv1beta1.ResolutionRequest{&tc.taskReq, &tc.pipelineReq},
}

prt := newPipelineRunTest(t, d)
defer prt.Cancel()

wantEvents := []string{
"Normal Started",
"Normal Running Tasks Completed: 0",
}
reconciledRun, _ := prt.reconcileRun(namespace, prName, wantEvents, false)
if len(reconciledRun.Labels) == 0 {
t.Errorf("the pipeline label in pr is not set")
}
pName := reconciledRun.Labels[pipeline.PipelineLabelKey]
if reconciledRun.Labels[pipeline.PipelineLabelKey] != tc.wantPipelineName {
t.Errorf("want pipeline name %s, but got %s", tc.wantPipelineName, pName)
}

// Verify the pipeline name label after the second `reconcile`, to prevent it from being overwritten again.
reconciledRun, _ = prt.reconcileRun(namespace, prName, wantEvents, false)
if len(reconciledRun.Labels) == 0 {
t.Errorf("the pipeline label in pr is not set")
}
pName = reconciledRun.Labels[pipeline.PipelineLabelKey]
if reconciledRun.Labels[pipeline.PipelineLabelKey] != tc.wantPipelineName {
t.Errorf("want pipeline name %s, but got %s", tc.wantPipelineName, pName)
}
}
}

// TestReconcile_OptionalWorkspacesOmitted checks that an optional workspace declared by
// a Task and a Pipeline can be omitted by a PipelineRun and the run will still start
// successfully without an error.
Expand Down

0 comments on commit 8a938fc

Please sign in to comment.