Use Patch() to update status of CnsRegisterVolume CR insteaf of Update()

Author: akankshapanse

manifests/supervisorcluster/1.29/cns-csi.yaml

@@ -59,7 +59,7 @@ rules:
     verbs: ["get", "list", "update", "create", "delete"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["cnsregistervolumes", "cnsregistervolumes/status", "cnsunregistervolumes", "cnsunregistervolumes/status"]
-    verbs: ["get", "list", "watch", "update", "delete"]
+    verbs: ["get", "list", "watch", "update", "delete", "patch"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["triggercsifullsyncs"]
     verbs: ["create", "get", "update", "watch", "list"]

manifests/supervisorcluster/1.30/cns-csi.yaml

@@ -59,7 +59,7 @@ rules:
     verbs: ["get", "list", "update", "create", "delete"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["cnsregistervolumes", "cnsregistervolumes/status", "cnsunregistervolumes", "cnsunregistervolumes/status"]
-    verbs: ["get", "list", "watch", "update", "delete"]
+    verbs: ["get", "list", "watch", "update", "delete", "patch"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["triggercsifullsyncs"]
     verbs: ["create", "get", "update", "watch", "list"]

manifests/supervisorcluster/1.31/cns-csi.yaml

@@ -59,7 +59,7 @@ rules:
     verbs: ["get", "list", "update", "create", "delete"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["cnsregistervolumes", "cnsregistervolumes/status", "cnsunregistervolumes", "cnsunregistervolumes/status"]
-    verbs: ["get", "list", "watch", "update", "delete"]
+    verbs: ["get", "list", "watch", "update", "delete", "patch"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["triggercsifullsyncs"]
     verbs: ["create", "get", "update", "watch", "list"]

manifests/supervisorcluster/1.32/cns-csi.yaml

@@ -59,7 +59,7 @@ rules:
     verbs: ["get", "list", "update", "create", "delete"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["cnsregistervolumes", "cnsregistervolumes/status", "cnsunregistervolumes", "cnsunregistervolumes/status"]
-    verbs: ["get", "list", "watch", "update", "delete"]
+    verbs: ["get", "list", "watch", "update", "delete", "patch"]
   - apiGroups: ["cns.vmware.com"]
     resources: ["triggercsifullsyncs"]
     verbs: ["create", "get", "update", "watch", "list"]

pkg/syncer/cnsoperator/controller/cnsregistervolume/cnsregistervolume_controller.go

@@ -1173,10 +1173,12 @@ func validateAndFixPVVolumeMode(ctx context.Context, k8sclient clientset.Interfa
 func setInstanceError(ctx context.Context, r *ReconcileCnsRegisterVolume,
 	instance *cnsregistervolumev1alpha1.CnsRegisterVolume, errMsg string) {
 	log := logger.GetLogger(ctx)
+	origInstance := instance.DeepCopy()
+	instance.Status.Registered = false
 	instance.Status.Error = errMsg
-	err := k8s.UpdateStatus(ctx, r.client, instance)
+	err := patchCnsRegisterVolumeStatus(ctx, r.client, origInstance, instance)
 	if err != nil {
-		log.Errorf("updateCnsRegisterVolume failed. err: %v", err)
+		log.Errorf("patchCnsRegisterVolumeStatus failed. err: %v", err)
 		return
 	}
 	recordEvent(ctx, r, instance, v1.EventTypeWarning, errMsg)
@@ -1205,11 +1207,12 @@ func setInstanceSuccess(ctx context.Context, r *ReconcileCnsRegisterVolume,
 		return err
 	}
 	// Now update the status subresource with the fresh object
+	origInstance := instance.DeepCopy()
 	instance.Status.Registered = true
 	instance.Status.Error = ""
-	err = k8s.UpdateStatus(ctx, r.client, instance)
+	err = patchCnsRegisterVolumeStatus(ctx, r.client, origInstance, instance)
 	if err != nil {
-		log.Errorf("updateCnsRegisterVolume status failed. err: %v", err)
+		log.Errorf("patchCnsRegisterVolumeStatus failed. err: %v", err)
 		return err
 	}
 	recordEvent(ctx, r, instance, v1.EventTypeNormal, msg)
@@ -1273,3 +1276,59 @@ func updateCnsRegisterVolume(ctx context.Context, client client.Client,
 	}
 	return err
 }
+
+// patchCnsRegisterVolumeStatus patches status field of CnsRegisterVolume instance in K8S.
+// For status subresources with required fields, we need to ensure those fields are always
+// included in the patch, even if they haven't changed. This is because Kubernetes validates
+// the patch against the CRD schema which marks 'registered' as required.
+func patchCnsRegisterVolumeStatus(ctx context.Context, cnsOperatorClient client.Client,
+	oldObj *cnsregistervolumev1alpha1.CnsRegisterVolume,
+	newObj *cnsregistervolumev1alpha1.CnsRegisterVolume) error {
+	log := logger.GetLogger(ctx)
+
+	// For status patches with required fields, we use a simple JSON patch that always
+	// includes required status fields to satisfy CRD validation
+	var statusPatch map[string]interface{}
+	if newObj.Status.Error != "" {
+		statusPatch = map[string]interface{}{
+			"status": map[string]interface{}{
+				"registered": newObj.Status.Registered,
+				"error":      newObj.Status.Error,
+			},
+		}
+	} else {
+		statusPatch = map[string]interface{}{
+			"status": map[string]interface{}{
+				"registered": newObj.Status.Registered,
+			},
+		}
+	}
+
+	patchBytes, err := json.Marshal(statusPatch)
+	if err != nil {
+		log.Errorf("failed to marshal status patch. err: %v", err)
+		return err
+	}
+
+	rawPatch := client.RawPatch(apitypes.MergePatchType, patchBytes)
+
+	// Try to patch CnsRegisterVolume CR for 3 times
+	allowedRetries := 3
+	attempt := 0
+	for {
+		attempt++
+		err := cnsOperatorClient.Status().Patch(ctx, oldObj, rawPatch)
+		if err != nil && attempt >= allowedRetries {
+			log.Errorf("failed to patch CnsRegisterVolume instance %q on namespace %q, Error: %+v",
+				oldObj.Name, oldObj.Namespace, err)
+			return err
+		} else if err == nil {
+			log.Debugf("Successfully patched CnsRegisterVolume instance %q on namespace %q",
+				oldObj.Name, oldObj.Namespace)
+			return nil
+		}
+		log.Warnf("attempt %d, failed to patch CnsRegisterVolume instance %q on namespace %q with error %+v, "+
+			"will retry...", attempt, oldObj.Name, oldObj.Namespace, err)
+		time.Sleep(100 * time.Millisecond)
+	}
+}