From a61358e8de607b08d96733068994f50ff64d0bf1 Mon Sep 17 00:00:00 2001 From: Vicente Cheng Date: Wed, 28 Sep 2022 14:58:30 +0800 Subject: [PATCH] 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 --- pkg/controller/blockdevice/controller.go | 17 +++++++++++++++-- pkg/controller/blockdevice/scanner.go | 15 +++++++++++---- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/pkg/controller/blockdevice/controller.go b/pkg/controller/blockdevice/controller.go index 6e7efbfd..2643193b 100644 --- a/pkg/controller/blockdevice/controller.go +++ b/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 diff --git a/pkg/controller/blockdevice/scanner.go b/pkg/controller/blockdevice/scanner.go index 57ff031f..4624f3dd 100644 --- a/pkg/controller/blockdevice/scanner.go +++ b/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 +}