Repository: dell/csi-baremetal
Pull request #448 (2021-06-29 14:01:33)

[ISSUE-447] Repeat Drive Replacement procedure
## Purpose
### Issue #447 

- Reset drive Usage from FAILED or RELEASED to IN_USE if driveCR is annotated with `replacement=restart`
- Delete annotation after restart
- Rename `delete` return code to avoid substitution of build-in function

## PR checklist
- [x] Add link to the issue
- [x] Choose PR label
- [ ] New unit tests added
- [x] Modified code has meaningful comments
- [ ] All TODOs are linked with the issues
- [ ] All comments are resolved

## Testing
_Link to custom CI build_



Patch:
diff --git a/Makefile.validation b/Makefile.validation
index 0c786f6..7d9cd3b 100644
--- a/Makefile.validation
+++ b/Makefile.validation
@@ -52,7 +52,7 @@ test-sanity:
 load-operator-image:
 	docker pull ${REGISTRY}/${PROJECT}-${OPERATOR}:${OPERATOR_VERSION}
 	docker tag ${REGISTRY}/${PROJECT}-${OPERATOR}:${OPERATOR_VERSION} ${PROJECT}-${OPERATOR}:${OPERATOR_VERSION}
-	kind load docker-image ${PROJECT}-${OPERATOR}:${OPERATOR_VERSION}
+	$(KIND) load docker-image ${PROJECT}-${OPERATOR}:${OPERATOR_VERSION}
 
 kind-pull-images: kind-pull-sidecar-images
 	docker pull ${REGISTRY}/${PROJECT}-${LOOPBACK_DRIVE_MGR}:${TAG}
@@ -93,19 +93,57 @@ kind-tag-images:
 	docker tag ${REGISTRY}/${PROJECT}-${OPERATOR}:${TAG} ${PROJECT}-${OPERATOR}:${TAG}
 
 kind-load-images:
-	kind load docker-image ${CSI_PROVISIONER}:${CSI_PROVISIONER_TAG}
-	kind load docker-image ${CSI_REGISTRAR}:${CSI_REGISTRAR_TAG}
-	kind load docker-image ${CSI_ATTACHER}:${CSI_ATTACHER_TAG}
-	kind load docker-image ${CSI_RESIZER}:${CSI_RESIZER_TAG}
-	kind load docker-image ${LIVENESS_PROBE}:${LIVENESS_PROBE_TAG}
-	kind load docker-image ${BUSYBOX}:${BUSYBOX_TAG}
-	kind load docker-image docker.io/library/nginx:1.14-alpine
-	kind load docker-image docker.io/library/centos:latest
-	kind load docker-image ${PROJECT}-${LOOPBACK_DRIVE_MGR}:${TAG}
-	kind load docker-image ${PROJECT}-${NODE}:${TAG}
-	kind load docker-image ${PROJECT}-${NODE}-kernel-5.4:${TAG}
-	kind load docker-image ${PROJECT}-${CONTROLLER}:${TAG}
-	kind load docker-image ${PROJECT}-${SCHEDULER}-${EXTENDER}:${TAG}
-	kind load docker-image ${PROJECT}-${EXTENDER_PATCHER}:${TAG}
-	kind load docker-image ${PROJECT}-${SCHEDULER}:${TAG}
-	kind load docker-image ${PROJECT}-${OPERATOR}:${TAG}
+	$(KIND) load docker-image ${CSI_PROVISIONER}:${CSI_PROVISIONER_TAG}
+	$(KIND) load docker-image ${CSI_REGISTRAR}:${CSI_REGISTRAR_TAG}
+	$(KIND) load docker-image ${CSI_ATTACHER}:${CSI_ATTACHER_TAG}
+	$(KIND) load docker-image ${CSI_RESIZER}:${CSI_RESIZER_TAG}
+	$(KIND) load docker-image ${LIVENESS_PROBE}:${LIVENESS_PROBE_TAG}
+	$(KIND) load docker-image ${BUSYBOX}:${BUSYBOX_TAG}
+	$(KIND) load docker-image docker.io/library/nginx:1.14-alpine
+	$(KIND) load docker-image docker.io/library/centos:latest
+	$(KIND) load docker-image ${PROJECT}-${LOOPBACK_DRIVE_MGR}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${NODE}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${NODE}-kernel-5.4:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${CONTROLLER}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${SCHEDULER}-${EXTENDER}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${EXTENDER_PATCHER}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${SCHEDULER}:${TAG}
+	$(KIND) load docker-image ${PROJECT}-${OPERATOR}:${TAG}
+
+kind-build:
+	# Get kind sources
+	wget -O kind-src https://codeload.github.com/kubernetes-sigs/kind/tar.gz/refs/tags/v0.11.1
+	tar -xzvf kind-src
+
+	# Add "--ipc=host" docker option and build binary
+	patch -p0 < $(KIND_DIR)/kind.patch
+	cd kind-0.11.1 && make build
+
+	# Copy file
+	cp kind-0.11.1/bin/kind $(KIND_DIR)
+
+	# Make executive
+	chmod +x $(KIND)
+
+	# Check
+	$(KIND) version
+
+	# Clean workdir
+	rm -rf kind-0.11.1
+	rm -f kind-src
+
+kind-install: kind-build
+	# Delete actual
+	rm -f /usr/bin/kind
+
+	# Copy executive file
+	mv $(KIND) /usr/bin
+
+	# Check
+	kind version
+
+kind-create-cluster:
+	$(KIND) create cluster --config $(KIND_DIR)/kind.yaml --image kindest/node:v1.18.19
+
+kind-delete-cluster:
+	$(KIND) delete cluster
diff --git a/docs/fake-attach.md b/docs/fake-attach.md
index 027f5a9..036e8cf 100644
--- a/docs/fake-attach.md
+++ b/docs/fake-attach.md
@@ -1,6 +1,6 @@
 # Proposal: Fake Attach Volume
 
-Last updated: June 11 2011
+Last updated: July 1 2021
 
 
 ## Abstract
@@ -29,19 +29,30 @@ This feature will require application Operator to put specific annotation on PVC
 
 If fake-attach volume is successfully created, CSI Volume CR will be annotated with `fake-attach:yes` and `Operational Status` will equal to `MISSING`. The pod must be restarted after `Operational Status` returns to `OPERATIVE`.
 
+
 ## Implementation
 
-When `pv.attach.kubernetes.io/ignore-if-inaccessible: yes` annotation is set CSI must ignore NodeStageVolume errors and put `fake-attach: yes` annotation on CSI Volume CR. 
+### NodeStageVolume
+When `pv.attach.kubernetes.io/ignore-if-inaccessible: yes` annotation is set CSI must:
+- ignore NodeStageVolume errors
+- put `fake-attach: yes` annotation on CSI Volume CR if error is and annotation wasn't set before
+- delete `fake-attach: yes` annotation on CSI Volume CR if error is not and annotation was set before
+
+Event FakeAttachInvolved generated when `fake-attach: yes` annotation is setting.
+
+Event FakeAttachCleared generated when `fake-attach: yes` annotation is deleting.
+
+### NodePublishVolume 
+CSI must check `fake-attach` annotation and mount tmpfs volume in read-only mode.
 
-On NodePublishVolume CSI must check `fake-attach` annotation and mount
-tmpfs volume in read-only mode.
 Command to mount tmpfs volume: `mount -t tmpfs -o size=1M,ro <volumeID> <destination folder>`
 
-CSI must generate event to notify user about this Fake-Attach Volume.
+### NodeUnpublishVolume 
+tmpfs volume must be unmounted usually.
 
-On NodeUnpublishVolume tmpfs volume must be unmounted usually.
+### NodeUnstageVolume 
+Do nothing.
 
-On NodeUnstageVolume `fake-attach` annotation must be deleted.
 
 ## Assumptions (if applicable)
 
diff --git a/docs/proposals/drive-replacement.md b/docs/proposals/drive-replacement.md
index 7196ecc..e88253c 100644
--- a/docs/proposals/drive-replacement.md
+++ b/docs/proposals/drive-replacement.md
@@ -66,6 +66,8 @@ To trigger physical drive replacement user must put the following annotation on
   - `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.*
+
+*DR procedure can be repeated if drive Usage is `FAILED` or `RELEASED`. User should annotate driveCR with `drive=add` and drive-controller will switch Usage value to IN_USE. After restart the annotation will be deleted.* 
 ### Detailed workflow
 * When drive health changed from `GOOD` to `SUSPECT` or `BAD` CSI will:
   - Set drive operational status to `RELEASING`
diff --git a/pkg/crcontrollers/drive/drivecontroller.go b/pkg/crcontrollers/drive/drivecontroller.go
index a12a495..adab0db 100644
--- a/pkg/crcontrollers/drive/drivecontroller.go
+++ b/pkg/crcontrollers/drive/drivecontroller.go
@@ -36,7 +36,13 @@ type Controller struct {
 const (
 	ignore uint8 = 0
 	update uint8 = 1
-	delete uint8 = 2
+	remove uint8 = 2
+)
+
+const (
+	// Annotations for driveCR to perform restart process
+	driveRestartReplacementAnnotationKey   = "drive"
+	driveRestartReplacementAnnotationValue = "add"
 )
 
 // NewController creates new instance of Controller structure
@@ -115,7 +121,7 @@ func (c *Controller) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 			log.Errorf("Failed to update Drive %s CR", driveName)
 			return ctrl.Result{}, client.IgnoreNotFound(err)
 		}
-	case delete:
+	case remove:
 		if err := c.client.DeleteCR(ctx, drive); err != nil {
 			log.Errorf("Failed to delete Drive %s CR", driveName)
 			return ctrl.Result{}, client.IgnoreNotFound(err)
@@ -166,6 +172,11 @@ func (c *Controller) handleDriveUpdate(ctx context.Context, log *logrus.Entry, d
 		}
 
 	case apiV1.DriveUsageReleased:
+		if c.restartReplacement(drive) {
+			toUpdate = true
+			break
+		}
+
 		status, found := drive.Annotations[apiV1.DriveAnnotationReplacement]
 		if !found || status != apiV1.DriveAnnotationReplacementReady {
 			break
@@ -214,7 +225,11 @@ func (c *Controller) handleDriveUpdate(ctx context.Context, log *logrus.Entry, d
 	case apiV1.DriveUsageRemoved:
 		if drive.Spec.Status == apiV1.DriveStatusOffline {
 			// drive was removed from the system. need to clean corresponding custom resource
-			return delete, nil
+			return remove, nil
+		}
+	case apiV1.DriveUsageFailed:
+		if c.restartReplacement(drive) {
+			toUpdate = true
 		}
 	}
 
@@ -258,3 +273,15 @@ func (c *Controller) checkAllVolsRemoved(volumes []*volumecrd.Volume) bool {
 	}
 	return true
 }
+
+// restartReplacement restores drive.Usage to IN_USE if CR is annotated
+// deletes the annotation to avoid event repeating
+func (c *Controller) restartReplacement(drive *drivecrd.Drive) bool {
+	if value, ok := drive.GetAnnotations()[driveRestartReplacementAnnotationKey]; ok && value == driveRestartReplacementAnnotationValue {
+		drive.Spec.Usage = apiV1.DriveUsageInUse
+		delete(drive.Annotations, driveRestartReplacementAnnotationKey)
+		return true
+	}
+
+	return false
+}
diff --git a/pkg/eventing/eventing.go b/pkg/eventing/eventing.go
index 33787b9..91cf8f9 100644
--- a/pkg/eventing/eventing.go
+++ b/pkg/eventing/eventing.go
@@ -45,4 +45,7 @@ const (
 	DriveHasData              = "DriveHasData"
 	DriveClean                = "DriveClean"
 	DriveHealthOverridden     = "DriveHealthOverridden"
+
+	FakeAttachInvolved = "FakeAttachInvolved"
+	FakeAttachCleared  = "FakeAttachCleared"
 )
diff --git a/pkg/node/node.go b/pkg/node/node.go
index 65a99d4..09788c5 100644
--- a/pkg/node/node.go
+++ b/pkg/node/node.go
@@ -46,6 +46,7 @@ import (
 	"github.com/dell/csi-baremetal/pkg/common"
 	"github.com/dell/csi-baremetal/pkg/controller"
 	csibmnodeconst "github.com/dell/csi-baremetal/pkg/crcontrollers/operator/common"
+	"github.com/dell/csi-baremetal/pkg/eventing"
 )
 
 const (
@@ -199,10 +200,10 @@ func (s *CSINodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStage
 
 	targetPath := getStagingPath(ll, req.GetStagingTargetPath())
 
-	isFakeAttach := false
+	isFakeAttachNeed := false
 	ignoreErrorIfFakeAttach := func(err error) {
 		if s.isPVCNeedFakeAttach(volumeID) {
-			isFakeAttach = true
+			isFakeAttachNeed = true
 		} else {
 			newStatus = apiV1.Failed
 			resp, errToReturn = nil, status.Error(codes.Internal, fmt.Sprintf("failed to stage volume: %s", err.Error()))
@@ -221,9 +222,18 @@ func (s *CSINodeService) NodeStageVolume(ctx context.Context, req *csi.NodeStage
 		}
 	}
 
-	if isFakeAttach {
-		volumeCR.Annotations["fake-attach"] = "yes"
-		ll.Warningf("Adding fake-attach annotation to the volume with ID %s", volumeID)
+	if isFakeAttachNeed {
+		if volumeCR.Annotations[fakeAttachVolumeAnnotation] != fakeAttachVolumeKey {
+			volumeCR.Annotations[fakeAttachVolumeAnnotation] = fakeAttachVolumeKey
+			ll.Warningf("Adding fake-attach annotation to the volume with ID %s", volumeID)
+			s.VolumeManager.recorder.Eventf(volumeCR, eventing.NormalType, eventing.FakeAttachInvolved,
+				"Fake-attach involved for volume with ID %s", volumeID)
+		}
+	} else if volumeCR.Annotations[fakeAttachVolumeAnnotation] == fakeAttachVolumeKey {
+		delete(volumeCR.Annotations, fakeAttachVolumeAnnotation)
+		ll.Warningf("Removing fake-attach annotation for volume %s", volumeID)
+		s.VolumeManager.recorder.Eventf(volumeCR, eventing.NormalType, eventing.FakeAttachCleared,
+			"Fake-attach cleared for volume with ID %s", volumeID)
 	}
 
 	if currStatus != apiV1.VolumeReady {
@@ -293,12 +303,11 @@ func (s *CSINodeService) NodeUnstageVolume(ctx context.Context, req *csi.NodeUns
 		errToReturn error
 	)
 
-	if volumeCR.Annotations[fakeAttachVolumeAnnotation] == fakeAttachVolumeKey {
-		ll.Warningf("Removing fake-attach annotation for volume %s", volumeID)
-		delete(volumeCR.Annotations, fakeAttachVolumeAnnotation)
-	} else if errToReturn = s.fsOps.UnmountWithCheck(getStagingPath(ll, req.GetStagingTargetPath())); errToReturn != nil {
-		volumeCR.Spec.CSIStatus = apiV1.Failed
-		resp = nil
+	if volumeCR.Annotations[fakeAttachVolumeAnnotation] != fakeAttachVolumeKey {
+		if errToReturn = s.fsOps.UnmountWithCheck(getStagingPath(ll, req.GetStagingTargetPath())); errToReturn != nil {
+			volumeCR.Spec.CSIStatus = apiV1.Failed
+			resp = nil
+		}
 	}
 
 	ctxWithID := context.WithValue(context.Background(), base.RequestUUID, req.GetVolumeId())
diff --git a/variables.mk b/variables.mk
index 7d81331..2a5db88 100644
--- a/variables.mk
+++ b/variables.mk
@@ -71,5 +71,9 @@ GOPROXY_PART    := GOPROXY=https://proxy.golang.org,direct
 METRICS_PACKAGE := github.com/dell/csi-baremetal/pkg/metrics
 LDFLAGS := -ldflags "-X ${METRICS_PACKAGE}.Revision=${RELEASE_STR} -X ${METRICS_PACKAGE}.Branch=${BRANCH}"
 
+### Kind
+KIND_DIR := test/kind
+KIND     := ${KIND_DIR}/kind
+
 # override some of variables, optional file
 -include variables.override.mk


Test patch:
diff --git a/pkg/node/node_test.go b/pkg/node/node_test.go
index 373fb12..2e22039 100644
--- a/pkg/node/node_test.go
+++ b/pkg/node/node_test.go
@@ -683,7 +683,7 @@ var _ = Describe("CSINodeService Fake-Attach", func() {
 		setVariables()
 	})
 
-	It("Should stage volume with fake-attach annotation", func() {
+	It("Should stage unhealthy volume with fake-attach annotation", func() {
 		req := getNodeStageRequest(testVolume1.Id, *testVolumeCap)
 		vol1 := testVolumeCR1
 		vol1.Spec.CSIStatus = apiV1.Created
@@ -723,7 +723,7 @@ var _ = Describe("CSINodeService Fake-Attach", func() {
 		Expect(vol1.Spec.CSIStatus).To(Equal(apiV1.VolumeReady))
 		Expect(vol1.Annotations[fakeAttachVolumeAnnotation]).To(Equal(fakeAttachVolumeKey))
 	})
-	It("Should publish volume with fake-attach annotation", func() {
+	It("Should publish unhealthy volume with fake-attach annotation", func() {
 		req := getNodePublishRequest(testV1ID, targetPath, *testVolumeCap)
 		req.VolumeContext[util.PodNameKey] = testPodName
 
@@ -746,7 +746,48 @@ var _ = Describe("CSINodeService Fake-Attach", func() {
 		Expect(err).To(BeNil())
 		Expect(volumeCR.Spec.Owners[0]).To(Equal(testPodName))
 	})
-	It("Should unstage volume with fake-attach", func() {
+	It("Should stage healthy volume with fake-attach annotation", func() {
+		req := getNodeStageRequest(testVolume1.Id, *testVolumeCap)
+		vol1 := testVolumeCR1
+		vol1.Spec.CSIStatus = apiV1.Created
+		err := node.k8sClient.UpdateCR(testCtx, &vol1)
+		Expect(err).To(BeNil())
+
+		pvcName := "pvcName"
+		pvcNamespace := "pvcNamespace"
+
+		pv := &corev1.PersistentVolume{}
+		pv.Name = vol1.Name
+		pv.Spec.ClaimRef = &corev1.ObjectReference{}
+		pv.Spec.ClaimRef.Name = pvcName
+		pv.Spec.ClaimRef.Namespace = pvcNamespace
+		err = node.k8sClient.Create(testCtx, pv)
+		Expect(err).To(BeNil())
+
+		pvc := &corev1.PersistentVolumeClaim{}
+		pvc.Name = pvcName
+		pvc.Namespace = pvcNamespace
+		pvc.Annotations = map[string]string{fakeAttachAnnotation: fakeAttachAllowKey}
+		err = node.k8sClient.Create(testCtx, pvc)
+		Expect(err).To(BeNil())
+
+		partitionPath := "/partition/path/for/volume1"
+		prov.On("GetVolumePath", vol1.Spec).Return(partitionPath, nil)
+		fsOps.On("PrepareAndPerformMount",
+			partitionPath, path.Join(req.GetStagingTargetPath(), stagingFileName), true, false).
+			Return(nil)
+
+		resp, err := node.NodeStageVolume(testCtx, req)
+		Expect(resp).NotTo(BeNil())
+		Expect(err).To(BeNil())
+
+		err = node.k8sClient.ReadCR(testCtx, testV1ID, "", &vol1)
+		Expect(err).To(BeNil())
+		Expect(vol1.Spec.CSIStatus).To(Equal(apiV1.VolumeReady))
+		_, ok := vol1.Annotations[fakeAttachVolumeAnnotation]
+		Expect(ok).To(Equal(false))
+	})
+	It("Should unstage volume with fake-attach annotation", func() {
 		req := getNodeUnstageRequest(testV1ID, stagePath)
 
 		vol1 := testVolumeCR1
@@ -763,7 +804,7 @@ var _ = Describe("CSINodeService Fake-Attach", func() {
 		Expect(err).To(BeNil())
 		Expect(volumeCR.Spec.CSIStatus).To(Equal(apiV1.Created))
 		_, ok := volumeCR.Annotations[fakeAttachVolumeAnnotation]
-		Expect(ok).To(Equal(false))
+		Expect(ok).To(Equal(true))
 	})
 })
 
diff --git a/test/kind/kind.patch b/test/kind/kind.patch
new file mode 100644
index 0000000..32d7653
--- /dev/null
+++ b/test/kind/kind.patch
@@ -0,0 +1,22 @@
+--- kind-0.11.1/pkg/cluster/internal/providers/docker/provision.go	2021-07-02 15:32:23.872508147 +0300
++++ kind-0.11.1-patched/pkg/cluster/internal/providers/docker/provision.go	2021-06-30 18:57:28.806000000 +0300
+@@ -243,6 +243,8 @@
+ 		"--volume", "/var",
+ 		// some k8s things want to read /lib/modules
+ 		"--volume", "/lib/modules:/lib/modules:ro",
++		// ipc host
++		"--ipc", "host",
+ 	},
+ 		args...,
+ 	)
+--- kind-0.11.1/pkg/cmd/kind/version/version.go	2021-05-28 02:18:13.000000000 +0300
++++ kind-0.11.1-patched/pkg/cmd/kind/version/version.go	2021-07-02 15:31:45.440367546 +0300
+@@ -50,7 +50,7 @@
+ }
+ 
+ // VersionCore is the core portion of the kind CLI version per Semantic Versioning 2.0.0
+-const VersionCore = "0.11.1"
++const VersionCore = "0.11.1-patched"
+ 
+ // VersionPreRelease is the pre-release portion of the kind CLI version per
+ // Semantic Versioning 2.0.0
diff --git a/test/kind/kind.yaml b/test/kind/kind.yaml
index 47f3833..5bf2cb4 100644
--- a/test/kind/kind.yaml
+++ b/test/kind/kind.yaml
@@ -4,8 +4,10 @@ apiVersion: kind.x-k8s.io/v1alpha4
 nodes:
 # the control plane node config
 - role: control-plane
+  image: kindest/node:v1.18.19
 # the two workers
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
   - hostPath: /dev
     containerPath: /dev
@@ -16,6 +18,7 @@ nodes:
   - hostPath: /run/lock
     containerPath: /run/lock
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
   - hostPath: /dev
     containerPath: /dev
@@ -26,6 +29,7 @@ nodes:
   - hostPath: /run/lock
     containerPath: /run/lock
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
     - hostPath: /dev
       containerPath: /dev
@@ -36,6 +40,7 @@ nodes:
     - hostPath: /run/lock
       containerPath: /run/lock
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
     - hostPath: /dev
       containerPath: /dev
@@ -46,6 +51,7 @@ nodes:
     - hostPath: /run/lock
       containerPath: /run/lock
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
     - hostPath: /dev
       containerPath: /dev
@@ -56,6 +62,7 @@ nodes:
     - hostPath: /run/lock
       containerPath: /run/lock
 - role: worker
+  image: kindest/node:v1.18.19
   extraMounts:
     - hostPath: /dev
       containerPath: /dev