etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #901 from karuturi/CLOUDSTACK-8808 

CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.propertiesWe 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

Testing:
added unittests for the changes in the file processors.
manual test:
setup: host xenserver 6.5, management server centos 6.7
template: disk created using the process specified by andy at https://issues.apache.org/jira/browse/CLOUDSTACK-8808?focusedCommentId=14933368
tried to register the template and it failed with an error. Template never moved to Ready state.
![screen shot 2015-09-30 at 3 53 34 pm](https://cloud.githubusercontent.com/assets/186833/10190608/76bcce92-678b-11e5-8f52-b449d149300b.png)

* pr/901:
  CLOUDSTACK-8808: Successfully registered VHD template is downloaded again due to missing virtualsize property in template.properties

Signed-off-by: Remi Bergsma <github@remi.nl>
This commit is contained in:
Remi Bergsma 2015-10-02 16:09:35 +02:00
parent 996c2f6dde 1056171aca
commit 78f74295e1
8 changed files with 393 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/src/com/cloud/storage/template/OVAProcessor.java
View File

@ -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();

17
core/src/com/cloud/storage/template/QCOW2Processor.java
View File

@ -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) {

4
core/src/com/cloud/storage/template/TemplateLocation.java
View File

@ -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;
}

17
core/src/com/cloud/storage/template/VhdProcessor.java
View File

@ -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];

138
core/test/com/cloud/storage/template/OVAProcessorTest.java Normal file
View File

@ -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();
}
}

109
core/test/com/cloud/storage/template/QCOW2ProcessorTest.java Normal file
View File

@ -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();
}
}

110
core/test/com/cloud/storage/template/VhdProcessorTest.java Normal file
View File

@ -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();
}
}

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

@ -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;