Skip to content

Commit b1f0fda

Browse files
vishesh92vishesh92
committed
Add additional checks to ensure different services & networks on the same VM
1 parent f4f7163 commit b1f0fda

File tree

4 files changed

+177
-52
lines changed

4 files changed

+177
-52
lines changed

‎engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java‎

Lines changed: 109 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,14 @@
3030
import java.nio.file.Files;
3131
import java.nio.file.Path;
3232
import java.nio.file.Paths;
33+
import java.util.HashSet;
3334
import java.util.List;
3435
import java.util.Map;
3536
import java.util.Set;
3637

38+
import com.cloud.network.Network;
3739
import com.cloud.vm.NicProfile;
40+
import com.google.gson.JsonParser;
3841
import com.googlecode.ipv6.IPv6Network;
3942
import org.apache.commons.codec.binary.Base64;
4043
import org.apache.commons.collections.MapUtils;
@@ -51,9 +54,13 @@
5154
import com.google.gson.JsonElement;
5255
import com.google.gson.JsonObject;
5356

57+
import javax.inject.Inject;
58+
5459
public class ConfigDriveBuilder {
5560

5661
protected static Logger LOGGER = LogManager.getLogger(ConfigDriveBuilder.class);
62+
@Inject
63+
NetworkModel _networkModel;
5764

5865
/**
5966
* This is for mocking the File class. We cannot mock the File class directly because Mockito uses it internally.
@@ -84,7 +91,7 @@ static void writeFile(File folder, String file, String content) {
8491

8592
/**
8693
* Read the content of a {@link File} and convert it to a String in base 64.
87-
* We expect the content of the file to be encoded using {@link StandardCharsets#US_ASC}
94+
* We expect the content of the file to be encoded using {@link StandardCharsets#US_ASCII}
8895
*/
8996
public static String fileToBase64String(File isoFile) throws IOException {
9097
byte[] encoded = Base64.encodeBase64(FileUtils.readFileToByteArray(isoFile));
@@ -111,7 +118,7 @@ public static File base64StringToFile(String encodedIsoData, String folder, Stri
111118
* This method will build the metadata files required by OpenStack driver. Then, an ISO is going to be generated and returned as a String in base 64.
112119
* If vmData is null, we throw a {@link CloudRuntimeException}. Moreover, {@link IOException} are captured and re-thrown as {@link CloudRuntimeException}.
113120
*/
114-
public static String buildConfigDrive(NicProfile nic, List<String[]> vmData, String isoFileName, String driveLabel, Map<String, String> customUserdataParams) {
121+
public static String buildConfigDrive(NicProfile nic, List<String[]> vmData, String isoFileName, String driveLabel, Map<String, String> customUserdataParams, List<Network.Service> supportedServices) {
115122
if (vmData == null && nic == null) {
116123
throw new CloudRuntimeException("No VM metadata and nic profile provided");
117124
}
@@ -124,11 +131,13 @@ public static String buildConfigDrive(NicProfile nic, List<String[]> vmData, Str
124131

125132
File openStackFolder = new File(tempDirName + ConfigDrive.openStackConfigDriveName);
126133

127-
writeVendorAndNetworkEmptyJsonFile(openStackFolder);
128-
writeNetworkData(nic, tempDirName, openStackFolder);
129-
writeVmMetadata(vmData, tempDirName, openStackFolder, customUserdataParams);
134+
writeVendorEmptyJsonFile(openStackFolder);
135+
writeNetworkData(nic, supportedServices, openStackFolder);
136+
if (supportedServices.contains(Network.Service.UserData)) {
137+
writeVmMetadata(vmData, tempDirName, openStackFolder, customUserdataParams);
130138

131-
linkUserData(tempDirName);
139+
linkUserData(tempDirName);
140+
}
132141

133142
return generateAndRetrieveIsoAsBase64Iso(isoFileName, driveLabel, tempDirName);
134143
} catch (IOException e) {
@@ -215,27 +224,70 @@ static void writeVmMetadata(List<String[]> vmData, String tempDirName, File open
215224
writeFile(openStackFolder, "meta_data.json", metaData.toString());
216225
}
217226

227+
static JsonObject getExistingNetworkData(File openStackFolder) {
228+
if (openStackFolder.exists()) {
229+
File networkDataFile = getFile(openStackFolder.getAbsolutePath(), "network_data.json");
230+
if (networkDataFile.exists()) {
231+
try {
232+
String networkDataFileContent = FileUtils.readFileToString(networkDataFile, com.cloud.utils.StringUtils.getPreferredCharset());
233+
if (StringUtils.isNotBlank(networkDataFileContent)) {
234+
return new JsonParser().parse(networkDataFileContent).getAsJsonObject();
235+
}
236+
} catch (IOException e) {
237+
LOGGER.warn("Failed to read existing network data file: {}", networkDataFile.getAbsolutePath(), e);
238+
}
239+
}
240+
}
241+
return new JsonObject();
242+
}
243+
218244
/**
219-
* First we generate a JSON object using {@link #createJsonObjectWithNic(NicProfile)}, then we write it to a file called "network_data.json".
245+
* First we generate a JSON object using {@link #getNetworkDataJsonObjectForNic(NicProfile, List)}, then we write it to a file called "network_data.json".
220246
*/
221-
static void writeNetworkData(NicProfile nic, String tempDirName, File openStackFolder) {
222-
JsonObject networkData = createJsonObjectWithNic(nic);
223-
writeFile(openStackFolder, "network_data.json", networkData.toString());
247+
static void writeNetworkData(NicProfile nic, List<Network.Service> supportedServices, File openStackFolder) {
248+
JsonObject networkData = getNetworkDataJsonObjectForNic(nic, supportedServices);
249+
JsonObject existingNetworkData = getExistingNetworkData(openStackFolder);
250+
251+
JsonObject finalNetworkData = new JsonObject();
252+
mergeJsonArraysAndUpdateObject(finalNetworkData, existingNetworkData, networkData, "links", "id", "type");
253+
mergeJsonArraysAndUpdateObject(finalNetworkData, existingNetworkData, networkData, "networks", "id", "type");
254+
mergeJsonArraysAndUpdateObject(finalNetworkData, existingNetworkData, networkData, "services", "address", "type");
255+
256+
writeFile(openStackFolder, "network_data.json", existingNetworkData.toString());
257+
}
258+
259+
static void mergeJsonArraysAndUpdateObject(JsonObject finalObject, JsonObject obj1, JsonObject obj2, String memberName, String keyPart1, String keyPart2) {
260+
JsonArray existingMembers = obj1.has(memberName) ? obj1.get(memberName).getAsJsonArray() : new JsonArray();
261+
JsonArray newMembers = obj2.has(memberName) ? obj2.get(memberName).getAsJsonArray() : new JsonArray();
262+
263+
if (existingMembers.size() > 0 || newMembers.size() > 0) {
264+
JsonArray finalMembers = new JsonArray();
265+
Set<String> idSet = new HashSet<>();
266+
for (JsonElement element : existingMembers.getAsJsonArray()) {
267+
JsonObject elementObject = element.getAsJsonObject();
268+
String key = String.format("%s-%s", elementObject.get(keyPart1).getAsString(), elementObject.get(keyPart2).getAsString());
269+
idSet.add(key);
270+
finalMembers.add(element);
271+
}
272+
for (JsonElement element : newMembers.getAsJsonArray()) {
273+
JsonObject elementObject = element.getAsJsonObject();
274+
String key = String.format("%s-%s", elementObject.get(keyPart1).getAsString(), elementObject.get(keyPart2).getAsString());
275+
if (!idSet.contains(key)) {
276+
finalMembers.add(element);
277+
}
278+
}
279+
finalObject.add(memberName, finalMembers);
280+
}
224281
}
225282

226283
/**
227-
* Writes the following empty JSON files:
228-
* <ul>
229-
* <li> vendor_data.json
230-
* <li> network_data.json
231-
* </ul>
284+
* Writes an empty JSON file named vendor_data.json in openStackFolder
232285
*
233-
* If the folder does not exist and we cannot create it, we throw a {@link CloudRuntimeException}.
286+
* If the folder does not exist, and we cannot create it, we throw a {@link CloudRuntimeException}.
234287
*/
235-
static void writeVendorAndNetworkEmptyJsonFile(File openStackFolder) {
288+
static void writeVendorEmptyJsonFile(File openStackFolder) {
236289
if (openStackFolder.exists() || openStackFolder.mkdirs()) {
237290
writeFile(openStackFolder, "vendor_data.json", "{}");
238-
writeFile(openStackFolder, "network_data.json", "{}");
239291
} else {
240292
throw new CloudRuntimeException("Failed to create folder " + openStackFolder);
241293
}
@@ -263,15 +315,39 @@ static JsonObject createJsonObjectWithVmData(List<String[]> vmData, String tempD
263315
}
264316

265317
/**
266-
* Creates the {@link JsonObject} with NIC's metadata. We expect the JSONObject to have the following entries:
318+
* Creates the {@link JsonObject} using @param nic's metadata. We expect the JSONObject to have the following entries:
319+
* <ul>
320+
* <li> links </li>
321+
* <li> networks </li>
322+
* <li> services </li>
323+
* </ul>
267324
*/
268-
static JsonObject createJsonObjectWithNic(NicProfile nic) {
269-
// TODO: Check if we actually need to read the existing file and upsert the data to support multiple networks
325+
static JsonObject getNetworkDataJsonObjectForNic(NicProfile nic, List<Network.Service> supportedServices) {
270326
JsonObject networkData = new JsonObject();
271-
JsonArray links = new JsonArray();
272-
JsonArray networks = new JsonArray();
273-
JsonArray services = new JsonArray();
274327

328+
if (supportedServices.contains(Network.Service.Dhcp)) {
329+
JsonArray links = getLinksJsonArrayForNic(nic);
330+
JsonArray networks = getNetworksJsonArrayForNic(nic);
331+
if (links.size() > 0) {
332+
networkData.add("links", links);
333+
}
334+
if (networks.size() > 0) {
335+
networkData.add("networks", networks);
336+
}
337+
}
338+
339+
if (supportedServices.contains(Network.Service.Dns)) {
340+
JsonArray services = getServicesJsonArrayForNic(nic);
341+
if (services.size() > 0) {
342+
networkData.add("services", services);
343+
}
344+
}
345+
346+
return networkData;
347+
}
348+
349+
static JsonArray getLinksJsonArrayForNic(NicProfile nic) {
350+
JsonArray links = new JsonArray();
275351
if (StringUtils.isNotBlank(nic.getMacAddress())) {
276352
JsonObject link = new JsonObject();
277353
link.addProperty("ethernet_mac_address", nic.getMacAddress());
@@ -280,7 +356,11 @@ static JsonObject createJsonObjectWithNic(NicProfile nic) {
280356
link.addProperty("type", "phy");
281357
links.add(link);
282358
}
359+
return links;
360+
}
283361

362+
static JsonArray getNetworksJsonArrayForNic(NicProfile nic) {
363+
JsonArray networks = new JsonArray();
284364
if (StringUtils.isNotBlank(nic.getIPv4Address())) {
285365
JsonObject ipv4Network = new JsonObject();
286366
ipv4Network.addProperty("id", String.format("eth%d", nic.getDeviceId()));
@@ -322,7 +402,11 @@ static JsonObject createJsonObjectWithNic(NicProfile nic) {
322402

323403
networks.add(ipv6Network);
324404
}
405+
return networks;
406+
}
325407

408+
static JsonArray getServicesJsonArrayForNic(NicProfile nic) {
409+
JsonArray services = new JsonArray();
326410
if (StringUtils.isNotBlank(nic.getIPv4Dns1())) {
327411
services.add(getDnsServiceObject(nic.getIPv4Dns1()));
328412
}
@@ -338,12 +422,7 @@ static JsonObject createJsonObjectWithNic(NicProfile nic) {
338422
if (StringUtils.isNotBlank(nic.getIPv6Dns2())) {
339423
services.add(getDnsServiceObject(nic.getIPv6Dns2()));
340424
}
341-
342-
networkData.add("links", links);
343-
networkData.add("networks", networks);
344-
networkData.add("services", services);
345-
346-
return networkData;
425+
return services;
347426
}
348427

349428
private static JsonObject getDnsServiceObject(String dnsAddress) {

‎engine/storage/configdrive/src/test/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilderTest.java‎

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@
3131
import java.util.List;
3232
import java.util.Map;
3333

34+
import com.cloud.network.Network;
3435
import org.apache.commons.io.FileUtils;
3536
import org.apache.commons.lang3.StringUtils;
3637
import org.junit.Assert;
38+
import org.junit.BeforeClass;
3739
import org.junit.Test;
3840
import org.junit.runner.RunWith;
3941
import org.mockito.InOrder;
@@ -49,6 +51,13 @@
4951
@RunWith(MockitoJUnitRunner.class)
5052
public class ConfigDriveBuilderTest {
5153

54+
private static List<Network.Service> supportedServices;
55+
56+
@BeforeClass
57+
public static void beforeClass() throws Exception {
58+
supportedServices = List.of(Network.Service.UserData, Network.Service.Dhcp, Network.Service.Dns);
59+
}
60+
5261
@Test
5362
public void writeFileTest() {
5463
try (MockedStatic<FileUtils> fileUtilsMocked = Mockito.mockStatic(FileUtils.class)) {
@@ -112,39 +121,39 @@ public void base64StringToFileTest() throws Exception {
112121
}
113122

114123
@Test(expected = CloudRuntimeException.class)
115-
public void buildConfigDriveTestNoVmData() {
116-
ConfigDriveBuilder.buildConfigDrive(null, null, "teste", "C:", null);
124+
public void buildConfigDriveTestNoVmDataAndNic() {
125+
ConfigDriveBuilder.buildConfigDrive(null, null, "teste", "C:", null, null);
117126
}
118127

119128
@Test(expected = CloudRuntimeException.class)
120129
public void buildConfigDriveTestIoException() {
121130
try (MockedStatic<ConfigDriveBuilder> configDriveBuilderMocked = Mockito.mockStatic(ConfigDriveBuilder.class)) {
122-
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(nullable(File.class))).thenThrow(CloudRuntimeException.class);
123-
Mockito.when(ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null)).thenCallRealMethod();
124-
ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null);
131+
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorEmptyJsonFile(nullable(File.class))).thenThrow(CloudRuntimeException.class);
132+
Mockito.when(ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null, supportedServices)).thenCallRealMethod();
133+
ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null, supportedServices);
125134
}
126135
}
127136

128137
@Test
129138
public void buildConfigDriveTest() {
130139
try (MockedStatic<ConfigDriveBuilder> configDriveBuilderMocked = Mockito.mockStatic(ConfigDriveBuilder.class)) {
131140

132-
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(Mockito.any(File.class))).then(invocationOnMock -> null);
141+
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorEmptyJsonFile(Mockito.any(File.class))).then(invocationOnMock -> null);
133142

134143
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVmMetadata(Mockito.anyList(), Mockito.anyString(), Mockito.any(File.class), anyMap())).then(invocationOnMock -> null);
135144

136145
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.linkUserData((Mockito.anyString()))).then(invocationOnMock -> null);
137146

138147
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.generateAndRetrieveIsoAsBase64Iso(Mockito.anyString(), Mockito.anyString(), Mockito.anyString())).thenAnswer(invocation -> "mockIsoDataBase64");
139148
//force execution of real method
140-
Mockito.when(ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null)).thenCallRealMethod();
149+
Mockito.when(ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null, supportedServices)).thenCallRealMethod();
141150

142-
String returnedIsoData = ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null);
151+
String returnedIsoData = ConfigDriveBuilder.buildConfigDrive(null, new ArrayList<>(), "teste", "C:", null, supportedServices);
143152

144153
Assert.assertEquals("mockIsoDataBase64", returnedIsoData);
145154

146155
configDriveBuilderMocked.verify(() -> {
147-
ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(Mockito.any(File.class));
156+
ConfigDriveBuilder.writeVendorEmptyJsonFile(Mockito.any(File.class));
148157
ConfigDriveBuilder.writeVmMetadata(Mockito.anyList(), Mockito.anyString(), Mockito.any(File.class), anyMap());
149158
ConfigDriveBuilder.linkUserData(Mockito.anyString());
150159
ConfigDriveBuilder.generateAndRetrieveIsoAsBase64Iso(Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
@@ -153,33 +162,33 @@ public void buildConfigDriveTest() {
153162
}
154163

155164
@Test(expected = CloudRuntimeException.class)
156-
public void writeVendorAndNetworkEmptyJsonFileTestCannotCreateOpenStackFolder() {
165+
public void writeVendorEmptyJsonFileTestCannotCreateOpenStackFolder() {
157166
File folderFileMock = Mockito.mock(File.class);
158167
Mockito.doReturn(false).when(folderFileMock).mkdirs();
159168

160-
ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(folderFileMock);
169+
ConfigDriveBuilder.writeVendorEmptyJsonFile(folderFileMock);
161170
}
162171

163172
@Test(expected = CloudRuntimeException.class)
164-
public void writeVendorAndNetworkEmptyJsonFileTest() {
173+
public void writeVendorEmptyJsonFileTest() {
165174
File folderFileMock = Mockito.mock(File.class);
166175
Mockito.doReturn(false).when(folderFileMock).mkdirs();
167176

168-
ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(folderFileMock);
177+
ConfigDriveBuilder.writeVendorEmptyJsonFile(folderFileMock);
169178
}
170179

171180
@Test
172-
public void writeVendorAndNetworkEmptyJsonFileTestCreatingFolder() {
181+
public void writeVendorEmptyJsonFileTestCreatingFolder() {
173182
try (MockedStatic<ConfigDriveBuilder> configDriveBuilderMocked = Mockito.mockStatic(ConfigDriveBuilder.class)) {
174183

175184
File folderFileMock = Mockito.mock(File.class);
176185
Mockito.doReturn(false).when(folderFileMock).exists();
177186
Mockito.doReturn(true).when(folderFileMock).mkdirs();
178187

179188
//force execution of real method
180-
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(folderFileMock)).thenCallRealMethod();
189+
configDriveBuilderMocked.when(() -> ConfigDriveBuilder.writeVendorEmptyJsonFile(folderFileMock)).thenCallRealMethod();
181190

182-
ConfigDriveBuilder.writeVendorAndNetworkEmptyJsonFile(folderFileMock);
191+
ConfigDriveBuilder.writeVendorEmptyJsonFile(folderFileMock);
183192

184193
Mockito.verify(folderFileMock).exists();
185194
Mockito.verify(folderFileMock).mkdirs();

0 commit comments

Comments
 (0)