blkdev/controller: do not re-provision the provisioned device

    - Avoid to do the redundant provision.
    - For auto-provision, do not enqueue again when device is
      already provisioned.
    - If provision, respect the disk tags from Longhorn.

Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>

pkg/controller/blockdevice/controller.go

@@ -143,7 +143,7 @@ func (c *Controller) OnBlockDeviceChange(key string, device *diskv1.BlockDevice)
 		return nil, fmt.Errorf("failed to resolve persistent dev path for block device %s", device.Name)
 	}
 	filesystem := c.BlockInfo.GetFileSystemInfoByDevPath(devPath)
-	logrus.Infof("Get filesystem info from device %s, fs type: %s", devPath, filesystem.Type)
+	logrus.Debugf("Get filesystem info from device %s, fs type: %s", devPath, filesystem.Type)
 
 	needFormat := deviceCpy.Spec.FileSystem.ForceFormatted && deviceCpy.Status.DeviceStatus.FileSystem.LastFormattedAt == nil
 	if needFormat {
@@ -177,9 +177,19 @@ func (c *Controller) OnBlockDeviceChange(key string, device *diskv1.BlockDevice)
 		return device, err
 	}
 
+	/*
+	 * We use the needProvision to control first time provision.
+	 * 1. `deviceCpy.Spec.FileSystem.Provisioned` is False.
+	 * 2. updateDeviceStatus() would made `deviceCpy.Spec.FileSystem.Provisioned` be true and trigger Update
+	 * 3. loop back and check `deviceCpy.Spec.FileSystem.Provisioned` again. (Now needProvision is true)
+	 * 4. provision
+	 *
+	 * NOTE: we do not need to provision again for provisioned device so we should do another
+	 *       check with `device.Status.ProvisionPhase`
+	 */
 	needProvision := deviceCpy.Spec.FileSystem.Provisioned
 	switch {
-	case needProvision:
+	case needProvision && device.Status.ProvisionPhase == diskv1.ProvisionPhaseUnprovisioned:
 		logrus.Infof("Prepare to provision device %s to node %s", device.Name, c.NodeName)
 		if err := c.provisionDeviceToNode(deviceCpy); err != nil {
 			err := fmt.Errorf("failed to provision device %s to node %s: %w", device.Name, c.NodeName, err)
@@ -355,6 +365,9 @@ func (c *Controller) provisionDeviceToNode(device *diskv1.BlockDevice) error {
 	}
 
 	if disk, ok := node.Spec.Disks[device.Name]; !ok || !reflect.DeepEqual(disk, diskSpec) {
+		/* we should respect the disk Tags from LH */
+		diskSpec.Tags = disk.Tags
+		logrus.Debugf("Previous disk tags on LH: %+v, we should respect it.", disk.Tags)
 		nodeCpy.Spec.Disks[device.Name] = diskSpec
 		if _, err = c.Nodes.Update(nodeCpy); err != nil {
 			return err

pkg/controller/blockdevice/scanner.go

@@ -129,12 +129,14 @@ func (s *Scanner) scanBlockDevicesOnNode() error {
 		bd := device.bd
 		autoProvisioned := device.AutoProvisioned
 		if oldBd, ok := oldBds[bd.Name]; ok {
-			if s.NeedsAutoProvision(oldBd, autoProvisioned) {
-				logrus.Debugf("Enqueue block device %s for auto-provisioning", bd.Name)
-				s.Blockdevices.Enqueue(s.Namespace, bd.Name)
-			} else if isDevPathChanged(oldBd, bd) {
+			if isDevPathChanged(oldBd, bd) {
 				logrus.Debugf("Enqueue block device %s for device path change", bd.Name)
 				s.Blockdevices.Enqueue(s.Namespace, bd.Name)
+			} else if isDevAlreadyProvisioned(bd) {
+				logrus.Debugf("Skip the provisioned device: %s", bd.Name)
+			} else if s.NeedsAutoProvision(oldBd, autoProvisioned) {
+				logrus.Debugf("Enqueue block device %s for auto-provisioning", bd.Name)
+				s.Blockdevices.Enqueue(s.Namespace, bd.Name)
 			} else {
 				logrus.Debugf("Skip updating device %s", bd.Name)
 			}
@@ -236,3 +238,8 @@ func (s *Scanner) NeedsAutoProvision(oldBd *diskv1.BlockDevice, autoProvisionPat
 func isDevPathChanged(oldBd *diskv1.BlockDevice, newBd *diskv1.BlockDevice) bool {
 	return oldBd.Status.DeviceStatus.DevPath != newBd.Status.DeviceStatus.DevPath
 }
+
+/* isDevAlreadyProvisioned would return true if the device is provisioned */
+func isDevAlreadyProvisioned(newBd *diskv1.BlockDevice) bool {
+	return newBd.Status.ProvisionPhase == diskv1.ProvisionPhaseProvisioned
+}