Use kmap.Union to merge two maps

Previously, we use self-defined helper function to merge two maps. Now, we use the existing [kmap.Union](https://pkg.go.dev/knative.dev/[email protected]/kmap#Union) method to achieve this functionality. Signed-off-by: Chuang Wang <[email protected]>
chuangw6 · Oct 19, 2022 · 2ed31e9 · 2ed31e9
1 parent b63c1e2
commit 2ed31e9
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 34 deletions.
diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -64,6 +64,7 @@ import (
 	"k8s.io/utils/clock"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/controller"
+	"knative.dev/pkg/kmap"
 	"knative.dev/pkg/kmeta"
 	"knative.dev/pkg/logging"
 	pkgreconciler "knative.dev/pkg/reconciler"
@@ -1245,26 +1246,13 @@ func (c *Reconciler) updateLabelsAndAnnotations(ctx context.Context, pr *v1beta1
 		// to deal with Patch (setting resourceVersion, and optimistic concurrency checks).
 		newPr = newPr.DeepCopy()
 		// Properly merge labels and annotations, as the labels *might* have changed during the reconciliation
-		newPr.Labels = merge(newPr.Labels, pr.Labels)
-		newPr.Annotations = merge(newPr.Annotations, pr.Annotations)
+		newPr.Labels = kmap.Union(newPr.Labels, pr.Labels)
+		newPr.Annotations = kmap.Union(newPr.Annotations, pr.Annotations)
 		return c.PipelineClientSet.TektonV1beta1().PipelineRuns(pr.Namespace).Update(ctx, newPr, metav1.UpdateOptions{})
 	}
 	return newPr, nil
 }
 
-func merge(new, old map[string]string) map[string]string {
-	if new == nil {
-		new = map[string]string{}
-	}
-	if old == nil {
-		return new
-	}
-	for k, v := range old {
-		new[k] = v
-	}
-	return new
-}
-
 func storePipelineSpecAndMergeMeta(pr *v1beta1.PipelineRun, ps *v1beta1.PipelineSpec, meta *metav1.ObjectMeta) error {
 	// Only store the PipelineSpec once, if it has never been set before.
 	if pr.Status.PipelineSpec == nil {

diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -5576,8 +5576,8 @@ metadata:
 func Test_storePipelineSpec_metadata(t *testing.T) {
 	pipelinerunlabels := map[string]string{"lbl1": "value1", "lbl2": "value2"}
 	pipelinerunannotations := map[string]string{"io.annotation.1": "value1", "io.annotation.2": "value2"}
-	pipelinelabels := map[string]string{"lbl1": "another value", "lbl3": "value3"}
-	pipelineannotations := map[string]string{"io.annotation.1": "another value", "io.annotation.3": "value3"}
+	pipelinelabels := map[string]string{"lbl1": "another value", "lbl2": "another value", "lbl3": "value3"}
+	pipelineannotations := map[string]string{"io.annotation.1": "another value", "io.annotation.2": "another value", "io.annotation.3": "value3"}
 	wantedlabels := map[string]string{"lbl1": "value1", "lbl2": "value2", "lbl3": "value3", pipeline.PipelineLabelKey: "bar"}
 	wantedannotations := map[string]string{"io.annotation.1": "value1", "io.annotation.2": "value2", "io.annotation.3": "value3"}
 

diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go
@@ -59,6 +59,7 @@ import (
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/changeset"
 	"knative.dev/pkg/controller"
+	"knative.dev/pkg/kmap"
 	"knative.dev/pkg/kmeta"
 	"knative.dev/pkg/logging"
 	pkgreconciler "knative.dev/pkg/reconciler"
@@ -599,26 +600,13 @@ func (c *Reconciler) updateLabelsAndAnnotations(ctx context.Context, tr *v1beta1
 		// If we want to switch this to Patch, then we will need to teach the utilities in test/controller.go
 		// to deal with Patch (setting resourceVersion, and optimistic concurrency checks).
 		newTr = newTr.DeepCopy()
-		newTr.Labels = merge(newTr.Labels, tr.Labels)
-		newTr.Annotations = merge(newTr.Annotations, tr.Annotations)
+		newTr.Labels = kmap.Union(newTr.Labels, tr.Labels)
+		newTr.Annotations = kmap.Union(newTr.Annotations, tr.Annotations)
 		return c.PipelineClientSet.TektonV1beta1().TaskRuns(tr.Namespace).Update(ctx, newTr, metav1.UpdateOptions{})
 	}
 	return newTr, nil
 }
 
-func merge(new, old map[string]string) map[string]string {
-	if new == nil {
-		new = map[string]string{}
-	}
-	if old == nil {
-		return new
-	}
-	for k, v := range old {
-		new[k] = v
-	}
-	return new
-}
-
 func (c *Reconciler) handlePodCreationError(tr *v1beta1.TaskRun, err error) error {
 	switch {
 	case isResourceQuotaConflictError(err):

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -4103,8 +4103,8 @@ spec:
 func Test_storeTaskSpec_metadata(t *testing.T) {
 	taskrunlabels := map[string]string{"lbl1": "value1", "lbl2": "value2"}
 	taskrunannotations := map[string]string{"io.annotation.1": "value1", "io.annotation.2": "value2"}
-	tasklabels := map[string]string{"lbl1": "another value", "lbl3": "value3"}
-	taskannotations := map[string]string{"io.annotation.1": "another value", "io.annotation.3": "value3"}
+	tasklabels := map[string]string{"lbl1": "another value", "lbl2": "another value", "lbl3": "value3"}
+	taskannotations := map[string]string{"io.annotation.1": "another value", "io.annotation.2": "another value", "io.annotation.3": "value3"}
 	wantedlabels := map[string]string{"lbl1": "value1", "lbl2": "value2", "lbl3": "value3"}
 	wantedannotations := map[string]string{"io.annotation.1": "value1", "io.annotation.2": "value2", "io.annotation.3": "value3"}