winterhazel commented on code in PR #9074:
URL: https://github.com/apache/cloudstack/pull/9074#discussion_r1955165738
##########
server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java:
##########
@@ -85,315 +80,205 @@ public class FirstFitAllocator extends AdapterBase
implements HostAllocator {
@Inject
protected ResourceManager _resourceMgr;
@Inject
- ClusterDao _clusterDao;
- @Inject
- ClusterDetailsDao _clusterDetailsDao;
- @Inject
ServiceOfferingDetailsDao _serviceOfferingDetailsDao;
@Inject
- CapacityManager _capacityMgr;
- @Inject
CapacityDao _capacityDao;
@Inject
UserVmDetailsDao _userVmDetailsDao;
boolean _checkHvm = true;
- protected String _allocationAlgorithm = "random";
+
+ protected AllocationAlgorithm allocationAlgorithm =
AllocationAlgorithm.random;
@Override
public List<Host> allocateTo(VirtualMachineProfile vmProfile,
DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo) {
- return allocateTo(vmProfile, plan, type, avoid, returnUpTo, true);
+ return allocateTo(vmProfile, plan, type, avoid, null, returnUpTo,
true);
}
@Override
- public List<Host> allocateTo(VirtualMachineProfile vmProfile,
DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo, boolean
considerReservedCapacity) {
+ public List<Host> allocateTo(VirtualMachineProfile vmProfile,
DeploymentPlan plan, Type type, ExcludeList avoid, List<? extends Host> hosts,
int returnUpTo,
+ boolean considerReservedCapacity) {
+ if (type == Host.Type.Storage) {
+ return null;
+ }
long dcId = plan.getDataCenterId();
Long podId = plan.getPodId();
Long clusterId = plan.getClusterId();
ServiceOffering offering = vmProfile.getServiceOffering();
- VMTemplateVO template = (VMTemplateVO)vmProfile.getTemplate();
+ VMTemplateVO template = (VMTemplateVO) vmProfile.getTemplate();
Account account = vmProfile.getOwner();
- boolean isVMDeployedWithUefi = false;
- UserVmDetailVO userVmDetailVO =
_userVmDetailsDao.findDetail(vmProfile.getId(), "UEFI");
- if(userVmDetailVO != null){
- if ("secure".equalsIgnoreCase(userVmDetailVO.getValue()) ||
"legacy".equalsIgnoreCase(userVmDetailVO.getValue())) {
- isVMDeployedWithUefi = true;
- }
- }
- logger.info(" Guest VM is requested with Custom[UEFI] Boot Type "+
isVMDeployedWithUefi);
+ String hostTagOnOffering = offering.getHostTag();
+ String hostTagOnTemplate = template.getTemplateTag();
+ String paramAsStringToLog = String.format("zone [%s], pod [%s],
cluster [%s]", dcId, podId, clusterId);
+ List<HostVO> suitableHosts = retrieveHosts(vmProfile, type,
(List<HostVO>) hosts, clusterId, podId, dcId, hostTagOnOffering,
hostTagOnTemplate);
- if (type == Host.Type.Storage) {
- // FirstFitAllocator should be used for user VMs only since it
won't care whether the host is capable of routing or not
- return new ArrayList<>();
+ if (suitableHosts.isEmpty()) {
+ logger.info("No suitable host found for VM [{}] in {}.",
vmProfile, paramAsStringToLog);
+ return null;
}
- logger.debug("Looking for hosts in zone [{}], pod [{}], cluster [{}]",
dcId, podId, clusterId);
+ if (CollectionUtils.isEmpty(hosts)) {
+ addHostsToAvoidSet(type, avoid, clusterId, podId, dcId,
suitableHosts);
+ }
- String hostTagOnOffering = offering.getHostTag();
- String hostTagOnTemplate = template.getTemplateTag();
- String hostTagUefi = "UEFI";
+ return allocateTo(plan, offering, template, avoid, suitableHosts,
returnUpTo, considerReservedCapacity, account);
+ }
- boolean hasSvcOfferingTag = hostTagOnOffering != null ? true : false;
- boolean hasTemplateTag = hostTagOnTemplate != null ? true : false;
+ protected List<HostVO> retrieveHosts(VirtualMachineProfile vmProfile, Type
type, List<HostVO> hostsToFilter, Long clusterId, Long podId, long dcId, String
hostTagOnOffering,
+ String hostTagOnTemplate) {
+ String haVmTag = (String)
vmProfile.getParameter(VirtualMachineProfile.Param.HaTag);
+ List<HostVO> clusterHosts;
- List<HostVO> clusterHosts = new ArrayList<>();
- List<HostVO> hostsMatchingUefiTag = new ArrayList<>();
- if(isVMDeployedWithUefi){
- hostsMatchingUefiTag = _hostDao.listByHostCapability(type,
clusterId, podId, dcId, Host.HOST_UEFI_ENABLE);
- if (logger.isDebugEnabled()) {
- logger.debug("Hosts with tag '" + hostTagUefi + "' are:" +
hostsMatchingUefiTag);
- }
+ if (CollectionUtils.isNotEmpty(hostsToFilter)) {
+ clusterHosts = new ArrayList<>(hostsToFilter);
+ } else {
+ clusterHosts = _resourceMgr.listAllUpAndEnabledHosts(type,
clusterId, podId, dcId);
}
-
- String haVmTag =
(String)vmProfile.getParameter(VirtualMachineProfile.Param.HaTag);
if (haVmTag != null) {
- clusterHosts = _hostDao.listByHostTag(type, clusterId, podId,
dcId, haVmTag);
+ clusterHosts.retainAll(hostDao.listByHostTag(type, clusterId,
podId, dcId, haVmTag));
+ } else if (ObjectUtils.allNull(hostTagOnOffering, hostTagOnTemplate)) {
+
clusterHosts.retainAll(_resourceMgr.listAllUpAndEnabledNonHAHosts(type,
clusterId, podId, dcId));
} else {
- if (hostTagOnOffering == null && hostTagOnTemplate == null) {
- clusterHosts =
_resourceMgr.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId);
- } else {
- List<HostVO> hostsMatchingOfferingTag = new ArrayList<>();
- List<HostVO> hostsMatchingTemplateTag = new ArrayList<>();
- if (hasSvcOfferingTag) {
- if (logger.isDebugEnabled()) {
- logger.debug("Looking for hosts having tag specified
on SvcOffering:" + hostTagOnOffering);
- }
- hostsMatchingOfferingTag = _hostDao.listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
- if (logger.isDebugEnabled()) {
- logger.debug("Hosts with tag '" + hostTagOnOffering +
"' are:" + hostsMatchingOfferingTag);
- }
- }
- if (hasTemplateTag) {
- if (logger.isDebugEnabled()) {
- logger.debug("Looking for hosts having tag specified
on Template:" + hostTagOnTemplate);
- }
- hostsMatchingTemplateTag = _hostDao.listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
- if (logger.isDebugEnabled()) {
- logger.debug("Hosts with tag '" + hostTagOnTemplate +
"' are:" + hostsMatchingTemplateTag);
- }
- }
-
- if (hasSvcOfferingTag && hasTemplateTag) {
-
hostsMatchingOfferingTag.retainAll(hostsMatchingTemplateTag);
- if (logger.isDebugEnabled()) {
- logger.debug("Found " +
hostsMatchingOfferingTag.size() + " Hosts satisfying both tags, host ids are:"
+ hostsMatchingOfferingTag);
- }
-
- clusterHosts = hostsMatchingOfferingTag;
- } else {
- if (hasSvcOfferingTag) {
- clusterHosts = hostsMatchingOfferingTag;
- } else {
- clusterHosts = hostsMatchingTemplateTag;
- }
- }
- }
+ retainHostsMatchingServiceOfferingAndTemplateTags(clusterHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
Review Comment:
```suggestion
retainHostsMatchingServiceOfferingAndTemplateTags(clusterHosts,
type, dcId, podId, clusterId, hostTagOnOffering, hostTagOnTemplate);
```
The parameters are flipped
##########
server/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java:
##########
@@ -0,0 +1,132 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import javax.inject.Inject;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.lang3.ObjectUtils;
+import org.springframework.stereotype.Component;
+
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner.ExcludeList;
+import com.cloud.host.Host;
+import com.cloud.host.Host.Type;
+import com.cloud.host.HostVO;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.resource.ResourceManager;
+import com.cloud.storage.VMTemplateVO;
+import com.cloud.vm.VirtualMachineProfile;
+
+@Component
+public class RandomAllocator extends BaseAllocator {
+ @Inject
+ private ResourceManager _resourceMgr;
+
+ protected List<Host> findSuitableHosts(VirtualMachineProfile vmProfile,
DeploymentPlan plan, Type type, ExcludeList avoid, List<? extends Host> hosts,
int returnUpTo,
+ boolean considerReservedCapacity) {
+ if (type == Host.Type.Storage) {
+ return null;
+ }
+
+ long dcId = plan.getDataCenterId();
+ Long podId = plan.getPodId();
+ Long clusterId = plan.getClusterId();
+ ServiceOffering offering = vmProfile.getServiceOffering();
+
+ String offeringHostTag = offering.getHostTag();
+ VMTemplateVO template = (VMTemplateVO) vmProfile.getTemplate();
+ logger.debug("Looking for hosts in zone [{}], pod [{}], cluster
[{}].", dcId, podId, clusterId);
+
+ List<? extends Host> availableHosts = retrieveHosts(type,
(List<HostVO>) hosts, template, offeringHostTag, clusterId, podId, dcId);
+
+ if (availableHosts.isEmpty()) {
+ logger.info("No suitable host found for VM [{}] in zone [{}], pod
[{}], cluster [{}].", vmProfile, dcId, podId, clusterId);
+ return null;
+ }
+
+ return filterAvailableHosts(avoid, returnUpTo,
considerReservedCapacity, availableHosts, offering);
+ }
+
+ protected List<Host> filterAvailableHosts(ExcludeList avoid, int
returnUpTo, boolean considerReservedCapacity, List<? extends Host>
availableHosts, ServiceOffering offering) {
+ logger.debug("Random Allocator found [{}] available hosts. They will
be checked if they are in the avoid set and for CPU capability and capacity.",
availableHosts::size);
+ List<Host> suitableHosts = new ArrayList<>();
+
+ Collections.shuffle(availableHosts);
+ for (Host host : availableHosts) {
+ if (suitableHosts.size() == returnUpTo) {
+ break;
+ }
+
+ if (avoid.shouldAvoid(host)) {
+ logger.debug("Host [{}] is in the avoid set, skipping it and
trying other available hosts.", () -> host);
+ continue;
+ }
+
+ if (!hostHasCpuCapabilityAndCapacity(considerReservedCapacity,
offering, host)) {
+ continue;
+ }
+
+ logger.debug("Found the suitable host [{}], adding to list.", ()
-> host);
+ suitableHosts.add(host);
+ }
+
+ logger.debug("Random Host Allocator returning {} suitable hosts.",
suitableHosts::size);
+ return suitableHosts;
+ }
+
+ /**
+ * @return all computing hosts, regardless of whether they support routing.
+ */
+ protected List<HostVO> retrieveHosts(Type type, List<HostVO> hosts,
VMTemplateVO template, String offeringHostTag, Long clusterId, Long podId, long
dcId) {
+ List<HostVO> availableHosts;
+ String templateTag = template.getTemplateTag();
+
+ if (CollectionUtils.isNotEmpty(hosts)) {
+ availableHosts = new ArrayList<>(hosts);
+ } else {
+ availableHosts = _resourceMgr.listAllUpAndEnabledHosts(type,
clusterId, podId, dcId);
+ }
+
+ if (ObjectUtils.anyNotNull(offeringHostTag, templateTag)) {
+ retainHostsMatchingServiceOfferingAndTemplateTags(availableHosts,
type, clusterId, podId, dcId, offeringHostTag, templateTag);
Review Comment:
```suggestion
retainHostsMatchingServiceOfferingAndTemplateTags(availableHosts, type, dcId,
podId, clusterId, offeringHostTag, templateTag);
```
Same issue here
##########
server/src/test/java/com/cloud/agent/manager/allocator/impl/BaseAllocatorTest.java:
##########
@@ -0,0 +1,219 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import com.cloud.capacity.CapacityManager;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VirtualMachineProfile;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BaseAllocatorTest {
+ @Mock
+ HostDao hostDaoMock;
+
+ @Mock
+ CapacityManager capacityManagerMock;
+
+ @InjectMocks
+ @Spy
+ BaseAllocator baseAllocator = new MockBaseAllocator();
+
+ private final Host.Type type = Host.Type.Routing;
+
+ private final Long clusterId = 1L;
+
+ private final Long podId = 1L;
+
+ private final Long dcId = 1L;
+
+ private final HostVO host1 = Mockito.mock(HostVO.class);
+
+ private final HostVO host2 = Mockito.mock(HostVO.class);
+
+ private final HostVO host3 = Mockito.mock(HostVO.class);
+
+ private final ServiceOfferingVO serviceOffering =
Mockito.mock(ServiceOfferingVO.class);
+
+ private final String hostTag = "hostTag";
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagShouldRetainHostsWithServiceOfferingTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = null;
+
+
Mockito.doReturn(hostsWithMathingTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(2, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ Assert.assertEquals(host3, suitableHosts.get(1));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagAndHasHostTagOnTemplateShouldRetainHostsWithServiceOfferingTagAndTemplateTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingServiceTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ List<HostVO> hostsWithMathingTemplateTags = new
ArrayList<>(Arrays.asList(host1, host2));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = "hostTagOnOffering";
+
+
Mockito.doReturn(hostsWithMathingTemplateTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
Mockito.doReturn(hostsWithMathingServiceTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
Review Comment:
```suggestion
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, dcId, podId, clusterId, hostTagOnOffering, hostTagOnTemplate);
```
##########
server/src/test/java/com/cloud/agent/manager/allocator/impl/BaseAllocatorTest.java:
##########
@@ -0,0 +1,219 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import com.cloud.capacity.CapacityManager;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VirtualMachineProfile;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BaseAllocatorTest {
+ @Mock
+ HostDao hostDaoMock;
+
+ @Mock
+ CapacityManager capacityManagerMock;
+
+ @InjectMocks
+ @Spy
+ BaseAllocator baseAllocator = new MockBaseAllocator();
+
+ private final Host.Type type = Host.Type.Routing;
+
+ private final Long clusterId = 1L;
+
+ private final Long podId = 1L;
+
+ private final Long dcId = 1L;
Review Comment:
It would be nice to use some different numbers for `clusterId`, `podId` and
`dcId` to catch errors like these
##########
server/src/test/java/com/cloud/agent/manager/allocator/impl/BaseAllocatorTest.java:
##########
@@ -0,0 +1,219 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import com.cloud.capacity.CapacityManager;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VirtualMachineProfile;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BaseAllocatorTest {
+ @Mock
+ HostDao hostDaoMock;
+
+ @Mock
+ CapacityManager capacityManagerMock;
+
+ @InjectMocks
+ @Spy
+ BaseAllocator baseAllocator = new MockBaseAllocator();
+
+ private final Host.Type type = Host.Type.Routing;
+
+ private final Long clusterId = 1L;
+
+ private final Long podId = 1L;
+
+ private final Long dcId = 1L;
+
+ private final HostVO host1 = Mockito.mock(HostVO.class);
+
+ private final HostVO host2 = Mockito.mock(HostVO.class);
+
+ private final HostVO host3 = Mockito.mock(HostVO.class);
+
+ private final ServiceOfferingVO serviceOffering =
Mockito.mock(ServiceOfferingVO.class);
+
+ private final String hostTag = "hostTag";
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagShouldRetainHostsWithServiceOfferingTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = null;
+
+
Mockito.doReturn(hostsWithMathingTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
Review Comment:
```suggestion
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, dcId, podId, clusterId, hostTagOnOffering, hostTagOnTemplate);
```
##########
server/src/test/java/com/cloud/agent/manager/allocator/impl/BaseAllocatorTest.java:
##########
@@ -0,0 +1,219 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import com.cloud.capacity.CapacityManager;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VirtualMachineProfile;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BaseAllocatorTest {
+ @Mock
+ HostDao hostDaoMock;
+
+ @Mock
+ CapacityManager capacityManagerMock;
+
+ @InjectMocks
+ @Spy
+ BaseAllocator baseAllocator = new MockBaseAllocator();
+
+ private final Host.Type type = Host.Type.Routing;
+
+ private final Long clusterId = 1L;
+
+ private final Long podId = 1L;
+
+ private final Long dcId = 1L;
+
+ private final HostVO host1 = Mockito.mock(HostVO.class);
+
+ private final HostVO host2 = Mockito.mock(HostVO.class);
+
+ private final HostVO host3 = Mockito.mock(HostVO.class);
+
+ private final ServiceOfferingVO serviceOffering =
Mockito.mock(ServiceOfferingVO.class);
+
+ private final String hostTag = "hostTag";
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagShouldRetainHostsWithServiceOfferingTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = null;
+
+
Mockito.doReturn(hostsWithMathingTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(2, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ Assert.assertEquals(host3, suitableHosts.get(1));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagAndHasHostTagOnTemplateShouldRetainHostsWithServiceOfferingTagAndTemplateTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingServiceTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ List<HostVO> hostsWithMathingTemplateTags = new
ArrayList<>(Arrays.asList(host1, host2));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = "hostTagOnOffering";
+
+
Mockito.doReturn(hostsWithMathingTemplateTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
Mockito.doReturn(hostsWithMathingServiceTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(1, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasHostTagOnTemplateShouldRetainHostsWithTemplateTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingServiceTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = null;
+ String hostTagOnOffering = "hostTagOnOffering";
+
+
Mockito.doReturn(hostsWithMathingServiceTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(2, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ Assert.assertEquals(host3, suitableHosts.get(1));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestNoServiceTagAndNoTemplateTagShouldHaveAllSuitableHosts()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ String hostTagOnTemplate = null;
+ String hostTagOnOffering = null;
+
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
Review Comment:
```suggestion
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, dcId, podId, clusterId, hostTagOnOffering, hostTagOnTemplate);
```
##########
server/src/test/java/com/cloud/agent/manager/allocator/impl/BaseAllocatorTest.java:
##########
@@ -0,0 +1,219 @@
+// 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 com.cloud.agent.manager.allocator.impl;
+
+import com.cloud.capacity.CapacityManager;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.host.Host;
+import com.cloud.host.HostVO;
+import com.cloud.host.dao.HostDao;
+import com.cloud.offering.ServiceOffering;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.utils.Pair;
+import com.cloud.vm.VirtualMachineProfile;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.Spy;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+@RunWith(MockitoJUnitRunner.class)
+public class BaseAllocatorTest {
+ @Mock
+ HostDao hostDaoMock;
+
+ @Mock
+ CapacityManager capacityManagerMock;
+
+ @InjectMocks
+ @Spy
+ BaseAllocator baseAllocator = new MockBaseAllocator();
+
+ private final Host.Type type = Host.Type.Routing;
+
+ private final Long clusterId = 1L;
+
+ private final Long podId = 1L;
+
+ private final Long dcId = 1L;
+
+ private final HostVO host1 = Mockito.mock(HostVO.class);
+
+ private final HostVO host2 = Mockito.mock(HostVO.class);
+
+ private final HostVO host3 = Mockito.mock(HostVO.class);
+
+ private final ServiceOfferingVO serviceOffering =
Mockito.mock(ServiceOfferingVO.class);
+
+ private final String hostTag = "hostTag";
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagShouldRetainHostsWithServiceOfferingTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = null;
+
+
Mockito.doReturn(hostsWithMathingTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(2, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ Assert.assertEquals(host3, suitableHosts.get(1));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasServiceOfferingTagAndHasHostTagOnTemplateShouldRetainHostsWithServiceOfferingTagAndTemplateTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingServiceTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ List<HostVO> hostsWithMathingTemplateTags = new
ArrayList<>(Arrays.asList(host1, host2));
+ String hostTagOnTemplate = "hostTagOnTemplate";
+ String hostTagOnOffering = "hostTagOnOffering";
+
+
Mockito.doReturn(hostsWithMathingTemplateTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnTemplate);
+
Mockito.doReturn(hostsWithMathingServiceTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
+
+ Assert.assertEquals(1, suitableHosts.size());
+ Assert.assertEquals(host1, suitableHosts.get(0));
+ }
+
+ @Test
+ public void
retainHostsMatchingServiceOfferingAndTemplateTagsTestHasHostTagOnTemplateShouldRetainHostsWithTemplateTag()
{
+ List<HostVO> suitableHosts = new ArrayList<>(Arrays.asList(host1,
host2, host3));
+ List<HostVO> hostsWithMathingServiceTags = new
ArrayList<>(Arrays.asList(host1, host3));
+ String hostTagOnTemplate = null;
+ String hostTagOnOffering = "hostTagOnOffering";
+
+
Mockito.doReturn(hostsWithMathingServiceTags).when(hostDaoMock).listByHostTag(type,
clusterId, podId, dcId, hostTagOnOffering);
+
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, clusterId, podId, dcId, hostTagOnTemplate, hostTagOnOffering);
Review Comment:
```suggestion
baseAllocator.retainHostsMatchingServiceOfferingAndTemplateTags(suitableHosts,
type, dcId, podId, clusterId, hostTagOnOffering, hostTagOnTemplate);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
