Skip to content

fix: always sync monut point instead of skipping #345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Yu-Jack merged 1 commit into from
Jul 14, 2026
Merged

fix: always sync monut point instead of skipping #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions pkg/controller/blockdevice/scanner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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...)
Expand Down
22 changes: 14 additions & 8 deletions tests/integration/test_0_single_disk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")

}

Expand Down
31 changes: 23 additions & 8 deletions tests/integration/test_1_disk_hotplug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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) {
Expand Down
Loading