Skip to content

Commit ef15cc9

Browse files
fix: Preserve user changes to deployment pod templates
Fixes OLM reverting user changes like kubectl rollout restart. OLM no longer stores pod template metadata, allowing user changes o annotations, labels, and other fields to persist. Generate-by: Cursor/Claude
1 parent 10c2841 commit ef15cc9

File tree

3 files changed

+365
-0
lines changed

3 files changed

+365
-0
lines changed

internal/operator-controller/applier/boxcutter.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"slices"
1313
"strings"
1414

15+
"github.com/go-logr/logr"
1516
"helm.sh/helm/v3/pkg/release"
1617
"helm.sh/helm/v3/pkg/storage/driver"
1718
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -153,8 +154,41 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
153154
return r.buildClusterExtensionRevision(objs, ext, revisionAnnotations), nil
154155
}
155156

157+
// removePodTemplateMetadata removes the metadata section from a Deployment's pod template.
158+
// This prevents OLM from claiming ownership of pod template metadata fields via Server-Side Apply.
159+
func removePodTemplateMetadata(unstr *unstructured.Unstructured, l logr.Logger) {
160+
if unstr.GetKind() != "Deployment" || unstr.GroupVersionKind().Group != "apps" {
161+
return
162+
}
163+
164+
obj := unstr.Object
165+
spec, ok := obj["spec"].(map[string]any)
166+
if !ok {
167+
return
168+
}
169+
170+
template, ok := spec["template"].(map[string]any)
171+
if !ok {
172+
return
173+
}
174+
175+
// Remove the entire metadata section from pod template.
176+
// This way OLM won't claim ownership of ANY pod template metadata fields.
177+
if _, hasMetadata := template["metadata"]; hasMetadata {
178+
delete(template, "metadata")
179+
l.V(1).Info("removed pod template metadata from Deployment to preserve all user changes",
180+
"deployment", unstr.GetName())
181+
}
182+
}
183+
156184
// sanitizedUnstructured takes an unstructured obj, removes status if present, and returns a sanitized copy containing only the allowed metadata entries set below.
157185
// If any unallowed entries are removed, a warning will be logged.
186+
//
187+
// For Deployment objects, this function removes the ENTIRE pod template metadata section.
188+
// This prevents OLM from overwriting ANY user changes to pod template metadata (annotations, labels, etc.).
189+
// Examples: "kubectl rollout restart", "kubectl annotate", "kubectl label", or any custom metadata users add.
190+
// This fixes the issue where user changes to pod templates would be undone by OLM.
191+
// See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
158192
func sanitizedUnstructured(ctx context.Context, unstr *unstructured.Unstructured) {
159193
l := log.FromContext(ctx)
160194
obj := unstr.Object
@@ -193,6 +227,13 @@ func sanitizedUnstructured(ctx context.Context, unstr *unstructured.Unstructured
193227
l.Info("warning: extraneous values removed from manifest metadata", "allowed metadata", allowedMetadata)
194228
}
195229
obj["metadata"] = metadataSanitized
230+
231+
// For Deployment objects, remove the ENTIRE pod template metadata section.
232+
// This prevents OLM from overwriting ANY user changes to pod templates.
233+
// Users can modify: annotations, labels, or any other metadata fields.
234+
// Examples: kubectl rollout restart, kubectl annotate, kubectl label, etc.
235+
// The deployment controller ensures required fields (like selector labels) stay in place.
236+
removePodTemplateMetadata(unstr, l)
196237
}
197238

198239
func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
Feature: Rollout Restart User Changes
2+
3+
# This test checks that OLMv1 does not undo changes made by users to deployed resources.
4+
# It uses "kubectl rollout restart" as an example, but the fix works for ANY pod template metadata changes.
5+
#
6+
# Background:
7+
# - In OLMv0, user changes to pod templates (annotations, labels, etc.) would be undone by OLM.
8+
# - In OLMv1, OLM should only manage the fields it owns, allowing user changes to stay.
9+
#
10+
# This test makes sure OLMv1 works correctly and does not have the OLMv0 problem.
11+
# See: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
12+
13+
Background:
14+
Given OLM is available
15+
And ClusterCatalog "test" serves bundles
16+
And ServiceAccount "olm-sa" with needed permissions is available in ${TEST_NAMESPACE}
17+
18+
# WHAT THIS TEST DOES:
19+
# This test checks that the fix for issue #3392 works correctly.
20+
# The test uses "kubectl rollout restart" as an example, but the fix is generic.
21+
#
22+
# THE PROBLEM (now fixed):
23+
# When users changed ANY pod template metadata, Boxcutter would undo the changes:
24+
# 1. User changes pod template (kubectl rollout restart, kubectl annotate, kubectl label, etc.)
25+
# 2. Boxcutter runs and removes ALL user changes
26+
# 3. User changes are lost
27+
#
28+
# THE FIX:
29+
# We changed the code in applier/boxcutter.go to NOT save pod template metadata at all.
30+
# This way, OLM won't overwrite ANY user changes to pod templates when it runs.
31+
# Now ANY user changes to pod templates (annotations, labels, etc.) will stay in place.
32+
#
33+
# UPSTREAM ISSUE: https://github.com/operator-framework/operator-lifecycle-manager/issues/3392
34+
@BoxcutterRuntime
35+
Scenario: User-initiated deployment changes persist after OLM reconciliation
36+
When ClusterExtension is applied
37+
"""
38+
apiVersion: olm.operatorframework.io/v1
39+
kind: ClusterExtension
40+
metadata:
41+
name: ${NAME}
42+
spec:
43+
namespace: ${TEST_NAMESPACE}
44+
serviceAccount:
45+
name: olm-sa
46+
source:
47+
sourceType: Catalog
48+
catalog:
49+
packageName: test
50+
selector:
51+
matchLabels:
52+
"olm.operatorframework.io/metadata.name": test-catalog
53+
"""
54+
Then ClusterExtension is rolled out
55+
And ClusterExtension is available
56+
And resource "deployment/test-operator" is installed
57+
And deployment "test-operator" is ready
58+
59+
# User runs "kubectl rollout restart deployment/test-operator"
60+
# This adds a restart annotation to trigger a rolling restart of pods.
61+
# Note: This test uses restart as an example, but the fix works for ANY pod template metadata changes.
62+
# In OLMv0, OLM would undo this change and stop the rollout.
63+
# In OLMv1, the change should stay because kubectl owns it.
64+
When user performs rollout restart on "deployment/test-operator"
65+
66+
# Wait for the rollout to finish - new pods created and running
67+
Then deployment "test-operator" rollout completes successfully
68+
And resource "deployment/test-operator" has restart annotation
69+
70+
# Wait for OLM to run its checks (OLM checks every 10 seconds)
71+
# This is the important part: does OLM undo what the user did?
72+
And I wait for "30" seconds
73+
74+
# After OLM runs, check that the rollout is STILL working
75+
# In OLMv0, this would fail because OLM undoes the annotation
76+
# and the new pods would be stopped and old pods would come back
77+
Then deployment "test-operator" rollout is still successful
78+
And resource "deployment/test-operator" has restart annotation
79+
And deployment "test-operator" has expected number of ready replicas

0 commit comments

Comments
 (0)