[cherry-pick-release-2.4] protect migrated volume from vm deletion (#1762)

Makefile

@@ -146,7 +146,7 @@ ifeq (, $(shell which controller-gen))
 	CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
 	cd $$CONTROLLER_GEN_TMP_DIR ;\
 	go mod init tmp ;\
-	go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.2 ;\
+	go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.2 ;\
 	rm -rf $$CONTROLLER_GEN_TMP_DIR ;\
 	}
 CONTROLLER_GEN=$(GOBIN)/controller-gen

go.mod

@@ -25,7 +25,7 @@ require (
 	github.com/stretchr/testify v1.7.0
 	github.com/thecodeteam/gofsutil v0.1.2 // indirect
 	github.com/vmware-tanzu/vm-operator-api v0.1.3
-	github.com/vmware/govmomi v0.27.0
+	github.com/vmware/govmomi v0.27.5
 	go.uber.org/zap v1.17.0
 	golang.org/x/crypto v0.0.0-20210220033148-5ea612d1eb83
 	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect

go.sum

@@ -691,10 +691,8 @@ github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae/go.mod h1:DD4vA1
 github.com/vmware-tanzu/vm-operator-api v0.1.3 h1:4vxewu0jAN3fSoCBI6FhjmRGJ7ci0R2WNu/I6hacTYs=
 github.com/vmware-tanzu/vm-operator-api v0.1.3/go.mod h1:mubK0QMyaA2TbeAmGsu2GVfiqDFppNUAUqoMPoKFgzM=
 github.com/vmware/govmomi v0.20.3/go.mod h1:URlwyTFZX72RmxtxuaFL2Uj3fD1JTvZdx59bHWk6aFU=
-github.com/vmware/govmomi v0.26.2-0.20210907233321-5196d831d88e h1:irZ+ONb0L2RGUxFkwt/5BAKxb2nvnP+w+U+6/asl4Qg=
-github.com/vmware/govmomi v0.26.2-0.20210907233321-5196d831d88e/go.mod h1:daTuJEcQosNMXYJOeku0qdBJP9SOLLWB3Mqz8THtv6o=
-github.com/vmware/govmomi v0.27.0 h1:KoQ8IsLAa7V78s5d7dgpZA8d039GBM83cVxgAq9uWuw=
-github.com/vmware/govmomi v0.27.0/go.mod h1:daTuJEcQosNMXYJOeku0qdBJP9SOLLWB3Mqz8THtv6o=
+github.com/vmware/govmomi v0.27.5 h1:/F8S4YfsLHy8J2e1X3rIl/Lup7R67zjTW9bXLv/q3ic=
+github.com/vmware/govmomi v0.27.5/go.mod h1:daTuJEcQosNMXYJOeku0qdBJP9SOLLWB3Mqz8THtv6o=
 github.com/vmware/vmw-guestinfo v0.0.0-20170707015358-25eff159a728/go.mod h1:x9oS4Wk2s2u4tS29nEaDLdzvuHdB19CvSGJjPgkZJNk=
 github.com/willf/bitset v1.1.11/go.mod h1:83CECat5yLh5zVOf4P1ErAgKA5UDvKtgyUABdr3+MjI=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2 h1:eY9dn8+vbi4tKz5Qo6v2eYzo7kUS51QINcR5jNpbZS8=

hack/check-staticcheck.sh

@@ -22,7 +22,7 @@ set -o pipefail
 # script is located.
 cd "$(dirname "${BASH_SOURCE[0]}")/.."
 
-go get honnef.co/go/tools/cmd/staticcheck@v0.2.0
+go install honnef.co/go/tools/cmd/staticcheck@2022.1.1
 
 # shellcheck disable=SC2046
 # shellcheck disable=SC1083

pkg/apis/migration/config/cns.vmware.com_cnsvspherevolumemigrations.yaml

@@ -45,6 +45,9 @@ spec:
               volumepath:
                 description: VolumePath is the vmdk path of the vSphere Volume
                 type: string
+              protectvolumefromvmdelete:
+                description: protect volume from vm deletion after vmdk is migrated to CSI
+                type: boolean
             required:
             - volumeid
             - volumepath

pkg/apis/migration/migration.go

@@ -73,6 +73,10 @@ type VolumeMigrationService interface {
 	// DeleteVolumeInfo helps delete mapping of volumePath to VolumeID for
 	// specified volumeID.
 	DeleteVolumeInfo(ctx context.Context, volumeID string) error
+
+	// ProtectVolumeFromVMDeletion sets keepAfterDeleteVm control flag on the migrated volume
+	// Returns an error if not able to set keepAfterDeleteVm control flag on the volume
+	ProtectVolumeFromVMDeletion(ctx context.Context, volumeID string) error
 }
 
 // volumeMigration holds migrated volume information and provides functionality
@@ -235,6 +239,39 @@ func (volumeMigration *volumeMigration) GetVolumeID(ctx context.Context, volumeS
 	return "", ErrVolumeIDNotFound
 }
 
+// ProtectVolumeFromVMDeletion sets keepAfterDeleteVm control flag on the migrated volume
+// Returns an error if not able to set keepAfterDeleteVm control flag on the volume
+func (volumeMigration *volumeMigration) ProtectVolumeFromVMDeletion(ctx context.Context, volumeID string) error {
+	log := logger.GetLogger(ctx)
+	volumeMigrationResource := &migrationv1alpha1.CnsVSphereVolumeMigration{}
+	var err error
+	err = volumeMigration.k8sClient.Get(ctx, client.ObjectKey{Name: volumeID}, volumeMigrationResource)
+	if err != nil {
+		log.Errorf("error while getting CnsVSphereVolumeMigration CR for VolumeID: %q, err: %v", volumeID, err)
+		return err
+	}
+	if !volumeMigrationResource.Spec.ProtectVolumeFromVMDelete {
+		log.Info("Set keepAfterDeleteVm control flag using Vslm APIs")
+		err = (*volumeMigration.volumeManager).ProtectVolumeFromVMDeletion(ctx, volumeID)
+		if err != nil {
+			log.Errorf("failed to protect migrated volume from vm deletion. Volume ID: %q, err: %v", volumeID, err)
+			return err
+		}
+		log.Infof("Migrated Volume with ID: %q is protected from VM deletion", volumeID)
+	} else {
+		log.Infof("Migrated volume with ID: %q is already protected from vm deletion", volumeID)
+		return nil
+	}
+	volumeMigrationResource.Spec.ProtectVolumeFromVMDelete = true
+	err = volumeMigration.k8sClient.Update(ctx, volumeMigrationResource)
+	if err != nil {
+		log.Errorf("error happened while updating volumeMigration CR to set ProtectVolumeFromVMDelete true "+
+			"for VolumeID: %q, err: %v", volumeID, err)
+		return err
+	}
+	return nil
+}
+
 // GetVolumePath returns VolumePath for given VolumeID.
 // Returns an error if not able to retrieve VolumePath.
 func (volumeMigration *volumeMigration) GetVolumePath(ctx context.Context, volumeID string) (string, error) {
@@ -255,7 +292,7 @@ func (volumeMigration *volumeMigration) GetVolumePath(ctx context.Context, volum
 	err := volumeMigration.k8sClient.Get(ctx, client.ObjectKey{Name: volumeID}, volumeMigrationResource)
 	if err != nil {
 		if !apierrors.IsNotFound(err) {
-			log.Errorf("error happened while getting CR for volumeMigration for VolumeID: %q, err: %v", volumeID, err)
+			log.Errorf("error while getting CnsVSphereVolumeMigration CR for VolumeID: %q, err: %v", volumeID, err)
 			return "", err
 		}
 	} else {

pkg/apis/migration/v1alpha1/types.go

@@ -37,6 +37,8 @@ type CnsVSphereVolumeMigrationSpec struct {
 	VolumePath string `json:"volumepath"`
 	// VolumeID is the FCD ID obtained after register volume with CNS.
 	VolumeID string `json:"volumeid"`
+	// ProtectVolumeFromVMDelete true means migrated volumes is protected from Node VM deletion
+	ProtectVolumeFromVMDelete bool `json:"protectvolumefromvmdelete"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/storagepool/config/cns.vmware.com_storagepools.yaml

@@ -1,10 +1,10 @@
 
 ---
-apiVersion: apiextensions.k8s.io/v1
+apiVersion: apiextensions.k8s.io/v1beta1
 kind: CustomResourceDefinition
 metadata:
   annotations:
-    controller-gen.kubebuilder.io/version: v0.6.2
+    controller-gen.kubebuilder.io/version: v0.2.5
   creationTimestamp: null
   name: storagepools.cns.vmware.com
 spec:
@@ -15,92 +15,93 @@ spec:
     plural: storagepools
     singular: storagepool
   scope: Cluster
-  versions:
-  - name: v1alpha1
-    schema:
-      openAPIV3Schema:
-        description: StoragePool is the Schema for the storagepools API
-        properties:
-          apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
-            type: string
-          kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
-            type: string
-          metadata:
-            type: object
-          spec:
-            description: StoragePoolSpec defines the desired state of StoragePool
-            properties:
-              driver:
-                description: Name of the driver
+  validation:
+    openAPIV3Schema:
+      description: StoragePool is the Schema for the storagepools API
+      properties:
+        apiVersion:
+          description: 'APIVersion defines the versioned schema of this representation
+            of an object. Servers should convert recognized schemas to the latest
+            internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
+          type: string
+        kind:
+          description: 'Kind is a string value representing the REST resource this
+            object represents. Servers may infer this from the endpoint the client
+            submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
+          type: string
+        metadata:
+          type: object
+        spec:
+          description: StoragePoolSpec defines the desired state of StoragePool
+          properties:
+            driver:
+              description: Name of the driver
+              type: string
+            parameters:
+              additionalProperties:
                 type: string
-              parameters:
-                additionalProperties:
-                  type: string
-                description: Opaque parameters describing attributes of the storage
-                  pool
-                type: object
-            required:
-            - driver
-            type: object
-          status:
-            description: StoragePoolStatus defines the observed state of StoragePool
-            properties:
-              accessibleNodes:
-                description: Nodes the storage pool has access to
-                items:
+              description: Opaque parameters describing attributes of the storage
+                pool
+              type: object
+          required:
+          - driver
+          type: object
+        status:
+          description: StoragePoolStatus defines the observed state of StoragePool
+          properties:
+            accessibleNodes:
+              description: Nodes the storage pool has access to
+              items:
+                type: string
+              type: array
+            capacity:
+              description: Total Capacity of the storage pool
+              properties:
+                allocatableSpace:
+                  anyOf:
+                  - type: integer
+                  - type: string
+                  description: allocatable capacity of storage pool
+                  pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                  x-kubernetes-int-or-string: true
+                freeSpace:
+                  anyOf:
+                  - type: integer
+                  - type: string
+                  description: Free Space of the storage pool
+                  pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                  x-kubernetes-int-or-string: true
+                total:
+                  anyOf:
+                  - type: integer
+                  - type: string
+                  description: Total capacity of the storage pool
+                  pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
+                  x-kubernetes-int-or-string: true
+              type: object
+            compatibleStorageClasses:
+              description: StorageClasses that can be used with this storage pool
+              items:
+                type: string
+              type: array
+            error:
+              description: Error that has occurred on the storage pool. Present only
+                when there is an error.
+              properties:
+                message:
+                  description: Message details of the encountered error
                   type: string
-                type: array
-              capacity:
-                description: Total Capacity of the storage pool
-                properties:
-                  allocatableSpace:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: allocatable capacity of storage pool
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                  freeSpace:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: Free Space of the storage pool
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                  total:
-                    anyOf:
-                    - type: integer
-                    - type: string
-                    description: Total capacity of the storage pool
-                    pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
-                    x-kubernetes-int-or-string: true
-                type: object
-              compatibleStorageClasses:
-                description: StorageClasses that can be used with this storage pool
-                items:
+                state:
+                  description: State indicates a single word description of the error
+                    state that has occurred on the StoragePool, "InMaintenance", "NotAccessible",
+                    etc.
                   type: string
-                type: array
-              error:
-                description: Error that has occurred on the storage pool. Present
-                  only when there is an error.
-                properties:
-                  message:
-                    description: Message details of the encountered error
-                    type: string
-                  state:
-                    description: State indicates a single word description of the
-                      error state that has occurred on the StoragePool, "InMaintenance",
-                      "NotAccessible", etc.
-                    type: string
-                type: object
-            type: object
-        type: object
+              type: object
+          type: object
+      type: object
+  version: v1alpha1
+  versions:
+  - name: v1alpha1
     served: true
     storage: true
 status:

pkg/common/cns-lib/volume/manager.go

@@ -108,6 +108,8 @@ type Manager interface {
 	RegisterDisk(ctx context.Context, path string, name string) (string, error)
 	// RetrieveVStorageObject helps in retreiving virtual disk information for a given volume id.
 	RetrieveVStorageObject(ctx context.Context, volumeID string) (*vim25types.VStorageObject, error)
+	// ProtectVolumeFromVMDeletion sets keepAfterDeleteVm control flag on migrated volume
+	ProtectVolumeFromVMDeletion(ctx context.Context, volumeID string) error
 	// CreateSnapshot helps create a snapshot for a block volume
 	CreateSnapshot(ctx context.Context, volumeID string, desc string) (*CnsSnapshotInfo, error)
 	// DeleteSnapshot helps delete a snapshot for a block volume
@@ -2185,3 +2187,28 @@ func (m *defaultManager) DeleteSnapshot(ctx context.Context, volumeID string, sn
 	}
 	return err
 }
+
+// ProtectVolumeFromVMDeletion helps set keepAfterDeleteVm control flag for given volumeID
+func (m *defaultManager) ProtectVolumeFromVMDeletion(ctx context.Context, volumeID string) error {
+	log := logger.GetLogger(ctx)
+	err := validateManager(ctx, m)
+	if err != nil {
+		log.Errorf("failed to validate volume manager with err: %+v", err)
+		return err
+	}
+	// Set up the VC connection
+	err = m.virtualCenter.ConnectVslm(ctx)
+	if err != nil {
+		log.Errorf("ConnectVslm failed with err: %+v", err)
+		return err
+	}
+	globalObjectManager := vslm.NewGlobalObjectManager(m.virtualCenter.VslmClient)
+	err = globalObjectManager.SetControlFlags(ctx, vim25types.ID{Id: volumeID}, []string{
+		string(vim25types.VslmVStorageObjectControlFlagKeepAfterDeleteVm)})
+	if err != nil {
+		log.Errorf("failed to set control flag keepAfterDeleteVm  for volumeID %q with err: %v", volumeID, err)
+		return err
+	}
+	log.Infof("Successfully set keepAfterDeleteVm control flag for volumeID: %q", volumeID)
+	return nil
+}

pkg/csi/service/vanilla/controller.go

@@ -1026,6 +1026,11 @@ func (c *controller) ControllerPublishVolume(ctx context.Context, req *csi.Contr
 					return nil, csifault.CSIInternalFault, logger.LogNewErrorCodef(log, codes.Internal,
 						"failed to get VolumeID from volumeMigrationService for volumePath: %q", volumePath)
 				}
+				err = volumeMigrationService.ProtectVolumeFromVMDeletion(ctx, req.VolumeId)
+				if err != nil {
+					return nil, csifault.CSIInternalFault, logger.LogNewErrorCodef(log, codes.Internal,
+						"failed to set keepAfterDeleteVm control flag for VolumeID %q", req.VolumeId)
+				}
 			}
 			node, err := c.nodeMgr.GetNodeByName(ctx, req.NodeId)
 			if err != nil {