Merge pull request kubernetes-sigs#3798 from skogta/topic/skogta/pvcBluePrint

Update PVC annotation with backing disk type for blueprint PVCs.

Author: divyenpatel

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

@@ -669,6 +669,8 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
 		return reconcile.Result{RequeueAfter: timeout}, nil
 	}
 
+	IsPVCCreatedByCNSRegisterVolumeReconciler := false
+
 	if pvc != nil {
 		if pvc.Status.Phase == v1.ClaimBound && pvc.Spec.VolumeName != pvName {
 			// This is handle cases where PVC with this name already exists and
@@ -730,6 +732,7 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
 			return reconcile.Result{RequeueAfter: timeout}, nil
 		} else {
 			log.Infof("PVC: %s is created successfully", instance.Spec.PvcName)
+			IsPVCCreatedByCNSRegisterVolumeReconciler = true
 		}
 	}
 	// Watch for PVC to be bound.
@@ -839,6 +842,18 @@ func (r *ReconcileCnsRegisterVolume) Reconcile(ctx context.Context,
 		return reconcile.Result{RequeueAfter: timeout}, nil
 	}
 
+	if isSharedDiskEnabled && !IsPVCCreatedByCNSRegisterVolumeReconciler {
+		// Ensure that the PVC has the correct backing disk annotation if PVC was already present on the cluster.
+		if pvc != nil {
+			pvc, err = setBackingDiskAnnotation(ctx, k8sclient, instance, pvc)
+			if err != nil {
+				msg := fmt.Sprintf("failed to update bakcing disk type on PVC. Error: %s", err)
+				setInstanceError(ctx, r, instance, msg)
+				return reconcile.Result{RequeueAfter: timeout}, nil
+			}
+		}
+	}
+
 	// Update the instance to indicate the volume registration is successful.
 	msg := fmt.Sprintf("Successfully registered the volume on namespace: %s", instance.Namespace)
 	err = setInstanceSuccess(ctx, r, instance, instance.Spec.PvcName, pvc.UID, msg)

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

@@ -41,6 +41,7 @@ import (
 	k8sfake "k8s.io/client-go/kubernetes/fake"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	restclient "k8s.io/client-go/rest"
+	k8stesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -2266,3 +2267,108 @@ var _ = Describe("patchCnsRegisterVolumeStatus Tests", func() {
 		})
 	})
 })
+
+func TestSetBackingDiskAnnotationNoChangeNeeded(t *testing.T) {
+	ctx := context.Background()
+
+	pvc := &corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-pvc",
+			Namespace: "default",
+			Annotations: map[string]string{
+				common.AnnKeyBackingDiskType: "backing-disk-type-1",
+			},
+		},
+	}
+
+	instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
+		Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
+			BackingType: "backing-disk-type-1",
+		},
+	}
+
+	client := k8sfake.NewSimpleClientset(pvc)
+
+	out, err := setBackingDiskAnnotation(ctx, client, instance, pvc)
+
+	assert.NoError(t, err)
+	assert.Equal(t, "backing-disk-type-1", out.Annotations[common.AnnKeyBackingDiskType])
+
+	// No patch should happen
+	actions := client.Actions()
+	assert.Len(t, actions, 0)
+}
+
+func TestSetBackingDiskAnnotationUpdateRequired(t *testing.T) {
+	ctx := context.Background()
+
+	pvc := &corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        "test-pvc",
+			Namespace:   "default",
+			Annotations: map[string]string{common.AnnKeyBackingDiskType: "backing-disk-type-1"},
+		},
+	}
+
+	instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
+		Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
+			BackingType: "backing-disk-type-2",
+		},
+	}
+
+	client := k8sfake.NewSimpleClientset(pvc)
+
+	out, err := setBackingDiskAnnotation(ctx, client, instance, pvc)
+	assert.NoError(t, err)
+	assert.NotNil(t, out)
+
+	actions := client.Actions()
+	assert.Len(t, actions, 1)
+
+	patchAction, ok := actions[0].(k8stesting.PatchAction)
+	assert.True(t, ok, "expected PatchAction")
+
+	assert.Equal(t, "persistentvolumeclaims", patchAction.GetResource().Resource)
+	assert.Equal(t, "test-pvc", patchAction.GetName())
+
+	var patch map[string]map[string]map[string]string
+	err = json.Unmarshal(patchAction.GetPatch(), &patch)
+	assert.NoError(t, err)
+
+	assert.Equal(t, "backing-disk-type-2", patch["metadata"]["annotations"][common.AnnKeyBackingDiskType])
+}
+
+func TestSetBackingDiskAnnotationNoAnnotationsInitially(t *testing.T) {
+	ctx := context.Background()
+
+	pvc := &corev1.PersistentVolumeClaim{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-pvc",
+			Namespace: "default",
+		},
+	}
+
+	instance := &cnsregistervolumev1alpha1.CnsRegisterVolume{
+		Spec: cnsregistervolumev1alpha1.CnsRegisterVolumeSpec{
+			BackingType: "backing-disk-type-1",
+		},
+	}
+
+	client := k8sfake.NewSimpleClientset(pvc)
+
+	out, err := setBackingDiskAnnotation(ctx, client, instance, pvc)
+	assert.NoError(t, err)
+	assert.NotNil(t, out)
+
+	actions := client.Actions()
+	assert.Len(t, actions, 1)
+
+	patchAction, ok := actions[0].(k8stesting.PatchAction)
+	assert.True(t, ok)
+
+	var patch map[string]map[string]map[string]string
+	err = json.Unmarshal(patchAction.GetPatch(), &patch)
+	assert.NoError(t, err)
+
+	assert.Equal(t, "backing-disk-type-1", patch["metadata"]["annotations"][common.AnnKeyBackingDiskType])
+}

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

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/types"
 	clientset "k8s.io/client-go/kubernetes"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	cnsregistervolumev1alpha1 "sigs.k8s.io/vsphere-csi-driver/v3/pkg/apis/cnsoperator/cnsregistervolume/v1alpha1"
@@ -391,3 +392,71 @@ func isPVCBound(ctx context.Context, client clientset.Interface, claim *v1.Persi
 	return false, fmt.Errorf("persistentVolumeClaim %s in namespace %s not in phase %s within %d seconds",
 		pvcName, ns, v1.ClaimBound, timeoutSeconds)
 }
+
+// setBackingDiskAnnotation checks if the backing disk annotation on the PVC matches
+// the backing disk type mentioned on CnsRegisterVolume instance.
+func setBackingDiskAnnotation(ctx context.Context, k8sClient clientset.Interface,
+	instance *cnsregistervolumev1alpha1.CnsRegisterVolume,
+	pvc *v1.PersistentVolumeClaim) (*v1.PersistentVolumeClaim, error) {
+	log := logger.GetLogger(ctx)
+	log.Infof("Checking BackingType for PVC %s.", pvc.Name)
+
+	if instance.Spec.BackingType == "" {
+		log.Infof("BackingType on instance %s is empty.", instance.Name)
+		return pvc, nil
+	}
+
+	if pvc.Annotations == nil {
+		pvc.Annotations = make(map[string]string)
+	}
+
+	backingDiskType, backingDiskAnnExists := pvc.Annotations[common.AnnKeyBackingDiskType]
+	if backingDiskAnnExists {
+		if backingDiskType == instance.Spec.BackingType {
+			log.Infof("BackingType for PVC %s is up to date. Skip updating.", pvc.Name)
+			return pvc, nil
+		}
+	}
+
+	log.Infof("Updating BackingType for PVC %s to %s",
+		pvc.Name, instance.Spec.BackingType)
+
+	currentPvcAnnotations := pvc.Annotations
+
+	patchAnnotations := make(map[string]interface{})
+	for k, v := range currentPvcAnnotations {
+		patchAnnotations[k] = v
+	}
+
+	backingDiskTypeOnSpec := instance.Spec.BackingType
+	patchAnnotations[common.AnnKeyBackingDiskType] = backingDiskTypeOnSpec
+
+	// Build patch structure
+	patch := map[string]interface{}{
+		"metadata": map[string]interface{}{
+			"annotations": patchAnnotations,
+		},
+	}
+	patchBytes, err := json.Marshal(patch)
+	if err != nil {
+		log.Errorf("failed to marshal with annotations for PVC %s. Err: %s", pvc.Name, err)
+		return pvc, fmt.Errorf("failed to marshal patch: %v", err)
+	}
+
+	log.Infof("Patching PVC %s with updated annotation", pvc.Name)
+
+	// Apply the patch
+	updatedpvc, err := k8sClient.CoreV1().PersistentVolumeClaims(pvc.Namespace).Patch(
+		ctx,
+		pvc.Name,
+		types.MergePatchType,
+		patchBytes,
+		metav1.PatchOptions{},
+	)
+	if err != nil {
+		log.Errorf("failed to patch PVC %s with annotations. Err: %s", pvc.Name, err)
+		return pvc, fmt.Errorf("failed to patch PVC %s: %v", pvc.Name, err)
+	}
+
+	return updatedpvc, nil
+}