issue-1256 fixing drive removal

api/v1/constants.go

@@ -69,6 +69,9 @@ const (
 	DriveAnnotationVolumeStatusPrefix = "status"
 	// Deprecated annotations
 	DriveAnnotationReplacement = "replacement"
+	// Disk auto eject
+	DriveAnnotationEject     = "eject"
+	DriveAnnotationEjectAuto = "auto"
 
 	// Volume operational status
 	OperationalStatusOperative   = "OPERATIVE"

pkg/crcontrollers/drive/drivecontroller.go

@@ -183,7 +183,7 @@ func (c *Controller) handleDriveUpdate(ctx context.Context, log *logrus.Entry, d
 
 		value, foundAllDRVolFakeAttach := drive.Annotations[allDRVolumesFakeAttachedAnnotation]
 		fakeAttachDR := !drive.Spec.IsClean && foundAllDRVolFakeAttach && value == allDRVolumesFakeAttachedKey
-		if drive.Spec.IsClean || fakeAttachDR {
+		if (drive.Spec.IsClean || fakeAttachDR) && c.driveEjectAuto(drive.Annotations) {
 			log.Infof("Initiating automatic removal of drive: %s", drive.GetName())
 		} else {
 			status, found := getDriveAnnotationRemoval(drive.Annotations)
@@ -607,3 +607,8 @@ func (c *Controller) handleDriveLableUpdate(ctx context.Context, log *logrus.Ent
 	log.Infof("Update AC %s labels to related drive %s successful", ac.GetName(), drive.GetName())
 	return nil
 }
+
+func (c *Controller) driveEjectAuto(annotations map[string]string) bool {
+	status, found := annotations[apiV1.DriveAnnotationEject]
+	return found && status == apiV1.DriveAnnotationEjectAuto
+}

pkg/crcontrollers/drive/drivecontroller_test.go

@@ -425,21 +425,45 @@ func TestDriveController_handleDriveUpdate(t *testing.T) {
 		assert.Nil(t, dc.client.DeleteCR(k8s.ListFailCtx, expectedD))
 		assert.Nil(t, dc.client.DeleteCR(k8s.ListFailCtx, expectedV))
 	})
-	t.Run("ReleasedUsage - transition to Removing state", func(t *testing.T) {
+	t.Run("ReleasedUsage - auto transition to Removing state", func(t *testing.T) {
 		expectedD := testBadCRDrive.DeepCopy()
 		assert.NotNil(t, expectedD)
 		expectedD.Spec.Usage = apiV1.DriveUsageReleased
 		expectedD.Spec.IsClean = true
+		expectedD.Annotations = map[string]string{"eject": "auto"}
+
 		assert.Nil(t, dc.client.CreateCR(testCtx, expectedD.Name, expectedD))
 
 		res, err := dc.handleDriveUpdate(testCtx, dc.log, expectedD)
 		assert.NotNil(t, res)
 		assert.Nil(t, err)
 		assert.Equal(t, res, update)
+		assert.Equal(t, expectedD.Spec.Usage, apiV1.DriveUsageRemoving)
+
+		assert.Nil(t, dc.client.DeleteCR(testCtx, expectedD))
+	})
+	t.Run("ReleasedUsage - manual transition to Removing state", func(t *testing.T) {
+		expectedD := testBadCRDrive.DeepCopy()
+		assert.NotNil(t, expectedD)
+		expectedD.Spec.Usage = apiV1.DriveUsageRemoving
+		expectedD.Spec.IsClean = true
+
+		assert.Nil(t, dc.client.CreateCR(testCtx, expectedD.Name, expectedD))
+
+		expectedV := failedVolCR.DeepCopy()
+		assert.NotNil(t, expectedV)
+		expectedV.Spec.CSIStatus = apiV1.Created
+		assert.Nil(t, dc.client.CreateCR(testCtx, expectedV.Name, expectedV))
+
+		res, err := dc.handleDriveUpdate(testCtx, dc.log, expectedD)
+		assert.NotNil(t, res)
+		assert.Nil(t, err)
+
 		assert.Equal(t, expectedD.Spec.Usage, apiV1.DriveUsageRemoving)
 		assert.Empty(t, expectedD.Annotations)
 
 		assert.Nil(t, dc.client.DeleteCR(testCtx, expectedD))
+		assert.Nil(t, dc.client.DeleteCR(testCtx, expectedV))
 	})
 	t.Run("ReleasedUsage - blocked transition to Removing state", func(t *testing.T) {
 		expectedD := testBadCRDrive.DeepCopy()
@@ -463,12 +487,12 @@ func TestDriveController_handleDriveUpdate(t *testing.T) {
 		assert.Nil(t, dc.client.DeleteCR(testCtx, expectedD))
 		assert.Nil(t, dc.client.DeleteCR(testCtx, expectedV))
 	})
-	t.Run("ReleasedUsage - has all-volumes-fake-attached annotation", func(t *testing.T) {
+	t.Run("ReleasedUsage - auto removal has all-volumes-fake-attached annotation", func(t *testing.T) {
 		expectedD := testBadCRDrive.DeepCopy()
 		assert.NotNil(t, expectedD)
 		expectedD.Spec.Usage = apiV1.DriveUsageReleased
 		expectedD.Spec.IsClean = false
-		expectedD.Annotations = map[string]string{allDRVolumesFakeAttachedAnnotation: allDRVolumesFakeAttachedKey}
+		expectedD.Annotations = map[string]string{allDRVolumesFakeAttachedAnnotation: allDRVolumesFakeAttachedKey, "eject": "auto"}
 		assert.Nil(t, dc.client.CreateCR(testCtx, expectedD.Name, expectedD))
 
 		expectedV := failedVolCR.DeepCopy()
@@ -1407,3 +1431,28 @@ func TestCheckLVGVolumeWithoutFakeAttachRemoved(t *testing.T) {
 		})
 	}
 }
+
+func TestDriveEjectAuto(t *testing.T) {
+	c := &Controller{}
+
+	// Test case when annotation is not found
+	annotations := make(map[string]string)
+	result := c.driveEjectAuto(annotations)
+	if result {
+		t.Errorf("Expected false, but got true")
+	}
+
+	// Test case when annotation is found and its value is not 'auto'
+	annotations[apiV1.DriveAnnotationEject] = "manual"
+	result = c.driveEjectAuto(annotations)
+	if result {
+		t.Errorf("Expected false, but got true")
+	}
+
+	// Test case when annotation is found and its value is 'auto'
+	annotations[apiV1.DriveAnnotationEject] = apiV1.DriveAnnotationEjectAuto
+	result = c.driveEjectAuto(annotations)
+	if !result {
+		t.Errorf("Expected true, but got false")
+	}
+}

tests/e2e-test-framework/framework/const.py

@@ -37,11 +37,13 @@
 DRIVE_HEALTH_ANNOTATION = "health"
 VOLUME_RELEASE_ANNOTATION = "release"
 FAKE_ATTACH_PVC_ANNOTATION_KEY = "pv.attach.kubernetes.io/ignore-if-inaccessible"
+DRIVE_EJECT = "eject"
 
 # annotation values
 DRIVE_HEALTH_BAD_ANNOTATION = "BAD"
 VOLUME_RELEASE_DONE_VALUE = "done"
 FAKE_ATTACH_PVC_ANNOTATION_VALUE = "yes"
+DRIVE_EJECT_AUTO = "auto"
 
 # health
 HEALTH_GOOD = "GOOD"

tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes.py

@@ -56,6 +56,13 @@ def test_5921_auto_drive_replacement_with_multiple_volumes_per_pod(self):
         # 2. simulate drive failure. Annotate drive used by pod with health=BAD
         for drive in drives:
             drive_name = drive["metadata"]["name"]
+            # mark drive as auto eject
+            self.utils.annotate_custom_resource(
+                resource_name=drive_name,
+                resource_type="drives",
+                annotation_key=const.DRIVE_EJECT,
+                annotation_value=const.DRIVE_EJECT_AUTO,
+            )
             self.utils.annotate_custom_resource(
                 resource_name=drive_name,
                 resource_type="drives",

tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes_single_fail.py

@@ -60,6 +60,13 @@ def test_5955_auto_drive_replacement_with_multiple_volumes_per_pod_single_failur
         failed_volume = volumes[0]
         # 2. simulate drive failure. Annotate drive used by pod with health=BAD
         failed_drive_name = failed_drive["metadata"]["name"]
+        # mark drive as auto eject
+        self.utils.annotate_custom_resource(
+            resource_name=failed_drive_name,
+            resource_type="drives",
+            annotation_key=const.DRIVE_EJECT,
+            annotation_value=const.DRIVE_EJECT_AUTO,
+        )
         self.utils.annotate_custom_resource(
             resource_name=failed_drive_name,
             resource_type="drives",

tests/e2e-test-framework/tests/test_drive_replacement_one_volume.py

@@ -55,6 +55,13 @@ def test_5771_auto_drive_replacement_with_one_volume_per_pod(self):
 
         # 2.1 simulate drive failure. Annotate drive used by pod with health=BAD
         drive_name = drive["metadata"]["name"]
+        # mark drive as auto eject
+        self.utils.annotate_custom_resource(
+            resource_name=drive_name,
+            resource_type="drives",
+            annotation_key=const.DRIVE_EJECT,
+            annotation_value=const.DRIVE_EJECT_AUTO,
+        )
         self.utils.annotate_custom_resource(
             resource_name=drive_name,
             resource_type="drives",