Skip to content

Commit 4217c14

Browse files
lmicciniclaude
andcommitted
Fix reconcile loops caused by server-defaulted fields in CreateOrPatch
Service, Deployment, DaemonSet, and StatefulSet CreateOrPatch functions were overwriting entire specs/templates, stripping Kubernetes server-defaulted fields (e.g. TargetPort, ImagePullPolicy, SessionAffinity). This caused controllerutil.CreateOrPatch to detect a diff on every reconcile, creating infinite reconcile loops. - Add shared pod.MergeContainersByName to preserve container-level server defaults (ImagePullPolicy, TerminationMessagePath, TerminationMessagePolicy) across Deployment, DaemonSet, StatefulSet - Add service.MergeServicePorts to preserve port-level server defaults (TargetPort, Protocol) - Change Service CreateOrPatch to copy only operator-controlled fields, keeping the existing spec as base so server-defaulted fields are preserved implicitly - Deprecate statefulset.MergeContainersByName in favor of the shared pod.MergeContainersByName Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fe8e60d commit 4217c14

8 files changed

Lines changed: 794 additions & 30 deletions

File tree

modules/common/daemonset/daemonset.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,30 @@ func (d *DaemonSet) CreateOrPatch(
6464
}
6565
daemonset.Annotations = util.MergeStringMaps(daemonset.Annotations, d.daemonset.Annotations)
6666
daemonset.Labels = util.MergeStringMaps(daemonset.Labels, d.daemonset.Labels)
67+
68+
// Save existing containers before overwriting the Template so we
69+
// can merge them below to preserve server-defaulted fields.
70+
existingContainers := daemonset.Spec.Template.Spec.Containers
71+
existingInitContainers := daemonset.Spec.Template.Spec.InitContainers
72+
6773
daemonset.Spec.Template = d.daemonset.Spec.Template
6874
pod.SetPullPolicyDefaults(&daemonset.Spec.Template.Spec)
6975
daemonset.Spec.UpdateStrategy = d.daemonset.Spec.UpdateStrategy
7076

77+
// Merge containers by name to preserve server-defaulted fields
78+
// (e.g. TerminationMessagePath, ImagePullPolicy) and avoid
79+
// unnecessary reconcile loops.
80+
daemonset.Spec.Template.Spec.Containers = existingContainers
81+
pod.MergeContainersByName(
82+
&daemonset.Spec.Template.Spec.Containers,
83+
d.daemonset.Spec.Template.Spec.Containers,
84+
)
85+
daemonset.Spec.Template.Spec.InitContainers = existingInitContainers
86+
pod.MergeContainersByName(
87+
&daemonset.Spec.Template.Spec.InitContainers,
88+
d.daemonset.Spec.Template.Spec.InitContainers,
89+
)
90+
7191
err := controllerutil.SetControllerReference(h.GetBeforeObject(), daemonset, h.GetScheme())
7292
if err != nil {
7393
return err

modules/common/deployment/deployment.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,31 @@ func (d *Deployment) CreateOrPatch(
6464
}
6565
deployment.Annotations = util.MergeStringMaps(deployment.Annotations, d.deployment.Annotations)
6666
deployment.Labels = util.MergeStringMaps(deployment.Labels, d.deployment.Labels)
67+
68+
// Save existing containers before overwriting the Template so we
69+
// can merge them below to preserve server-defaulted fields.
70+
existingContainers := deployment.Spec.Template.Spec.Containers
71+
existingInitContainers := deployment.Spec.Template.Spec.InitContainers
72+
6773
deployment.Spec.Template = d.deployment.Spec.Template
6874
pod.SetPullPolicyDefaults(&deployment.Spec.Template.Spec)
6975
deployment.Spec.Replicas = d.deployment.Spec.Replicas
7076
deployment.Spec.Strategy = d.deployment.Spec.Strategy
7177

78+
// Merge containers by name to preserve server-defaulted fields
79+
// (e.g. TerminationMessagePath, ImagePullPolicy) and avoid
80+
// unnecessary reconcile loops.
81+
deployment.Spec.Template.Spec.Containers = existingContainers
82+
pod.MergeContainersByName(
83+
&deployment.Spec.Template.Spec.Containers,
84+
d.deployment.Spec.Template.Spec.Containers,
85+
)
86+
deployment.Spec.Template.Spec.InitContainers = existingInitContainers
87+
pod.MergeContainersByName(
88+
&deployment.Spec.Template.Spec.InitContainers,
89+
d.deployment.Spec.Template.Spec.InitContainers,
90+
)
91+
7292
err := controllerutil.SetControllerReference(h.GetBeforeObject(), deployment, h.GetScheme())
7393
if err != nil {
7494
return err

modules/common/pod/merge.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/*
2+
Copyright 2026 Red Hat
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package pod
18+
19+
import (
20+
corev1 "k8s.io/api/core/v1"
21+
)
22+
23+
// MergeContainersByName merges desired container specs into existing containers
24+
// matched by name. It starts from the desired container and preserves only the
25+
// server-defaulted fields (TerminationMessagePath, TerminationMessagePolicy,
26+
// ImagePullPolicy) from the existing container. All other fields come from the
27+
// desired spec, which ensures that new fields added in future Kubernetes
28+
// versions are not silently dropped.
29+
//
30+
// When container counts differ or a desired container name is not found in
31+
// existing, the existing slice is replaced with the desired containers.
32+
func MergeContainersByName(existing *[]corev1.Container, desired []corev1.Container) {
33+
if len(*existing) != len(desired) {
34+
*existing = desired
35+
return
36+
}
37+
38+
existingByName := make(map[string]int, len(*existing))
39+
for i := range *existing {
40+
existingByName[(*existing)[i].Name] = i
41+
}
42+
43+
for _, d := range desired {
44+
idx, ok := existingByName[d.Name]
45+
if !ok {
46+
*existing = desired
47+
return
48+
}
49+
// Preserve server-defaulted fields from the existing container
50+
// only when the desired spec doesn't explicitly set them.
51+
if d.ImagePullPolicy == "" {
52+
d.ImagePullPolicy = (*existing)[idx].ImagePullPolicy
53+
}
54+
if d.TerminationMessagePath == "" {
55+
d.TerminationMessagePath = (*existing)[idx].TerminationMessagePath
56+
}
57+
if d.TerminationMessagePolicy == "" {
58+
d.TerminationMessagePolicy = (*existing)[idx].TerminationMessagePolicy
59+
}
60+
(*existing)[idx] = d
61+
}
62+
}

0 commit comments

Comments
 (0)