Skip to content

Commit 78f7429

Browse files
remibergsmaremibergsma
committed
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>
2 parents 996c2f6 + 1056171 commit 78f7429

8 files changed

Lines changed: 393 additions & 9 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
7171
String result = command.execute();
7272
if (result != null) {
7373
s_logger.info("failed to untar OVA package due to " + result + ". templatePath: " + templatePath + ", templateName: " + templateName);
74-
return null;
74+
throw new InternalErrorException("failed to untar OVA package");
7575
}
7676

7777
FormatInfo info = new FormatInfo();

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import javax.ejb.Local;
2828
import javax.naming.ConfigurationException;
2929

30+
import com.cloud.exception.InternalErrorException;
3031
import org.apache.log4j.Logger;
3132

3233
import com.cloud.storage.Storage.ImageFormat;
@@ -42,7 +43,7 @@ public class QCOW2Processor extends AdapterBase implements Processor {
4243
private StorageLayer _storage;
4344

4445
@Override
45-
public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
46+
public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
4647
if (format != null) {
4748
s_logger.debug("We currently don't handle conversion from " + format + " to QCOW2.");
4849
return null;
@@ -64,17 +65,27 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
6465
info.size = _storage.getSize(qcow2Path);
6566

6667
try {
67-
info.virtualSize = getVirtualSize(qcow2File);
68+
info.virtualSize = getTemplateVirtualSize(qcow2File);
6869
} catch (IOException e) {
6970
s_logger.error("Unable to get virtual size from " + qcow2File.getName());
70-
return null;
71+
throw new InternalErrorException("unable to get virtual size from qcow2 file");
7172
}
7273

7374
return info;
7475
}
7576

7677
@Override
7778
public long getVirtualSize(File file) throws IOException {
79+
try {
80+
long size = getTemplateVirtualSize(file);
81+
return size;
82+
} catch (Exception e) {
83+
s_logger.info("[ignored]" + "failed to get template virtual size for QCOW2: " + e.getLocalizedMessage());
84+
}
85+
return file.length();
86+
}
87+
88+
protected long getTemplateVirtualSize(File file) throws IOException {
7889
byte[] b = new byte[8];
7990
try (FileInputStream strm = new FileInputStream(file)) {
8091
if (strm.skip(VIRTUALSIZE_HEADER_LOCATION) != VIRTUALSIZE_HEADER_LOCATION) {

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,9 @@ public boolean addFormat(FormatInfo newInfo) {
180180
deleteFormat(newInfo.format);
181181

182182
if (!checkFormatValidity(newInfo)) {
183-
s_logger.warn("Format is invalid ");
183+
s_logger.warn("Format is invalid");
184+
s_logger.debug("Format: " + newInfo.format + " size: " + newInfo.size + " virtualsize: " + newInfo.virtualSize + " filename: " + newInfo.filename);
185+
s_logger.debug("format, filename cannot be null and size, virtual size should be > 0 ");
184186
return false;
185187
}
186188

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

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import javax.ejb.Local;
2828
import javax.naming.ConfigurationException;
2929

30+
import com.cloud.exception.InternalErrorException;
3031
import org.apache.log4j.Logger;
3132

3233
import com.cloud.storage.Storage.ImageFormat;
@@ -52,7 +53,7 @@ public class VhdProcessor extends AdapterBase implements Processor {
5253
private byte[][] citrixCreatorApp = { {0x74, 0x61, 0x70, 0x00}, {0x43, 0x54, 0x58, 0x53}}; /*"tap ", and "CTXS"*/
5354

5455
@Override
55-
public FormatInfo process(String templatePath, ImageFormat format, String templateName) {
56+
public FormatInfo process(String templatePath, ImageFormat format, String templateName) throws InternalErrorException {
5657
if (format != null) {
5758
s_logger.debug("We currently don't handle conversion from " + format + " to VHD.");
5859
return null;
@@ -72,17 +73,27 @@ public FormatInfo process(String templatePath, ImageFormat format, String templa
7273
info.size = _storage.getSize(vhdPath);
7374

7475
try {
75-
info.virtualSize = getVirtualSize(vhdFile);
76+
info.virtualSize = getTemplateVirtualSize(vhdFile);
7677
} catch (IOException e) {
7778
s_logger.error("Unable to get the virtual size for " + vhdPath);
78-
return null;
79+
throw new InternalErrorException("unable to get virtual size from vhd file");
7980
}
8081

8182
return info;
8283
}
8384

8485
@Override
8586
public long getVirtualSize(File file) throws IOException {
87+
try {
88+
long size = getTemplateVirtualSize(file);
89+
return size;
90+
} catch (Exception e) {
91+
s_logger.info("[ignored]" + "failed to get template virtual size for VHD: " + e.getLocalizedMessage());
92+
}
93+
return file.length();
94+
}
95+
96+
protected long getTemplateVirtualSize(File file) throws IOException {
8697
byte[] currentSize = new byte[8];
8798
byte[] creatorApp = new byte[4];
8899

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

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package com.cloud.storage.template;
21+
22+
import com.cloud.exception.InternalErrorException;
23+
import com.cloud.storage.Storage;
24+
import com.cloud.storage.StorageLayer;
25+
import com.cloud.utils.script.Script;
26+
import org.junit.Assert;
27+
import org.junit.Before;
28+
import org.junit.Test;
29+
import org.junit.runner.RunWith;
30+
import org.mockito.Mock;
31+
import org.mockito.Mockito;
32+
import org.powermock.api.mockito.PowerMockito;
33+
import org.powermock.core.classloader.annotations.PrepareForTest;
34+
import org.powermock.modules.junit4.PowerMockRunner;
35+
36+
import java.io.File;
37+
import java.util.HashMap;
38+
import java.util.Map;
39+
40+
@RunWith(PowerMockRunner.class)
41+
@PrepareForTest(OVAProcessor.class)
42+
public class OVAProcessorTest {
43+
OVAProcessor processor;
44+
45+
@Mock
46+
StorageLayer mockStorageLayer;
47+
48+
@Before
49+
public void setUp() throws Exception {
50+
processor = PowerMockito.spy(new OVAProcessor());
51+
Map<String, Object> params = new HashMap<String, Object>();
52+
params.put(StorageLayer.InstanceConfigKey, mockStorageLayer);
53+
processor.configure("OVA Processor", params);
54+
}
55+
56+
@Test(expected = InternalErrorException.class)
57+
public void testProcessWhenUntarFails() throws Exception {
58+
String templatePath = "/tmp";
59+
String templateName = "template";
60+
61+
Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
62+
63+
Script mockScript = Mockito.mock(Script.class);
64+
PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
65+
PowerMockito.when(mockScript.execute()).thenReturn("error while untaring the file");
66+
67+
processor.process(templatePath, null, templateName);
68+
}
69+
70+
@Test(expected = InternalErrorException.class)
71+
public void testProcessWhenVirtualSizeThrowsException() throws Exception {
72+
String templatePath = "/tmp";
73+
String templateName = "template";
74+
75+
Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
76+
Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000l);
77+
Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString());
78+
79+
Script mockScript = Mockito.mock(Script.class);
80+
PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
81+
PowerMockito.when(mockScript.execute()).thenReturn(null);
82+
83+
processor.process(templatePath, null, templateName);
84+
}
85+
86+
@Test
87+
public void testProcess() throws Exception {
88+
String templatePath = "/tmp";
89+
String templateName = "template";
90+
long virtualSize = 2000;
91+
long actualSize = 1000;
92+
93+
Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
94+
Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize);
95+
Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(Mockito.anyString(), Mockito.anyString());
96+
97+
Script mockScript = Mockito.mock(Script.class);
98+
PowerMockito.whenNew(Script.class).withAnyArguments().thenReturn(mockScript);
99+
PowerMockito.when(mockScript.execute()).thenReturn(null);
100+
101+
Processor.FormatInfo info = processor.process(templatePath, null, templateName);
102+
Assert.assertEquals(Storage.ImageFormat.OVA, info.format);
103+
Assert.assertEquals("actual size:", actualSize, info.size);
104+
Assert.assertEquals("virtual size:", virtualSize, info.virtualSize);
105+
Assert.assertEquals("template name:", templateName + ".ova", info.filename);
106+
}
107+
108+
@Test
109+
public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception {
110+
long virtualSize = 2000;
111+
long actualSize = 1000;
112+
String templatePath = "/tmp";
113+
String templateName = "template";
114+
File mockFile = Mockito.mock(File.class);
115+
Mockito.when(mockFile.length()).thenReturn(actualSize);
116+
Mockito.when(mockFile.getParent()).thenReturn(templatePath);
117+
Mockito.when(mockFile.getName()).thenReturn(templateName);
118+
Mockito.doThrow(new InternalErrorException("virtual size calculation failed")).when(processor).getTemplateVirtualSize(templatePath, templateName);
119+
Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile));
120+
Mockito.verify(mockFile, Mockito.times(1)).length();
121+
}
122+
123+
@Test
124+
public void testGetVirtualSize() throws Exception {
125+
long virtualSize = 2000;
126+
long actualSize = 1000;
127+
String templatePath = "/tmp";
128+
String templateName = "template";
129+
File mockFile = Mockito.mock(File.class);
130+
Mockito.when(mockFile.length()).thenReturn(actualSize);
131+
Mockito.when(mockFile.getParent()).thenReturn(templatePath);
132+
Mockito.when(mockFile.getName()).thenReturn(templateName);
133+
Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize(templatePath, templateName);
134+
Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile));
135+
Mockito.verify(mockFile, Mockito.times(0)).length();
136+
}
137+
138+
}

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

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package com.cloud.storage.template;
20+
21+
import com.cloud.exception.InternalErrorException;
22+
import com.cloud.storage.Storage;
23+
import com.cloud.storage.StorageLayer;
24+
import org.junit.Assert;
25+
import org.junit.Before;
26+
import org.junit.Test;
27+
import org.junit.runner.RunWith;
28+
import org.mockito.Mock;
29+
import org.mockito.Mockito;
30+
import org.mockito.runners.MockitoJUnitRunner;
31+
32+
import java.io.File;
33+
import java.io.IOException;
34+
import java.util.HashMap;
35+
import java.util.Map;
36+
37+
@RunWith(MockitoJUnitRunner.class)
38+
public class QCOW2ProcessorTest {
39+
QCOW2Processor processor;
40+
41+
@Mock
42+
StorageLayer mockStorageLayer;
43+
44+
@Before
45+
public void setUp() throws Exception {
46+
processor = Mockito.spy(new QCOW2Processor());
47+
Map<String, Object> params = new HashMap<String, Object>();
48+
params.put(StorageLayer.InstanceConfigKey, mockStorageLayer);
49+
processor.configure("VHD Processor", params);
50+
}
51+
52+
@Test(expected = InternalErrorException.class)
53+
public void testProcessWhenVirtualSizeThrowsException() throws Exception {
54+
String templatePath = "/tmp";
55+
String templateName = "template";
56+
57+
Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
58+
File mockFile = Mockito.mock(File.class);
59+
60+
Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
61+
Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(1000L);
62+
Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any());
63+
64+
processor.process(templatePath, null, templateName);
65+
}
66+
67+
@Test
68+
public void testProcess() throws Exception {
69+
String templatePath = "/tmp";
70+
String templateName = "template";
71+
long virtualSize = 2000;
72+
long actualSize = 1000;
73+
74+
Mockito.when(mockStorageLayer.exists(Mockito.anyString())).thenReturn(true);
75+
File mockFile = Mockito.mock(File.class);
76+
77+
Mockito.when(mockStorageLayer.getFile(Mockito.anyString())).thenReturn(mockFile);
78+
Mockito.when(mockStorageLayer.getSize(Mockito.anyString())).thenReturn(actualSize);
79+
Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any());
80+
81+
Processor.FormatInfo info = processor.process(templatePath, null, templateName);
82+
Assert.assertEquals(Storage.ImageFormat.QCOW2, info.format);
83+
Assert.assertEquals(actualSize, info.size);
84+
Assert.assertEquals(virtualSize, info.virtualSize);
85+
Assert.assertEquals(templateName + ".qcow2", info.filename);
86+
}
87+
88+
@Test
89+
public void testGetVirtualSizeWhenVirtualSizeThrowsException() throws Exception {
90+
long virtualSize = 2000;
91+
long actualSize = 1000;
92+
File mockFile = Mockito.mock(File.class);
93+
Mockito.when(mockFile.length()).thenReturn(actualSize);
94+
Mockito.doThrow(new IOException("virtual size calculation failed")).when(processor).getTemplateVirtualSize((File)Mockito.any());
95+
Assert.assertEquals(actualSize, processor.getVirtualSize(mockFile));
96+
Mockito.verify(mockFile, Mockito.times(1)).length();
97+
}
98+
99+
@Test
100+
public void testGetVirtualSize() throws Exception {
101+
long virtualSize = 2000;
102+
long actualSize = 1000;
103+
File mockFile = Mockito.mock(File.class);
104+
Mockito.when(mockFile.length()).thenReturn(actualSize);
105+
Mockito.doReturn(virtualSize).when(processor).getTemplateVirtualSize((File)Mockito.any());
106+
Assert.assertEquals(virtualSize, processor.getVirtualSize(mockFile));
107+
Mockito.verify(mockFile, Mockito.times(0)).length();
108+
}
109+
}

0 commit comments

Comments
 (0)