diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go index 66a9314584..9d87bdb24c 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go @@ -113,14 +113,26 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, saName strin pullPolicy = v1.PullAlways } + // make a copy of the labels and annotations to avoid mutating the input parameters + podLabels := make(map[string]string) + podAnnotations := make(map[string]string) + + for key, value := range labels { + podLabels[key] = value + } + + for key, value := range annotations { + podAnnotations[key] = value + } + readOnlyRootFilesystem := false pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ GenerateName: source.GetName() + "-", Namespace: source.GetNamespace(), - Labels: labels, - Annotations: annotations, + Labels: podLabels, + Annotations: podAnnotations, }, Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -207,25 +219,17 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, saName strin } // Set priorityclass if its annotation exists - if prio, ok := annotations[CatalogPriorityClassKey]; ok && prio != "" { + if prio, ok := podAnnotations[CatalogPriorityClassKey]; ok && prio != "" { pod.Spec.PriorityClassName = prio } // Add PodSpec hash // This hash info will be used to detect PodSpec changes - if labels == nil { - labels = make(map[string]string) - } - labels[PodHashLabelKey] = hashPodSpec(pod.Spec) - pod.SetLabels(labels) + podLabels[PodHashLabelKey] = hashPodSpec(pod.Spec) // add eviction annotation to enable the cluster autoscaler to evict the pod in order to drain the node // since catalog pods are not backed by a controller, they cannot be evicted by default - if annotations == nil { - annotations = make(map[string]string) - } - annotations[ClusterAutoscalingAnnotationKey] = "true" - pod.SetAnnotations(annotations) + podAnnotations[ClusterAutoscalingAnnotationKey] = "true" return pod } diff --git a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler_test.go b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler_test.go index f73b6f3762..ad9c82c6de 100644 --- a/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler_test.go +++ b/staging/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler_test.go @@ -95,6 +95,37 @@ func TestPodContainerSecurityContext(t *testing.T) { require.Equal(t, expectedContainerSecCtx, gotContainerSecCtx) } +// TestPodAvoidsConcurrentWrite is a regression test for +// https://bugzilla.redhat.com/show_bug.cgi?id=2101357 +// we were mutating the input annotations and labels parameters causing +// concurrent write issues +func TestPodAvoidsConcurrentWrite(t *testing.T) { + catsrc := &v1alpha1.CatalogSource{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testns", + }, + } + + labels := map[string]string{ + "label": "something", + } + + annotations := map[string]string{ + "annotation": "somethingelse", + } + + gotPod := Pod(catsrc, "hello", "busybox", "", labels, annotations, int32(0), int32(0)) + + // check labels and annotations point to different addresses between parameters and what's in the pod + require.NotEqual(t, &labels, &gotPod.Labels) + require.NotEqual(t, &annotations, &gotPod.Annotations) + + // check that labels and annotations from the parameters were copied down to the pod's + require.Equal(t, labels["label"], gotPod.Labels["label"]) + require.Equal(t, annotations["annotation"], gotPod.Annotations["annotation"]) +} + func TestPodSchedulingOverrides(t *testing.T) { // This test ensures that any overriding pod scheduling configuration elements // defined in spec.grpcPodConfig are applied to the catalog source pod created diff --git a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go index 66a9314584..9d87bdb24c 100644 --- a/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go +++ b/vendor/github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler/reconciler.go @@ -113,14 +113,26 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, saName strin pullPolicy = v1.PullAlways } + // make a copy of the labels and annotations to avoid mutating the input parameters + podLabels := make(map[string]string) + podAnnotations := make(map[string]string) + + for key, value := range labels { + podLabels[key] = value + } + + for key, value := range annotations { + podAnnotations[key] = value + } + readOnlyRootFilesystem := false pod := &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ GenerateName: source.GetName() + "-", Namespace: source.GetNamespace(), - Labels: labels, - Annotations: annotations, + Labels: podLabels, + Annotations: podAnnotations, }, Spec: v1.PodSpec{ Containers: []v1.Container{ @@ -207,25 +219,17 @@ func Pod(source *v1alpha1.CatalogSource, name string, image string, saName strin } // Set priorityclass if its annotation exists - if prio, ok := annotations[CatalogPriorityClassKey]; ok && prio != "" { + if prio, ok := podAnnotations[CatalogPriorityClassKey]; ok && prio != "" { pod.Spec.PriorityClassName = prio } // Add PodSpec hash // This hash info will be used to detect PodSpec changes - if labels == nil { - labels = make(map[string]string) - } - labels[PodHashLabelKey] = hashPodSpec(pod.Spec) - pod.SetLabels(labels) + podLabels[PodHashLabelKey] = hashPodSpec(pod.Spec) // add eviction annotation to enable the cluster autoscaler to evict the pod in order to drain the node // since catalog pods are not backed by a controller, they cannot be evicted by default - if annotations == nil { - annotations = make(map[string]string) - } - annotations[ClusterAutoscalingAnnotationKey] = "true" - pod.SetAnnotations(annotations) + podAnnotations[ClusterAutoscalingAnnotationKey] = "true" return pod }