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>

Author: lmiccini

modules/common/daemonset/daemonset.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	appsv1 "k8s.io/api/apps/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -63,9 +64,29 @@ func (d *DaemonSet) CreateOrPatch(
 		}
 		daemonset.Annotations = util.MergeStringMaps(daemonset.Annotations, d.daemonset.Annotations)
 		daemonset.Labels = util.MergeStringMaps(daemonset.Labels, d.daemonset.Labels)
+
+		// Save existing containers before overwriting the Template so we
+		// can merge them below to preserve server-defaulted fields.
+		existingContainers := daemonset.Spec.Template.Spec.Containers
+		existingInitContainers := daemonset.Spec.Template.Spec.InitContainers
+
 		daemonset.Spec.Template = d.daemonset.Spec.Template
 		daemonset.Spec.UpdateStrategy = d.daemonset.Spec.UpdateStrategy
 
+		// Merge containers by name to preserve server-defaulted fields
+		// (e.g. TerminationMessagePath, ImagePullPolicy) and avoid
+		// unnecessary reconcile loops.
+		daemonset.Spec.Template.Spec.Containers = existingContainers
+		pod.MergeContainersByName(
+			&daemonset.Spec.Template.Spec.Containers,
+			d.daemonset.Spec.Template.Spec.Containers,
+		)
+		daemonset.Spec.Template.Spec.InitContainers = existingInitContainers
+		pod.MergeContainersByName(
+			&daemonset.Spec.Template.Spec.InitContainers,
+			d.daemonset.Spec.Template.Spec.InitContainers,
+		)
+
 		err := controllerutil.SetControllerReference(h.GetBeforeObject(), daemonset, h.GetScheme())
 		if err != nil {
 			return err

modules/common/deployment/deployment.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
+	"github.com/openstack-k8s-operators/lib-common/modules/common/pod"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/util"
 	appsv1 "k8s.io/api/apps/v1"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
@@ -63,10 +64,30 @@ func (d *Deployment) CreateOrPatch(
 		}
 		deployment.Annotations = util.MergeStringMaps(deployment.Annotations, d.deployment.Annotations)
 		deployment.Labels = util.MergeStringMaps(deployment.Labels, d.deployment.Labels)
+
+		// Save existing containers before overwriting the Template so we
+		// can merge them below to preserve server-defaulted fields.
+		existingContainers := deployment.Spec.Template.Spec.Containers
+		existingInitContainers := deployment.Spec.Template.Spec.InitContainers
+
 		deployment.Spec.Template = d.deployment.Spec.Template
 		deployment.Spec.Replicas = d.deployment.Spec.Replicas
 		deployment.Spec.Strategy = d.deployment.Spec.Strategy
 
+		// Merge containers by name to preserve server-defaulted fields
+		// (e.g. TerminationMessagePath, ImagePullPolicy) and avoid
+		// unnecessary reconcile loops.
+		deployment.Spec.Template.Spec.Containers = existingContainers
+		pod.MergeContainersByName(
+			&deployment.Spec.Template.Spec.Containers,
+			d.deployment.Spec.Template.Spec.Containers,
+		)
+		deployment.Spec.Template.Spec.InitContainers = existingInitContainers
+		pod.MergeContainersByName(
+			&deployment.Spec.Template.Spec.InitContainers,
+			d.deployment.Spec.Template.Spec.InitContainers,
+		)
+
 		err := controllerutil.SetControllerReference(h.GetBeforeObject(), deployment, h.GetScheme())
 		if err != nil {
 			return err

modules/common/pod/merge.go

@@ -0,0 +1,62 @@
+/*
+Copyright 2026 Red Hat
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package pod
+
+import (
+	corev1 "k8s.io/api/core/v1"
+)
+
+// MergeContainersByName merges desired container specs into existing containers
+// matched by name. It starts from the desired container and preserves only the
+// server-defaulted fields (TerminationMessagePath, TerminationMessagePolicy,
+// ImagePullPolicy) from the existing container. All other fields come from the
+// desired spec, which ensures that new fields added in future Kubernetes
+// versions are not silently dropped.
+//
+// When container counts differ or a desired container name is not found in
+// existing, the existing slice is replaced with the desired containers.
+func MergeContainersByName(existing *[]corev1.Container, desired []corev1.Container) {
+	if len(*existing) != len(desired) {
+		*existing = desired
+		return
+	}
+
+	existingByName := make(map[string]int, len(*existing))
+	for i := range *existing {
+		existingByName[(*existing)[i].Name] = i
+	}
+
+	for _, d := range desired {
+		idx, ok := existingByName[d.Name]
+		if !ok {
+			*existing = desired
+			return
+		}
+		// Preserve server-defaulted fields from the existing container
+		// only when the desired spec doesn't explicitly set them.
+		if d.ImagePullPolicy == "" {
+			d.ImagePullPolicy = (*existing)[idx].ImagePullPolicy
+		}
+		if d.TerminationMessagePath == "" {
+			d.TerminationMessagePath = (*existing)[idx].TerminationMessagePath
+		}
+		if d.TerminationMessagePolicy == "" {
+			d.TerminationMessagePolicy = (*existing)[idx].TerminationMessagePolicy
+		}
+		(*existing)[idx] = d
+	}
+}