From ba0503d000250e233701a71bd8e406e52b6ad110 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Maharana Date: Mon, 14 Sep 2015 22:02:37 +0530 Subject: [PATCH] CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged offerings when called with VM id Fixed the subset and superset issue. Added unit test for the same. --- .../cloud/vm/VirtualMachineManagerImpl.java | 8 +++--- .../vm/VirtualMachineManagerImplTest.java | 28 +++++++++++++++++++ .../com/cloud/api/query/QueryManagerImpl.java | 6 ++-- 3 files changed, 35 insertions(+), 7 deletions(-) diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index 5c5838951f9..cda695dc8ad 100644 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2908,12 +2908,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac newServiceOffering.getCpu() + " cpu(s) at " + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory"); } - // Check that the service offering being upgraded to has storage tags subset of the current service offering storage tags, since volume is not migrated. + // Check that the service offering being upgraded to has all the tags of the current service offering. final List currentTags = StringUtils.csvTagsToList(currentServiceOffering.getTags()); final List newTags = StringUtils.csvTagsToList(newServiceOffering.getTags()); - if (!currentTags.containsAll(newTags)) { - throw new InvalidParameterValueException("Unable to upgrade virtual machine; the new service offering " + " should have tags as subset of " + - "current service offering tags. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + newTags); + if (!newTags.containsAll(currentTags)) { + throw new InvalidParameterValueException("Unable to upgrade virtual machine; the current service offering " + " should have tags as subset of " + + "the new service offering tags. Current service offering tags: " + currentTags + "; " + "new service " + "offering tags: " + newTags); } } diff --git a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java index 865b0662ec0..d849eabaaf4 100644 --- a/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -29,7 +29,9 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.ArrayList; +import com.cloud.service.dao.ServiceOfferingDao; import junit.framework.Assert; import org.junit.Before; @@ -135,6 +137,8 @@ public class VirtualMachineManagerImplTest { @Mock VMInstanceDao _vmInstanceDao; @Mock + ServiceOfferingDao _offeringDao; + @Mock VMTemplateDao _templateDao; @Mock VolumeDao _volsDao; @@ -149,6 +153,8 @@ public class VirtualMachineManagerImplTest { @Mock VMInstanceVO _vmInstance; @Mock + ServiceOfferingVO _serviceOfferingMock; + @Mock HostVO _host; @Mock VMTemplateVO _templateMock; @@ -227,6 +233,8 @@ public class VirtualMachineManagerImplTest { _vmMgr._uservmDetailsDao = _vmDetailsDao; _vmMgr._entityMgr = _entityMgr; _vmMgr._configDepot = _configDepot; + _vmMgr._offeringDao = _offeringDao; + _vmMgr.hostAllocators = new ArrayList<>(); when(_vmMock.getId()).thenReturn(314l); when(_vmInstance.getId()).thenReturn(1L); @@ -505,4 +513,24 @@ public class VirtualMachineManagerImplTest { Assert.assertFalse(actual); } + + @Test + public void testCheckIfCanUpgrade() throws Exception { + when(_vmInstance.getState()).thenReturn(State.Stopped); + when(_serviceOfferingMock.isDynamic()).thenReturn(true); + when(_vmInstance.getServiceOfferingId()).thenReturn(1l); + when(_serviceOfferingMock.getId()).thenReturn(2l); + + ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class); + + when(_offeringDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); + when(mockCurrentServiceOffering.getUseLocalStorage()).thenReturn(true); + when(_serviceOfferingMock.getUseLocalStorage()).thenReturn(true); + when(mockCurrentServiceOffering.getSystemUse()).thenReturn(true); + when(_serviceOfferingMock.getSystemUse()).thenReturn(true); + when(mockCurrentServiceOffering.getTags()).thenReturn("x,y"); + when(_serviceOfferingMock.getTags()).thenReturn("z,x,y"); + + _vmMgr.checkIfCanUpgrade(_vmInstance, _serviceOfferingMock); + } } diff --git a/server/src/com/cloud/api/query/QueryManagerImpl.java b/server/src/com/cloud/api/query/QueryManagerImpl.java index a87d9fb1f0d..94929577a39 100644 --- a/server/src/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/com/cloud/api/query/QueryManagerImpl.java @@ -2620,11 +2620,11 @@ public class QueryManagerImpl extends ManagerBase implements QueryService, Confi if(currentVmOffering == null) return offerings; List currentTagsList = StringUtils.csvTagsToList(currentVmOffering.getTags()); - // New offerings should be a subset of existing storage tags. Discard offerings who are not. + // New service offering should have all the tags of the current service offering. List filteredOfferings = new ArrayList<>(); for (ServiceOfferingJoinVO offering : offerings){ - List tags = StringUtils.csvTagsToList(offering.getTags()); - if(currentTagsList.containsAll(tags)){ + List newTagsList = StringUtils.csvTagsToList(offering.getTags()); + if(newTagsList.containsAll(currentTagsList)){ filteredOfferings.add(offering); } }