Merge pull request #1424 from syed/create-template-api-bug

[CLOUDSTACK-8973] Fix create template from snapshot returning null in case of region storeThis PR fixes a case where when we create a template from a snpashot in a region wide store, we don't set the cross zone flag which causes a null response to be returned
Tests:

Before fix
```
(local)  > create template snapshotid=33aa3f3b-5a47-4d2a-8d27-12952c01ebed displaytext=t2 ostypeid=20c8ead6-d750-11e5-9f8c-06524200007c name=t9

accountid = 1b13d7c2-d750-11e5-9f8c-06524200007c
cmd = org.apache.cloudstack.api.command.admin.template.CreateTemplateCmdByAdmin
created = 2016-02-23T16:09:24+0000
jobid = 4f9f5ff9-e7f0-4af6-999c-799431fd47de
jobinstanceid = a08a9711-bd31-43bb-80a2-49cf9d722a19
jobinstancetype = Template
jobprocstatus = 0
jobresult:
null:
crossZones = False
isfeatured = False
ispublic = False
isready = False
tags:
jobresultcode = 0
jobresulttype = object
jobstatus = 1
userid = 1b140f08-d750-11e5-9f8c-06524200007c
```

See the *null* in response

After fix:

```
(local)  > create template snapshotid=33aa3f3b-5a47-4d2a-8d27-12952c01ebed displaytext=t2 ostypeid=20c8ead6-d750-11e5-9f8c-06524200007c name=t11

accountid = 1b13d7c2-d750-11e5-9f8c-06524200007c
cmd = org.apache.cloudstack.api.command.admin.template.CreateTemplateCmdByAdmin
created = 2016-02-25T21:47:03+0000
jobid = 1b74209b-b3c1-4168-a243-f559aa0c081b
jobinstanceid = 06ecee5a-b1f2-4e67-80fb-f0f44b0aa198
jobinstancetype = Template
jobprocstatus = 0
jobresult:
template:
id = 06ecee5a-b1f2-4e67-80fb-f0f44b0aa198
name = t11
account = admin
created = 2016-02-25T21:47:03+0000
crossZones = True
displaytext = t2
domain = ROOT
domainid = 1b13ab80-d750-11e5-9f8c-06524200007c
format = VHD
hypervisor = XenServer
isdynamicallyscalable = False
isextractable = True
isfeatured = False
ispublic = False
isready = True
ostypeid = 20c8ead6-d750-11e5-9f8c-06524200007c
ostypename = CentOS 5 (64-bit)
passwordenabled = False
size = 21474836480
sourcetemplateid = 1af0f0cc-d750-11e5-9f8c-06524200007c
sshkeyenabled = False
status = Download Complete
tags:
templatetype = USER
jobresultcode = 0
jobresulttype = object
jobstatus = 1
userid = 1b140f08-d750-11e5-9f8c-06524200007c
```
Works correctly

* pr/1424:
  Fix create template from snapshot returning null in case of region store

Signed-off-by: Will Stevens <williamstevens@gmail.com>

server/src/com/cloud/api/ApiResponseHelper.java

@@ -1392,7 +1392,7 @@ public class ApiResponseHelper implements ResponseGenerator {
     @Override
     public List<TemplateResponse> createTemplateResponses(ResponseView view, VirtualMachineTemplate result, Long zoneId, boolean readyOnly) {
         List<TemplateJoinVO> tvo = null;
-        if (zoneId == null || zoneId == -1) {
+        if (zoneId == null || zoneId == -1 || result.isCrossZones()) {
             tvo = ApiDBUtils.newTemplateView(result);
         } else {
             tvo = ApiDBUtils.newTemplateView(result, zoneId, readyOnly);

server/src/com/cloud/template/TemplateManagerImpl.java

@@ -40,6 +40,8 @@ import com.google.gson.GsonBuilder;
 import org.apache.cloudstack.api.command.user.template.GetUploadParamsForTemplateCmd;
 import org.apache.cloudstack.framework.async.AsyncCallFuture;
 import org.apache.cloudstack.storage.command.TemplateOrVolumePostUploadCommand;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
@@ -263,7 +265,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
     private UserVmJoinDao _userVmJoinDao;
     @Inject
     private SnapshotDataStoreDao _snapshotStoreDao;
-
+    @Inject
+    private ImageStoreDao _imgStoreDao;
     @Inject
     MessageBus _messageBus;
 
@@ -277,6 +280,7 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
     @Inject
     private EndPointSelector selector;
 
+
     private TemplateAdapter getAdapter(HypervisorType type) {
         TemplateAdapter adapter = null;
         if (type == HypervisorType.BareMetal) {
@@ -1719,6 +1723,14 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
                 s_logger.debug("This template is getting created from other template, setting source template Id to: " + sourceTemplateId);
             }
         }
+
+
+        // for region wide storage, set cross zones flag
+        List<ImageStoreVO> stores = _imgStoreDao.findRegionImageStores();
+        if (!CollectionUtils.isEmpty(stores)) {
+            privateTemplate.setCrossZones(true);
+        }
+
         privateTemplate.setSourceTemplateId(sourceTemplateId);
 
         VMTemplateVO template = _tmpltDao.persist(privateTemplate);

server/test/com/cloud/template/TemplateManagerImplTest.java

@@ -21,13 +21,19 @@ package com.cloud.template;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.api.query.dao.UserVmJoinDao;
+import com.cloud.configuration.Resource;
 import com.cloud.dc.dao.DataCenterDao;
 import com.cloud.domain.dao.DomainDao;
 import com.cloud.event.dao.UsageEventDao;
 import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.ResourceAllocationException;
 import com.cloud.host.Status;
 import com.cloud.host.dao.HostDao;
+import com.cloud.hypervisor.Hypervisor;
 import com.cloud.projects.ProjectManager;
+import com.cloud.storage.GuestOSVO;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
 import com.cloud.storage.StoragePoolStatus;
@@ -54,6 +60,7 @@ import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.dao.VMInstanceDao;
+import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -66,6 +73,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.framework.messagebus.MessageBus;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreDao;
+import org.apache.cloudstack.storage.datastore.db.ImageStoreVO;
 import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
@@ -77,6 +86,8 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.ComponentScan;
 import org.springframework.context.annotation.Configuration;
@@ -102,11 +113,13 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyLong;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.eq;
 
 @RunWith(SpringJUnit4ClassRunner.class)
 @ContextConfiguration(loader = AnnotationConfigContextLoader.class)
@@ -133,6 +146,21 @@ public class TemplateManagerImplTest {
     @Inject
     PrimaryDataStoreDao primaryDataStoreDao;
 
+    @Inject
+    ResourceLimitService resourceLimitMgr;
+
+    @Inject
+    ImageStoreDao imgStoreDao;
+
+    @Inject
+    GuestOSDao guestOSDao;
+
+    @Inject
+    VMTemplateDao tmpltDao;
+
+    @Inject
+    SnapshotDao snapshotDao;
+
     public class CustomThreadPoolExecutor extends ThreadPoolExecutor {
         AtomicInteger ai = new AtomicInteger(0);
         public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
@@ -357,6 +385,58 @@ public class TemplateManagerImplTest {
         assertTrue("Test template is scheduled for seeding to on pool", ((CustomThreadPoolExecutor) preloadExecutor).getCount() == 2);
     }
 
+    @Test
+    public void testCreatePrivateTemplateRecordForRegionStore() throws ResourceAllocationException {
+
+        CreateTemplateCmd mockCreateCmd = mock(CreateTemplateCmd.class);
+        when(mockCreateCmd.getTemplateName()).thenReturn("test");
+        when(mockCreateCmd.getTemplateTag()).thenReturn(null);
+        when(mockCreateCmd.getBits()).thenReturn(64);
+        when(mockCreateCmd.getRequiresHvm()).thenReturn(true);
+        when(mockCreateCmd.isPasswordEnabled()).thenReturn(false);
+        when(mockCreateCmd.isPublic()).thenReturn(false);
+        when(mockCreateCmd.isFeatured()).thenReturn(false);
+        when(mockCreateCmd.isDynamicallyScalable()).thenReturn(false);
+        when(mockCreateCmd.getVolumeId()).thenReturn(null);
+        when(mockCreateCmd.getSnapshotId()).thenReturn(1L);
+        when(mockCreateCmd.getOsTypeId()).thenReturn(1L);
+        when(mockCreateCmd.getEventDescription()).thenReturn("test");
+        when(mockCreateCmd.getDetails()).thenReturn(null);
+
+        Account mockTemplateOwner = mock(Account.class);
+
+        SnapshotVO mockSnapshot = mock(SnapshotVO.class);
+        when(snapshotDao.findById(anyLong())).thenReturn(mockSnapshot);
+
+        when(mockSnapshot.getVolumeId()).thenReturn(1L);
+        when(mockSnapshot.getState()).thenReturn(Snapshot.State.BackedUp);
+        when(mockSnapshot.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.XenServer);
+
+        doNothing().when(resourceLimitMgr).checkResourceLimit(any(Account.class), eq(Resource.ResourceType.template));
+        doNothing().when(resourceLimitMgr).checkResourceLimit(any(Account.class), eq(Resource.ResourceType.secondary_storage), anyLong());
+
+        GuestOSVO mockGuestOS = mock(GuestOSVO.class);
+        when(guestOSDao.findById(anyLong())).thenReturn(mockGuestOS);
+
+        when(tmpltDao.getNextInSequence(eq(Long.class), eq("id"))).thenReturn(1L);
+
+        List<ImageStoreVO> mockRegionStores = new ArrayList<>();
+        ImageStoreVO mockRegionStore = mock(ImageStoreVO.class);
+        mockRegionStores.add(mockRegionStore);
+        when(imgStoreDao.findRegionImageStores()).thenReturn(mockRegionStores);
+
+        when(tmpltDao.persist(any(VMTemplateVO.class))).thenAnswer(new Answer<VMTemplateVO>() {
+            @Override
+            public VMTemplateVO answer(InvocationOnMock invocationOnMock) throws Throwable {
+                Object[] args = invocationOnMock.getArguments();
+                return (VMTemplateVO)args[0];
+            }
+        });
+
+        VMTemplateVO template = templateManager.createPrivateTemplateRecord(mockCreateCmd, mockTemplateOwner);
+        assertTrue("Template in a region store should have cross zones set", template.isCrossZones());
+    }
+
     @Configuration
     @ComponentScan(basePackageClasses = {TemplateManagerImpl.class},
             includeFilters = {@ComponentScan.Filter(value = TestConfiguration.Library.class, type = FilterType.CUSTOM)},
@@ -523,6 +603,11 @@ public class TemplateManagerImplTest {
             return Mockito.mock(SnapshotDataStoreDao.class);
         }
 
+        @Bean
+        public ImageStoreDao imageStoreDao() {
+            return Mockito.mock(ImageStoreDao.class);
+        }
+
         @Bean
         public MessageBus messageBus() {
             return Mockito.mock(MessageBus.class);