Repository: dell/csi-baremetal
Pull request #445 (2021-06-25 15:06:24)

[ISSUE-441] Override Drive Health via annotation
## Purpose
### Issue #441

- Override Drive Health if `health` annotation is set
- Generate event `DriveHealthOverridden`
- Generate event `DriveHealth<>` after deleting annotation

## PR checklist
- [x] Add link to the issue
- [x] Choose PR label
- [x] New unit tests added
- [x] Modified code has meaningful comments
- [ ] All TODOs are linked with the issues
- [x] All comments are resolved

## Testing
```
user@user-vm ~/g/s/g/d/csi-baremetal-operator (master)> kubectl annotate drive 11a64144-c873-435f-b8c5-51aa05268593 health=SUSPECT
drive.csi-baremetal.dell.com/11a64144-c873-435f-b8c5-51aa05268593 annotated

user@user-vm ~/g/s/g/d/csi-baremetal-operator (master)> kubectl get events --sort-by='.lastTimestamp' | grep LOOPBACK2955441738
3m27s       Normal    DriveDiscovered            drive/05bfbccd-0526-4167-8c98-090590f6b67d           New drive discovered SN: LOOPBACK2955441738, Node: 77a7e96c-ee8e-4e35-9110-938686735333.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
3m27s       Normal    DriveHealthGood            drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive health is: GOOD, previous state: UNKNOWN.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
2m27s       Warning   DriveHealthOverridden      drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive health is overridden with: SUSPECT, real state: GOOD.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
2m27s       Normal    DriveReadyForReplacement   drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive is ready for replacement, Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''

user@user-vm ~/g/s/g/d/csi-baremetal-operator (master) [1]> kubectl annotate drive 05bfbccd-0526-4167-8c98-090590f6b67d health-
drive.csi-baremetal.dell.com/05bfbccd-0526-4167-8c98-090590f6b67d annotated

user@user-vm ~/g/s/g/d/csi-baremetal-operator (master)> kubectl get events --sort-by='.lastTimestamp' | grep LOOPBACK2955441738
3m27s       Normal    DriveDiscovered            drive/05bfbccd-0526-4167-8c98-090590f6b67d           New drive discovered SN: LOOPBACK2955441738, Node: 77a7e96c-ee8e-4e35-9110-938686735333.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
3m27s       Normal    DriveHealthGood            drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive health is: GOOD, previous state: UNKNOWN.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
2m27s       Warning   DriveHealthOverridden      drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive health is overridden with: SUSPECT, real state: GOOD.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
2m27s       Normal    DriveReadyForReplacement   drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive is ready for replacement, Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
56s         Normal    DriveHealthGood            drive/05bfbccd-0526-4167-8c98-090590f6b67d           Drive health is: GOOD, previous state: SUSPECT.Drive Details: SN='LOOPBACK2955441738', Node='77a7e96c-ee8e-4e35-9110-938686735333', Type='HDD', Model='Test Loopback', Size='105906176', Firmware=''
```




Patch:
diff --git a/docs/proposals/drive-replacement.md b/docs/proposals/drive-replacement.md
index b7d2538..7196ecc 100644
--- a/docs/proposals/drive-replacement.md
+++ b/docs/proposals/drive-replacement.md
@@ -64,6 +64,8 @@ Negotiation between CSI and Operator is optional and will be done though annotat
 #### User
 To trigger physical drive replacement user must put the following annotation on the corresponding Drive custom resource:
   - `driveremove.csi-baremetal/replacement: ready` - informs that drive replacement is ready
+
+*User can trigger drive releasing himself. If user annotates driveCR with `health=<SUSPECT/BAD>`, drive health will be overridden with passed value.*
 ### Detailed workflow
 * When drive health changed from `GOOD` to `SUSPECT` or `BAD` CSI will:
   - Set drive operational status to `RELEASING`
diff --git a/pkg/eventing/eventing.go b/pkg/eventing/eventing.go
index 3b49adb..33787b9 100644
--- a/pkg/eventing/eventing.go
+++ b/pkg/eventing/eventing.go
@@ -44,4 +44,5 @@ const (
 	DriveSuccessfullyReplaced = "DriveSuccessfullyReplaced"
 	DriveHasData              = "DriveHasData"
 	DriveClean                = "DriveClean"
+	DriveHealthOverridden     = "DriveHealthOverridden"
 )
diff --git a/pkg/node/volumemgr.go b/pkg/node/volumemgr.go
index d9c5104..ae86c79 100644
--- a/pkg/node/volumemgr.go
+++ b/pkg/node/volumemgr.go
@@ -67,6 +67,10 @@ const (
 
 	fakeAttachAnnotation = "pv.attach.kubernetes.io/ignore-if-inaccessible"
 	fakeAttachAllowKey   = "yes"
+
+	// Annotation key for health overriding
+	// Discover function replaces drive health with passed value if the annotation is set
+	driveHealthOverrideAnnotation = "health"
 )
 
 // eventRecorder interface for sending events
@@ -602,6 +606,9 @@ func (m *VolumeManager) updateDrivesCRs(ctx context.Context, drivesFromMgr []*ap
 				if searchSystemDrives && driveCR.Spec.IsSystem {
 					m.systemDrivesUUIDs = append(m.systemDrivesUUIDs, driveCR.Spec.UUID)
 				}
+				if value, ok := driveCR.GetAnnotations()[driveHealthOverrideAnnotation]; ok {
+					m.overrideDriveHealth(drivePtr, value, driveCR.Name)
+				}
 				if driveCR.Equals(drivePtr) {
 					updates.AddNotChanged(&driveCR)
 				} else {
@@ -979,14 +986,20 @@ func (m *VolumeManager) createEventsForDriveUpdates(updates *driveUpdates) {
 			createdDrive, apiV1.HealthUnknown, createdDrive.Spec.Health)
 	}
 	for _, updDrive := range updates.Updated {
-		if updDrive.CurrentState.Spec.Health != updDrive.PreviousState.Spec.Health {
-			m.createEventForDriveHealthChange(
-				updDrive.CurrentState, updDrive.PreviousState.Spec.Health, updDrive.CurrentState.Spec.Health)
-		}
 		if updDrive.CurrentState.Spec.Status != updDrive.PreviousState.Spec.Status {
 			m.createEventForDriveStatusChange(
 				updDrive.CurrentState, updDrive.PreviousState.Spec.Status, updDrive.CurrentState.Spec.Status)
 		}
+		// skip HealthChange event if drive health was overridden
+		if _, ok := updDrive.CurrentState.Annotations[driveHealthOverrideAnnotation]; ok {
+			m.createEventForDriveHealthOverridden(
+				updDrive.CurrentState, updDrive.PreviousState.Spec.Health, updDrive.CurrentState.Spec.Health)
+			continue
+		}
+		if updDrive.CurrentState.Spec.Health != updDrive.PreviousState.Spec.Health {
+			m.createEventForDriveHealthChange(
+				updDrive.CurrentState, updDrive.PreviousState.Spec.Health, updDrive.CurrentState.Spec.Health)
+		}
 	}
 }
 
@@ -1031,6 +1044,16 @@ func (m *VolumeManager) createEventForDriveStatusChange(
 		statusMsgTemplate, currentStatus, prevStatus)
 }
 
+// createEventForDriveHealthOverridden creates DriveHealthOverridden with Warning type
+func (m *VolumeManager) createEventForDriveHealthOverridden(
+	drive *drivecrd.Drive, realHealth, overriddenHealth string) {
+	msgTemplate := "Drive health is overridden with: %s, real state: %s."
+	eventType := eventing.WarningType
+	reason := eventing.DriveHealthOverridden
+	m.sendEventForDrive(drive, eventType, reason,
+		msgTemplate, overriddenHealth, realHealth)
+}
+
 func (m *VolumeManager) sendEventForDrive(drive *drivecrd.Drive, eventtype, reason, messageFmt string,
 	args ...interface{}) {
 	messageFmt += drive.GetDriveDescription()
@@ -1136,3 +1159,19 @@ func (m *VolumeManager) isPVCNeedFakeAttach(volumeID string) bool {
 
 	return false
 }
+
+// overrideDriveHealth replaces drive health with passed value,
+// generates error message if value is not valid
+func (m *VolumeManager) overrideDriveHealth(drive *api.Drive, overriddenHealth, driveCRName string) {
+	if (overriddenHealth == apiV1.HealthGood) ||
+		(overriddenHealth == apiV1.HealthSuspect) ||
+		(overriddenHealth == apiV1.HealthBad) ||
+		(overriddenHealth == apiV1.HealthUnknown) {
+		m.log.Warnf("Drive %s has health annotation. Health %s has been overridden with %s.",
+			driveCRName, drive.Health, overriddenHealth)
+		drive.Health = overriddenHealth
+	} else {
+		m.log.Errorf("Drive %s has health annotation, but value %s is not %s/%s/%s/%s. Health is not overridden.",
+			driveCRName, overriddenHealth, apiV1.HealthGood, apiV1.HealthSuspect, apiV1.HealthBad, apiV1.HealthUnknown)
+	}
+}


Test patch:
diff --git a/pkg/node/volumemgr_test.go b/pkg/node/volumemgr_test.go
index db63883..09a9742 100644
--- a/pkg/node/volumemgr_test.go
+++ b/pkg/node/volumemgr_test.go
@@ -778,6 +778,26 @@ func TestVolumeManager_updatesDrivesCRs_Fail(t *testing.T) {
 	assert.Equal(t, 1, len(res.Created))
 }
 
+func TestVolumeManager_updatesDrivesCRs_Override(t *testing.T) {
+	client, err := k8s.GetFakeKubeClient(testNs, testLogger)
+	assert.Nil(t, err)
+	vm := NewVolumeManager(nil, nil, testLogger, client, client, new(mocks.NoOpRecorder), nodeID)
+
+	testDriveCR1 := testDriveCR.DeepCopy()
+	testDriveCR1.Annotations = map[string]string{driveHealthOverrideAnnotation: apiV1.HealthSuspect}
+	err = client.CreateCR(testCtx, testDriveCR1.Name, testDriveCR1)
+	assert.Nil(t, err)
+
+	drive := drive1
+	drives := []*api.Drive{&drive}
+
+	updates, err := vm.updateDrivesCRs(testCtx, drives)
+	assert.Nil(t, err)
+	assert.Equal(t, len(updates.Updated), 1)
+	assert.Equal(t, updates.Updated[0].PreviousState.Spec.Health, drive1.Health)
+	assert.Equal(t, updates.Updated[0].CurrentState.Spec.Health, apiV1.HealthSuspect)
+}
+
 func TestVolumeManager_handleDriveStatusChange(t *testing.T) {
 	vm := prepareSuccessVolumeManagerWithDrives(nil, t)
 
diff --git a/test/e2e/common/deploy.go b/test/e2e/common/deploy.go
index c650cf5..30e4798 100644
--- a/test/e2e/common/deploy.go
+++ b/test/e2e/common/deploy.go
@@ -103,7 +103,7 @@ func DeployOperator(f *framework.Framework) (func(), error) {
 	return cleanup, nil
 }
 
-// DeployCSIWithArgs deploys csi-baremetal-deployment with CmdHelmExecutor
+// DeployCSI deploys csi-baremetal-deployment with CmdHelmExecutor
 // After install - waiting all pods ready, checking kubernetes-scheduler restart
 // Cleanup - deleting csi-chart, cleaning all csi custom resources
 // Helm command - "helm install csi-baremetal <CHARTS_DIR>/csi-baremetal-deployment