This is an automated email from the ASF dual-hosted git repository.
weizhou pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.18 by this push:
new 4ef7ebbded8 ssvm: pass all accessible secondary storage to ssvm (#7410)
4ef7ebbded8 is described below
commit 4ef7ebbded87bcc5db04678d0b63e590aca7e128
Author: Abhishek Kumar <[email protected]>
AuthorDate: Mon Jun 12 12:32:42 2023 +0530
ssvm: pass all accessible secondary storage to ssvm (#7410)
* ssvm: pass all accessible secondary storage to ssvm
Fixes #7162
Signed-off-by: Abhishek Kumar <[email protected]>
* changes
Signed-off-by: Abhishek Kumar <[email protected]>
* test
Signed-off-by: Abhishek Kumar <[email protected]>
* license
Signed-off-by: Abhishek Kumar <[email protected]>
---------
Signed-off-by: Abhishek Kumar <[email protected]>
---
.../SecondaryStorageManagerImpl.java | 49 ++++++++----
.../SecondaryStorageManagerImplTest.java | 89 ++++++++++++++++++++++
systemvm/agent/scripts/ssvm-check.sh | 27 ++++---
3 files changed, 139 insertions(+), 26 deletions(-)
diff --git
a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
index aa03211d9c0..f93d3e28a38 100644
---
a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
+++
b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java
@@ -26,6 +26,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
@@ -1065,11 +1066,12 @@ public class SecondaryStorageManagerImpl extends
ManagerBase implements Secondar
Map<String, String> details =
_vmDetailsDao.listDetailsKeyPairs(vm.getId());
vm.setDetails(details);
- DataStore secStore =
_dataStoreMgr.getImageStoreWithFreeCapacity(dest.getDataCenter().getId());
- if (secStore == null) {
+ List<DataStore> secStores=
_dataStoreMgr.listImageStoresWithFreeCapacity(dest.getDataCenter().getId());
+ if (CollectionUtils.isEmpty(secStores)) {
s_logger.warn(String.format("Unable to finalize virtual machine
profile [%s] as it has no secondary storage available to satisfy storage needs
for zone [%s].", profile.toString(), dest.getDataCenter().getUuid()));
return false;
}
+ Collections.shuffle(secStores);
final Map<String, String> sshAccessDetails =
_networkMgr.getSystemVMAccessDetails(profile.getVirtualMachine());
final Map<String, String> ipAddressDetails = new
HashMap<>(sshAccessDetails);
@@ -1163,7 +1165,7 @@ public class SecondaryStorageManagerImpl extends
ManagerBase implements Secondar
if (dc.getDns2() != null) {
buf.append(" dns2=").append(dc.getDns2());
}
- String nfsVersion = imageStoreDetailsUtil != null ?
imageStoreDetailsUtil.getNfsVersion(secStore.getId()) : null;
+ String nfsVersion = imageStoreDetailsUtil != null ?
imageStoreDetailsUtil.getNfsVersion(secStores.get(0).getId()) : null;
buf.append(" nfsVersion=").append(nfsVersion);
buf.append("
keystore_password=").append(VirtualMachineGuru.getEncodedString(PasswordGenerator.generateRandomPassword(16)));
String bootArgs = buf.toString();
@@ -1175,27 +1177,44 @@ public class SecondaryStorageManagerImpl extends
ManagerBase implements Secondar
s_logger.debug(String.format("Setting UseHttpsToUpload config on
cmdline with [%s] value.", useHttpsToUpload));
buf.append(" useHttpsToUpload=").append(useHttpsToUpload);
- addSecondaryStorageServerAddressToBuffer(buf, secStore, vmName);
+ addSecondaryStorageServerAddressToBuffer(buf, secStores, vmName);
return true;
}
/**
- * Adds the secondary storage address to the buffer if it is in the
following pattern: <protocol>//<address>/...
+ * Adds the secondary storages address to the buffer if it is in the
following pattern: <protocol>//<address>/...
*/
- protected void addSecondaryStorageServerAddressToBuffer(StringBuilder
buffer, DataStore dataStore, String vmName) {
- String url = dataStore.getTO().getUrl();
- String[] urlArray = url.split("/");
+ protected void addSecondaryStorageServerAddressToBuffer(StringBuilder
buffer, List<DataStore> dataStores, String vmName) {
+ List<String> addresses = new ArrayList<>();
+ for (DataStore dataStore: dataStores) {
+ String url = dataStore.getTO().getUrl();
+ String[] urlArray = url.split("/");
+
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("Found [%s] as secondary storage
[%s] URL for SSVM [%s].", dataStore.getName(), url, vmName));
+ }
+ if (ArrayUtils.getLength(urlArray) < 3) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("Could not retrieve secondary
storage [%s] address from URL [%s] of SSVM [%s].", dataStore.getName(), url,
vmName));
+ }
+ continue;
+ }
+
+ String address = urlArray[2];
+ s_logger.info(String.format("Using [%s] as address of secondary
storage [%s] of SSVM [%s].", address, dataStore.getName(), vmName));
+ if (!addresses.contains(address)) {
+ addresses.add(address);
+ }
- s_logger.debug(String.format("Found [%s] as secondary storage's URL
for SSVM [%s].", url, vmName));
- if (ArrayUtils.getLength(urlArray) < 3) {
- s_logger.debug(String.format("Could not retrieve secondary storage
address from URL [%s] of SSVM [%s].", url, vmName));
+ }
+ if (addresses.isEmpty()) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("No address found for the
secondary storages: [%s] of SSVM: [%s]",
StringUtils.join(dataStores.stream().map(DataStore::getName).collect(Collectors.toList()),
","), vmName));
+ }
return;
}
-
- String address = urlArray[2];
- s_logger.info(String.format("Using [%s] as address of secondary
storage of SSVM [%s].", address, vmName));
- buffer.append(" secondaryStorageServerAddress=").append(address);
+ buffer.append("
secondaryStorageServerAddress=").append(StringUtils.join(addresses, ","));
}
@Override
diff --git
a/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java
b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java
new file mode 100644
index 00000000000..6df205e1aac
--- /dev/null
+++
b/services/secondary-storage/controller/src/test/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImplTest.java
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.secondarystorage;
+
+import java.security.SecureRandom;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import org.apache.commons.lang3.StringUtils;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.utils.net.NetUtils;
+import com.google.common.net.InetAddresses;
+
+@RunWith(MockitoJUnitRunner.class)
+public class SecondaryStorageManagerImplTest {
+ private final SecureRandom secureRandom = new SecureRandom();
+
+ @Spy
+ @InjectMocks
+ private SecondaryStorageManagerImpl secondaryStorageManager;
+
+ private List<DataStore>
mockDataStoresForTestAddSecondaryStorageServerAddressToBuffer(List<String>
addresses) {
+ List<DataStore> dataStores = new ArrayList<>();
+ for (String address: addresses) {
+ DataStore dataStore = Mockito.mock(DataStore.class);
+ DataStoreTO dataStoreTO = Mockito.mock(DataStoreTO.class);
+
Mockito.when(dataStoreTO.getUrl()).thenReturn(NetUtils.isValidIp4(address) ?
String.format("http://%s";, address) : address);
+ Mockito.when(dataStore.getTO()).thenReturn(dataStoreTO);
+ dataStores.add(dataStore);
+ }
+ return dataStores;
+ }
+
+ private void runAddSecondaryStorageServerAddressToBufferTest(List<String>
addresses, String expected) {
+ List<DataStore> dataStores =
mockDataStoresForTestAddSecondaryStorageServerAddressToBuffer(addresses);
+ StringBuilder builder = new StringBuilder();
+
secondaryStorageManager.addSecondaryStorageServerAddressToBuffer(builder,
dataStores, "VM");
+ String result = builder.toString();
+ result = result.contains("=") ? result.split("=")[1] : null;
+ Assert.assertEquals(expected, result);
+ }
+
+ @Test
+ public void testAddSecondaryStorageServerAddressToBufferDifferentAddress()
{
+ String randomIp1 =
InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress();
+ String randomIp2 =
InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress();
+ List<String> addresses = List.of(randomIp1, randomIp2);
+ String expected = StringUtils.join(addresses, ",");
+ runAddSecondaryStorageServerAddressToBufferTest(addresses, expected);
+ }
+
+ @Test
+ public void testAddSecondaryStorageServerAddressToBufferSameAddress() {
+ String randomIp1 =
InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress();
+ List<String> addresses = List.of(randomIp1, randomIp1);
+ runAddSecondaryStorageServerAddressToBufferTest(addresses, randomIp1);
+ }
+
+ @Test
+ public void testAddSecondaryStorageServerAddressToBufferInvalidAddress() {
+ String randomIp1 =
InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress();
+ String randomIp2 =
InetAddresses.fromInteger(secureRandom.nextInt()).getHostAddress();
+ List<String> addresses = List.of(randomIp1, "garbage", randomIp2);
+ runAddSecondaryStorageServerAddressToBufferTest(addresses,
StringUtils.join(List.of(randomIp1, randomIp2), ","));
+ }
+}
diff --git a/systemvm/agent/scripts/ssvm-check.sh
b/systemvm/agent/scripts/ssvm-check.sh
index 5fdc244debd..b2721a93b3f 100644
--- a/systemvm/agent/scripts/ssvm-check.sh
+++ b/systemvm/agent/scripts/ssvm-check.sh
@@ -101,7 +101,7 @@ then
else
echo "ERROR: Storage $storage is not currently mounted"
echo "Verifying if we can at least ping the storage"
- STORAGE_ADDRESS=`grep "secondaryStorageServerAddress" $CMDLINE | sed -E
's/.*secondaryStorageServerAddress=([^ ]*).*/\1/g'`
+ STORAGE_ADDRESSES=`grep "secondaryStorageServerAddress" $CMDLINE | sed -E
's/.*secondaryStorageServerAddress=([^ ]*).*/\1/g'`
if [[ -z "$STORAGE_ADDRESS" ]]
then
@@ -117,16 +117,21 @@ else
route -n
fi
else
- echo "Storage address is $STORAGE_ADDRESS, trying to ping it"
- ping -c 2 $STORAGE_ADDRESS
- if [ $? -eq 0 ]
- then
- echo "Good: Can ping $storage storage address"
- else
- echo "WARNING: Cannot ping $storage storage address"
- echo routing table follows
- route -n
- fi
+ echo "Storage address(s): $STORAGE_ADDRESSES, trying to ping"
+ STORAGE_ADDRESS_LIST=$(echo $STORAGE_ADDRESSES | tr ",")
+ for STORAGE_ADDRESS in $STORAGE_ADDRESS_LIST
+ do
+ echo "Pinging storage address: $STORAGE_ADDRESS"
+ ping -c 2 $STORAGE_ADDRESS
+ if [ $? -eq 0 ]
+ then
+ echo "Good: Can ping $storage storage address"
+ else
+ echo "WARNING: Cannot ping $storage storage address"
+ echo routing table follows
+ route -n
+ fi
+ done
fi
fi
