fix(linstor): wait for temporary probe devices

Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>

packages/system/linstor/images/piraeus-server/patches/README.md

@@ -18,3 +18,7 @@ Custom patches for piraeus-server (linstor-server) v1.33.3.
 - **retry-secondary-after-mkfs.diff** — Retry `drbdadm secondary` after mkfs when DRBD reports the device as held open by an external probe (Talos block-controller, udev, multipathd, etc.). Without this retry, a transient `Device is held open by someone` aborts resource initialization, leaves the satellite in an intermediate state, and prevents the controller from receiving the final `UpToDate` event — orphan PVs in `Released` then cannot be cleaned up.
   - Related upstream issue: [#268](https://github.com/LINBIT/linstor-server/issues/268)
   - Related upstream issue: [drbd #74](https://github.com/LINBIT/drbd/issues/74) (same EBUSY pattern with multipathd)
+
+- **fix-min-io-probe-device-wait.diff** — Wait for temporary probe devices before reading pool I/O properties; retry failures and fall back to a probe when ZFS volumes have no usable device path.
+  - Source PR: [#528](https://github.com/LINBIT/linstor-server/pull/528), fixes [#527](https://github.com/LINBIT/linstor-server/issues/527)
+  - Adapted for v1.33.3; inactive LVM volumes retain the existing no-probe behavior.

packages/system/linstor/images/piraeus-server/patches/fix-min-io-probe-device-wait.diff

@@ -0,0 +1,660 @@
+diff --git a/satellite/src/main/java/com/linbit/linstor/layer/storage/AbsStorageProvider.java b/satellite/src/main/java/com/linbit/linstor/layer/storage/AbsStorageProvider.java
+--- a/satellite/src/main/java/com/linbit/linstor/layer/storage/AbsStorageProvider.java
++++ b/satellite/src/main/java/com/linbit/linstor/layer/storage/AbsStorageProvider.java
+@@ -163,6 +163,8 @@
+     }
+
+     private static final long DFLT_WAIT_UNTIL_DEVICE_CREATED_TIMEOUT_IN_MS = 5000;
++    private static final int PROBE_VLM_ATTEMPTS = 3;
++    private static final long PROBE_VLM_RETRY_DELAY_IN_MS = 1000;
+     protected static final int DFLT_STRIPES = 1;
+     public static final long SIZE_OF_NOT_FOUND_STOR_POOL = ApiConsts.VAL_STOR_POOL_SPACE_NOT_FOUND;
+
+@@ -184,6 +186,7 @@
+     protected final Set<String> changedStoragePoolStrings = new HashSet<>();
+     private final String typeDescr;
+     private final FileSystemWatch fsWatch;
++    private final Object probeVlmLock = new Object();
+     protected final DeviceProviderKind kind;
+     private final DrbdInvalidateUtils drbdInvalidateUtils;
+
+@@ -2022,61 +2025,126 @@
+         final StorPoolName storPoolObjName = storPoolObj.getName();
+         errorReporter.logDebug("ENTER updateMinIoSize method: Storage pool \"%s\"", storPoolObjName.displayValue);
+         Collection<VlmProviderObject<Resource>> vlmProviderList = storPoolObj.getVolumes(storDriverAccCtx);
++        boolean haveInfo = false;
+         if (vlmProviderList != null && vlmProviderList.size() >= 1)
+         {
+             errorReporter.logDebug("updateMinIoSize: Have vlmProviderList with %d items", vlmProviderList.size());
+             Iterator<VlmProviderObject<Resource>> vlmProviderIter = vlmProviderList.iterator();
+-            boolean haveInfo = false;
+             while (vlmProviderIter.hasNext() && !haveInfo)
+             {
+                 VlmProviderObject<Resource> vlmProvider = vlmProviderIter.next();
+                 errorReporter.logDebug("updateMinIoSize: Have vlmProvider");
+                 final String storDevicePath = vlmProvider.getDevicePath();
++                if (storDevicePath != null)
++                {
++                    try
++                    {
++                        updateMinIoSizeByDevice(storPoolObj, storDevicePath, propsChange);
++                        haveInfo = true;
++                    }
++                    catch (IOException ignored)
++                    {
++                    }
++                }
++            }
++        }
++        // An inactive shared LVM volume can be owned by another node; do not probe it by creating a volume.
++        if (!haveInfo && this instanceof ProbeVlmStorageProvider &&
++            (vlmProviderList == null || vlmProviderList.isEmpty() ||
++                kind == DeviceProviderKind.ZFS || kind == DeviceProviderKind.ZFS_THIN))
++        {
++            updateMinIoSizeByProbeVlm((ProbeVlmStorageProvider) this, storPoolObj, propsChange);
++        }
++        errorReporter.logDebug("EXIT updateMinIoSize method");
++    }
++
++    private void updateMinIoSizeByProbeVlm(
++        ProbeVlmStorageProvider storPrv,
++        StorPool storPoolObj,
++        LocalPropsChangePojo propsChange
++    )
++        throws AccessDeniedException
++    {
++        // Providers use a fixed probe name. Keep creation, probing and cleanup in one critical section.
++        synchronized (probeVlmLock)
++        {
++            boolean haveInfo = false;
++            boolean interrupted = false;
++            String lastFailure = "";
++            int attempts = 0;
++            while (attempts < PROBE_VLM_ATTEMPTS && !haveInfo && !interrupted)
++            {
++                if (attempts > 0)
++                {
++                    try
++                    {
++                        Thread.sleep(PROBE_VLM_RETRY_DELAY_IN_MS);
++                    }
++                    catch (InterruptedException exc)
++                    {
++                        Thread.currentThread().interrupt();
++                        lastFailure = "interrupted before the next probe attempt";
++                        break;
++                    }
++                }
++                attempts++;
++                boolean cleaned = false;
+                 try
+                 {
+-                    updateMinIoSizeByDevice(storPoolObj, storDevicePath, propsChange);
+-                    haveInfo = true;
+-                }
+-                catch (IOException ignored)
+-                {
+-                }
+-            }
+-        }
+-        else
+-        {
+-            errorReporter.logDebug("updateMinIoSize: Don't have vlmProviderList, using temporary volumes");
+-            if (this instanceof ProbeVlmStorageProvider)
+-            {
+-                final ProbeVlmStorageProvider storPrv = (ProbeVlmStorageProvider) this;
+-                try
+-                {
+-                    final String storDevicePath = storPrv.createTmpProbeVlm(storPoolObj);
++                    @Nullable String storDevicePath = storPrv.createTmpProbeVlm(storPoolObj);
++                    if (storDevicePath == null)
++                    {
++                        lastFailure = "temporary volume has no device path";
++                    }
++                    else
++                    {
++                        waitUntilDeviceCreated(storDevicePath, getWaitTimeoutAfterCreate(storPoolObj));
++                        updateMinIoSizeByDevice(storPoolObj, storDevicePath, propsChange);
++                        haveInfo = true;
++                    }
++                }
++                catch (StorageException | IOException exc)
++                {
++                    lastFailure = exc.getMessage();
++                    interrupted = exc.getCause() instanceof InterruptedException;
++                    errorReporter.logDebug("Temporary volume probe attempt %d failed: %s", attempts, lastFailure);
++                }
++                finally
++                {
+                     try
+                     {
+-                        updateMinIoSizeByDevice(storPoolObj, storDevicePath, propsChange);
+-                    }
+-                    catch (IOException ignored)
+-                    {
+-                    }
+-                }
+-                catch (StorageException ignored)
+-                {
+-                    errorReporter.logDebug("updateMinIoSize: Temporary volume creation failed");
+-                }
+-                finally
+-                {
+-                    try
+-                    {
+                         storPrv.deleteTmpProbeVlm(storPoolObj);
+-                    }
+-                    catch (StorageException ignored)
+-                    {
+-                        errorReporter.logDebug("updateMinIoSize: Temporary volume deletion failed");
+-                    }
+-                }
+-            }
+-        }
+-        errorReporter.logDebug("EXIT updateMinIoSize method");
++                        cleaned = true;
++                    }
++                    catch (StorageException exc)
++                    {
++                        interrupted |= exc.getCause() instanceof InterruptedException;
++                        errorReporter.logWarning(
++                            "Failed to delete temporary probe volume for storage pool '%s': %s",
++                            storPoolObj.getName().displayValue,
++                            exc.getMessage()
++                        );
++                    }
++                }
++                if (interrupted)
++                {
++                    Thread.currentThread().interrupt();
++                }
++                if (!cleaned)
++                {
++                    break;
++                }
++            }
++            if (!haveInfo)
++            {
++                errorReporter.logWarning(
++                    "Could not determine block device information for storage pool '%s' after %d probe attempts: %s",
++                    storPoolObj.getName().displayValue,
++                    attempts,
++                    lastFailure
++                );
++            }
++        }
+     }
+
+     private void updateMinIoSizeByDevice(
+diff --git a/src/test/java/com/linbit/fsevent/ProbeVolumeDeviceWaitTest.java b/src/test/java/com/linbit/fsevent/ProbeVolumeDeviceWaitTest.java
+new file mode 100644
+--- /dev/null
++++ b/src/test/java/com/linbit/fsevent/ProbeVolumeDeviceWaitTest.java
+@@ -0,0 +1,467 @@
++package com.linbit.fsevent;
++
++import com.linbit.Platform;
++import com.linbit.linstor.core.identifier.StorPoolName;
++import com.linbit.linstor.core.objects.Resource;
++import com.linbit.linstor.core.objects.StorPool;
++import com.linbit.linstor.core.pojos.LocalPropsChangePojo;
++import com.linbit.linstor.layer.storage.AbsStorageProvider.AbsStorageProviderInit;
++import com.linbit.linstor.layer.storage.BlockSizeConsts;
++import com.linbit.linstor.layer.storage.lvm.LvmProvider;
++import com.linbit.linstor.layer.storage.zfs.ZfsThinProvider;
++import com.linbit.linstor.logging.ErrorReporter;
++import com.linbit.linstor.propscon.Props;
++import com.linbit.linstor.storage.StorageConstants;
++import com.linbit.linstor.storage.StorageException;
++import com.linbit.linstor.storage.interfaces.categories.resource.VlmProviderObject;
++
++import java.io.IOException;
++import java.nio.file.Files;
++import java.nio.file.Path;
++import java.util.Collections;
++import java.util.List;
++import java.util.Map;
++import java.util.concurrent.Callable;
++import java.util.concurrent.CountDownLatch;
++import java.util.concurrent.ExecutorService;
++import java.util.concurrent.Executors;
++import java.util.concurrent.Future;
++import java.util.concurrent.ScheduledExecutorService;
++import java.util.concurrent.ScheduledFuture;
++import java.util.concurrent.TimeUnit;
++import java.util.concurrent.atomic.AtomicInteger;
++
++import org.junit.After;
++import org.junit.Before;
++import org.junit.Rule;
++import org.junit.Test;
++import org.junit.rules.TemporaryFolder;
++import org.junit.rules.Timeout;
++
++import static org.junit.Assert.assertEquals;
++import static org.junit.Assert.assertFalse;
++import static org.junit.Assert.assertTrue;
++import static org.junit.Assume.assumeTrue;
++import static org.mockito.Mockito.doReturn;
++import static org.mockito.Mockito.mock;
++import static org.mockito.Mockito.mockingDetails;
++import static org.mockito.Mockito.when;
++
++public class ProbeVolumeDeviceWaitTest
++{
++    @Rule
++    public final TemporaryFolder directory = new TemporaryFolder();
++
++    @Rule
++    public final Timeout timeout = Timeout.seconds(15);
++
++    private FileSystemWatch watch;
++    private Path device;
++    private Path link;
++    private StorPool pool;
++    private ProbeProvider provider;
++    private ErrorReporter reporter;
++    private ScheduledExecutorService executor;
++
++    @Before
++    public void setUp() throws Exception
++    {
++        assumeTrue(Platform.isLinux());
++        device = directory.newFile("zd-test").toPath();
++        link = directory.getRoot().toPath().resolve("probe");
++        executor = Executors.newSingleThreadScheduledExecutor();
++        reporter = mock(ErrorReporter.class);
++        watch = new FileSystemWatch(reporter);
++        watch.start();
++        pool = mock(StorPool.class);
++        when(pool.getName()).thenReturn(new StorPoolName("data"));
++        when(pool.getVolumes(null)).thenReturn(Collections.emptyList());
++        when(pool.getProps(null)).thenReturn(mock(Props.class));
++        provider = new ProbeProvider(providerInit());
++    }
++
++    @After
++    public void tearDown() throws Exception
++    {
++        if (executor != null)
++        {
++            executor.shutdownNow();
++            assertTrue(executor.awaitTermination(2, TimeUnit.SECONDS));
++        }
++        if (watch != null)
++        {
++            watch.shutdown(false);
++            watch.awaitShutdown(2000);
++            watch.cancelAllWatchKeys();
++        }
++    }
++
++    @Test
++    public void delayedDevicePublishesPropertiesBeforeCleanup() throws Exception
++    {
++        provider.delayMillis = 50;
++        assertPublished(probe());
++        assertCleaned(1);
++        assertEquals(0, warningCount());
++    }
++
++    @Test
++    public void existingProbeDevicePublishesProperties() throws Exception
++    {
++        assertPublished(probe());
++        assertCleaned(1);
++    }
++
++    @Test
++    public void missingDeviceTimesOutAndCleansEveryAttempt() throws Exception
++    {
++        provider.delayMillis = -1;
++        provider.timeoutMillis = 20;
++        assertTrue(probe().isEmpty());
++        assertCleaned(3);
++        assertEquals(1, warningCount());
++    }
++
++    @Test
++    public void failedCreationStillCleansEveryAttempt() throws Exception
++    {
++        provider.failCreateTimes = Integer.MAX_VALUE;
++        assertTrue(probe().isEmpty());
++        assertCleaned(3);
++        assertEquals(1, warningCount());
++    }
++
++    @Test
++    public void transientCreationFailureIsRetried() throws Exception
++    {
++        provider.failCreateTimes = 1;
++        provider.delayMillis = 50;
++        assertPublished(probe());
++        assertCleaned(2);
++        assertEquals(0, warningCount());
++    }
++
++    @Test
++    public void nullProbePathDoesNotPublishProperties() throws Exception
++    {
++        provider.nullPath = true;
++        assertTrue(probe().isEmpty());
++        assertCleaned(3);
++        assertEquals(1, warningCount());
++    }
++
++    @Test
++    public void nullFirstPathDoesNotSkipUsableVolume() throws Exception
++    {
++        Files.createSymbolicLink(link, device);
++        doReturn(List.of(volume(null), volume(link))).when(pool).getVolumes(null);
++        assertPublished(probe());
++        assertEquals(0, provider.createCalls);
++        assertEquals(0, provider.deleteCalls);
++    }
++
++    @Test
++    public void missingFirstPathDoesNotSkipUsableVolume() throws Exception
++    {
++        Files.createSymbolicLink(link, device);
++        doReturn(List.of(volume(link.resolveSibling("missing")), volume(link))).when(pool).getVolumes(null);
++        assertPublished(probe());
++        assertEquals(0, provider.createCalls);
++        assertEquals(0, provider.deleteCalls);
++    }
++
++    @Test
++    public void allNullPathsFallBackToProbe() throws Exception
++    {
++        doReturn(List.of(volume(null))).when(pool).getVolumes(null);
++        provider.delayMillis = 50;
++        assertPublished(probe());
++        assertCleaned(1);
++    }
++
++    @Test
++    public void allMissingPathsFallBackToProbe() throws Exception
++    {
++        doReturn(List.of(
++            volume(link.resolveSibling("missing-a")), volume(link.resolveSibling("missing-b"))
++        )).when(pool).getVolumes(null);
++        provider.delayMillis = 50;
++        assertPublished(probe());
++        assertCleaned(1);
++    }
++
++    @Test
++    public void cleanupFailureAfterSuccessIsReported() throws Exception
++    {
++        provider.failDelete = true;
++        assertPublished(probe());
++        assertEquals(1, provider.createCalls);
++        assertEquals(1, provider.deleteCalls);
++        assertEquals(1, warningCount());
++    }
++
++    @Test
++    public void cleanupFailurePreventsAnotherCreationAttempt() throws Exception
++    {
++        provider.delayMillis = -1;
++        provider.timeoutMillis = 20;
++        provider.failDelete = true;
++        assertTrue(probe().isEmpty());
++        assertEquals(1, provider.createCalls);
++        assertEquals(1, provider.deleteCalls);
++        assertEquals(2, warningCount());
++    }
++
++    @Test
++    public void interruptedDeviceWaitCleansUpAndPreservesInterrupt() throws Exception
++    {
++        provider.delayMillis = -1;
++        Thread callingThread = Thread.currentThread();
++        executor.schedule(callingThread::interrupt, 50, TimeUnit.MILLISECONDS);
++        try
++        {
++            assertTrue(probe().isEmpty());
++            assertTrue(callingThread.isInterrupted());
++            assertCleaned(1);
++            assertEquals(1, warningCount());
++        }
++        finally
++        {
++            Thread.interrupted();
++        }
++    }
++
++    @Test
++    public void interruptedRetryDelayDoesNotCreateAnotherProbe() throws Exception
++    {
++        provider.failCreateTimes = Integer.MAX_VALUE;
++        Thread callingThread = Thread.currentThread();
++        executor.schedule(callingThread::interrupt, 100, TimeUnit.MILLISECONDS);
++        try
++        {
++            assertTrue(probe().isEmpty());
++            assertTrue(callingThread.isInterrupted());
++            assertCleaned(1);
++            assertEquals(1, warningCount());
++        }
++        finally
++        {
++            Thread.interrupted();
++        }
++    }
++
++    @Test
++    public void interruptedCleanupPreservesInterruptAndStopsRetries() throws Exception
++    {
++        provider.failCreateTimes = Integer.MAX_VALUE;
++        provider.interruptDelete = true;
++        try
++        {
++            assertTrue(probe().isEmpty());
++            assertTrue(Thread.currentThread().isInterrupted());
++            assertEquals(1, provider.createCalls);
++            assertEquals(1, provider.deleteCalls);
++        }
++        finally
++        {
++            Thread.interrupted();
++        }
++    }
++
++    @Test
++    public void concurrentProbesDoNotShareTheTemporaryVolume() throws Exception
++    {
++        provider.creationDelayMillis = 200;
++        ExecutorService callers = Executors.newFixedThreadPool(2);
++        CountDownLatch ready = new CountDownLatch(2);
++        CountDownLatch start = new CountDownLatch(1);
++        Callable<LocalPropsChangePojo> call = () ->
++        {
++            ready.countDown();
++            start.await();
++            return probe();
++        };
++        try
++        {
++            Future<LocalPropsChangePojo> first = callers.submit(call);
++            Future<LocalPropsChangePojo> second = callers.submit(call);
++            assertTrue(ready.await(2, TimeUnit.SECONDS));
++            start.countDown();
++            assertFalse(first.get(10, TimeUnit.SECONDS).isEmpty());
++            assertFalse(second.get(10, TimeUnit.SECONDS).isEmpty());
++            assertEquals(1, provider.peakActiveProbes.get());
++            assertCleaned(2);
++        }
++        finally
++        {
++            callers.shutdownNow();
++            assertTrue(callers.awaitTermination(2, TimeUnit.SECONDS));
++        }
++    }
++
++    @Test
++    public void inactiveLvmVolumesDoNotCreateProbe() throws Exception
++    {
++        doReturn(List.of(volume(null))).when(pool).getVolumes(null);
++        LvmProvider lvm = new LvmProvider(providerInit())
++        {
++            @Override
++            public String createTmpProbeVlm(StorPool ignored)
++            {
++                throw new AssertionError("Inactive shared LVM volumes must not trigger a probe volume");
++            }
++
++            @Override
++            public void deleteTmpProbeVlm(StorPool ignored)
++            {
++                throw new AssertionError("No probe volume was created");
++            }
++        };
++        LocalPropsChangePojo changes = new LocalPropsChangePojo();
++        lvm.updateMinIoSize(pool, changes);
++        assertTrue(changes.isEmpty());
++    }
++
++    private AbsStorageProviderInit providerInit()
++    {
++        return new AbsStorageProviderInit(
++            reporter, null, null, null, null, null, null, null, null, null, watch, null, null, null
++        );
++    }
++
++    private LocalPropsChangePojo probe() throws Exception
++    {
++        LocalPropsChangePojo changes = new LocalPropsChangePojo();
++        provider.updateMinIoSize(pool, changes);
++        return changes;
++    }
++
++    private void assertPublished(LocalPropsChangePojo changesRef)
++    {
++        String namespace = StorageConstants.NAMESPACE_INTERNAL + '/';
++        assertEquals(Map.of(
++            namespace + StorageConstants.BLK_DEV_MIN_IO_SIZE, Long.toString(BlockSizeConsts.DFLT_IO_SIZE),
++            namespace + StorageConstants.BLK_DEV_OPT_IO_SIZE, Long.toString(BlockSizeConsts.DFLT_OPT_IO_SIZE)
++        ), changesRef.changedStorPoolProps.get(pool.getName()));
++    }
++
++    private void assertCleaned(int attemptsRef)
++    {
++        assertEquals(attemptsRef, provider.createCalls);
++        assertEquals(attemptsRef, provider.deleteCalls);
++        assertFalse(Files.exists(link));
++    }
++
++    @SuppressWarnings("unchecked")
++    private VlmProviderObject<Resource> volume(Path pathRef)
++    {
++        VlmProviderObject<Resource> volume = mock(VlmProviderObject.class);
++        when(volume.getDevicePath()).thenReturn(pathRef == null ? null : pathRef.toString());
++        return volume;
++    }
++
++    private long warningCount()
++    {
++        return mockingDetails(reporter).getInvocations().stream()
++            .filter(invocation -> invocation.getMethod().getName().equals("logWarning"))
++            .count();
++    }
++
++    private class ProbeProvider extends ZfsThinProvider
++    {
++        private long delayMillis;
++        private long creationDelayMillis;
++        private long timeoutMillis = 4000;
++        private int failCreateTimes;
++        private boolean nullPath;
++        private boolean failDelete;
++        private boolean interruptDelete;
++        private int createCalls;
++        private int deleteCalls;
++        private ScheduledFuture<?> create;
++        private final AtomicInteger activeProbes = new AtomicInteger();
++        private final AtomicInteger peakActiveProbes = new AtomicInteger();
++
++        ProbeProvider(AbsStorageProviderInit initRef)
++        {
++            super(initRef);
++        }
++
++        @Override
++        protected long getWaitTimeoutAfterCreate(StorPool ignored)
++        {
++            return timeoutMillis;
++        }
++
++        @Override
++        public String createTmpProbeVlm(StorPool ignored) throws StorageException
++        {
++            peakActiveProbes.accumulateAndGet(activeProbes.incrementAndGet(), Math::max);
++            createCalls++;
++            if (creationDelayMillis > 0)
++            {
++                try
++                {
++                    Thread.sleep(creationDelayMillis);
++                }
++                catch (InterruptedException exc)
++                {
++                    throw new StorageException("Test probe creation interrupted", exc);
++                }
++            }
++            if (failCreateTimes > 0)
++            {
++                failCreateTimes--;
++                throw new StorageException("Probe creation failed");
++            }
++            if (nullPath)
++            {
++                return null;
++            }
++            if (delayMillis == 0)
++            {
++                try
++                {
++                    Files.createSymbolicLink(link, device);
++                }
++                catch (IOException exc)
++                {
++                    throw new StorageException("Test device creation failed", exc);
++                }
++            }
++            else if (delayMillis > 0)
++            {
++                create = executor.schedule(() -> Files.createSymbolicLink(link, device),
++                    delayMillis, TimeUnit.MILLISECONDS);
++            }
++            return link.toString();
++        }
++
++        @Override
++        public void deleteTmpProbeVlm(StorPool ignored) throws StorageException
++        {
++            deleteCalls++;
++            if (interruptDelete)
++            {
++                throw new StorageException("Probe cleanup interrupted", new InterruptedException());
++            }
++            if (failDelete)
++            {
++                throw new StorageException("Probe cleanup failed");
++            }
++            try
++            {
++                if (create != null && !create.cancel(false))
++                {
++                    create.get();
++                }
++                Files.deleteIfExists(link);
++                activeProbes.decrementAndGet();
++            }
++            catch (Exception exc)
++            {
++                throw new StorageException("Test device cleanup failed", exc);
++            }
++        }
++    }
++}