diff --git a/api/v1/constants.go b/api/v1/constants.go index 41e2c1253..12c83f093 100644 --- a/api/v1/constants.go +++ b/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" diff --git a/pkg/crcontrollers/drive/drivecontroller.go b/pkg/crcontrollers/drive/drivecontroller.go index d915dcb75..60078c698 100644 --- a/pkg/crcontrollers/drive/drivecontroller.go +++ b/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 +} diff --git a/pkg/crcontrollers/drive/drivecontroller_test.go b/pkg/crcontrollers/drive/drivecontroller_test.go index f646cc621..9986221f7 100644 --- a/pkg/crcontrollers/drive/drivecontroller_test.go +++ b/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") + } +} diff --git a/tests/e2e-test-framework/framework/const.py b/tests/e2e-test-framework/framework/const.py index 9dfcd3f7d..b770300a0 100644 --- a/tests/e2e-test-framework/framework/const.py +++ b/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" diff --git a/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes.py b/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes.py index aa2195e82..216667b26 100644 --- a/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes.py +++ b/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", diff --git a/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes_single_fail.py b/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes_single_fail.py index 8574c10af..44cccc2ad 100644 --- a/tests/e2e-test-framework/tests/test_drive_replacement_multi_volumes_single_fail.py +++ b/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", diff --git a/tests/e2e-test-framework/tests/test_drive_replacement_one_volume.py b/tests/e2e-test-framework/tests/test_drive_replacement_one_volume.py index 63ee48986..de7a8ca3b 100644 --- a/tests/e2e-test-framework/tests/test_drive_replacement_one_volume.py +++ b/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",