andreaturli commented on this pull request.
> @@ -383,39 +391,94 @@ private OSProfile createOsProfile(String computerName,
> Template template) {
return builder.build();
}
- private NetworkInterfaceCard createNetworkInterfaceCard(String subnetId,
String name, String locationName,
- String azureGroup,
TemplateOptions options) {
- final PublicIPAddressApi ipApi = api.getPublicIPAddressApi(azureGroup);
+ private List<NetworkInterfaceCard> createNICs(String nodeName, String
location, AzureTemplateOptions options) {
[minor] better `createNetworkInterfaceCards()` ?
>
- PublicIPAddressProperties properties =
PublicIPAddressProperties.builder().publicIPAllocationMethod("Static")
- .idleTimeoutInMinutes(4).build();
+ for (IpOptions ipConfig : publicIpsFirst(options.getIpOptions())) {
maybe this logic deserves to live in a private method? or a Function name
`ipOptionsTo IpConfiguration` maybe?
> }
- String networkInterfaceCardName = "jc-nic-" + name;
- return
api.getNetworkInterfaceCardApi(azureGroup).createOrUpdate(networkInterfaceCardName,
locationName,
- networkInterfaceCardProperties.build(),
ImmutableMap.of("jclouds", name));
+ return nics.build();
+ }
+
+ private NetworkProfile createNetworkProfile(List<NetworkInterfaceCard>
nics) {
+ List<NetworkInterface> nicAttachments = new
ArrayList<NetworkInterface>(nics.size());
+ for (int i = 0; i < nics.size(); i++) {
+ nicAttachments.add(NetworkInterface.create(nics.get(i).id(),
NetworkInterfaceProperties.create(i == 0)));
maybe a javadoc to explain the method as it is not obvious, especially the `i
== 0` part
> }
- String networkInterfaceCardName = "jc-nic-" + name;
- return
api.getNetworkInterfaceCardApi(azureGroup).createOrUpdate(networkInterfaceCardName,
locationName,
- networkInterfaceCardProperties.build(),
ImmutableMap.of("jclouds", name));
+ return nics.build();
+ }
+
+ private NetworkProfile createNetworkProfile(List<NetworkInterfaceCard>
nics) {
+ List<NetworkInterface> nicAttachments = new
ArrayList<NetworkInterface>(nics.size());
+ for (int i = 0; i < nics.size(); i++) {
+ nicAttachments.add(NetworkInterface.create(nics.get(i).id(),
NetworkInterfaceProperties.create(i == 0)));
+ }
+ return NetworkProfile.create(nicAttachments);
+ }
+
+ private static List<IpOptions> publicIpsFirst(List<IpOptions> ipOptions) {
javadoc maybe to explain the reason why you are sorting this list?
> List<NetworkSecurityGroup> networkGroups = new
> ArrayList<NetworkSecurityGroup>();
- for (IdReference networkInterfaceCardIdReference :
networkInterfacesIdReferences) {
- String nicName = networkInterfaceCardIdReference.name();
- String nicResourceGroup =
networkInterfaceCardIdReference.resourceGroup();
+ for (NetworkInterface networkInterfaceCardIdReference :
networkInterfacesIdReferences) {
maybe `networkInterfaceCardIdReference` is now `networkInterface` ?
> @@ -135,12 +137,12 @@ public boolean apply(SecurityGroup input) {
if (vm == null) {
throw new IllegalArgumentException("Node " + nodeId + " was not
found");
}
- List<IdReference> networkInterfacesIdReferences =
vm.properties().networkProfile().networkInterfaces();
+ List<NetworkInterface> networkInterfacesIdReferences =
vm.properties().networkProfile().networkInterfaces();
[minor] is maybe `networkInterfaces` rather than
`networkInterfacesIdReferences` ?
> @@ -136,9 +138,9 @@ public NodeMetadata apply(VirtualMachine virtualMachine) {
return builder.build();
}
- private Iterable<String> getPrivateIpAddresses(List<IdReference>
idReferences) {
+ private Iterable<String> getPrivateIpAddresses(List<NetworkInterface>
idReferences) {
[minor] `networkInterfaces` instead of `idReferences` ?
> private AvailabilitySet availabilitySet;
private String availabilitySetName;
private List<DataDisk> dataDisks = ImmutableList.of();
private String resourceGroup;
-
- /**
- * Sets the virtual network name
- */
- public AzureTemplateOptions virtualNetworkName(String virtualNetworkName) {
do we need to deprecate them first or because it is still in labs we can change
the public api this way?
> @@ -86,10 +89,10 @@ public boolean cleanupNode(final String id) {
}
public void cleanupVirtualMachineNICs(VirtualMachine virtualMachine) {
- for (IdReference nicRef :
virtualMachine.properties().networkProfile().networkInterfaces()) {
- String nicResourceGroup = nicRef.resourceGroup();
- String nicName = nicRef.name();
- NetworkInterfaceCard nic =
api.getNetworkInterfaceCardApi(nicRef.resourceGroup()).get(nicName);
+ for (NetworkInterface nicRef :
virtualMachine.properties().networkProfile().networkInterfaces()) {
just `nic` instead of `nicRef` ?
> @@ -196,4 +197,35 @@ private void createResourceGroupIfNeeded(String group,
> String location, AzureTem
ImmutableMap.of("description", "jclouds default resource
group"));
}
}
+
+ private void normalizeNetworkOptions(AzureTemplateOptions options) {
+ if (!options.getNetworks().isEmpty() &&
!options.getIpOptions().isEmpty()) {
+ throw new IllegalArgumentException(
+ "The options.networks and options.ipOptions are exclusive. Just
configure the networking options once.");
+ }
+
+ if (!options.getNetworks().isEmpty() &&
options.getIpOptions().isEmpty()) {
+ // The portable interface allows to configure network IDs (subnet
IDs),
move this comment to javadoc maybe?
> @@ -196,4 +197,35 @@ private void createResourceGroupIfNeeded(String group,
> String location, AzureTem
ImmutableMap.of("description", "jclouds default resource
group"));
}
}
+
+ private void normalizeNetworkOptions(AzureTemplateOptions options) {
+ if (!options.getNetworks().isEmpty() &&
!options.getIpOptions().isEmpty()) {
+ throw new IllegalArgumentException(
+ "The options.networks and options.ipOptions are exclusive. Just
configure the networking options once.");
+ }
+
+ if (!options.getNetworks().isEmpty() &&
options.getIpOptions().isEmpty()) {
+ // The portable interface allows to configure network IDs (subnet
IDs),
we need a good testing coverage for this method :D
> @@ -29,6 +31,8 @@
@AutoValue
public abstract class Subnet {
+ private static final Pattern NETWORK_PATTERN =
Pattern.compile("^.*/virtualNetworks/([^/]+)(/.*)?$");
maybe `VIRTUAL_NETWORK_PATTERN` ?
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/386#pullrequestreview-34960288
