linstor: grow cloned volumes inside the clone instead of resizing afterwards

Deploying a VM with a root disk larger than its template failed with

  Cannot resize volume, because we have a non-UpToDate DRBD device.

whenever the resource group has Clone/BalanceAfterClone=true. The clone
reports COMPLETE as soon as every replica can access UpToDate data, which is
while the additional balance replica is still doing its initial sync; the
resize that followed CloneWaiter.waitFor() was rejected by LINSTOR's
all-replicas-UpToDate precheck. Waiting for the sync client-side would block
the deploy for the whole sync of the template size, so the fix is in LINSTOR:
REST API 1.29.1 (LINSTOR 1.35.0) accepts volume_sizes on the clone request and
grows the volume inside the clone flux before the balance placement.

Pass the requested size that way when the controller supports it, detected
once per controller URL via its REST API version, and skip the post-clone
resize. Older controllers keep the clone-then-resize sequence unchanged.

Author: rp-

plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java

@@ -403,6 +403,29 @@ private void deleteTemplateForProps(
         }
     }
 
+    /**
+     * Request the target size as part of the clone if the controller supports it.
+     *
+     * A clone reports COMPLETE as soon as every replica can access UpToDate data, which with
+     * Clone/BalanceAfterClone is while the additional replica is still syncing. A resize issued at that
+     * point is rejected with "Cannot resize volume, because we have a non-UpToDate DRBD device".
+     * Controllers with REST API 1.29.1+ take the size in the clone request and grow the volume inside the
+     * clone before the balance placement, so the race cannot occur. Older controllers keep the
+     * clone-then-resize sequence.
+     *
+     * @return true if the caller still has to resize the resource after the clone finished
+     */
+    static boolean applyCloneSize(DevelopersApi api, ResourceDefinitionCloneRequest cloneRequest, Long sizeByte) {
+        if (sizeByte == null || sizeByte <= 0) {
+            return false;
+        }
+        if (LinstorUtil.supportsCloneVolumeSizes(api)) {
+            cloneRequest.setVolumeSizes(Collections.singletonList(sizeByte / 1024));
+            return false;
+        }
+        return true;
+    }
+
     private String cloneResource(long csCloneId, VolumeInfo volumeInfo, StoragePoolVO storagePoolVO) {
         // get the cached template on this storage
         VMTemplateStoragePoolVO tmplPoolRef = _vmTemplatePoolDao.findByPoolTemplate(
@@ -436,6 +459,7 @@ private String cloneResource(long csCloneId, VolumeInfo volumeInfo, StoragePoolV
                         cloneRequest.setVolumePassphrases(Collections.singletonList(utf8Passphrase));
                     }
                 }
+                final boolean resizeAfterClone = applyCloneSize(linstorApi, cloneRequest, volumeInfo.getSize());
                 ResourceDefinitionCloneStarted cloneStarted = linstorApi.resourceDefinitionClone(
                     cloneRes, cloneRequest);
 
@@ -447,7 +471,7 @@ private String cloneResource(long csCloneId, VolumeInfo volumeInfo, StoragePoolV
 
                 logger.info("Clone resource definition " + cloneRes + " to " + rscName + " finished");
 
-                if (volumeInfo.getSize() != null && volumeInfo.getSize() > 0) {
+                if (resizeAfterClone) {
                     resizeResource(linstorApi, rscName, volumeInfo.getSize());
                 }
 

plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java

@@ -101,6 +101,50 @@ public static DevelopersApi getLinstorAPI(String linstorUrl, String apiToken, bo
         return new DevelopersApi(client);
     }
 
+    /**
+     * The REST API version of the connected controller, e.g. "1.29.1", or null if it could not be queried.
+     */
+    @Nullable
+    public static String getRestApiVersion(DevelopersApi api) {
+        try {
+            return api.controllerVersion().getRestApiVersion();
+        } catch (ApiException apiExc) {
+            LOGGER.warn("Unable to query controller API version: {}", apiExc.getBestMessage());
+            return null;
+        }
+    }
+
+    /**
+     * Check if the connected controller accepts volume_sizes on a resource-definition clone request
+     * (REST API 1.29.1, LINSTOR 1.35.0). With it the grow happens inside the clone, before an optional
+     * Clone/BalanceAfterClone placement, so it cannot race the balance replica's sync.
+     */
+    public static boolean supportsCloneVolumeSizes(DevelopersApi api) {
+        return isVersionAtLeast(getRestApiVersion(api), 1, 29, 1);
+    }
+
+    static boolean isVersionAtLeast(String version, int major, int minor, int patch) {
+        if (version == null || version.isEmpty()) {
+            return false;
+        }
+        String[] parts = version.split("\\.");
+        try {
+            int maj = Integer.parseInt(parts[0]);
+            int min = parts.length > 1 ? Integer.parseInt(parts[1]) : 0;
+            int pat = parts.length > 2 ? Integer.parseInt(parts[2]) : 0;
+            if (maj != major) {
+                return maj > major;
+            }
+            if (min != minor) {
+                return min > minor;
+            }
+            return pat >= patch;
+        } catch (NumberFormatException nfExc) {
+            LOGGER.warn("Unable to parse controller API version '{}'", version);
+            return false;
+        }
+    }
+
     public static String getBestErrorMessage(ApiCallRcList answers) {
         return answers != null && !answers.isEmpty() ?
             answers.stream()

plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java

@@ -19,7 +19,9 @@
 import com.linbit.linstor.api.ApiException;
 import com.linbit.linstor.api.DevelopersApi;
 import com.linbit.linstor.api.model.AutoSelectFilter;
+import com.linbit.linstor.api.model.ControllerVersion;
 import com.linbit.linstor.api.model.LayerType;
+import com.linbit.linstor.api.model.ResourceDefinitionCloneRequest;
 import com.linbit.linstor.api.model.ResourceGroup;
 
 import java.util.Arrays;
@@ -35,6 +37,8 @@
 import org.mockito.junit.MockitoJUnitRunner;
 
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -85,4 +89,45 @@ public void testGetEncryptedLayerList() throws ApiException  {
         layers = LinstorUtil.getEncryptedLayerList(api, "EncryptedGrp");
         Assert.assertEquals(Arrays.asList(LayerType.DRBD, LayerType.LUKS, LayerType.STORAGE), layers);
     }
+
+    private DevelopersApi mockApiWithRestVersion(String restApiVersion) throws ApiException {
+        DevelopersApi apiMock = mock(DevelopersApi.class);
+        ControllerVersion version = new ControllerVersion();
+        version.setRestApiVersion(restApiVersion);
+        when(apiMock.controllerVersion()).thenReturn(version);
+        return apiMock;
+    }
+
+    @Test
+    public void testApplyCloneSizeNewController() throws ApiException {
+        DevelopersApi newCtrl = mockApiWithRestVersion("1.29.1");
+        ResourceDefinitionCloneRequest req = new ResourceDefinitionCloneRequest();
+
+        boolean resizeAfter = LinstorPrimaryDataStoreDriverImpl.applyCloneSize(newCtrl, req, 40L * 1024 * 1024 * 1024);
+
+        Assert.assertFalse(resizeAfter);
+        Assert.assertEquals(Collections.singletonList(40L * 1024 * 1024), req.getVolumeSizes());
+    }
+
+    @Test
+    public void testApplyCloneSizeOldController() throws ApiException {
+        DevelopersApi oldCtrl = mockApiWithRestVersion("1.28.0");
+        ResourceDefinitionCloneRequest req = new ResourceDefinitionCloneRequest();
+
+        boolean resizeAfter = LinstorPrimaryDataStoreDriverImpl.applyCloneSize(oldCtrl, req, 40L * 1024 * 1024 * 1024);
+
+        Assert.assertTrue(resizeAfter);
+        Assert.assertNull(req.getVolumeSizes());
+    }
+
+    @Test
+    public void testApplyCloneSizeWithoutSize() throws ApiException {
+        DevelopersApi newCtrl = mockApiWithRestVersion("1.29.1");
+        ResourceDefinitionCloneRequest req = new ResourceDefinitionCloneRequest();
+
+        Assert.assertFalse(LinstorPrimaryDataStoreDriverImpl.applyCloneSize(newCtrl, req, null));
+        Assert.assertFalse(LinstorPrimaryDataStoreDriverImpl.applyCloneSize(newCtrl, req, 0L));
+        Assert.assertNull(req.getVolumeSizes());
+        verify(newCtrl, never()).controllerVersion();
+    }
 }

plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java

@@ -19,6 +19,7 @@
 import com.linbit.linstor.api.ApiException;
 import com.linbit.linstor.api.DevelopersApi;
 import com.linbit.linstor.api.model.AutoSelectFilter;
+import com.linbit.linstor.api.model.ControllerVersion;
 import com.linbit.linstor.api.model.Node;
 import com.linbit.linstor.api.model.Properties;
 import com.linbit.linstor.api.model.ProviderKind;
@@ -124,4 +125,57 @@ public void testGetRscGroupStoragePools() throws ApiException {
                 .collect(Collectors.toList());
         Assert.assertEquals(names, Arrays.asList("nodeA::thinpool", "nodeB::thinpool", "nodeC::thinpool"));
     }
+
+    @Test
+    public void testIsVersionAtLeast() {
+        Assert.assertTrue(LinstorUtil.isVersionAtLeast("1.29.1", 1, 29, 1));
+        Assert.assertTrue(LinstorUtil.isVersionAtLeast("1.29.2", 1, 29, 1));
+        Assert.assertTrue(LinstorUtil.isVersionAtLeast("1.30.0", 1, 29, 1));
+        Assert.assertTrue(LinstorUtil.isVersionAtLeast("2.0.0", 1, 29, 1));
+        Assert.assertTrue(LinstorUtil.isVersionAtLeast("1.30", 1, 29, 1));
+
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("1.29.0", 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("1.29", 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("1.28.5", 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("0.99.9", 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast(null, 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("", 1, 29, 1));
+        Assert.assertFalse(LinstorUtil.isVersionAtLeast("garbage", 1, 29, 1));
+    }
+
+    private DevelopersApi mockApi() {
+        return mock(DevelopersApi.class);
+    }
+
+    private ControllerVersion controllerVersion(String restApiVersion) {
+        ControllerVersion version = new ControllerVersion();
+        version.setRestApiVersion(restApiVersion);
+        return version;
+    }
+
+    @Test
+    public void testGetRestApiVersion() throws ApiException {
+        DevelopersApi ctrl = mockApi();
+        when(ctrl.controllerVersion()).thenReturn(controllerVersion("1.29.1"));
+        Assert.assertEquals("1.29.1", LinstorUtil.getRestApiVersion(ctrl));
+
+        DevelopersApi down = mockApi();
+        when(down.controllerVersion()).thenThrow(new ApiException(503, "unavailable"));
+        Assert.assertNull(LinstorUtil.getRestApiVersion(down));
+    }
+
+    @Test
+    public void testSupportsCloneVolumeSizes() throws ApiException {
+        DevelopersApi newCtrl = mockApi();
+        when(newCtrl.controllerVersion()).thenReturn(controllerVersion("1.29.1"));
+        Assert.assertTrue(LinstorUtil.supportsCloneVolumeSizes(newCtrl));
+
+        DevelopersApi oldCtrl = mockApi();
+        when(oldCtrl.controllerVersion()).thenReturn(controllerVersion("1.29.0"));
+        Assert.assertFalse(LinstorUtil.supportsCloneVolumeSizes(oldCtrl));
+
+        DevelopersApi unreachable = mockApi();
+        when(unreachable.controllerVersion()).thenThrow(new ApiException(503, "unavailable"));
+        Assert.assertFalse(LinstorUtil.supportsCloneVolumeSizes(unreachable));
+    }
 }