Merge pull request #1321 from nitin-maharana/CloudStack-Nitin5_4.7

CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged offerings when called with VM idWhen calling listServiceOfferings with VM id as parameter. It is returning incompatible tagged offerings. It should only list all compatible tagged offerings. Compatible means the new service offering should contain all the tags of the existing service offering(Existing offering SUBSET of new offering). If that is the case It should list in the result and can be upgraded to that offering.

* pr/1321:
  CLOUDSTACK-8847: ListServiceOfferings is returning incompatible tagged offerings when called with VM id

Signed-off-by: Will Stevens <[email protected]>

Author: swill

engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -2908,12 +2908,12 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe
                     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<String> currentTags = StringUtils.csvTagsToList(currentServiceOffering.getTags());
         final List<String> 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);
         }
     }
 

engine/orchestration/test/com/cloud/vm/VirtualMachineManagerImplTest.java

@@ -29,7 +29,9 @@
 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 void setup() {
         _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 void testSendStopWithNullAnswer() throws Exception {
 
         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);
+    }
 }

server/src/com/cloud/api/query/QueryManagerImpl.java

@@ -2620,11 +2620,11 @@ private List<ServiceOfferingJoinVO> filterOfferingsOnCurrentTags(List<ServiceOff
         if(currentVmOffering == null) return offerings;
         List<String> 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<ServiceOfferingJoinVO> filteredOfferings = new ArrayList<>();
         for (ServiceOfferingJoinVO offering : offerings){
-            List<String> tags = StringUtils.csvTagsToList(offering.getTags());
-            if(currentTagsList.containsAll(tags)){
+            List<String> newTagsList = StringUtils.csvTagsToList(offering.getTags());
+            if(newTagsList.containsAll(currentTagsList)){
                 filteredOfferings.add(offering);
             }
         }