You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by vi...@apache.org on 2024/05/07 07:48:00 UTC
(cloudstack) 04/05: fixup
This is an automated email from the ASF dual-hosted git repository.
vishesh pushed a commit to branch enforce-strict-host-tag-check
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 5ffe5e2d75214010f750c18f4b909042c858f2b2
Author: Vishesh <vi...@gmail.com>
AuthorDate: Thu May 2 16:58:01 2024 +0530
fixup
---
engine/schema/src/main/java/com/cloud/host/HostVO.java | 10 +++++++---
.../com/cloud/deploy/DeploymentPlanningManagerImpl.java | 2 +-
server/src/main/java/com/cloud/vm/UserVmManager.java | 13 +++++++++----
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 8 +++++---
test/integration/smoke/test_vm_strict_host_tags.py | 8 ++++----
5 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/engine/schema/src/main/java/com/cloud/host/HostVO.java b/engine/schema/src/main/java/com/cloud/host/HostVO.java
index 58e93f9d7aa..1a507da7957 100644
--- a/engine/schema/src/main/java/com/cloud/host/HostVO.java
+++ b/engine/schema/src/main/java/com/cloud/host/HostVO.java
@@ -45,6 +45,7 @@ import javax.persistence.Transient;
import org.apache.cloudstack.util.HypervisorTypeConverter;
import org.apache.cloudstack.utils.jsinterpreter.TagAsRuleHelper;
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
@@ -772,7 +773,8 @@ public class HostVO implements Host {
if (StringUtils.isEmpty(serviceOffering.getHostTag()) && StringUtils.isEmpty(template.getTemplateTag())) {
return new HashSet<>();
}
- HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
+ List<String> hostTagsList = getHostTags();
+ HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
HashSet<String> tags = new HashSet<>();
if (StringUtils.isNotEmpty(serviceOffering.getHostTag())) {
tags.addAll(Arrays.asList(serviceOffering.getHostTag().split(",")));
@@ -793,7 +795,8 @@ public class HostVO implements Host {
if (tags.isEmpty()) {
return true;
}
- HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
+ List<String> hostTagsList = getHostTags();
+ HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
return hostTagsSet.containsAll(tags);
}
@@ -802,7 +805,8 @@ public class HostVO implements Host {
if (tags.isEmpty()) {
return new HashSet<>();
}
- HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
+ List<String> hostTagsList = getHostTags();
+ HashSet<String> hostTagsSet = CollectionUtils.isNotEmpty(hostTagsList) ? new HashSet<>(hostTagsList) : new HashSet<>();
tags.removeAll(hostTagsSet);
return tags;
}
diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
index b5a916143db..629cb807185 100644
--- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
+++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
@@ -283,7 +283,7 @@ StateListener<State, VirtualMachine.Event, VirtualMachine>, Configurable {
boolean storageMigrationNeededDuringClusterMigration = false;
for (Volume volume : volumes) {
StoragePoolVO pool = _storagePoolDao.findById(volume.getPoolId());
- if (List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
+ if (pool != null && List.of(ScopeType.HOST, ScopeType.CLUSTER).contains(pool.getScope())) {
storageMigrationNeededDuringClusterMigration = true;
break;
}
diff --git a/server/src/main/java/com/cloud/vm/UserVmManager.java b/server/src/main/java/com/cloud/vm/UserVmManager.java
index ec4fa873397..9b1fc30fdd3 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManager.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManager.java
@@ -22,6 +22,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
+import com.cloud.utils.StringUtils;
import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -148,11 +149,15 @@ public interface UserVmManager extends UserVmService {
void generateUsageEvent(VirtualMachine vm, boolean isDisplay, String eventType);
static Set<String> getStrictHostTags() {
- Set<String> strictHostTags = new HashSet<>(List.of(StrictHostTags.value().split(",")));
- if (EnforceStrictResourceLimitHostTagCheck.value()) {
- strictHostTags.addAll(List.of(ResourceLimitHostTags.value().split(",")));
+ String strictHostTags = StrictHostTags.value();
+ Set<String> strictHostTagsSet = new HashSet<>();
+ if (StringUtils.isNotEmpty(strictHostTags)) {
+ strictHostTagsSet.addAll(List.of(strictHostTags.split(",")));
}
- return strictHostTags;
+ if (EnforceStrictResourceLimitHostTagCheck.value() && StringUtils.isNotEmpty(ResourceLimitHostTags.value())) {
+ strictHostTagsSet.addAll(List.of(ResourceLimitHostTags.value().split(",")));
+ }
+ return strictHostTagsSet;
}
void persistDeviceBusInfo(UserVmVO paramUserVmVO, String paramString);
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index a90183c0b83..50dc9c36e1f 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -1964,9 +1964,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
HostVO instanceHost = _hostDao.findById(vmInstance.getHostId());
_hostDao.loadHostTags(instanceHost);
- if (!instanceHost.checkHostServiceOfferingAndTemplateTags(newServiceOfferingVO, template, UserVmManager.getStrictHostTags())) {
- logger.error(String.format("Cannot upgrade VM [%s] as the new service offering [%s] does not have the required host tags %s.",
- vmInstance, newServiceOfferingVO, instanceHost.getHostTags()));
+ Set<String> strictHostTags = UserVmManager.getStrictHostTags();
+ if (!instanceHost.checkHostServiceOfferingAndTemplateTags(newServiceOfferingVO, template, strictHostTags)) {
+ logger.error("Cannot upgrade VM {} as the new service offering {} does not have the required host tags {}.",
+ vmInstance, newServiceOfferingVO,
+ instanceHost.getHostServiceOfferingAndTemplateMissingTags(newServiceOfferingVO, template, strictHostTags));
return false;
}
}
diff --git a/test/integration/smoke/test_vm_strict_host_tags.py b/test/integration/smoke/test_vm_strict_host_tags.py
index af1eba5f887..e63db571da5 100644
--- a/test/integration/smoke/test_vm_strict_host_tags.py
+++ b/test/integration/smoke/test_vm_strict_host_tags.py
@@ -166,7 +166,7 @@ class TestVMDeploymentPlannerStrictTags(cloudstackTestCase):
self._cleanup.append(vm)
self.fail("VM should not be deployed")
except Exception as e:
- self.assertTrue("No suitable host found for follow compute offering tags: t2" in str(e))
+ self.assertTrue("No suitable host found for vm " in str(e))
class TestScaleVMStrictTags(cloudstackTestCase):
@@ -261,7 +261,7 @@ class TestScaleVMStrictTags(cloudstackTestCase):
vm.start(self.apiclient)
self.fail("VM should not be be able scale and start")
except Exception as e:
- self.assertTrue("No suitable host found for follow compute offering tags: h2" in str(e))
+ self.assertTrue("No suitable host found for vm " in str(e))
class TestRestoreVMStrictTags(cloudstackTestCase):
@@ -333,7 +333,7 @@ class TestRestoreVMStrictTags(cloudstackTestCase):
vm.restore(self.apiclient, templateid=self.template_t2.id, expunge=True)
restored_vm = VirtualMachine.list(self.apiclient, id=vm.id, listall=True)[0]
- self.assertEqual(restored_vm.templateid, self.template_t2.id, "VM was not scaled")
+ self.assertEqual(restored_vm.templateid, self.template_t2.id, "VM was not restored")
@attr(tags=["advanced", "advancedns", "ssh", "smoke"], required_hardware="false")
def test_02_restore_vm_strict_tags_failure(self):
@@ -349,7 +349,7 @@ class TestRestoreVMStrictTags(cloudstackTestCase):
vm.restore(self.apiclient, templateid=self.template_t2.id, expunge=True)
self.fail("VM should not be restored")
except Exception as e:
- self.assertTrue("No suitable host found for follow compute offering tags: t2" in str(e))
+ self.assertTrue("No suitable host found for vm " in str(e))
class TestMigrateVMStrictTags(cloudstackTestCase):
@classmethod