diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java index c3b4e73ead03..d3463b1cf2bc 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java @@ -251,7 +251,7 @@ private void deleteSnapshot(@Nonnull DataStore dataStore, @Nonnull String rscDef try { - ApiCallRcList answers = linstorApi.resourceSnapshotDelete(rscDefName, snapshotName, Collections.emptyList()); + ApiCallRcList answers = linstorApi.resourceSnapshotDelete(rscDefName, snapshotName, Collections.emptyList(), null); if (answers.hasError()) { for (ApiCallRc answer : answers) @@ -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()); } diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java index 67c070f84eb0..14cb73cc3514 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorUtil.java +++ b/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() diff --git a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java index d0f82484694b..ee3e17b64090 100644 --- a/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java +++ b/plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/snapshot/LinstorVMSnapshotStrategy.java @@ -214,7 +214,7 @@ private StrategyPriority allVolumesOnLinstor(Long vmId) { private String linstorDeleteSnapshot(final DevelopersApi api, final String rscName, final String snapshotName) { String resultMsg = null; try { - ApiCallRcList answers = api.resourceSnapshotDelete(rscName, snapshotName, Collections.emptyList()); + ApiCallRcList answers = api.resourceSnapshotDelete(rscName, snapshotName, Collections.emptyList(), null); if (answers.hasError()) { resultMsg = LinstorUtil.getBestErrorMessage(answers); } diff --git a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java index 4653cfa358b0..99bb66fa2eb5 100644 --- a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImplTest.java +++ b/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(); + } } diff --git a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java b/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java index 55f0c6ebe6dc..cca4ef69d7b1 100644 --- a/plugins/storage/volume/linstor/src/test/java/org/apache/cloudstack/storage/datastore/util/LinstorUtilTest.java +++ b/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)); + } } diff --git a/pom.xml b/pom.xml index 1c7ae233d789..751212a7df99 100644 --- a/pom.xml +++ b/pom.xml @@ -173,7 +173,7 @@ 10.1 2.6.6 0.6.0 - 0.7.0 + 0.8.1 0.10.2 3.4.4_1 4.0.1 diff --git a/test/integration/plugins/linstor/test_linstor_volumes.py b/test/integration/plugins/linstor/test_linstor_volumes.py index c2c220bfe9e9..94291af85405 100644 --- a/test/integration/plugins/linstor/test_linstor_volumes.py +++ b/test/integration/plugins/linstor/test_linstor_volumes.py @@ -857,7 +857,10 @@ def test_07_detach_volume_reboot_vm(self): # STEP 3: Reboot VM with detached vol # ####################################### - self.virtual_machine.reboot(self.apiClient) + # via the helper, which waits for the guest to come back: CloudStack reports + # Running as soon as the domain exists, so leaving it un-awaited lets the next + # test attach and detach a volume against a guest that has not enumerated it yet + TestLinstorVolumes._reboot_vm(self.virtual_machine) vm = self._get_vm(self.virtual_machine.id)