Skip to content

Commit a9db8ca

Browse files
chethanv28chethanv28
committed
Use Patch operation for PVC updates
1 parent 3c7214e commit a9db8ca

File tree

12 files changed

+437
-53
lines changed

12 files changed

+437
-53
lines changed

‎pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator.go‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2175,15 +2175,35 @@ func (c *K8sOrchestrator) PreLinkedCloneCreateAction(ctx context.Context, pvcNam
21752175
return err
21762176
}
21772177

2178-
if linkedClonePVC.Labels == nil {
2179-
linkedClonePVC.Labels = make(map[string]string)
2178+
oldData, err := json.Marshal(linkedClonePVC)
2179+
if err != nil {
2180+
log.Errorf("Failed to marshal PVC %s/%s: %v", pvcNamespace, pvcName, err)
2181+
return err
2182+
}
2183+
2184+
newPVC := linkedClonePVC.DeepCopy()
2185+
if newPVC.Labels == nil {
2186+
newPVC.Labels = make(map[string]string)
21802187
}
21812188
// Add label
2182-
if _, ok := linkedClonePVC.Labels[common.AnnKeyLinkedClone]; !ok {
2183-
linkedClonePVC.Labels[common.LinkedClonePVCLabel] = linkedClonePVC.Annotations[common.AttributeIsLinkedClone]
2189+
if _, ok := newPVC.Labels[common.AnnKeyLinkedClone]; !ok {
2190+
newPVC.Labels[common.LinkedClonePVCLabel] = newPVC.Annotations[common.AttributeIsLinkedClone]
21842191
}
21852192

2186-
_, err = c.k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, linkedClonePVC, metav1.UpdateOptions{})
2193+
newData, err := json.Marshal(newPVC)
2194+
if err != nil {
2195+
log.Errorf("Failed to marshal updated PVC %s/%s with labels: %v", pvcNamespace, pvcName, err)
2196+
return err
2197+
}
2198+
2199+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, linkedClonePVC)
2200+
if err != nil {
2201+
log.Errorf("Error creating two way merge patch for PVC %s/%s with error: %v", pvcNamespace, pvcName, err)
2202+
return err
2203+
}
2204+
2205+
_, err = c.k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Patch(ctx, linkedClonePVC.Name, k8stypes.StrategicMergePatchType,
2206+
patchBytes, metav1.PatchOptions{})
21872207
if err != nil {
21882208
log.Errorf("failed to add linked clone label for PVC %s/%s. Error: %+v, retrying...",
21892209
pvcNamespace, pvcName, err)

‎pkg/csi/service/common/commonco/k8sorchestrator/k8sorchestrator_helper.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"time"
2424

2525
k8stypes "k8s.io/apimachinery/pkg/types"
26+
"k8s.io/apimachinery/pkg/util/strategicpatch"
2627
"k8s.io/apimachinery/pkg/util/wait"
2728

2829
v1 "k8s.io/api/core/v1"
@@ -83,17 +84,37 @@ func (c *K8sOrchestrator) updatePVCAnnotations(ctx context.Context,
8384
return err
8485
}
8586

87+
oldData, err := json.Marshal(pvcObj)
88+
if err != nil {
89+
log.Errorf("failed to marshal PVC %s/%s: %v", pvcNamespace, pvcName, err)
90+
return err
91+
}
92+
93+
newPVC := pvcObj.DeepCopy()
8694
for key, val := range annotations {
8795
// If value is not set, remove the annotation.
8896
if val == "" {
89-
delete(pvcObj.ObjectMeta.Annotations, key)
97+
delete(newPVC.ObjectMeta.Annotations, key)
9098
log.Debugf("Removing annotation %s on pvc %s/%s", key, pvcNamespace, pvcName)
9199
} else {
92-
metav1.SetMetaDataAnnotation(&pvcObj.ObjectMeta, key, val)
100+
metav1.SetMetaDataAnnotation(&newPVC.ObjectMeta, key, val)
93101
log.Debugf("Updating annotation %s on pvc %s/%s to value: %s", key, pvcNamespace, pvcName, val)
94102
}
95103
}
96-
_, err = c.k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, pvcObj, metav1.UpdateOptions{})
104+
105+
newData, err := json.Marshal(newPVC)
106+
if err != nil {
107+
log.Errorf("failed to marshal updated PVC %s/%s with annotations: %v", pvcNamespace, pvcName, err)
108+
return err
109+
}
110+
111+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvcObj)
112+
if err != nil {
113+
log.Errorf("error creating two way merge patch for PVC %s/%s: %v", pvcNamespace, pvcName, err)
114+
return err
115+
}
116+
117+
_, err = c.k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Patch(ctx, pvcObj.Name, k8stypes.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
97118
if err != nil {
98119
log.Errorf("failed to update pvc annotations %s/%s with err:%+v", pvcNamespace, pvcName, err)
99120
return err

‎pkg/csi/service/wcpguest/controller.go‎

Lines changed: 59 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package wcpguest
1818

1919
import (
2020
"context"
21+
"encoding/json"
2122
"fmt"
2223
"math"
2324
"net/http"
@@ -48,6 +49,7 @@ import (
4849
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4950
"k8s.io/apimachinery/pkg/fields"
5051
"k8s.io/apimachinery/pkg/types"
52+
"k8s.io/apimachinery/pkg/util/strategicpatch"
5153
"k8s.io/apimachinery/pkg/watch"
5254
clientset "k8s.io/client-go/kubernetes"
5355
"k8s.io/client-go/rest"
@@ -622,10 +624,36 @@ func (c *controller) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequ
622624
if finalizer == cnsoperatortypes.CNSVolumeFinalizer {
623625
log.Infof("Removing %q finalizer from PersistentVolumeClaim with name: %q on namespace: %q",
624626
cnsoperatortypes.CNSVolumeFinalizer, svPVC.Name, svPVC.Namespace)
625-
svPVC.ObjectMeta.Finalizers = slices.Delete(svPVC.ObjectMeta.Finalizers, i, i+1)
627+
628+
oldData, err := json.Marshal(svPVC)
629+
if err != nil {
630+
msg := fmt.Sprintf("failed to marshal supervisor PVC %q in %q namespace. Error: %+v",
631+
req.VolumeId, c.supervisorNamespace, err)
632+
log.Error(msg)
633+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
634+
}
635+
636+
newPVC := svPVC.DeepCopy()
637+
newPVC.ObjectMeta.Finalizers = slices.Delete(newPVC.ObjectMeta.Finalizers, i, i+1)
638+
639+
newData, err := json.Marshal(newPVC)
640+
if err != nil {
641+
msg := fmt.Sprintf("failed to marshal updated supervisor PVC %q in %q namespace. Error: %+v",
642+
req.VolumeId, c.supervisorNamespace, err)
643+
log.Error(msg)
644+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
645+
}
646+
647+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, svPVC)
648+
if err != nil {
649+
msg := fmt.Sprintf("error creating two way merge patch for supervisor PVC %q in %q namespace. Error: %+v",
650+
req.VolumeId, c.supervisorNamespace, err)
651+
log.Error(msg)
652+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
653+
}
654+
626655
// Update the instance after removing finalizer
627-
_, err := c.supervisorClient.CoreV1().PersistentVolumeClaims(c.supervisorNamespace).Update(ctx, svPVC,
628-
metav1.UpdateOptions{})
656+
_, err = c.supervisorClient.CoreV1().PersistentVolumeClaims(c.supervisorNamespace).Patch(ctx, svPVC.Name, types.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
629657
if err != nil {
630658
msg := fmt.Sprintf("failed to update supervisor PVC %q in %q namespace. Error: %+v",
631659
req.VolumeId, c.supervisorNamespace, err)
@@ -1466,14 +1494,38 @@ func (c *controller) ControllerExpandVolume(ctx context.Context, req *csi.Contro
14661494
switch (gcPvcRequestSize).Cmp(svPvcRequestSize) {
14671495
case 1:
14681496
// Update requested storage in SV PVC spec
1469-
svPvcClone := svPVC.DeepCopy()
1470-
svPvcClone.Spec.Resources.Requests[corev1.ResourceName(corev1.ResourceStorage)] = *gcPvcRequestSize
1497+
oldExpandData, err := json.Marshal(svPVC)
1498+
if err != nil {
1499+
msg := fmt.Sprintf("failed to marshal supervisor PVC for expansion %q in %q namespace. Error: %+v",
1500+
volumeID, c.supervisorNamespace, err)
1501+
log.Error(msg)
1502+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
1503+
}
1504+
1505+
newExpandPVC := svPVC.DeepCopy()
1506+
newExpandPVC.Spec.Resources.Requests[corev1.ResourceName(corev1.ResourceStorage)] = *gcPvcRequestSize
1507+
1508+
newExpandData, err := json.Marshal(newExpandPVC)
1509+
if err != nil {
1510+
msg := fmt.Sprintf("failed to marshal updated supervisor PVC for expansion %q in %q namespace. Error: %+v",
1511+
volumeID, c.supervisorNamespace, err)
1512+
log.Error(msg)
1513+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
1514+
}
1515+
1516+
expandPatchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldExpandData, newExpandData, svPVC)
1517+
if err != nil {
1518+
msg := fmt.Sprintf("error creating two way merge patch for supervisor PVC expansion %q in %q namespace. Error: %+v",
1519+
volumeID, c.supervisorNamespace, err)
1520+
log.Error(msg)
1521+
return nil, csifault.CSIInternalFault, status.Error(codes.Internal, msg)
1522+
}
14711523

14721524
// Make an update call to SV API server
14731525
log.Infof("Increasing the size of supervisor PVC %s in namespace %s to %s",
14741526
volumeID, c.supervisorNamespace, gcPvcRequestSize.String())
1475-
svPVC, err = c.supervisorClient.CoreV1().PersistentVolumeClaims(c.supervisorNamespace).Update(
1476-
ctx, svPvcClone, metav1.UpdateOptions{})
1527+
svPVC, err = c.supervisorClient.CoreV1().PersistentVolumeClaims(c.supervisorNamespace).Patch(
1528+
ctx, svPVC.Name, types.StrategicMergePatchType, expandPatchBytes, metav1.PatchOptions{})
14771529
if err != nil {
14781530
msg := fmt.Sprintf("failed to update supervisor PVC %q in %q namespace. Error: %+v",
14791531
volumeID, c.supervisorNamespace, err)

‎pkg/kubernetes/kubernetes.go‎

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -832,14 +832,34 @@ func AddFinalizerOnPVC(ctx context.Context, k8sClient clientset.Interface, pvcNa
832832
return err
833833
}
834834

835+
oldData, err := json.Marshal(pvc)
836+
if err != nil {
837+
log.Errorf("Failed to marshal PVC: %v", err)
838+
return err
839+
}
840+
841+
newPVC := pvc.DeepCopy()
835842
// If the finalizer is already present, no action is needed
836-
if !controllerutil.AddFinalizer(pvc, finalizer) {
843+
if !controllerutil.AddFinalizer(newPVC, finalizer) {
837844
log.Info("Finalizer already present on PVC. No action needed.")
838845
return nil
839846
}
840847

848+
newData, err := json.Marshal(newPVC)
849+
if err != nil {
850+
log.Errorf("Failed to marshal updated PVC with finalizer: %v", err)
851+
return err
852+
}
853+
854+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvc)
855+
if err != nil {
856+
log.Errorf("Error creating two way merge patch for PVC %s/%s with error: %v", pvcNamespace, pvcName, err)
857+
return err
858+
}
859+
841860
// Update the PVC with the new finalizer
842-
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, pvc, metav1.UpdateOptions{})
861+
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Patch(ctx, pvc.Name, k8stypes.StrategicMergePatchType,
862+
patchBytes, metav1.PatchOptions{})
843863
if err != nil {
844864
log.Errorf("Failed to add finalizer on PVC. Error: %s", err.Error())
845865
return err
@@ -866,14 +886,34 @@ func RemoveFinalizerFromPVC(ctx context.Context, k8sClient clientset.Interface,
866886
return err
867887
}
868888

889+
oldData, err := json.Marshal(pvc)
890+
if err != nil {
891+
log.Errorf("Failed to marshal PVC: %v", err)
892+
return err
893+
}
894+
895+
newPVC := pvc.DeepCopy()
869896
// If the finalizer is not present, no action is needed
870-
if !controllerutil.RemoveFinalizer(pvc, finalizer) {
897+
if !controllerutil.RemoveFinalizer(newPVC, finalizer) {
871898
log.Info("Finalizer not present on PVC. No action needed.")
872899
return nil
873900
}
874901

902+
newData, err := json.Marshal(newPVC)
903+
if err != nil {
904+
log.Errorf("Failed to marshal updated PVC with finalizer removed: %v", err)
905+
return err
906+
}
907+
908+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvc)
909+
if err != nil {
910+
log.Errorf("Error creating two way merge patch for PVC %s/%s with error: %v", pvcNamespace, pvcName, err)
911+
return err
912+
}
913+
875914
// Update the PVC to remove the finalizer
876-
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Update(ctx, pvc, metav1.UpdateOptions{})
915+
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvcNamespace).Patch(ctx, pvc.Name, k8stypes.StrategicMergePatchType,
916+
patchBytes, metav1.PatchOptions{})
877917
if err != nil {
878918
log.Errorf("Failed to remove finalizer from PVC. Error: %s", err.Error())
879919
return err

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3434
"k8s.io/apimachinery/pkg/runtime"
3535
apitypes "k8s.io/apimachinery/pkg/types"
36+
"k8s.io/apimachinery/pkg/util/strategicpatch"
3637
"k8s.io/client-go/kubernetes/scheme"
3738
typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1"
3839
"k8s.io/client-go/tools/record"
@@ -837,18 +838,38 @@ func validatePVCTopologyCompatibility(ctx context.Context, k8sclient clientset.I
837838
}
838839
topologyAnnotation = "[" + strings.Join(segmentsArray, ",") + "]"
839840

841+
oldData, err := json.Marshal(pvc)
842+
if err != nil {
843+
return logger.LogNewErrorf(log, "failed to marshal PVC %s/%s: %v", pvc.Namespace, pvc.Name, err)
844+
}
845+
846+
// Update the original PVC object's annotations first
847+
// This ensures tests that expect the original PVC object to be modified work correctly
840848
if pvc.Annotations == nil {
841849
pvc.Annotations = make(map[string]string)
842850
}
843851
pvc.Annotations[common.AnnVolumeAccessibleTopology] = topologyAnnotation
844852

853+
newData, err := json.Marshal(pvc)
854+
if err != nil {
855+
return logger.LogNewErrorf(log, "failed to marshal updated PVC %s/%s with topology annotation: %v", pvc.Namespace, pvc.Name, err)
856+
}
857+
858+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvc)
859+
if err != nil {
860+
return logger.LogNewErrorf(log, "error creating two way merge patch for PVC %s/%s: %v", pvc.Namespace, pvc.Name, err)
861+
}
862+
845863
// Update the PVC in Kubernetes to persist the topology annotation
846-
_, err := k8sclient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Update(ctx, pvc, metav1.UpdateOptions{})
864+
updatedPVC, err := k8sclient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch(ctx, pvc.Name, apitypes.StrategicMergePatchType, patchBytes, metav1.PatchOptions{})
847865
if err != nil {
848866
return logger.LogNewErrorf(log, "failed to update PVC %s/%s with topology annotation: %+v",
849867
pvc.Namespace, pvc.Name, err)
850868
}
851869

870+
// Update the PVC's resource version to match the patched PVC
871+
pvc.ResourceVersion = updatedPVC.ResourceVersion
872+
852873
log.Infof("Successfully added topology annotation %s to PVC %s/%s",
853874
topologyAnnotation, pvc.Namespace, pvc.Name)
854875
// Return nil as we just added the topology annotation based on actual volume placement

‎pkg/syncer/fullsync.go‎

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,17 @@ func setFileShareAnnotationsOnPVC(ctx context.Context, k8sClient clientset.Inter
683683
pv.Spec.CSI.VolumeHandle, err)
684684
return err
685685
}
686+
oldData, err := json.Marshal(pvc)
687+
if err != nil {
688+
log.Errorf("setFileShareAnnotationsOnPVC: Failed to marshal PVC %q in namespace %q: %v", pvc.Name, pvc.Namespace, err)
689+
return err
690+
}
691+
686692
vSANFileBackingDetails := volume.BackingObjectDetails.(*cnstypes.CnsVsanFileShareBackingDetails)
687693
accessPoints := make(map[string]string)
694+
695+
// Update the original PVC object's annotations first
696+
// This ensures tests that expect the original PVC object to be modified work correctly
688697
for _, kv := range vSANFileBackingDetails.AccessPoints {
689698
if kv.Key == common.Nfsv3AccessPointKey {
690699
pvc.Annotations[common.Nfsv3ExportPathAnnotationKey] = kv.Value
@@ -696,14 +705,30 @@ func setFileShareAnnotationsOnPVC(ctx context.Context, k8sClient clientset.Inter
696705
log.Debugf("setFileShareAnnotationsOnPVC: Access point details for PVC: %q, namespace: %q are %+v",
697706
pvc.Name, pvc.Namespace, accessPoints)
698707

708+
newData, err := json.Marshal(pvc)
709+
if err != nil {
710+
log.Errorf("setFileShareAnnotationsOnPVC: Failed to marshal updated PVC %q in namespace %q with annotations: %v", pvc.Name, pvc.Namespace, err)
711+
return err
712+
}
713+
714+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvc)
715+
if err != nil {
716+
log.Errorf("setFileShareAnnotationsOnPVC: Error creating two way merge patch for PVC %q in namespace %q with error: %v", pvc.Name, pvc.Namespace, err)
717+
return err
718+
}
719+
699720
// Update PVC to add annotation on it
700-
pvc, err = k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Update(ctx, pvc,
701-
metav1.UpdateOptions{})
721+
updatedPVC, err := k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch(ctx, pvc.Name, types.StrategicMergePatchType,
722+
patchBytes, metav1.PatchOptions{})
702723
if err != nil {
703724
log.Errorf("setFileShareAnnotationsOnPVC: Error updating PVC %q in namespace %q, Err: %v",
704725
pvc.Name, pvc.Namespace, err)
705726
return err
706727
}
728+
729+
// Update the PVC's resource version to match the patched PVC
730+
pvc.ResourceVersion = updatedPVC.ResourceVersion
731+
707732
log.Infof("setFileShareAnnotationsOnPVC: Added file share export paths annotation successfully on PVC %q, "+
708733
"namespce %q", pvc.Name, pvc.Namespace)
709734
return nil
@@ -1727,9 +1752,33 @@ func RemoveCNSFinalizerFromPVCIfTKGClusterDeleted(ctx context.Context, k8sClient
17271752
log.Infof("RemoveCNSFinalizerFromPVCIfTKGClusterDeleted: Removing %q finalizer from PVC "+
17281753
"with name: %q on namespace: %q in Terminating state",
17291754
cnsoperatortypes.CNSVolumeFinalizer, pvc.Name, pvc.Namespace)
1730-
controllerutil.RemoveFinalizer(pvc, finalizerToRemove)
1731-
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Update(ctx,
1732-
pvc, metav1.UpdateOptions{})
1755+
1756+
oldData, err := json.Marshal(pvc)
1757+
if err != nil {
1758+
msg := fmt.Sprintf("RemoveCNSFinalizerFromPVCIfTKGClusterDeleted: Failed to marshal PVC %q in namespace %q: %v", pvc.Name, pvc.Namespace, err)
1759+
log.Error(msg)
1760+
return
1761+
}
1762+
1763+
newPVC := pvc.DeepCopy()
1764+
controllerutil.RemoveFinalizer(newPVC, finalizerToRemove)
1765+
1766+
newData, err := json.Marshal(newPVC)
1767+
if err != nil {
1768+
msg := fmt.Sprintf("RemoveCNSFinalizerFromPVCIfTKGClusterDeleted: Failed to marshal updated PVC %q in namespace %q with finalizer removed: %v", pvc.Name, pvc.Namespace, err)
1769+
log.Error(msg)
1770+
return
1771+
}
1772+
1773+
patchBytes, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, pvc)
1774+
if err != nil {
1775+
msg := fmt.Sprintf("RemoveCNSFinalizerFromPVCIfTKGClusterDeleted: Error creating two way merge patch for PVC %q in namespace %q with error: %v", pvc.Name, pvc.Namespace, err)
1776+
log.Error(msg)
1777+
return
1778+
}
1779+
1780+
_, err = k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch(ctx, pvc.Name, types.StrategicMergePatchType,
1781+
patchBytes, metav1.PatchOptions{})
17331782
if err != nil {
17341783
msg := fmt.Sprintf("RemoveCNSFinalizerFromPVCIfTKGClusterDeleted: failed to update "+
17351784
"supervisor PVC %q in %q namespace. Err: %+v", pvc.Name, pvc.Namespace, err)

0 commit comments

Comments
 (0)