harikrishna-patnala commented on code in PR #9329:
URL: https://github.com/apache/cloudstack/pull/9329#discussion_r1672122749
##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java:
##########
@@ -1821,13 +1823,19 @@ protected boolean prepareElement(final NetworkElement
element, final Network net
if (!sp.addDnsEntry(network, profile, vmProfile, dest,
context)) {
return false;
}
+ buildConfigDrive = true;
}
if (_networkModel.areServicesSupportedInNetwork(network.getId(),
Service.UserData)
&&
_networkModel.isProviderSupportServiceInNetwork(network.getId(),
Service.UserData, element.getProvider()) && element instanceof
UserDataServiceProvider) {
final UserDataServiceProvider sp = (UserDataServiceProvider)
element;
if (!sp.addPasswordAndUserdata(network, profile, vmProfile,
dest, context)) {
return false;
}
+ buildConfigDrive = true;
+ }
+ if (buildConfigDrive && element instanceof
ConfigDriveNetworkElement) {
Review Comment:
Do we need to this flag ? is nt it implicit that if the element is of
configdrive, we need to prepare the drive at this point ?
##########
server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java:
##########
@@ -590,7 +599,35 @@ private boolean deleteConfigDriveIsoOnHostCache(final
VirtualMachine vm, final L
return true;
}
- private boolean createConfigDriveIso(VirtualMachineProfile profile,
DeployDestination dest, DiskTO disk) throws ResourceUnavailableException {
+ private List<NicProfile> getNicProfilesForNic(VirtualMachine vm, List<?
extends Nic> nics) {
Review Comment:
not sure if this is helpful but NetworkOrchestrator has already this method
public List<NicProfile> getNicProfiles(final VirtualMachine vm) {
##########
engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java:
##########
@@ -212,18 +223,55 @@ static void writeVmMetadata(List<String[]> vmData, String
tempDirName, File open
}
/**
- * Writes the following empty JSON files:
- * <ul>
- * <li> vendor_data.json
- * <li> network_data.json
- * </ul>
+ * First we generate a JSON object using {@link
#getNetworkDataJsonObjectForNic(NicProfile, List)}, then we write it to a file
called "network_data.json".
+ */
+ static void writeNetworkData(List<NicProfile> nics, Map<Long,
List<Network.Service>> supportedServices, File openStackFolder) {
+
+ JsonObject finalNetworkData = new JsonObject();
+ for (NicProfile nic: nics) {
+ List<Network.Service> supportedService =
supportedServices.get(nic.getId());
+ JsonObject networkData = getNetworkDataJsonObjectForNic(nic,
supportedService);
+
+ mergeJsonArraysAndUpdateObject(finalNetworkData, networkData,
"links", "id", "type");
+ mergeJsonArraysAndUpdateObject(finalNetworkData, networkData,
"networks", "id", "type");
+ mergeJsonArraysAndUpdateObject(finalNetworkData, networkData,
"services", "address", "type");
+ }
+
+ writeFile(openStackFolder, "network_data.json",
finalNetworkData.toString());
+ }
+
+ static void mergeJsonArraysAndUpdateObject(JsonObject finalObject,
JsonObject newObj, String memberName, String keyPart1, String keyPart2) {
Review Comment:
can we move this to any Utils class ? seems like a generic implementation
for merging Json arrays
##########
engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java:
##########
@@ -108,9 +113,9 @@ public static File base64StringToFile(String
encodedIsoData, String folder, Stri
* 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.
* If vmData is null, we throw a {@link CloudRuntimeException}. Moreover,
{@link IOException} are captured and re-thrown as {@link CloudRuntimeException}.
*/
- public static String buildConfigDrive(List<String[]> vmData, String
isoFileName, String driveLabel, Map<String, String> customUserdataParams) {
- if (vmData == null) {
- throw new CloudRuntimeException("No VM metadata provided");
+ public static String buildConfigDrive(List<NicProfile> nics,
List<String[]> vmData, String isoFileName, String driveLabel, Map<String,
String> customUserdataParams, Map<Long, List<Network.Service>>
supportedServices) {
+ if (vmData == null && nics == null) {
Review Comment:
does it mean we are always expecting nics information everytime we build the
config drive ? prior to this PR it used to work without this information also
right! please correct me if I'm wrong here
##########
engine/storage/configdrive/src/main/java/org/apache/cloudstack/storage/configdrive/ConfigDriveBuilder.java:
##########
@@ -121,10 +126,16 @@ public static String buildConfigDrive(List<String[]>
vmData, String isoFileName,
File openStackFolder = new File(tempDirName +
ConfigDrive.openStackConfigDriveName);
- writeVendorAndNetworkEmptyJsonFile(openStackFolder);
- writeVmMetadata(vmData, tempDirName, openStackFolder,
customUserdataParams);
+ writeVendorEmptyJsonFile(openStackFolder);
+ writeNetworkData(nics, supportedServices, openStackFolder);
+ for (NicProfile nic: nics) {
Review Comment:
looks like this answers my previous question, we are adding the metadata per
nic right!
Another doubt here, I hope this won't break the existing config drive which
are already built before this PR changes ? for example after we start stop the
VM or when MS had to reapply the metadata to the VM in other case ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
