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 <[email protected]> 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
