Skip to content

Commit ccf3568

Browse files
vishesh92vishesh92
committed
Address comments
1 parent e438a3f commit ccf3568

File tree

7 files changed

+51
-40
lines changed

7 files changed

+51
-40
lines changed

‎engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import java.util.Map;
2222

2323
import com.cloud.dc.DataCenter;
24+
import com.cloud.hypervisor.Hypervisor;
2425
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
2526
import org.apache.cloudstack.framework.config.ConfigKey;
2627
import org.apache.cloudstack.framework.config.ConfigKey.Scope;
@@ -144,6 +145,8 @@ void prepare(VirtualMachineProfile profile, DeployDestination dest, ReservationC
144145

145146
List<NicProfile> getNicProfiles(VirtualMachine vm);
146147

148+
List<NicProfile> getNicProfiles(Long vmId, Hypervisor.HypervisorType hypervisorType);
149+
147150
Map<String, String> getSystemVMAccessDetails(VirtualMachine vm);
148151

149152
Pair<? extends NetworkGuru, ? extends Network> implementNetwork(long networkId, DeployDestination dest, ReservationContext context)

‎engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4445,25 +4445,30 @@ private boolean getNicProfileDefaultNic(NicProfile nicProfile) {
44454445
}
44464446

44474447
@Override
4448-
public List<NicProfile> getNicProfiles(final VirtualMachine vm) {
4449-
final List<NicVO> nics = _nicDao.listByVmId(vm.getId());
4448+
public List<NicProfile> getNicProfiles(final Long vmId, HypervisorType hypervisorType) {
4449+
final List<NicVO> nics = _nicDao.listByVmId(vmId);
44504450
final List<NicProfile> profiles = new ArrayList<NicProfile>();
44514451

44524452
if (nics != null) {
44534453
for (final Nic nic : nics) {
44544454
final NetworkVO network = _networksDao.findById(nic.getNetworkId());
4455-
final Integer networkRate = _networkModel.getNetworkRate(network.getId(), vm.getId());
4455+
final Integer networkRate = _networkModel.getNetworkRate(network.getId(), vmId);
44564456

44574457
final NetworkGuru guru = AdapterBase.getAdapterByName(networkGurus, network.getGuruName());
44584458
final NicProfile profile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), networkRate,
4459-
_networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vm.getHypervisorType(), network));
4459+
_networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(hypervisorType, network));
44604460
guru.updateNicProfile(profile, network);
44614461
profiles.add(profile);
44624462
}
44634463
}
44644464
return profiles;
44654465
}
44664466

4467+
@Override
4468+
public List<NicProfile> getNicProfiles(final VirtualMachine vm) {
4469+
return getNicProfiles(vm.getId(), vm.getHypervisorType());
4470+
}
4471+
44674472
@Override
44684473
public Map<String, String> getSystemVMAccessDetails(final VirtualMachine vm) {
44694474
final Map<String, String> accessDetails = new HashMap<>();

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

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ public static String buildConfigDrive(List<NicProfile> nics, List<String[]> vmDa
130130
writeNetworkData(nics, supportedServices, openStackFolder);
131131
for (NicProfile nic: nics) {
132132
if (supportedServices.get(nic.getId()).contains(Network.Service.UserData)) {
133+
if (vmData == null) {
134+
throw new CloudRuntimeException("No VM metadata provided");
135+
}
133136
writeVmMetadata(vmData, tempDirName, openStackFolder, customUserdataParams);
134137

135138
linkUserData(tempDirName);
@@ -226,20 +229,25 @@ static void writeVmMetadata(List<String[]> vmData, String tempDirName, File open
226229
* First we generate a JSON object using {@link #getNetworkDataJsonObjectForNic(NicProfile, List)}, then we write it to a file called "network_data.json".
227230
*/
228231
static void writeNetworkData(List<NicProfile> nics, Map<Long, List<Network.Service>> supportedServices, File openStackFolder) {
229-
230232
JsonObject finalNetworkData = new JsonObject();
231-
for (NicProfile nic: nics) {
232-
List<Network.Service> supportedService = supportedServices.get(nic.getId());
233-
JsonObject networkData = getNetworkDataJsonObjectForNic(nic, supportedService);
234-
235-
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "links", "id", "type");
236-
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "networks", "id", "type");
237-
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "services", "address", "type");
233+
if (needForGeneratingNetworkData(supportedServices)) {
234+
for (NicProfile nic : nics) {
235+
List<Network.Service> supportedService = supportedServices.get(nic.getId());
236+
JsonObject networkData = getNetworkDataJsonObjectForNic(nic, supportedService);
237+
238+
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "links", "id", "type");
239+
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "networks", "id", "type");
240+
mergeJsonArraysAndUpdateObject(finalNetworkData, networkData, "services", "address", "type");
241+
}
238242
}
239243

240244
writeFile(openStackFolder, "network_data.json", finalNetworkData.toString());
241245
}
242246

247+
static boolean needForGeneratingNetworkData(Map<Long, List<Network.Service>> supportedServices) {
248+
return supportedServices.values().stream().anyMatch(services -> services.contains(Network.Service.Dhcp) || services.contains(Network.Service.Dns));
249+
}
250+
243251
static void mergeJsonArraysAndUpdateObject(JsonObject finalObject, JsonObject newObj, String memberName, String keyPart1, String keyPart2) {
244252
JsonArray existingMembers = finalObject.has(memberName) ? finalObject.get(memberName).getAsJsonArray() : new JsonArray();
245253
JsonArray newMembers = newObj.has(memberName) ? newObj.get(memberName).getAsJsonArray() : new JsonArray();
@@ -309,22 +317,18 @@ static JsonObject createJsonObjectWithVmData(List<String[]> vmData, String tempD
309317
static JsonObject getNetworkDataJsonObjectForNic(NicProfile nic, List<Network.Service> supportedServices) {
310318
JsonObject networkData = new JsonObject();
311319

312-
if (supportedServices.contains(Network.Service.Dhcp)) {
313-
JsonArray links = getLinksJsonArrayForNic(nic);
314-
JsonArray networks = getNetworksJsonArrayForNic(nic);
315-
if (links.size() > 0) {
316-
networkData.add("links", links);
317-
}
318-
if (networks.size() > 0) {
319-
networkData.add("networks", networks);
320-
}
320+
JsonArray links = getLinksJsonArrayForNic(nic);
321+
JsonArray networks = getNetworksJsonArrayForNic(nic);
322+
if (links.size() > 0) {
323+
networkData.add("links", links);
324+
}
325+
if (networks.size() > 0) {
326+
networkData.add("networks", networks);
321327
}
322328

323-
if (supportedServices.contains(Network.Service.Dns)) {
324-
JsonArray services = getServicesJsonArrayForNic(nic);
325-
if (services.size() > 0) {
326-
networkData.add("services", services);
327-
}
329+
JsonArray services = getServicesJsonArrayForNic(nic);
330+
if (services.size() > 0) {
331+
networkData.add("services", services);
328332
}
329333

330334
return networkData;

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,8 +636,6 @@ public void testWriteNetworkData() throws Exception {
636636
public void testWriteNetworkDataEmptyJson() throws Exception {
637637
// Setup
638638
NicProfile nicp = mock(NicProfile.class);
639-
Mockito.when(nicp.getId()).thenReturn(1L);
640-
641639
List<Network.Service> services1 = Collections.emptyList();
642640

643641
Map<Long, List<Network.Service>> supportedServices = new HashMap<>();

‎server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525

2626
import javax.inject.Inject;
2727

28+
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
2829
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
2930
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
3031
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
@@ -112,6 +113,8 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle
112113
@Inject
113114
NetworkModel _networkModel;
114115
@Inject
116+
NetworkOrchestrationService _networkOrchestrationService;
117+
@Inject
115118
GuestOSCategoryDao _guestOSCategoryDao;
116119
@Inject
117120
GuestOSDao _guestOSDao;
@@ -546,8 +549,7 @@ private boolean createConfigDriveIsoOnHostCache(NicProfile nic, VirtualMachinePr
546549

547550
final String isoFileName = ConfigDrive.configIsoFileName(profile.getInstanceName());
548551
final String isoPath = ConfigDrive.createConfigDrivePath(profile.getInstanceName());
549-
List<? extends Nic> nics = _networkModel.getNics(nic.getVirtualMachineId());
550-
List<NicProfile> nicProfiles = getNicProfilesForNic(profile.getVirtualMachine(), nics);
552+
List<NicProfile> nicProfiles = _networkOrchestrationService.getNicProfiles(nic.getVirtualMachineId(), profile.getHypervisorType());
551553
final Map<Long, List<Service>> supportedServices = getSupportedServicesByElementForNetwork(nicProfiles);
552554
final String isoData = ConfigDriveBuilder.buildConfigDrive(nicProfiles, profile.getVmData(), isoFileName, profile.getConfigDriveLabel(), customUserdataParamMap, supportedServices);
553555
final HandleConfigDriveIsoCommand configDriveIsoCommand = new HandleConfigDriveIsoCommand(isoPath, isoData, null, false, true, true);
@@ -599,14 +601,6 @@ private boolean deleteConfigDriveIsoOnHostCache(final VirtualMachine vm, final L
599601
return true;
600602
}
601603

602-
private List<NicProfile> getNicProfilesForNic(VirtualMachine vm, List<? extends Nic> nics) {
603-
List<NicProfile> nicProfiles = new ArrayList<>();
604-
for (Nic nic: nics) {
605-
nicProfiles.add(_networkModel.getNicProfile(vm, nic.getNetworkId(), null));
606-
}
607-
return nicProfiles;
608-
}
609-
610604
private Map<Long, List<Network.Service>> getSupportedServicesByElementForNetwork(List<NicProfile> nics) {
611605

612606
Map<Long, List<Network.Service>> supportedServices = new HashMap<>();
@@ -642,8 +636,7 @@ public boolean createConfigDriveIso(NicProfile nic, VirtualMachineProfile profil
642636

643637
final String isoFileName = ConfigDrive.configIsoFileName(profile.getInstanceName());
644638
final String isoPath = ConfigDrive.createConfigDrivePath(profile.getInstanceName());
645-
List<? extends Nic> nics = _networkModel.getNics(profile.getId());
646-
List<NicProfile> nicProfiles = getNicProfilesForNic(profile.getVirtualMachine(), nics);
639+
List<NicProfile> nicProfiles = _networkOrchestrationService.getNicProfiles(nic.getVirtualMachineId(), profile.getHypervisorType());
647640
final Map<Long, List<Service>> supportedServices = getSupportedServicesByElementForNetwork(nicProfiles);
648641
final String isoData = ConfigDriveBuilder.buildConfigDrive(
649642
nicProfiles, profile.getVmData(), isoFileName, profile.getConfigDriveLabel(), customUserdataParamMap, supportedServices);

‎server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import com.cloud.vm.dao.VMInstanceDao;
6262
import com.google.common.collect.Maps;
6363
import org.apache.cloudstack.context.CallContext;
64+
import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
6465
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
6566
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
6667
import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
@@ -149,6 +150,7 @@ public class ConfigDriveNetworkElementTest {
149150
@Mock private AgentManager agentManager;
150151
@Mock private CallContext callContextMock;
151152
@Mock private DomainVO domainVO;
153+
@Mock private NetworkOrchestrationService _networkOrchestrationService;
152154

153155
@Spy @InjectMocks
154156
private ConfigDriveNetworkElement _configDrivesNetworkElement = new ConfigDriveNetworkElement();

‎server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import javax.naming.ConfigurationException;
2626

2727
import com.cloud.dc.DataCenter;
28+
import com.cloud.hypervisor.Hypervisor;
2829
import com.cloud.network.PublicIpQuarantine;
2930
import com.cloud.network.VirtualRouterProvider;
3031
import com.cloud.utils.fsm.NoTransitionException;
@@ -640,6 +641,11 @@ public List<NicProfile> getNicProfiles(VirtualMachine vm) {
640641
return null;
641642
}
642643

644+
@Override
645+
public List<NicProfile> getNicProfiles(Long vmId, Hypervisor.HypervisorType hypervisorType) {
646+
return List.of();
647+
}
648+
643649
@Override
644650
public Map<String, String> getSystemVMAccessDetails(VirtualMachine vm) {
645651
return null;

0 commit comments

Comments
 (0)