fix(collector): remove prometheus annotations on disablePrometheusAnnotations update

.chloggen/fix-disable-prometheus-annotations-removal.yaml

@@ -0,0 +1,24 @@
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: bug_fix
+
+# The name of the component, or a single word describing the area of concern, (e.g. collector, target allocator, auto-instrumentation, opamp, github action)
+component: collector
+
+# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Honor `spec.observability.metrics.disablePrometheusAnnotations: true` on update by removing existing prometheus.io annotations from the pod template, not just stopping new ones from being added.
+
+# One or more tracking issues related to the change
+issues: [5043]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext: |
+  The pod-template mutate path preserves any annotation that exists on the
+  current resource but is absent from the desired render, so toggling
+  disablePrometheusAnnotations from false to true on an existing
+  OpenTelemetryCollector previously left the prometheus.io/scrape, port, and
+  path annotations stuck on the rolled pods. The mutate path now explicitly
+  strips a small fixed set of operator-managed keys (currently the
+  prometheus.io annotations) when they are absent from the desired pod
+  template, while continuing to preserve all other externally-set annotations.

internal/manifests/mutate.go

@@ -386,11 +386,40 @@ func mutateIssuer(existing, desired *cmv1.Issuer) {
 	existing.Spec = desired.Spec
 }
 
+// operatorManagedPodAnnotationKeys is the set of pod-template annotation keys
+// whose presence is exclusively governed by the operator. When mutating an
+// existing pod template, any key in this set that is absent from the desired
+// pod template is stripped from the existing pod template before the
+// preserve-external-annotations merge runs. This lets operator-managed feature
+// toggles (e.g. spec.observability.metrics.disablePrometheusAnnotations) take
+// effect on already-running collectors without requiring a manual edit of the
+// rendered Deployment, StatefulSet, or DaemonSet.
+//
+// Keys that are NOT in this set are preserved if previously set, matching the
+// long-standing "external annotations on the pod template should survive a
+// reconcile" contract enforced by the Ingress/Service mutate tests.
+var operatorManagedPodAnnotationKeys = []string{
+	"prometheus.io/scrape",
+	"prometheus.io/port",
+	"prometheus.io/path",
+}
+
 func mutatePodTemplate(existing, desired *corev1.PodTemplateSpec) error {
 	if err := mergeWithOverride(&existing.Labels, desired.Labels); err != nil {
 		return err
 	}
 
+	// Strip operator-managed annotation keys from existing when they're absent
+	// from desired so that disabling the feature actually removes them. See the
+	// comment on operatorManagedPodAnnotationKeys for the rationale.
+	if existing.Annotations != nil {
+		for _, key := range operatorManagedPodAnnotationKeys {
+			if _, inDesired := desired.Annotations[key]; !inDesired {
+				delete(existing.Annotations, key)
+			}
+		}
+	}
+
 	if err := mergeWithOverride(&existing.Annotations, desired.Annotations); err != nil {
 		return err
 	}

internal/manifests/mutate_test.go

@@ -2625,3 +2625,117 @@ func TestGetMutateFunc_MutateNetworkPolicy(t *testing.T) {
 	require.Exactly(t, got.Annotations, want.Annotations)
 	require.Exactly(t, got.Spec, want.Spec)
 }
+
+// TestMutatePodTemplateStripsOperatorManagedAnnotations exercises the disable-after-create
+// scenario described in https://github.com/open-telemetry/opentelemetry-operator/issues/5043.
+// When the operator re-renders a Deployment with prometheus annotations disabled, the
+// existing prometheus.io/* annotations on the pod template must be removed; merely omitting
+// them from the desired pod template was not enough because the surrounding merge preserves
+// any key only present on the existing side.
+func TestMutatePodTemplateStripsOperatorManagedAnnotations(t *testing.T) {
+	const (
+		userKept       = "ops/internal-bookkeeping"
+		userValue      = "should-be-preserved-across-reconciles"
+		nonProm        = "non.prom/key-with-prefix-but-not-prom"
+		nonPromValue   = "should-also-be-preserved"
+		sha256Key      = "opentelemetry-operator-config/sha256"
+		oldHash        = "old-hash"
+		newHash        = "new-hash"
+	)
+
+	tests := []struct {
+		name              string
+		existingTemplate  corev1.PodTemplateSpec
+		desiredTemplate   corev1.PodTemplateSpec
+		expectAnnotations map[string]string
+	}{
+		{
+			name: "disable removes prometheus annotations but keeps user keys",
+			existingTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"prometheus.io/scrape": "true",
+						"prometheus.io/port":   "8888",
+						"prometheus.io/path":   "/metrics",
+						sha256Key:              oldHash,
+						userKept:               userValue,
+						nonProm:                nonPromValue,
+					},
+				},
+			},
+			desiredTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						sha256Key: newHash,
+					},
+				},
+			},
+			expectAnnotations: map[string]string{
+				sha256Key:    newHash,
+				userKept:     userValue,
+				nonProm:      nonPromValue,
+			},
+		},
+		{
+			name: "enable keeps prometheus annotations when desired sets them",
+			existingTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						sha256Key: oldHash,
+						userKept:  userValue,
+					},
+				},
+			},
+			desiredTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						"prometheus.io/scrape": "true",
+						"prometheus.io/port":   "8888",
+						"prometheus.io/path":   "/metrics",
+						sha256Key:              newHash,
+					},
+				},
+			},
+			expectAnnotations: map[string]string{
+				"prometheus.io/scrape": "true",
+				"prometheus.io/port":   "8888",
+				"prometheus.io/path":   "/metrics",
+				sha256Key:              newHash,
+				userKept:               userValue,
+			},
+		},
+		{
+			name: "no operator-managed keys on either side leaves user annotations intact",
+			existingTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						userKept: userValue,
+						nonProm:  nonPromValue,
+					},
+				},
+			},
+			desiredTemplate: corev1.PodTemplateSpec{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						sha256Key: newHash,
+					},
+				},
+			},
+			expectAnnotations: map[string]string{
+				userKept:  userValue,
+				nonProm:   nonPromValue,
+				sha256Key: newHash,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			existing := tt.existingTemplate.DeepCopy()
+			desired := tt.desiredTemplate.DeepCopy()
+			err := mutatePodTemplate(existing, desired)
+			require.NoError(t, err)
+			require.Exactly(t, tt.expectAnnotations, existing.Annotations)
+		})
+	}
+}