Skip to content

Commit 3c53cf4

Browse files
committed
Add e2e tests
1 parent 3bedcc9 commit 3c53cf4

File tree

9 files changed

+542
-26
lines changed

9 files changed

+542
-26
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ jobs:
132132
smoke/test_usage
133133
smoke/test_usage_events
134134
smoke/test_vm_deployment_planner
135+
smoke/test_vm_strict_host_tags
135136
smoke/test_vm_schedule
136137
smoke/test_vm_life_cycle
137138
smoke/test_vm_lifecycle_unmanage_import

engine/schema/src/main/java/com/cloud/host/HostVO.java

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -768,12 +768,9 @@ public void setUuid(String uuid) {
768768
this.uuid = uuid;
769769
}
770770

771-
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
772-
if (serviceOffering == null || template == null) {
773-
return false;
774-
}
771+
private Set<String> getHostServiceOfferingAndTemplateStrictTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
775772
if (StringUtils.isEmpty(serviceOffering.getHostTag()) && StringUtils.isEmpty(template.getTemplateTag())) {
776-
return true;
773+
return new HashSet<>();
777774
}
778775
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
779776
HashSet<String> tags = new HashSet<>();
@@ -784,12 +781,32 @@ public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOf
784781
tags.add(template.getTemplateTag());
785782
}
786783
tags.removeIf(tag -> !strictHostTags.contains(tag));
784+
tags.removeAll(hostTagsSet);
785+
return tags;
786+
}
787+
788+
public boolean checkHostServiceOfferingAndTemplateTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
789+
if (serviceOffering == null || template == null) {
790+
return false;
791+
}
792+
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
787793
if (tags.isEmpty()) {
788794
return true;
789795
}
796+
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
790797
return hostTagsSet.containsAll(tags);
791798
}
792799

800+
public Set<String> getHostServiceOfferingAndTemplateMissingTags(ServiceOffering serviceOffering, VirtualMachineTemplate template, Set<String> strictHostTags) {
801+
Set<String> tags = getHostServiceOfferingAndTemplateStrictTags(serviceOffering, template, strictHostTags);
802+
if (tags.isEmpty()) {
803+
return new HashSet<>();
804+
}
805+
HashSet<String> hostTagsSet = new HashSet<>(getHostTags());
806+
tags.removeAll(hostTagsSet);
807+
return tags;
808+
}
809+
793810
public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering) {
794811
if (serviceOffering == null) {
795812
return false;

engine/schema/src/test/java/com/cloud/host/HostVOTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.List;
3030
import java.util.Set;
3131

32+
import static org.junit.Assert.assertEquals;
3233
import static org.junit.Assert.assertFalse;
3334
import static org.junit.Assert.assertTrue;
3435

@@ -97,23 +98,35 @@ public void testEitherNoSOOrTemplate() {
9798
@Test
9899
public void testNoTagOfferingTemplate() {
99100
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()));
101+
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Collections.emptySet()).isEmpty());
100102
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")));
103+
assertTrue(host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1", "tag2")).isEmpty());
101104
}
102105

103106
@Test
104107
public void testRightTagOfferingTemplate() {
105108
host.setHostTags(Arrays.asList("tag1", "tag2"), false);
106109
offering.setHostTag("tag2,tag1");
107110
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1")));
111+
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, Mockito.mock(VirtualMachineTemplate.class), Set.of("tag1"));
112+
assertTrue(actualMissingTags.isEmpty());
113+
108114
host.setHostTags(Arrays.asList("tag1", "tag2", "tag3"), false);
109115
offering.setHostTag("tag2,tag1");
110116
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
111117
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
112118
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag3")));
119+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag3"));
120+
assertTrue(actualMissingTags.isEmpty());
113121
host.setHostTags(List.of("tag3"), false);
114122
offering.setHostTag(null);
115123
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag3")));
124+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag3"));
125+
assertTrue(actualMissingTags.isEmpty());
126+
116127
assertTrue(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag2", "tag1")));
128+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag2", "tag1"));
129+
assertTrue(actualMissingTags.isEmpty());
117130
}
118131

119132
@Test
@@ -123,9 +136,14 @@ public void testWrongOfferingTag() {
123136
VirtualMachineTemplate template = Mockito.mock(VirtualMachineTemplate.class);
124137
Mockito.when(template.getTemplateTag()).thenReturn("tag1");
125138
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
139+
Set<String> actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
140+
assertEquals(Set.of("tag4"), actualMissingTags);
141+
126142
offering.setHostTag("tag1,tag2");
127143
template = Mockito.mock(VirtualMachineTemplate.class);
128144
Mockito.when(template.getTemplateTag()).thenReturn("tag3");
145+
actualMissingTags = host.getHostServiceOfferingAndTemplateMissingTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4"));
129146
assertFalse(host.checkHostServiceOfferingAndTemplateTags(offering, template, Set.of("tag1", "tag2", "tag3", "tag4")));
147+
assertEquals(Set.of("tag3"), actualMissingTags);
130148
}
131149
}

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,6 +2075,7 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI
20752075

20762076
// Check disable threshold for cluster is not crossed
20772077
HostVO host = _hostDao.findById(vmInstance.getHostId());
2078+
_hostDao.loadDetails(host);
20782079
if (_capacityMgr.checkIfClusterCrossesThreshold(host.getClusterId(), cpuDiff, memoryDiff)) {
20792080
throw new CloudRuntimeException(String.format("Unable to scale %s due to insufficient resources.", vmInstance.toString()));
20802081
}
@@ -2087,12 +2088,14 @@ private boolean upgradeRunningVirtualMachine(Long vmId, Long newServiceOfferingI
20872088
_resourceLimitMgr.updateVmResourceCountForServiceOfferingChange(caller.getAccountId(), vmInstance.isDisplay(),
20882089
(long) currentCpu, (long) newCpu, (long) currentMemory, (long) newMemory,
20892090
currentServiceOffering, newServiceOffering, template);
2090-
// #1 Check existing host has capacity
2091+
2092+
// #1 Check existing host has capacity & and the correct tags
20912093
if (!excludes.shouldAvoid(ApiDBUtils.findHostById(vmInstance.getHostId()))) {
20922094
existingHostHasCapacity = _capacityMgr.checkIfHostHasCpuCapability(vmInstance.getHostId(), newCpu, newSpeed)
20932095
&& _capacityMgr.checkIfHostHasCapacity(vmInstance.getHostId(), cpuDiff, ByteScaleUtils.mebibytesToBytes(memoryDiff), false,
20942096
_capacityMgr.getClusterOverProvisioningFactor(host.getClusterId(), Capacity.CAPACITY_TYPE_CPU),
2095-
_capacityMgr.getClusterOverProvisioningFactor(host.getClusterId(), Capacity.CAPACITY_TYPE_MEMORY), false);
2097+
_capacityMgr.getClusterOverProvisioningFactor(host.getClusterId(), Capacity.CAPACITY_TYPE_MEMORY), false)
2098+
&& checkEnforceStrictHostTagCheck(vmInstance, host);
20962099
excludes.addHost(vmInstance.getHostId());
20972100
}
20982101

@@ -5490,7 +5493,7 @@ public Pair<UserVmVO, Map<VirtualMachineProfile.Param, Object>> startVirtualMach
54905493
if (destinationHost != null) {
54915494
logger.debug("Destination Host to deploy the VM is specified, specifying a deployment plan to deploy the VM");
54925495
_hostDao.loadHostTags(destinationHost);
5493-
checkEnforceStrictHostTagCheck(vm, destinationHost);
5496+
validateStrictHostTagCheck(vm, destinationHost);
54945497

54955498
final ServiceOfferingVO offering = serviceOfferingDao.findById(vm.getId(), vm.getServiceOfferingId());
54965499
Pair<Boolean, Boolean> cpuCapabilityAndCapacity = _capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(destinationHost, offering, false);
@@ -6705,16 +6708,26 @@ private DeployDestination chooseVmMigrationDestination(VMInstanceVO vm, Host src
67056708
}
67066709
}
67076710

6708-
protected void checkEnforceStrictHostTagCheck(VMInstanceVO vm, HostVO host) {
6711+
protected boolean checkEnforceStrictHostTagCheck(VMInstanceVO vm, HostVO host) {
67096712
ServiceOffering serviceOffering = serviceOfferingDao.findByIdIncludingRemoved(vm.getServiceOfferingId());
67106713
VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId());
6714+
return checkEnforceStrictHostTagCheck(host, serviceOffering, template);
6715+
}
67116716

6717+
private boolean checkEnforceStrictHostTagCheck(HostVO host, ServiceOffering serviceOffering, VirtualMachineTemplate template) {
67126718
Set<String> strictHostTags = UserVmManager.getStrictHostTags();
6713-
if (!host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, strictHostTags)) {
6714-
s_logger.error(String.format(
6715-
"Cannot deploy VM: %s to host : %s due to tag mismatch." +
6716-
" strictHosts: %s serviceOffering tags: %s, template tags: %s",
6717-
vm, host, strictHostTags, serviceOffering.getHostTag(), template.getTemplateTag()));
6719+
return host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, strictHostTags);
6720+
}
6721+
6722+
protected void validateStrictHostTagCheck(VMInstanceVO vm, HostVO host) {
6723+
ServiceOffering serviceOffering = serviceOfferingDao.findByIdIncludingRemoved(vm.getServiceOfferingId());
6724+
VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId());
6725+
6726+
if (!checkEnforceStrictHostTagCheck(host, serviceOffering, template)) {
6727+
Set<String> missingTags = host.getHostServiceOfferingAndTemplateMissingTags(serviceOffering, template, UserVmManager.getStrictHostTags());
6728+
logger.error("Cannot deploy VM: {} to host : {} due to tag mismatch. host tags: {}, " +
6729+
"strict host tags: {} serviceOffering tags: {}, template tags: {}, missing tags: {}",
6730+
vm, host, host.getHostTags(), UserVmManager.getStrictHostTags(), serviceOffering.getHostTag(), template.getTemplateTag(), missingTags);
67186731
throw new InvalidParameterValueException(String.format("Cannot deploy VM, destination host: %s " +
67196732
"is not compatible for the VM", host.getName()));
67206733
}
@@ -6747,7 +6760,7 @@ private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcH
67476760

67486761
HostVO destinationHostVO = _hostDao.findById(destinationHost.getId());
67496762
_hostDao.loadHostTags(destinationHostVO);
6750-
checkEnforceStrictHostTagCheck(vm, destinationHostVO);
6763+
validateStrictHostTagCheck(vm, destinationHostVO);
67516764

67526765
checkHostsDedication(vm, srcHost.getId(), destinationHost.getId());
67536766

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,7 +1565,7 @@ public void testCheckVolumesLimits() {
15651565
}
15661566

15671567
@Test
1568-
public void testCheckEnforceStrictHostTagCheckPass() {
1568+
public void testValidateStrictHostTagCheckPass() {
15691569
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
15701570
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
15711571

@@ -1580,15 +1580,15 @@ public void testCheckEnforceStrictHostTagCheckPass() {
15801580

15811581
Mockito.when(destinationHostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet())).thenReturn(true);
15821582

1583-
userVmManagerImpl.checkEnforceStrictHostTagCheck(vm, destinationHostVO);
1583+
userVmManagerImpl.validateStrictHostTagCheck(vm, destinationHostVO);
15841584

15851585
Mockito.verify(
15861586
destinationHostVO, Mockito.times(1)
15871587
).checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet());
15881588
}
15891589

15901590
@Test(expected = InvalidParameterValueException.class)
1591-
public void testCheckEnforceStrictHostTagCheckFail() {
1591+
public void testValidateStrictHostTagCheckFail() {
15921592
ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class);
15931593
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
15941594

@@ -1602,6 +1602,6 @@ public void testCheckEnforceStrictHostTagCheckFail() {
16021602
Mockito.when(vm.getTemplateId()).thenReturn(2L);
16031603

16041604
Mockito.when(destinationHostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(ServiceOffering.class), Mockito.any(VirtualMachineTemplate.class), Mockito.anySet())).thenReturn(false);
1605-
userVmManagerImpl.checkEnforceStrictHostTagCheck(vm, destinationHostVO);
1605+
userVmManagerImpl.validateStrictHostTagCheck(vm, destinationHostVO);
16061606
}
16071607
}

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,6 @@ public void setUp() throws Exception {
302302
HostVO hostVO = Mockito.mock(HostVO.class);
303303
when(hostVO.isInMaintenanceStates()).thenReturn(false);
304304
hosts.add(hostVO);
305-
when(hostVO.checkHostServiceOfferingAndTemplateTags(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(true);
306305
when(resourceManager.listHostsInClusterByStatus(anyLong(), any(Status.class))).thenReturn(hosts);
307306
when(resourceManager.listAllUpAndEnabledHostsInOneZoneByHypervisor(any(Hypervisor.HypervisorType.class), anyLong())).thenReturn(hosts);
308307
List<VMTemplateStoragePoolVO> templates = new ArrayList<>();

0 commit comments

Comments
 (0)