CLOUDSTACK-8808: Successfully registered VHD template is downloaded

again due to missing virtualsize property in template.properties

We have multiple file processors to process different types of image
formats. The processor interface has two methods getVirtualSize() and
process().

    1. getVirtualSize() as the name says, returns the virtual size of
the file and is used at get the size while copying files from NFS to s3
    2. process() returns FormatInfo struct which has fileType, size,
virutalSize, filename.  on successfully downloading a template, each
file is passed to all the processors.process() and whichever returns a
FormatInfo, that will be used to create template.properties file.  If
process() throws an InternalErrorException, template installation fails.
But, if process() returns null, template registration is successful with
template.properties missing some attributes like virtualSize, file
format etc. which results in this bug on restart of ssvm/cloud
service/management server.

failing the template download if virutalsize or some other properties
cannot be determined.

The following changes are done:
getVirtualSize() to always return size(if it can calculate, get virtual
size else return file size). This would mean the following changes

    1. QCOW2Processor.getVirtualSize() to return file size if virtual
size calculation fails
    2. VHDProcessor.getVirtualSize() to return file size if virtual size
calculation fails

process() to throw InternalErrorException if virtual size calculation
fails or any other exceptions occur. This would mean the following
changes

    1. OVAProcessor to throw InternalErrorException if untar fails
    2. QCOW2Processor to throw InternalErrorException if virtual size
calculation fails
    3. VHDProcessor to throw InternalErrorException if virtual size
calculation fails

core/src/com/cloud/storage/template/OVAProcessor.java

@@ -71,7 +71,7 @@ public class OVAProcessor extends AdapterBase implements Processor {
         String result = command.execute();
         if (result != null) {
             s_logger.info("failed to untar OVA package due to " + result + ". templatePath: " + templatePath + ", templateName: " + templateName);
-            return null;
+            throw new InternalErrorException("failed to untar OVA package");
         }
 
         FormatInfo info = new FormatInfo();

core/src/com/cloud/storage/template/QCOW2Processor.java

@@ -27,6 +27,7 @@ import java.util.Map;
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
+import com.cloud.exception.InternalErrorException;
 import org.apache.log4j.Logger;
 
 import com.cloud.storage.Storage.ImageFormat;
@@ -42,7 +43,7 @@ public class QCOW2Processor extends AdapterBase implements Processor {
     private StorageLayer _storage;
 
     @Override
-    public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to QCOW2.");
             return null;
@@ -64,10 +65,10 @@ public class QCOW2Processor extends AdapterBase implements Processor {
         info.size = _storage.getSize(qcow2Path);
 
         try {
-            info.virtualSize = getVirtualSize(qcow2File);
+            info.virtualSize = getTemplateVirtualSize(qcow2File);
         } catch (IOException e) {
             s_logger.error("Unable to get virtual size from " + qcow2File.getName());
-            return null;
+            throw new InternalErrorException("unable to get virtual size from qcow2 file");
         }
 
         return info;
@@ -75,6 +76,16 @@ public class QCOW2Processor extends AdapterBase implements Processor {
 
     @Override
     public long getVirtualSize(File file) throws IOException {
+        try {
+            long size = getTemplateVirtualSize(file);
+            return size;
+        } catch (Exception e) {
+            s_logger.info("[ignored]" + "failed to get template virtual size for QCOW2: " + e.getLocalizedMessage());
+        }
+        return file.length();
+    }
+
+    protected long getTemplateVirtualSize(File file) throws IOException {
         byte[] b = new byte[8];
         try (FileInputStream strm = new FileInputStream(file)) {
             if (strm.skip(VIRTUALSIZE_HEADER_LOCATION) != VIRTUALSIZE_HEADER_LOCATION) {

core/src/com/cloud/storage/template/TemplateLocation.java

@@ -180,7 +180,9 @@ public class TemplateLocation {
         deleteFormat(newInfo.format);
 
         if (!checkFormatValidity(newInfo)) {
-            s_logger.warn("Format is invalid ");
+            s_logger.warn("Format is invalid");
+            s_logger.debug("Format: " + newInfo.format + " size: " + newInfo.size + " virtualsize: " + newInfo.virtualSize + " filename: " + newInfo.filename);
+            s_logger.debug("format, filename cannot be null and size, virtual size should be  > 0 ");
             return false;
         }
 

core/src/com/cloud/storage/template/VhdProcessor.java

@@ -27,6 +27,7 @@ import java.util.Map;
 import javax.ejb.Local;
 import javax.naming.ConfigurationException;
 
+import com.cloud.exception.InternalErrorException;
 import org.apache.log4j.Logger;
 
 import com.cloud.storage.Storage.ImageFormat;
@@ -52,7 +53,7 @@ public class VhdProcessor extends AdapterBase implements Processor {
     private byte[][] citrixCreatorApp = { {0x74, 0x61, 0x70, 0x00}, {0x43, 0x54, 0x58, 0x53}}; /*"tap ", and "CTXS"*/
 
     @Override
-    public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
+    public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
         if (format != null) {
             s_logger.debug("We currently don't handle conversion from " + format + " to VHD.");
             return null;
@@ -72,10 +73,10 @@ public class VhdProcessor extends AdapterBase implements Processor {
         info.size = _storage.getSize(vhdPath);
 
         try {
-            info.virtualSize = getVirtualSize(vhdFile);
+            info.virtualSize = getTemplateVirtualSize(vhdFile);
         } catch (IOException e) {
             s_logger.error("Unable to get the virtual size for " + vhdPath);
-            return null;
+            throw new InternalErrorException("unable to get virtual size from vhd file");
         }
 
         return info;
@@ -83,6 +84,16 @@ public class VhdProcessor extends AdapterBase implements Processor {
 
     @Override
     public long getVirtualSize(File file) throws IOException {
+        try {
+            long size = getTemplateVirtualSize(file);
+            return size;
+        } catch (Exception e) {
+            s_logger.info("[ignored]" + "failed to get template virtual size for VHD: " + e.getLocalizedMessage());
+        }
+        return file.length();
+    }
+
+    protected long getTemplateVirtualSize(File file) throws IOException {
         byte[] currentSize = new byte[8];
         byte[] creatorApp = new byte[4];
 

core/test/com/cloud/storage/template/OVAProcessorTest.java

@@ -0,0 +1,138 @@
+/*
+ * 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.storage.template;
+
+import com.cloud.exception.InternalErrorException;
+import com.cloud.storage.Storage;
+import com.cloud.storage.StorageLayer;
+import com.cloud.utils.script.Script;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+
+import java.io.File;
+import java.util.HashMap;
+import java.util.Map;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest(OVAProcessor.class)
+public class OVAProcessorTest {
+    OVAProcessor processor;
+
+    @Mock
+    StorageLayer mockStorageLayer;
+
+    @Before
+    public void setUp() throws Exception {
+        processor = PowerMockito.spy(new OVAProcessor());
+        Map<String, Object> params = new HashMap<String, Object>();
+        params.put(StorageLayer.InstanceConfigKey, mockStorageLayer);
+        processor.configure("OVA Processor", params);
+    }
+
+    @Test(expected = InternalErrorException.class)
+    public void testProcessWhenUntarFails() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+
+        Script mockScript = Mockito.mock(Script.class);
+        PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
+        PowerMockito.when(mockScript.execute()).thenReturn("error while untaring the file");
+
+        processor.process(templatePath, null, templateName);
+    }
+
+    @Test(expected = InternalErrorException.class)
+    public void testProcessWhenVirtualSizeThrowsException() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000l);
+        Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString());
+
+        Script mockScript = Mockito.mock(Script.class);
+        PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
+        PowerMockito.when(mockScript.execute()).thenReturn(null);
+
+        processor.process(templatePath, null, templateName);
+    }
+
+    @Test
+    public void testProcess() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+        long virtualSize = 2000;
+        long actualSize = 1000;
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString());
+
+        Script mockScript = Mockito.mock(Script.class);
+        PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
+        PowerMockito.when(mockScript.execute()).thenReturn(null);
+
+        Processor.FormatInfo info = processor.process(templatePath, null, templateName);
+        Assert.assertEquals(Storage.ImageFormat.OVA, info.format);
+        Assert.assertEquals("actual size:", actualSize, info.size);
+        Assert.assertEquals("virtual size:", virtualSize, info.virtualSize);
+        Assert.assertEquals("template name:", templateName + ".ova", info.filename);
+    }
+
+    @Test
+    public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception {
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        String templatePath = "/tmp";
+        String templateName = "template";
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.when(mockFile.getParent()).thenReturn(templatePath);
+        Mockito.when(mockFile.getName()).thenReturn(templateName);
+        Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(templatePath, templateName);
+        Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile, Mockito.times(1)).length();
+    }
+
+    @Test
+    public void testGetVirtualSize() throws Exception {
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        String templatePath = "/tmp";
+        String templateName = "template";
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.when(mockFile.getParent()).thenReturn(templatePath);
+        Mockito.when(mockFile.getName()).thenReturn(templateName);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(templatePath, templateName);
+        Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile, Mockito.times(0)).length();
+    }
+
+}

core/test/com/cloud/storage/template/QCOW2ProcessorTest.java

@@ -0,0 +1,109 @@
+/*
+ * 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.storage.template;
+
+import com.cloud.exception.InternalErrorException;
+import com.cloud.storage.Storage;
+import com.cloud.storage.StorageLayer;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+@RunWith(MockitoJUnitRunner.class)
+public class QCOW2ProcessorTest {
+    QCOW2Processor processor;
+
+    @Mock
+    StorageLayer mockStorageLayer;
+
+    @Before
+    public void setUp() throws Exception {
+        processor = Mockito.spy(new QCOW2Processor());
+        Map<String, Object> params = new HashMap<String, Object>();
+        params.put(StorageLayer.InstanceConfigKey, mockStorageLayer);
+        processor.configure("VHD Processor", params);
+    }
+
+    @Test(expected = InternalErrorException.class)
+    public void testProcessWhenVirtualSizeThrowsException() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        File mockFile = Mockito.mock(File.class);
+
+        Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000L);
+        Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any());
+
+        processor.process(templatePath, null, templateName);
+    }
+
+    @Test
+    public void testProcess() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+        long virtualSize = 2000;
+        long actualSize = 1000;
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        File mockFile = Mockito.mock(File.class);
+
+        Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any());
+
+        Processor.FormatInfo info = processor.process(templatePath, null, templateName);
+        Assert.assertEquals(Storage.ImageFormat.QCOW2, info.format);
+        Assert.assertEquals(actualSize, info.size);
+        Assert.assertEquals(virtualSize, info.virtualSize);
+        Assert.assertEquals(templateName + ".qcow2", info.filename);
+    }
+
+    @Test
+    public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception {
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any());
+        Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile, Mockito.times(1)).length();
+    }
+
+    @Test
+    public void testGetVirtualSize() throws Exception {
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any());
+        Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile, Mockito.times(0)).length();
+    }
+}

core/test/com/cloud/storage/template/VhdProcessorTest.java

@@ -0,0 +1,110 @@
+/*
+ * 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.storage.template;
+
+import com.cloud.exception.InternalErrorException;
+import com.cloud.storage.Storage;
+import com.cloud.storage.StorageLayer;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
+
+@RunWith(MockitoJUnitRunner.class)
+public class VhdProcessorTest {
+    VhdProcessor processor;
+
+    @Mock
+    StorageLayer mockStorageLayer;
+
+    @Before
+    public void setUp() throws Exception {
+        processor = Mockito.spy(new VhdProcessor());
+        Map<String, Object> params = new HashMap<String, Object>();
+        params.put(StorageLayer.InstanceConfigKey, mockStorageLayer);
+        processor.configure("VHD Processor", params);
+    }
+
+    @Test(expected = InternalErrorException.class)
+    public void testProcessWhenVirtualSizeThrowsException() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        File mockFile = Mockito.mock(File.class);
+
+        Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000L);
+        Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File) Mockito.any());
+
+        processor.process(templatePath, null, templateName);
+    }
+
+    @Test
+    public void testProcess() throws Exception {
+        String templatePath = "/tmp";
+        String templateName = "template";
+        long virtualSize = 2000;
+        long actualSize = 1000;
+
+        Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
+        File mockFile = Mockito.mock(File.class);
+
+        Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
+        Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File) Mockito.any());
+
+        Processor.FormatInfo info = processor.process(templatePath, null, templateName);
+        Assert.assertEquals(Storage.ImageFormat.VHD, info.format);
+        Assert.assertEquals(actualSize, info.size);
+        Assert.assertEquals(virtualSize, info.virtualSize);
+        Assert.assertEquals(templateName + ".vhd", info.filename);
+    }
+
+    @Test
+    public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception {
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File) Mockito.any());
+        Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile,Mockito.times(1)).length();
+    }
+
+    @Test
+    public void testGetVirtualSize() throws Exception{
+        long virtualSize = 2000;
+        long actualSize = 1000;
+        File mockFile = Mockito.mock(File.class);
+        Mockito.when(mockFile.length()).thenReturn(actualSize);
+        Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File) Mockito.any());
+        Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile));
+        Mockito.verify(mockFile,Mockito.times(0)).length();
+    }
+}

services/secondary-storage/server/src/org/apache/cloudstack/storage/template/DownloadManagerImpl.java

@@ -482,7 +482,10 @@ public class DownloadManagerImpl extends ManagerBase implements DownloadManager
                 return e.toString();
             }
             if (info != null) {
-                loc.addFormat(info);
+                if(!loc.addFormat(info)) {
+                    loc.purge();
+                    return "Unable to install due to invalid file format";
+                }
                 dnld.setTemplatesize(info.virtualSize);
                 dnld.setTemplatePhysicalSize(info.size);
                 break;