chore: patch metadata-only pod updates

pkg/controller/instance/in_place_update_utils.go

@@ -297,13 +297,16 @@ func equalResourcesInPlaceFields(old, new *corev1.Pod) bool {
 			return false
 		}
 		oc := old.Spec.Containers[index]
-		realRequests := nc.Resources.Requests
-		// 'requests' defaults to Limits if that is explicitly specified, see: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#resources
-		if realRequests == nil {
-			realRequests = nc.Resources.Limits
-		}
-		if !equalField(oc.Resources.Requests, realRequests) {
-			return false
+		for _, resourceName := range []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory} {
+			if request, ok := nc.Resources.Requests[resourceName]; ok {
+				oldRequest := corev1.ResourceList{}
+				if existing, ok := oc.Resources.Requests[resourceName]; ok {
+					oldRequest[resourceName] = existing
+				}
+				if !equalField(oldRequest, corev1.ResourceList{resourceName: request}) {
+					return false
+				}
+			}
 		}
 		if !equalField(oc.Resources.Limits, nc.Resources.Limits) {
 			return false

pkg/controller/instance/reconciler_update.go

@@ -159,15 +159,14 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 				return kubebuilderx.Continue, err
 			}
 
-			// if already updating using subresource, don't update it again, because without subresource, those fields are considered immutable.
-			// Another reconciliation will be triggered since pod status will be updated.
-			if !equalResourcesInPlaceFields(pod, newPod) && supportResizeSubResource {
+			switch {
+			case !equalResourcesInPlaceFields(pod, newPod) && supportResizeSubResource:
 				err = tree.Update(newMergedPod, kubebuilderx.WithSubResource("resize"))
-			} else {
-				if !safeMetadataOnlyInPlaceUpdate(pod, newPod) {
-					if err = r.switchover(tree, inst, newMergedPod.(*corev1.Pod)); err != nil {
-						return kubebuilderx.Continue, err
-					}
+			case safeMetadataOnlyInPlaceUpdate(pod, newPod):
+				err = tree.Update(newMergedPod, kubebuilderx.WithPatch(true))
+			default:
+				if err = r.switchover(tree, inst, newMergedPod.(*corev1.Pod)); err != nil {
+					return kubebuilderx.Continue, err
 				}
 				err = tree.Update(newMergedPod)
 			}

pkg/controller/instance/utils_test.go

@@ -182,6 +182,49 @@ func TestSafeMetadataOnlyInPlaceUpdate(t *testing.T) {
 	}
 }
 
+func TestEqualResourcesInPlaceFieldsSkipsOmittedRequests(t *testing.T) {
+	oldPod := builder.NewPodBuilder("default", "mysql-0").
+		SetContainers([]corev1.Container{{
+			Name: "mysql",
+			Resources: corev1.ResourceRequirements{
+				Limits: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("1"),
+					corev1.ResourceMemory: resource.MustParse("2Gi"),
+				},
+				Requests: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("500m"),
+					corev1.ResourceMemory: resource.MustParse("1Gi"),
+				},
+			},
+		}}).
+		GetObject()
+
+	omittedRequests := oldPod.DeepCopy()
+	omittedRequests.Spec.Containers[0].Resources = corev1.ResourceRequirements{
+		Limits: corev1.ResourceList{
+			corev1.ResourceCPU:    resource.MustParse("1"),
+			corev1.ResourceMemory: resource.MustParse("2Gi"),
+		},
+	}
+	if !equalResourcesInPlaceFields(oldPod, omittedRequests) {
+		t.Fatal("omitted desired CPU/memory requests should not force resource inequality")
+	}
+
+	explicitRequestChanged := omittedRequests.DeepCopy()
+	explicitRequestChanged.Spec.Containers[0].Resources.Requests = corev1.ResourceList{
+		corev1.ResourceCPU: resource.MustParse("1"),
+	}
+	if equalResourcesInPlaceFields(oldPod, explicitRequestChanged) {
+		t.Fatal("explicit desired CPU request must still be compared")
+	}
+
+	limitChanged := omittedRequests.DeepCopy()
+	limitChanged.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory] = resource.MustParse("3Gi")
+	if equalResourcesInPlaceFields(oldPod, limitChanged) {
+		t.Fatal("limits must still be compared")
+	}
+}
+
 func TestConfigsToUpdateStillReportsRealConfigHashMismatch(t *testing.T) {
 	inst := builder.NewInstanceBuilder("default", "valkey-0").
 		SetConfigs([]workloads.ConfigTemplate{{

pkg/controller/instanceset/in_place_update_util.go

@@ -306,13 +306,16 @@ func equalResourcesInPlaceFields(old, new *corev1.Pod) bool {
 			return false
 		}
 		oc := old.Spec.Containers[index]
-		realRequests := nc.Resources.Requests
-		// 'requests' defaults to Limits if that is explicitly specified, see: https://kubernetes.io/docs/reference/kubernetes-api/workload-resources/pod-v1/#resources
-		if realRequests == nil {
-			realRequests = nc.Resources.Limits
-		}
-		if !equalField(oc.Resources.Requests, realRequests) {
-			return false
+		for _, resourceName := range []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory} {
+			if request, ok := nc.Resources.Requests[resourceName]; ok {
+				oldRequest := corev1.ResourceList{}
+				if existing, ok := oc.Resources.Requests[resourceName]; ok {
+					oldRequest[resourceName] = existing
+				}
+				if !equalField(oldRequest, corev1.ResourceList{resourceName: request}) {
+					return false
+				}
+			}
 		}
 		if !equalField(oc.Resources.Limits, nc.Resources.Limits) {
 			return false

pkg/controller/instanceset/in_place_update_util_test.go

@@ -88,6 +88,43 @@ var _ = Describe("instance util test", func() {
 			mergeInPlaceFields(newPod, oldPod)
 			Expect(equalBasicInPlaceFields(oldPod, newPod)).Should(BeTrue())
 		})
+
+		It("skips omitted CPU and memory requests in resource comparison", func() {
+			oldPod := builder.NewPodBuilder(namespace, "foo-0").
+				SetContainers([]corev1.Container{{
+					Name: "foo",
+					Resources: corev1.ResourceRequirements{
+						Limits: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("1"),
+							corev1.ResourceMemory: resource.MustParse("2Gi"),
+						},
+						Requests: corev1.ResourceList{
+							corev1.ResourceCPU:    resource.MustParse("500m"),
+							corev1.ResourceMemory: resource.MustParse("1Gi"),
+						},
+					},
+				}}).
+				GetObject()
+
+			omittedRequests := oldPod.DeepCopy()
+			omittedRequests.Spec.Containers[0].Resources = corev1.ResourceRequirements{
+				Limits: corev1.ResourceList{
+					corev1.ResourceCPU:    resource.MustParse("1"),
+					corev1.ResourceMemory: resource.MustParse("2Gi"),
+				},
+			}
+			Expect(equalResourcesInPlaceFields(oldPod, omittedRequests)).Should(BeTrue())
+
+			explicitRequestChanged := omittedRequests.DeepCopy()
+			explicitRequestChanged.Spec.Containers[0].Resources.Requests = corev1.ResourceList{
+				corev1.ResourceCPU: resource.MustParse("1"),
+			}
+			Expect(equalResourcesInPlaceFields(oldPod, explicitRequestChanged)).Should(BeFalse())
+
+			limitChanged := omittedRequests.DeepCopy()
+			limitChanged.Spec.Containers[0].Resources.Limits[corev1.ResourceMemory] = resource.MustParse("3Gi")
+			Expect(equalResourcesInPlaceFields(oldPod, limitChanged)).Should(BeFalse())
+		})
 	})
 
 	Context("container image comparison", func() {

pkg/controller/instanceset/reconciler_update.go

@@ -188,15 +188,14 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 				return kubebuilderx.Continue, err
 			}
 
-			// if already updating using subresource, don't update it again, because without subresource, those fields are considered immutable.
-			// Another reconciliation will be triggered since pod status will be updated.
-			if !equalResourcesInPlaceFields(pod, newPod) && supportResizeSubResource {
+			switch {
+			case !equalResourcesInPlaceFields(pod, newPod) && supportResizeSubResource:
 				err = tree.Update(newMergedPod, kubebuilderx.WithSubResource("resize"))
-			} else {
-				if !safeMetadataOnlyInPlaceUpdate(pod, newPod) {
-					if err = r.switchover(tree, its, newMergedPod.(*corev1.Pod)); err != nil {
-						return kubebuilderx.Continue, err
-					}
+			case safeMetadataOnlyInPlaceUpdate(pod, newPod):
+				err = tree.Update(newMergedPod, kubebuilderx.WithPatch(true))
+			default:
+				if err = r.switchover(tree, its, newMergedPod.(*corev1.Pod)); err != nil {
+					return kubebuilderx.Continue, err
 				}
 				err = tree.Update(newMergedPod)
 			}

pkg/controller/instanceset/reconciler_update_test.go

@@ -495,6 +495,10 @@ var _ = Describe("update reconciler test", func() {
 			updatedPod := postPods[0].(*corev1.Pod)
 			Expect(updatedPod.Annotations).Should(HaveKeyWithValue(constant.CMInsConfigurationHashLabelKey, "new-hash"),
 				"pod should be patched with the new config-hash annotation even though switchover is skipped")
+			_, option, err := tree.GetWithOption(updatedPod)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(option.Patch).Should(BeTrue(),
+				"metadata-only pod updates should use a patch to avoid stale full-update conflicts")
 			Expect(spy.switchoverCalls).Should(Equal(0),
 				"switchover must not be invoked when only the config-hash annotation differs")
 		})

pkg/controller/kubebuilderx/plan_builder.go

@@ -187,12 +187,18 @@ func buildOrderedVertices(transCtx *transformContext, currentTree *ObjectTree, d
 					transCtx.logger.Error(err, "can't get GVKName from object", "object", newObj.GetName())
 					return
 				}
-				var v *model.ObjectVertex
-				subResource := desiredTree.childrenOptions[*name].SubResource
+				var (
+					v           *model.ObjectVertex
+					subResource = desiredTree.childrenOptions[*name].SubResource
+					action      = model.ActionUpdatePtr()
+				)
+				if desiredTree.childrenOptions[*name].Patch {
+					action = model.ActionPatchPtr()
+				}
 				if subResource != "" {
-					v = model.NewObjectVertex(oldObj, newObj, model.ActionUpdatePtr(), inDataContext4G(), model.WithSubResource(subResource))
+					v = model.NewObjectVertex(oldObj, newObj, action, inDataContext4G(), model.WithSubResource(subResource))
 				} else {
-					v = model.NewObjectVertex(oldObj, newObj, model.ActionUpdatePtr(), inDataContext4G())
+					v = model.NewObjectVertex(oldObj, newObj, action, inDataContext4G())
 				}
 				findAndAppend(v)
 			}

pkg/controller/kubebuilderx/plan_builder_test.go

@@ -98,6 +98,27 @@ var _ = Describe("plan builder test", func() {
 			Expect(planBuilder.defaultWalkFunc(v)).Should(Succeed())
 		})
 
+		It("should patch object", func() {
+			svcOrig := builder.NewServiceBuilder(namespace, name).GetObject()
+			svc := svcOrig.DeepCopy()
+			svc.Labels = map[string]string{"patched": "true"}
+			v := &model.ObjectVertex{
+				OriObj: svcOrig,
+				Obj:    svc,
+				Action: model.ActionPatchPtr(),
+			}
+			k8sMock.EXPECT().
+				Patch(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
+				DoAndReturn(func(_ context.Context, obj *corev1.Service, _ client.Patch, _ ...client.PatchOption) error {
+					Expect(obj).ShouldNot(BeNil())
+					Expect(obj.Namespace).Should(Equal(svc.Namespace))
+					Expect(obj.Name).Should(Equal(svc.Name))
+					Expect(obj.Labels).Should(Equal(svc.Labels))
+					return nil
+				}).Times(1)
+			Expect(planBuilder.defaultWalkFunc(v)).Should(Succeed())
+		})
+
 		It("should update pvc object", func() {
 			pvcOrig := builder.NewPVCBuilder(namespace, name).GetObject()
 			pvc := pvcOrig.DeepCopy()
@@ -286,6 +307,30 @@ var _ = Describe("plan builder test", func() {
 				}
 				Expect(found).To(BeTrue())
 			})
+
+			It("should append a patch vertex when requested by object options", func() {
+				oldPod := builder.NewPodBuilder("ns", "pod").GetObject()
+				newPod := oldPod.DeepCopy()
+				newPod.Labels = map[string]string{"patched": "true"}
+
+				currentTree := NewObjectTree()
+				desiredTree := NewObjectTree()
+				currentTree.SetRoot(builder.NewInstanceSetBuilder("ns", "root").GetObject())
+				desiredTree.SetRoot(currentTree.GetRoot())
+				Expect(currentTree.Add(oldPod)).Should(Succeed())
+				Expect(desiredTree.Update(newPod, WithPatch(true))).Should(Succeed())
+
+				vertices := buildOrderedVertices(transCtx, currentTree, desiredTree)
+
+				found := false
+				for _, v := range vertices {
+					if pod, ok := v.Obj.(*corev1.Pod); ok && pod.Name == "pod" {
+						found = true
+						Expect(*v.Action).To(Equal(model.PATCH))
+					}
+				}
+				Expect(found).To(BeTrue())
+			})
 		})
 	})
 })

pkg/controller/kubebuilderx/reconciler.go

@@ -41,6 +41,9 @@ type ObjectOptions struct {
 	// if not empty, action should be done on the specified subresource
 	SubResource string
 
+	// if true, the object should be patched instead of fully updated.
+	Patch bool
+
 	// if true, the object should not be reconciled
 	SkipToReconcile bool
 }
@@ -51,6 +54,12 @@ func (w WithSubResource) ApplyToObject(opts *ObjectOptions) {
 	opts.SubResource = string(w)
 }
 
+type WithPatch bool
+
+func (w WithPatch) ApplyToObject(opts *ObjectOptions) {
+	opts.Patch = bool(w)
+}
+
 type SkipToReconcile bool
 
 func (o SkipToReconcile) ApplyToObject(opts *ObjectOptions) {