diff --git a/pkg/controller/blockdevice/scanner.go b/pkg/controller/blockdevice/scanner.go index cb5661aeb..019448a99 100644 --- a/pkg/controller/blockdevice/scanner.go +++ b/pkg/controller/blockdevice/scanner.go @@ -173,6 +173,9 @@ func (s *Scanner) handleExistingDev(oldBd *diskv1.BlockDevice, newBd *diskv1.Blo oldBdCp.Status.DeviceStatus.Capacity = newBd.Status.DeviceStatus.Capacity oldBdCp.Status.DeviceStatus.Details = newBd.Status.DeviceStatus.Details oldBdCp.Status.DeviceStatus.Partitioned = newBd.Status.DeviceStatus.Partitioned + oldBdCp.Status.DeviceStatus.FileSystem.MountPoint = newBd.Status.DeviceStatus.FileSystem.MountPoint + oldBdCp.Status.DeviceStatus.FileSystem.Type = newBd.Status.DeviceStatus.FileSystem.Type + oldBdCp.Status.DeviceStatus.FileSystem.IsReadOnly = newBd.Status.DeviceStatus.FileSystem.IsReadOnly } else { // The BD is inactive. This can happen if a provisioned device is // temporarily gone and has come back. It can also happen for provisioned @@ -216,6 +219,9 @@ func (s *Scanner) handleExistingDev(oldBd *diskv1.BlockDevice, newBd *diskv1.Blo oldBdCp.Status.DeviceStatus.Capacity = newBd.Status.DeviceStatus.Capacity oldBdCp.Status.DeviceStatus.Details = newBd.Status.DeviceStatus.Details oldBdCp.Status.DeviceStatus.Partitioned = newBd.Status.DeviceStatus.Partitioned + oldBdCp.Status.DeviceStatus.FileSystem.MountPoint = newBd.Status.DeviceStatus.FileSystem.MountPoint + oldBdCp.Status.DeviceStatus.FileSystem.Type = newBd.Status.DeviceStatus.FileSystem.Type + oldBdCp.Status.DeviceStatus.FileSystem.IsReadOnly = newBd.Status.DeviceStatus.FileSystem.IsReadOnly } } @@ -406,19 +412,6 @@ func (s *Scanner) scanBlockDevicesOnNode(ctx context.Context) error { } } - // A device with a mount point is only tracked when it was provisioned by NDM - // (meaning Longhorn mounted it as part of the normal flow). If it is mounted - // but not yet provisioned, an external process mounted it and NDM should leave - // it alone. For unprovisioned existing BDs in that case, deactivateOrDeleteBlockDevices - // will clean up the stale CR. - if mp := newBd.Status.DeviceStatus.FileSystem.MountPoint; mp != "" { - if existingBd == nil || existingBd.Status.ProvisionPhase != diskv1.ProvisionPhaseProvisioned { - logrus.Infof("block device %s is mounted at %s but not provisioned by NDM, ignoring", - newBd.Status.DeviceStatus.DevPath, mp) - continue - } - } - if existingBd != nil { // Pick up the name of the existing block device we found (not strictly necessary, // but just in case we try to use newBd.name in handleExistingDev...) diff --git a/tests/integration/test_0_single_disk_test.go b/tests/integration/test_0_single_disk_test.go index 67191f5fc..a0a03a1ac 100644 --- a/tests/integration/test_0_single_disk_test.go +++ b/tests/integration/test_0_single_disk_test.go @@ -152,14 +152,20 @@ func (s *SingleDiskSuite) Test_1_UnprovisionSingleDisk() { _, err = bdi.Update(context.TODO(), newBlockdevice, v1.UpdateOptions{}) require.Equal(s.T(), nil, err, "Update Blockdevices should not get error") - // sleep 30 seconds to wait controller handle. jitter is between 7~13 seconds so 30 seconds would be enough to run twice - time.Sleep(30 * time.Second) - - // check for the removed status - curBlockdevice, err = bdi.Get(context.TODO(), s.targetDiskName, v1.GetOptions{}) - require.Equal(s.T(), nil, err, "Get BlockdevicesList should not get error before we want to check remove") - require.Equal(s.T(), "", curBlockdevice.Status.DeviceStatus.FileSystem.MountPoint, "Mountpoint should be empty after we remove disk!") - require.Equal(s.T(), diskv1.ProvisionPhaseUnprovisioned, curBlockdevice.Status.ProvisionPhase, "Block device provisionPhase should be Provisioned") + // Poll until the controller finishes unprovisioning and Longhorn unmounts the disk + // (up to 90 seconds). The disk is still physically present, so the CR will not be + // deleted — we just wait for MountPoint to be cleared and phase to be Unprovisioned. + require.Eventually(s.T(), func() bool { + bd, getErr := bdi.Get(context.TODO(), s.targetDiskName, v1.GetOptions{}) + if getErr != nil { + s.T().Logf("poll: Get error: %v", getErr) + return false + } + s.T().Logf("poll: MountPoint=%q ProvisionPhase=%q", + bd.Status.DeviceStatus.FileSystem.MountPoint, bd.Status.ProvisionPhase) + return bd.Status.DeviceStatus.FileSystem.MountPoint == "" && + bd.Status.ProvisionPhase == diskv1.ProvisionPhaseUnprovisioned + }, 90*time.Second, 5*time.Second, "Mountpoint should be empty and provisionPhase Unprovisioned after unprovision") } diff --git a/tests/integration/test_1_disk_hotplug_test.go b/tests/integration/test_1_disk_hotplug_test.go index a6314e629..c8a6ae2b1 100644 --- a/tests/integration/test_1_disk_hotplug_test.go +++ b/tests/integration/test_1_disk_hotplug_test.go @@ -15,6 +15,7 @@ import ( "github.com/melbahja/goph" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/clientcmd" @@ -244,14 +245,28 @@ func (s *HotPlugTestSuite) Test_4_RemoveInactiveDisk() { _, err = bdi.Update(context.TODO(), newBlockdevice, v1.UpdateOptions{}) require.Equal(s.T(), nil, err, "Update Blockdevices should not get error") - // sleep 30 seconds to wait controller handle. jitter is between 7~13 seconds so 30 seconds would be enough to run twice - time.Sleep(30 * time.Second) - - // check for the removed status - curBlockdevice, err = bdi.Get(context.TODO(), s.targetDiskName, v1.GetOptions{}) - require.Equal(s.T(), nil, err, "Get BlockdevicesList should not get error before we want to check remove") - require.Equal(s.T(), "", curBlockdevice.Status.DeviceStatus.FileSystem.MountPoint, "Mountpoint should be empty after we remove disk!") - require.Equal(s.T(), diskv1.ProvisionPhaseUnprovisioned, curBlockdevice.Status.ProvisionPhase, "Block device provisionPhase should be Unprovisioned") + // Poll until the controller finishes unprovisioning (up to 90 seconds). + // Two valid terminal states: + // 1. CR still exists with ProvisionPhase=Unprovisioned and MountPoint="" — the + // controller has finished but the scanner hasn't cleaned it up yet. + // 2. CR is deleted — the scanner already cleaned up the Unprovisioned+absent disk, + // which is also a correct outcome. + require.Eventually(s.T(), func() bool { + bd, getErr := bdi.Get(context.TODO(), s.targetDiskName, v1.GetOptions{}) + if getErr != nil { + if errors.IsNotFound(getErr) { + s.T().Logf("poll: CR deleted (fully cleaned up)") + // CR deleted = fully cleaned up, that's also success + return true + } + s.T().Logf("poll: unexpected Get error: %v", getErr) + return false + } + s.T().Logf("poll: MountPoint=%q ProvisionPhase=%q", + bd.Status.DeviceStatus.FileSystem.MountPoint, bd.Status.ProvisionPhase) + return bd.Status.DeviceStatus.FileSystem.MountPoint == "" && + bd.Status.ProvisionPhase == diskv1.ProvisionPhaseUnprovisioned + }, 90*time.Second, 5*time.Second, "Disk should be unprovisioned (CR deleted or MountPoint empty with Unprovisioned phase)") } func doCommand(cmdString string) (string, string, error) {