This is an automated email from the ASF dual-hosted git repository. pearl11594 pushed a commit to branch nsx-fix-code-smells in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 05be11a54f585e1f19e249223392baf650f2f68f Author: Pearl Dsilva <pearl1...@gmail.com> AuthorDate: Wed Dec 27 11:27:53 2023 -0500 NSX: Fix code smells and reported bugs --- .../admin/network/CreateNetworkOfferingCmd.java | 4 +- .../command/admin/vpc/CreateVPCOfferingCmd.java | 4 +- .../cloud/configuration/ConfigurationManager.java | 29 +++--- .../engine/orchestration/NetworkOrchestrator.java | 8 +- .../cloud/vm/VirtualMachineManagerImplTest.java | 5 +- .../com/cloud/network/element/NsxProviderVO.java | 1 + .../agent/api/CreateNsxDhcpRelayConfigCommand.java | 15 +++ .../CreateNsxDistributedFirewallRulesCommand.java | 15 +++ .../api/CreateNsxLoadBalancerRuleCommand.java | 15 +++ .../agent/api/CreateNsxPortForwardRuleCommand.java | 16 ++++ .../api/CreateOrUpdateNsxTier1NatRuleCommand.java | 16 ++++ .../api/DeleteNsxLoadBalancerRuleCommand.java | 15 +++ .../agent/api/DeleteNsxNatRuleCommand.java | 16 ++++ .../agent/api/DeleteNsxSegmentCommand.java | 16 ++++ .../agent/api/DeleteNsxTier1GatewayCommand.java | 16 ++++ .../apache/cloudstack/resource/NsxNetworkRule.java | 1 + .../apache/cloudstack/resource/NsxResource.java | 14 +-- .../apache/cloudstack/service/NsxApiClient.java | 30 +++--- .../org/apache/cloudstack/service/NsxElement.java | 29 ++---- .../cloudstack/service/NsxGuestNetworkGuru.java | 103 +++++++++++---------- .../cloudstack/service/NsxProviderServiceImpl.java | 17 ++-- .../cloudstack/service/NsxPublicNetworkGuru.java | 26 ++---- .../org/apache/cloudstack/utils/NsxHelper.java | 3 + .../cloudstack/service/NsxApiClientTest.java | 4 +- .../service/NsxProviderServiceImplTest.java | 8 +- .../configuration/ConfigurationManagerImpl.java | 4 +- .../com/cloud/hypervisor/HypervisorGuruBase.java | 2 +- 27 files changed, 274 insertions(+), 158 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java index 5d0a87ef0df..93de44f9291 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/network/CreateNetworkOfferingCmd.java @@ -286,7 +286,7 @@ public class CreateNetworkOfferingCmd extends BaseCmd { return forVpc; } - public Boolean isForNsx() { + public boolean isForNsx() { return BooleanUtils.isTrue(forNsx); } @@ -314,7 +314,7 @@ public class CreateNetworkOfferingCmd extends BaseCmd { } public Map<String, List<String>> getServiceProviders() { - Map<String, List<String>> serviceProviderMap = new HashMap<String, List<String>>(); + Map<String, List<String>> serviceProviderMap = new HashMap<>(); if (serviceProviderList != null && !serviceProviderList.isEmpty() && !isForNsx()) { Collection servicesCollection = serviceProviderList.values(); Iterator iter = servicesCollection.iterator(); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java index e514ddb508b..a6601626803 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java @@ -163,7 +163,7 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd { return supportedServices; } - public Boolean isForNsx() { + public boolean isForNsx() { return BooleanUtils.isTrue(forNsx); } @@ -172,7 +172,7 @@ public class CreateVPCOfferingCmd extends BaseAsyncCreateCmd { } public Map<String, List<String>> getServiceProviders() { - Map<String, List<String>> serviceProviderMap = new HashMap<String, List<String>>();; + Map<String, List<String>> serviceProviderMap = new HashMap<>(); if (serviceProviderList != null && !serviceProviderList.isEmpty() && !isForNsx()) { Collection<? extends Map<String, String>> servicesCollection = serviceProviderList.values(); Iterator<? extends Map<String, String>> iter = servicesCollection.iterator(); diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index c975e8f351f..bbddd8fd471 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -63,9 +63,9 @@ public interface ConfigurationManager { static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length"; static final ConfigKey<Integer> VM_USERDATA_MAX_LENGTH = new ConfigKey<>("Advanced", Integer.class, VM_USERDATA_MAX_LENGTH_STRING, "32768", "Max length of vm userdata after base64 decoding. Default is 32768 and maximum is 1048576", true); - public static final ConfigKey<Boolean> AllowNonRFC1918CompliantIPs = new ConfigKey<Boolean>(Boolean.class, + public static final ConfigKey<Boolean> AllowNonRFC1918CompliantIPs = new ConfigKey<>(Boolean.class, "allow.non.rfc1918.compliant.ips", "Advanced", "false", - "Allows non-compliant RFC 1918 IPs for Shared, Isolated networks and VPCs", true); + "Allows non-compliant RFC 1918 IPs for Shared, Isolated networks and VPCs", true, null); /** * @param offering @@ -100,7 +100,6 @@ public interface ConfigurationManager { // * @param volatileVm // * @param hostTag // * @param networkRate -// * TODO // * @param id // * @param useVirtualNetwork // * @param deploymentPlanner @@ -170,11 +169,9 @@ public interface ConfigurationManager { * @param zoneType * @param allocationState * @param networkDomain - * TODO * @param isSecurityGroupEnabled - * TODO - * @param ip6Dns1 TODO - * @param ip6Dns2 TODO + * @param ip6Dns1 + * @param ip6Dns2 * @return * @throws * @throws @@ -189,7 +186,7 @@ public interface ConfigurationManager { * * @param userId * @param vlanDbId - * @param caller TODO + * @param caller * @return success/failure */ boolean deleteVlanAndPublicIpRange(long userId, long vlanDbId, Account caller); @@ -206,16 +203,16 @@ public interface ConfigurationManager { * @param trafficType * @param tags * @param specifyVlan - * @param networkRate TODO - * @param serviceProviderMap TODO - * @param isDefault TODO - * @param type TODO - * @param systemOnly TODO + * @param networkRate + * @param serviceProviderMap + * @param isDefault + * @param type + * @param systemOnly * @param serviceOfferingId * @param conserveMode ; - * @param specifyIpRanges TODO + * @param specifyIpRanges * @param isPersistent ; - * @param details TODO + * @param details * @param forVpc * @param forTungsten * @param forNsx @@ -228,7 +225,7 @@ public interface ConfigurationManager { Integer networkRate, Map<Service, Set<Provider>> serviceProviderMap, boolean isDefault, Network.GuestType type, boolean systemOnly, Long serviceOfferingId, boolean conserveMode, Map<Service, Map<Capability, String>> serviceCapabilityMap, boolean specifyIpRanges, boolean isPersistent, Map<NetworkOffering.Detail, String> details, boolean egressDefaultPolicy, Integer maxconn, boolean enableKeepAlive, Boolean forVpc, - Boolean forTungsten, Boolean forNsx, String mode, List<Long> domainIds, List<Long> zoneIds, boolean enableOffering, final NetUtils.InternetProtocol internetProtocol); + Boolean forTungsten, boolean forNsx, String mode, List<Long> domainIds, List<Long> zoneIds, boolean enableOffering, final NetUtils.InternetProtocol internetProtocol); Vlan createVlanAndPublicIpRange(long zoneId, long networkId, long physicalNetworkId, boolean forVirtualNetwork, boolean forSystemVms, Long podId, String startIP, String endIP, String vlanGateway, String vlanNetmask, String vlanId, boolean bypassVlanOverlapCheck, Domain domain, Account vlanOwner, String startIPv6, String endIPv6, String vlanIp6Gateway, String vlanIp6Cidr, boolean forNsx) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 93c5a6020fc..b9873470a53 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -1036,7 +1036,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra if (profile == null) { return null; } - // TODO: Fix with the public network PR + if (isNicAllocatedForNsxPublicNetworkOnVR(network, profile, vm)) { String guruName = "NsxPublicNetworkGuru"; NetworkGuru nsxGuru = AdapterBase.getAdapterByName(networkGurus, guruName); @@ -1090,7 +1090,6 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } Long vpcId = _ipAddressDao.findByIp(requested.getIPv4Address()).getVpcId(); - // TODO: Need to fix isolated network List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpcId, true); if (CollectionUtils.isEmpty(ips)) { @@ -2860,10 +2859,9 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra } // Check if cidr is RFC1918 compliant if the network is Guest Isolated for IPv4 - if (cidr != null && ntwkOff.getGuestType() == Network.GuestType.Isolated && ntwkOff.getTrafficType() == TrafficType.Guest) { - if (!NetUtils.validateGuestCidr(cidr, !ConfigurationManager.AllowNonRFC1918CompliantIPs.value())) { + if (cidr != null && (ntwkOff.getGuestType() == Network.GuestType.Isolated && ntwkOff.getTrafficType() == TrafficType.Guest) && + !NetUtils.validateGuestCidr(cidr, !ConfigurationManager.AllowNonRFC1918CompliantIPs.value())) { throw new InvalidParameterValueException("Virtual Guest Cidr " + cidr + " is not RFC 1918 or 6598 compliant"); - } } final String networkDomainFinal = networkDomain; diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 452f6cd1f4b..9e9a0df53b0 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -992,10 +992,11 @@ public class VirtualMachineManagerImplTest { Mockito.when(vpcVO.getId()).thenReturn(4L); Mockito.when(networkVO.getId()).thenReturn(5L); virtualMachineManagerImpl.setVmNetworkDetails(vm, vmTO); - assertEquals(vmTO.getNetworkIdToNetworkNameMap().size(), 1); - assertEquals(vmTO.getNetworkIdToNetworkNameMap().get(5L), "D3-A2-Z1-V4-S5"); + assertEquals(1, vmTO.getNetworkIdToNetworkNameMap().size()); + assertEquals("D3-A2-Z1-V4-S5", vmTO.getNetworkIdToNetworkNameMap().get(5L)); } + @Test public void testOrchestrateStartNonNullPodId() throws Exception { VMInstanceVO vmInstance = new VMInstanceVO(); ReflectionTestUtils.setField(vmInstance, "id", 1L); diff --git a/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java b/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java index 274038b74bb..f08e08b1ca0 100644 --- a/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java +++ b/engine/schema/src/main/java/com/cloud/network/element/NsxProviderVO.java @@ -213,6 +213,7 @@ public class NsxProviderVO implements NsxProvider { public Builder() { + // Default constructor } public Builder setZoneId(long zoneId) { diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java index 5b986628628..80f723380e1 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDhcpRelayConfigCommand.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.agent.api; import java.util.List; +import java.util.Objects; public class CreateNsxDhcpRelayConfigCommand extends NsxCommand { @@ -56,4 +57,18 @@ public class CreateNsxDhcpRelayConfigCommand extends NsxCommand { public List<String> getAddresses() { return addresses; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + CreateNsxDhcpRelayConfigCommand that = (CreateNsxDhcpRelayConfigCommand) o; + return networkId == that.networkId && Objects.equals(vpcId, that.vpcId) && Objects.equals(vpcName, that.vpcName) && Objects.equals(networkName, that.networkName) && Objects.equals(addresses, that.addresses); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), vpcId, vpcName, networkId, networkName, addresses); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java index 9283d1292cd..3b730909c9c 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxDistributedFirewallRulesCommand.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api; import org.apache.cloudstack.resource.NsxNetworkRule; import java.util.List; +import java.util.Objects; public class CreateNsxDistributedFirewallRulesCommand extends NsxCommand { @@ -46,4 +47,18 @@ public class CreateNsxDistributedFirewallRulesCommand extends NsxCommand { public List<NsxNetworkRule> getRules() { return rules; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + CreateNsxDistributedFirewallRulesCommand that = (CreateNsxDistributedFirewallRulesCommand) o; + return networkId == that.networkId && Objects.equals(vpcId, that.vpcId) && Objects.equals(rules, that.rules); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), vpcId, networkId, rules); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java index b35bda26c19..8a51f96fd67 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxLoadBalancerRuleCommand.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api; import org.apache.cloudstack.resource.NsxLoadBalancerMember; import java.util.List; +import java.util.Objects; public class CreateNsxLoadBalancerRuleCommand extends NsxNetworkCommand { @@ -66,4 +67,18 @@ public class CreateNsxLoadBalancerRuleCommand extends NsxNetworkCommand { public String getProtocol() { return protocol; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + CreateNsxLoadBalancerRuleCommand command = (CreateNsxLoadBalancerRuleCommand) o; + return lbId == command.lbId && Objects.equals(publicPort, command.publicPort) && Objects.equals(privatePort, command.privatePort) && Objects.equals(algorithm, command.algorithm) && Objects.equals(protocol, command.protocol) && Objects.equals(memberList, command.memberList); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), publicPort, privatePort, algorithm, protocol, memberList, lbId); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java index 96d5213abd5..35ea1121738 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateNsxPortForwardRuleCommand.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.agent.api; +import java.util.Objects; + public class CreateNsxPortForwardRuleCommand extends NsxNetworkCommand { private final String publicPort; private final String privatePort; @@ -49,4 +51,18 @@ public class CreateNsxPortForwardRuleCommand extends NsxNetworkCommand { public String getProtocol() { return protocol; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + CreateNsxPortForwardRuleCommand that = (CreateNsxPortForwardRuleCommand) o; + return ruleId == that.ruleId && Objects.equals(publicPort, that.publicPort) && Objects.equals(privatePort, that.privatePort) && Objects.equals(protocol, that.protocol); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), publicPort, privatePort, protocol, ruleId); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java index 7e3c03fd28c..cf2c156e3ca 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/CreateOrUpdateNsxTier1NatRuleCommand.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.agent.api; +import java.util.Objects; + public class CreateOrUpdateNsxTier1NatRuleCommand extends NsxCommand { private String tier1GatewayName; @@ -47,4 +49,18 @@ public class CreateOrUpdateNsxTier1NatRuleCommand extends NsxCommand { public String getNatRuleId() { return natRuleId; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + CreateOrUpdateNsxTier1NatRuleCommand that = (CreateOrUpdateNsxTier1NatRuleCommand) o; + return Objects.equals(tier1GatewayName, that.tier1GatewayName) && Objects.equals(action, that.action) && Objects.equals(translatedIpAddress, that.translatedIpAddress) && Objects.equals(natRuleId, that.natRuleId); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), tier1GatewayName, action, translatedIpAddress, natRuleId); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java index 21296aff991..a89842881ed 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxLoadBalancerRuleCommand.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.agent.api; import org.apache.cloudstack.resource.NsxLoadBalancerMember; import java.util.List; +import java.util.Objects; public class DeleteNsxLoadBalancerRuleCommand extends NsxNetworkCommand { private long lbId; @@ -37,4 +38,18 @@ public class DeleteNsxLoadBalancerRuleCommand extends NsxNetworkCommand { } public List<NsxLoadBalancerMember> getMemberList() { return memberList; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + DeleteNsxLoadBalancerRuleCommand that = (DeleteNsxLoadBalancerRuleCommand) o; + return lbId == that.lbId && Objects.equals(memberList, that.memberList); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), lbId, memberList); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java index 82ca54d5b0d..f054c24f6c0 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxNatRuleCommand.java @@ -18,6 +18,8 @@ package org.apache.cloudstack.agent.api; import com.cloud.network.Network; +import java.util.Objects; + public class DeleteNsxNatRuleCommand extends NsxNetworkCommand { private Long ruleId; private Network.Service service; @@ -51,4 +53,18 @@ public class DeleteNsxNatRuleCommand extends NsxNetworkCommand { public String getProtocol() { return protocol; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + DeleteNsxNatRuleCommand that = (DeleteNsxNatRuleCommand) o; + return Objects.equals(ruleId, that.ruleId) && Objects.equals(service, that.service) && Objects.equals(privatePort, that.privatePort) && Objects.equals(protocol, that.protocol); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), ruleId, service, privatePort, protocol); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java index 06af4d8c31d..bcb00680f31 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxSegmentCommand.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.agent.api; +import java.util.Objects; + public class DeleteNsxSegmentCommand extends NsxCommand { private Long vpcId; @@ -48,4 +50,18 @@ public class DeleteNsxSegmentCommand extends NsxCommand { public String getNetworkName() { return networkName; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + DeleteNsxSegmentCommand command = (DeleteNsxSegmentCommand) o; + return networkId == command.networkId && Objects.equals(vpcId, command.vpcId) && Objects.equals(vpcName, command.vpcName) && Objects.equals(networkName, command.networkName); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), vpcId, vpcName, networkId, networkName); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java index 2cfb3c3fe08..30af305954c 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/agent/api/DeleteNsxTier1GatewayCommand.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.agent.api; +import java.util.Objects; + public class DeleteNsxTier1GatewayCommand extends NsxCommand { private Long networkResourceId; @@ -41,4 +43,18 @@ public class DeleteNsxTier1GatewayCommand extends NsxCommand { public boolean isResourceVpc() { return isResourceVpc; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + DeleteNsxTier1GatewayCommand that = (DeleteNsxTier1GatewayCommand) o; + return isResourceVpc == that.isResourceVpc && Objects.equals(networkResourceId, that.networkResourceId) && Objects.equals(networkResourceName, that.networkResourceName); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), networkResourceId, networkResourceName, isResourceVpc); + } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java index bceee68389c..c11141db9d4 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxNetworkRule.java @@ -252,6 +252,7 @@ public class NsxNetworkRule { private Network.Service service; public Builder() { + // Default constructor } public Builder setDomainId(long domainId) { diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java index d36e9ee5809..a90266f7164 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxResource.java @@ -252,7 +252,7 @@ public class NsxResource implements ServerResource { } private Answer executeRequest(CreateNsxDhcpRelayConfigCommand cmd) { - long zoneId = cmd.getZoneId(); + long datacenterId = cmd.getZoneId(); long domainId = cmd.getDomainId(); long accountId = cmd.getAccountId(); Long vpcId = cmd.getVpcId(); @@ -261,7 +261,7 @@ public class NsxResource implements ServerResource { String networkName = cmd.getNetworkName(); List<String> addresses = cmd.getAddresses(); - String dhcpRelayConfigName = NsxControllerUtils.getNsxDhcpRelayConfigId(zoneId, domainId, accountId, vpcId, networkId); + String dhcpRelayConfigName = NsxControllerUtils.getNsxDhcpRelayConfigId(datacenterId, domainId, accountId, vpcId, networkId); String msg = String.format("Creating DHCP relay config with name %s on network %s of VPC %s", dhcpRelayConfigName, networkName, vpcName); @@ -275,7 +275,7 @@ public class NsxResource implements ServerResource { return new NsxAnswer(cmd, e); } - String segmentName = NsxControllerUtils.getNsxSegmentId(domainId, accountId, zoneId, vpcId, networkId); + String segmentName = NsxControllerUtils.getNsxSegmentId(domainId, accountId, datacenterId, vpcId, networkId); String dhcpConfigPath = String.format("%s/%s", DHCP_RELAY_CONFIGS_PATH_PREFIX, dhcpRelayConfigName); try { Segment segment = nsxApiClient.getSegmentById(segmentName); @@ -295,13 +295,13 @@ public class NsxResource implements ServerResource { } private Answer executeRequest(CreateNsxTier1GatewayCommand cmd) { - String name = NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc()); + String tier1GatewayName = NsxControllerUtils.getTier1GatewayName(cmd.getDomainId(), cmd.getAccountId(), cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc()); boolean sourceNatEnabled = cmd.isSourceNatEnabled(); try { - nsxApiClient.createTier1Gateway(name, tier0Gateway, edgeCluster, sourceNatEnabled); + nsxApiClient.createTier1Gateway(tier1GatewayName, tier0Gateway, edgeCluster, sourceNatEnabled); return new NsxAnswer(cmd, true, ""); } catch (CloudRuntimeException e) { - String msg = String.format("Cannot create tier 1 gateway %s (%s: %s): %s", name, + String msg = String.format("Cannot create tier 1 gateway %s (%s: %s): %s", tier1GatewayName, (cmd.isResourceVpc() ? "VPC" : "NETWORK"), cmd.getNetworkResourceName(), e.getMessage()); LOGGER.error(msg); return new NsxAnswer(cmd, e); @@ -457,7 +457,7 @@ public class NsxResource implements ServerResource { cmd.getZoneId(), cmd.getNetworkResourceId(), cmd.isResourceVpc()); String ruleName = NsxControllerUtils.getLoadBalancerRuleName(tier1GatewayName, cmd.getLbId()); try { - nsxApiClient.deleteNsxLbResources(tier1GatewayName, cmd.getLbId(), cmd.getVmId()); + nsxApiClient.deleteNsxLbResources(tier1GatewayName, cmd.getLbId()); } catch (Exception e) { LOGGER.error(String.format("Failed to add NSX load balancer rule %s for network: %s", ruleName, cmd.getNetworkResourceName())); return new NsxAnswer(cmd, new CloudRuntimeException(e.getMessage())); diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java index 5a3cf48aa04..ef42c754c6a 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxApiClient.java @@ -105,7 +105,7 @@ public class NsxApiClient { // Constants private static final String TIER_1_RESOURCE_TYPE = "Tier1"; - private static final String Tier_1_LOCALE_SERVICE_ID = "default"; + private static final String TIER_1_LOCALE_SERVICE_ID = "default"; private static final String SEGMENT_RESOURCE_TYPE = "Segment"; private static final String TIER_0_GATEWAY_PATH_PREFIX = "/infra/tier-0s/"; private static final String TIER_1_GATEWAY_PATH_PREFIX = "/infra/tier-1s/"; @@ -115,8 +115,6 @@ public class NsxApiClient { private enum PoolAllocation { ROUTING, LB_SMALL, LB_MEDIUM, LB_LARGE, LB_XLARGE } - private enum TYPE { ROUTED, NATTED } - private enum HAMode { ACTIVE_STANDBY, ACTIVE_ACTIVE } private enum FailoverMode { PREEMPTIVE, NON_PREEMPTIVE } @@ -155,10 +153,6 @@ public class NsxApiClient { JUMP_TO_APPLICATION } - private Map<String,String> actionMap = Map.of( - "Allow", FirewallActions.ALLOW.name(), - "Deny", FirewallActions.DROP.name()); - public enum RouteAdvertisementType { TIER1_STATIC_ROUTES, TIER1_CONNECTED, TIER1_NAT, TIER1_LB_VIP, TIER1_LB_SNAT, TIER1_DNS_FORWARDER_IP, TIER1_IPSEC_LOCAL_ENDPOINT } @@ -272,7 +266,7 @@ public class NsxApiClient { com.vmware.nsx_policy.infra.tier_1s.LocaleServices tier1LocalService = (com.vmware.nsx_policy.infra.tier_1s.LocaleServices) nsxService.apply(com.vmware.nsx_policy.infra.tier_1s.LocaleServices.class); com.vmware.nsx_policy.model.LocaleServices localeService = new com.vmware.nsx_policy.model.LocaleServices.Builder() .setEdgeClusterPath(localeServices.get(0).getEdgeClusterPath()).build(); - tier1LocalService.patch(tier1Id, Tier_1_LOCALE_SERVICE_ID, localeService); + tier1LocalService.patch(tier1Id, TIER_1_LOCALE_SERVICE_ID, localeService); } catch (Error error) { throw new CloudRuntimeException(String.format("Failed to instantiate tier-1 gateway %s in edge cluster %s", tier1Id, edgeCluster)); } @@ -329,7 +323,7 @@ public class NsxApiClient { return; } removeTier1GatewayNatRules(tier1Id); - localeService.delete(tier1Id, Tier_1_LOCALE_SERVICE_ID); + localeService.delete(tier1Id, TIER_1_LOCALE_SERVICE_ID); Tier1s tier1service = (Tier1s) nsxService.apply(Tier1s.class); tier1service.delete(tier1Id); } @@ -355,7 +349,7 @@ public class NsxApiClient { Sites sites = (Sites) nsxService.apply(Sites.class); return sites.list(null, false, null, null, null, null); } catch (Exception e) { - throw new CloudRuntimeException(String.format("Failed to fetch service segment list due to %s", e.getMessage())); + throw new CloudRuntimeException(String.format("Failed to fetch sites list due to %s", e.getMessage())); } } @@ -364,7 +358,7 @@ public class NsxApiClient { EnforcementPoints enforcementPoints = (EnforcementPoints) nsxService.apply(EnforcementPoints.class); return enforcementPoints.list(siteId, null, false, null, null, null, null); } catch (Exception e) { - throw new CloudRuntimeException(String.format("Failed to fetch service segment list due to %s", e.getMessage())); + throw new CloudRuntimeException(String.format("Failed to fetch enforcement points due to %s", e.getMessage())); } } @@ -373,7 +367,7 @@ public class NsxApiClient { com.vmware.nsx.TransportZones transportZones = (com.vmware.nsx.TransportZones) nsxService.apply(com.vmware.nsx.TransportZones.class); return transportZones.list(null, null, true, null, null, null, null, null, TransportType.OVERLAY.name(), null); } catch (Exception e) { - throw new CloudRuntimeException(String.format("Failed to fetch service segment list due to %s", e.getMessage())); + throw new CloudRuntimeException(String.format("Failed to fetch transport zones due to %s", e.getMessage())); } } @@ -532,7 +526,7 @@ public class NsxApiClient { } } - public void createNsxLoadBalancer(String tier1GatewayName, long lbId) { + public void createNsxLoadBalancer(String tier1GatewayName) { try { String lbName = getLoadBalancerName(tier1GatewayName); LbServices lbServices = (LbServices) nsxService.apply(LbServices.class); @@ -561,7 +555,7 @@ public class NsxApiClient { try { String lbServerPoolName = getServerPoolName(tier1GatewayName, lbId); createNsxLbServerPool(memberList, tier1GatewayName, lbServerPoolName, algorithm); - createNsxLoadBalancer(tier1GatewayName, lbId); + createNsxLoadBalancer(tier1GatewayName); String lbVirtualServerName = getVirtualServerName(tier1GatewayName, lbId); String lbServiceName = getLoadBalancerName(tier1GatewayName); @@ -584,7 +578,7 @@ public class NsxApiClient { } } - public void deleteNsxLbResources(String tier1GatewayName, long lbId, long vmId) { + public void deleteNsxLbResources(String tier1GatewayName, long lbId) { try { // Delete associated Virtual servers LbVirtualServers lbVirtualServers = (LbVirtualServers) nsxService.apply(LbVirtualServers.class); @@ -879,12 +873,14 @@ public class NsxApiClient { List<String> segmentGroup = List.of(String.format("%s/%s", GROUPS_PATH_PREFIX, segmentName)); List<String> sourceCidrList = rule.getSourceCidrList(); List<String> destCidrList = rule.getDestinationCidrList(); + List<String> ingressSource = (rule.getService() == Network.Service.NetworkACL ? segmentGroup : destCidrList); + List<String> egressSource = (rule.getService() == Network.Service.NetworkACL ? sourceCidrList : destCidrList); String trafficType = rule.getTrafficType(); if (trafficType.equalsIgnoreCase("ingress")) { - return source ? sourceCidrList : (rule.getService() == Network.Service.NetworkACL ? segmentGroup : destCidrList); + return source ? sourceCidrList : ingressSource; } else if (trafficType.equalsIgnoreCase("egress")) { - return source ? segmentGroup : (rule.getService() == Network.Service.NetworkACL ? sourceCidrList : destCidrList); + return source ? segmentGroup : egressSource; } String err = String.format("Unsupported traffic type %s", trafficType); LOGGER.error(err); diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java index c398312563d..bce62848e3e 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxElement.java @@ -234,9 +234,6 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns @Override public boolean implement(Network network, NetworkOffering offering, DeployDestination dest, ReservationContext context) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { -// Account account = accountMgr.getAccount(network.getAccountId()); -// DomainVO domain = domainDao.findById(network.getDomainId()); -// return nsxService.createNetwork(network.getDataCenterId(), account.getId(), domain.getId(), network.getId(), network.getName()); // TODO: Check if the network is NSX based (was already implemented as part of the guru.setup() return true; } @@ -485,8 +482,6 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns if (vm == null || networkModel.getNicInNetworkIncludingRemoved(vm.getId(), config.getId()) == null) { continue; } - Nic nic = networkModel.getNicInNetworkIncludingRemoved(vm.getId(), config.getId()); - Network publicNetwork = networkModel.getSystemNetworkByZoneAndTrafficType(config.getDataCenterId(), Networks.TrafficType.Public); Pair<VpcVO, NetworkVO> vpcOrNetwork = getVpcOrNetwork(config.getVpcId(), config.getId()); VpcVO vpc = vpcOrNetwork.first(); NetworkVO network = vpcOrNetwork.second(); @@ -544,14 +539,10 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns .setRuleId(rule.getId()) .setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT)) .build(); - if (rule.getState() == FirewallRule.State.Add) { - if (!nsxService.createPortForwardRule(networkRule)) { - return false; - } - } else if (rule.getState() == FirewallRule.State.Revoke) { - if (!nsxService.deletePortForwardRule(networkRule)) { - return false; - } + if (rule.getState() == FirewallRule.State.Add && !nsxService.createPortForwardRule(networkRule)) { + return false; + } else if (rule.getState() == FirewallRule.State.Revoke && !nsxService.deletePortForwardRule(networkRule)) { + return false; } } return true; @@ -632,14 +623,10 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns .setProtocol(loadBalancingRule.getProtocol().toUpperCase(Locale.ROOT)) .setAlgorithm(loadBalancingRule.getAlgorithm()) .build(); - if (loadBalancingRule.getState() == FirewallRule.State.Add) { - if (!nsxService.createLbRule(networkRule)) { - return false; - } - } else if (loadBalancingRule.getState() == FirewallRule.State.Revoke) { - if (!nsxService.deleteLbRule(networkRule)) { - return false; - } + if (loadBalancingRule.getState() == FirewallRule.State.Add && !nsxService.createLbRule(networkRule)) { + return false; + } else if (loadBalancingRule.getState() == FirewallRule.State.Revoke && !nsxService.deleteLbRule(networkRule)) { + return false; } } return true; diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java index 064bf404f47..8cf1d283fc7 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxGuestNetworkGuru.java @@ -197,64 +197,65 @@ public class NsxGuestNetworkGuru extends GuestNetworkGuru implements NetworkMigr @Override public NicProfile allocate(Network network, NicProfile nic, VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException { NicProfile nicProfile = super.allocate(network, nic, vm); + if (vm.getType() != VirtualMachine.Type.DomainRouter) { + return nicProfile; + } - if (vm.getType() == VirtualMachine.Type.DomainRouter) { - final DataCenter zone = _dcDao.findById(network.getDataCenterId()); - long zoneId = network.getDataCenterId(); - if (Objects.isNull(zone)) { - String msg = String.format("Unable to find zone with id: %s", zoneId); - LOGGER.error(msg); - throw new CloudRuntimeException(msg); - } - Account account = accountDao.findById(network.getAccountId()); - if (Objects.isNull(account)) { - String msg = String.format("Unable to find account with id: %s", network.getAccountId()); - LOGGER.error(msg); - throw new CloudRuntimeException(msg); - } - VpcVO vpc = _vpcDao.findById(network.getVpcId()); - if (Objects.isNull(vpc)) { - String msg = String.format("Unable to find VPC with id: %s, allocating for network %s", network.getVpcId(), network.getName()); - LOGGER.debug(msg); - } - - DomainVO domain = domainDao.findById(account.getDomainId()); - if (Objects.isNull(domain)) { - String msg = String.format("Unable to find domain with id: %s", account.getDomainId()); - LOGGER.error(msg); - throw new CloudRuntimeException(msg); - } + final DataCenter zone = _dcDao.findById(network.getDataCenterId()); + long zoneId = network.getDataCenterId(); + if (Objects.isNull(zone)) { + String msg = String.format("Unable to find zone with id: %s", zoneId); + LOGGER.error(msg); + throw new CloudRuntimeException(msg); + } + Account account = accountDao.findById(network.getAccountId()); + if (Objects.isNull(account)) { + String msg = String.format("Unable to find account with id: %s", network.getAccountId()); + LOGGER.error(msg); + throw new CloudRuntimeException(msg); + } + VpcVO vpc = _vpcDao.findById(network.getVpcId()); + if (Objects.isNull(vpc)) { + String msg = String.format("Unable to find VPC with id: %s, allocating for network %s", network.getVpcId(), network.getName()); + LOGGER.debug(msg); + } - if (isNull(network.getVpcId())) { - long domainId = domain.getId(); - long accountId = account.getId(); - long dataCenterId = zone.getId(); - long resourceId = network.getId(); - PublicIpAddress ipAddress = networkModel.getSourceNatIpAddressForGuestNetwork(account, network); - String translatedIp = ipAddress.getAddress().addr(); - String tier1GatewayName = NsxControllerUtils.getTier1GatewayName(domainId, accountId, dataCenterId, resourceId, false); - LOGGER.debug(String.format("Creating NSX NAT Rule for Tier1 GW %s for translated IP %s for Isolated network %s", tier1GatewayName, translatedIp, network.getName())); - String natRuleId = NsxControllerUtils.getNsxNatRuleId(domainId, accountId, dataCenterId, resourceId, false); - CreateOrUpdateNsxTier1NatRuleCommand cmd = NsxHelper.createOrUpdateNsxNatRuleCommand(domainId, accountId, dataCenterId, tier1GatewayName, "SNAT", translatedIp, natRuleId); - NsxAnswer nsxAnswer = nsxControllerUtils.sendNsxCommand(cmd, dataCenterId); - if (!nsxAnswer.getResult()) { - String msg = String.format("Could not create NSX NAT Rule on Tier1 Gateway %s for IP %s for Isolated network %s", tier1GatewayName, translatedIp, network.getName()); - LOGGER.error(msg); - throw new CloudRuntimeException(msg); - } - } + DomainVO domain = domainDao.findById(account.getDomainId()); + if (Objects.isNull(domain)) { + String msg = String.format("Unable to find domain with id: %s", account.getDomainId()); + LOGGER.error(msg); + throw new CloudRuntimeException(msg); + } - // Create the DHCP relay config for the segment - String iPv4Address = nicProfile.getIPv4Address(); - List<String> addresses = List.of(iPv4Address); - CreateNsxDhcpRelayConfigCommand command = NsxHelper.createNsxDhcpRelayConfigCommand(domain, account, zone, vpc, network, addresses); - NsxAnswer answer = nsxControllerUtils.sendNsxCommand(command, zone.getId()); - if (!answer.getResult()) { - String msg = String.format("Error creating DHCP relay config for network %s and nic %s: %s", network.getName(), nic.getName(), answer.getDetails()); + if (isNull(network.getVpcId())) { + long domainId = domain.getId(); + long accountId = account.getId(); + long dataCenterId = zone.getId(); + long resourceId = network.getId(); + PublicIpAddress ipAddress = networkModel.getSourceNatIpAddressForGuestNetwork(account, network); + String translatedIp = ipAddress.getAddress().addr(); + String tier1GatewayName = NsxControllerUtils.getTier1GatewayName(domainId, accountId, dataCenterId, resourceId, false); + LOGGER.debug(String.format("Creating NSX NAT Rule for Tier1 GW %s for translated IP %s for Isolated network %s", tier1GatewayName, translatedIp, network.getName())); + String natRuleId = NsxControllerUtils.getNsxNatRuleId(domainId, accountId, dataCenterId, resourceId, false); + CreateOrUpdateNsxTier1NatRuleCommand cmd = NsxHelper.createOrUpdateNsxNatRuleCommand(domainId, accountId, dataCenterId, tier1GatewayName, "SNAT", translatedIp, natRuleId); + NsxAnswer nsxAnswer = nsxControllerUtils.sendNsxCommand(cmd, dataCenterId); + if (!nsxAnswer.getResult()) { + String msg = String.format("Could not create NSX NAT Rule on Tier1 Gateway %s for IP %s for Isolated network %s", tier1GatewayName, translatedIp, network.getName()); LOGGER.error(msg); throw new CloudRuntimeException(msg); } } + + // Create the DHCP relay config for the segment + String iPv4Address = nicProfile.getIPv4Address(); + List<String> addresses = List.of(iPv4Address); + CreateNsxDhcpRelayConfigCommand command = NsxHelper.createNsxDhcpRelayConfigCommand(domain, account, zone, vpc, network, addresses); + NsxAnswer answer = nsxControllerUtils.sendNsxCommand(command, zone.getId()); + if (!answer.getResult()) { + String msg = String.format("Error creating DHCP relay config for network %s and nic %s: %s", network.getName(), nic.getName(), answer.getDetails()); + LOGGER.error(msg); + throw new CloudRuntimeException(msg); + } return nicProfile; } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java index b5c0c6f3ff4..773e44cf4c3 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxProviderServiceImpl.java @@ -17,6 +17,7 @@ package org.apache.cloudstack.service; import com.amazonaws.util.CollectionUtils; +import com.cloud.configuration.ConfigurationManager; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.exception.InvalidParameterValueException; @@ -42,6 +43,7 @@ import org.apache.cloudstack.api.command.ListNsxControllersCmd; import org.apache.cloudstack.api.BaseResponse; import org.apache.cloudstack.api.command.AddNsxControllerCmd; import org.apache.cloudstack.api.response.NsxControllerResponse; +import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.resource.NsxResource; import org.apache.commons.lang3.StringUtils; @@ -191,20 +193,21 @@ public class NsxProviderServiceImpl implements NsxProviderService { @Override public List<Class<?>> getCommands() { - List<Class<?>> cmdList = new ArrayList<Class<?>>(); - cmdList.add(AddNsxControllerCmd.class); - cmdList.add(ListNsxControllersCmd.class); - cmdList.add(DeleteNsxControllerCmd.class); + List<Class<?>> cmdList = new ArrayList<>(); + if (Boolean.TRUE.equals(NetworkOrchestrationService.NSX_ENABLED.value())) { + cmdList.add(AddNsxControllerCmd.class); + cmdList.add(ListNsxControllersCmd.class); + cmdList.add(DeleteNsxControllerCmd.class); + } return cmdList; } @VisibleForTesting void validateNetworkState(List<NetworkVO> networkList) { for (NetworkVO network : networkList) { - if (network.getBroadcastDomainType() == Networks.BroadcastDomainType.NSX) { - if ((network.getState() != Network.State.Shutdown) && (network.getState() != Network.State.Destroy)) { + if (network.getBroadcastDomainType() == Networks.BroadcastDomainType.NSX && + ((network.getState() != Network.State.Shutdown) && (network.getState() != Network.State.Destroy))) { throw new CloudRuntimeException("This NSX Controller cannot be deleted as there are one or more logical networks provisioned by CloudStack on it."); - } } } } diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java index 74926ffd380..2e8127526b3 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/service/NsxPublicNetworkGuru.java @@ -74,6 +74,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { super(); } + @Override protected boolean canHandle(NetworkOffering offering) { return isMyTrafficType(offering.getTrafficType()) && offering.isSystemOnly() && offering.isForNsx(); } @@ -85,19 +86,15 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { } if (offering.getTrafficType() == Networks.TrafficType.Public) { - NetworkVO ntwk = - new NetworkVO(offering.getTrafficType(), Networks.Mode.Static, network.getBroadcastDomainType(), offering.getId(), Network.State.Setup, plan.getDataCenterId(), + return new NetworkVO(offering.getTrafficType(), Networks.Mode.Static, network.getBroadcastDomainType(), offering.getId(), Network.State.Setup, plan.getDataCenterId(), plan.getPhysicalNetworkId(), offering.isRedundantRouter()); - return ntwk; - } else { - return null; } + return null; } @Override public NicProfile allocate(Network network, NicProfile nic, VirtualMachineProfile vm) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException { s_logger.debug("NSX Public network guru: allocate"); - // NicProfile createdProfile = super.allocate(network, nic, vm); IPAddressVO ipAddress = _ipAddressDao.findByIp(nic.getIPv4Address()); if (ipAddress == null) { @@ -107,10 +104,6 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { } Long vpcId = ipAddress.getVpcId(); boolean isForVpc = vpcId != null; - if (vpcId == null) { - // TODO: Pass network.getId() to support isolated networks - throw new CloudRuntimeException("TODO: Add support for isolated networks public network"); - } VpcVO vpc = vpcDao.findById(vpcId); if (vpc == null) { String err = String.format("Cannot find a VPC with ID %s", vpcId); @@ -119,7 +112,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { } // For NSX, use VR Public IP != Source NAT - List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpcId, true); + List<IPAddressVO> ips = _ipAddressDao.listByAssociatedVpc(vpc.getId(), true); if (CollectionUtils.isEmpty(ips)) { String err = String.format("Cannot find a source NAT IP for the VPC %s", vpc.getName()); s_logger.error(err); @@ -134,7 +127,7 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { long accountId = vpc.getAccountId(); long domainId = vpc.getDomainId(); long dataCenterId = vpc.getZoneId(); - long resourceId = isForVpc ? vpc.getId() : network.getId(); + long resourceId = vpc.getId(); Network.Service[] services = { Network.Service.SourceNat }; boolean sourceNatEnabled = vpcOfferingServiceMapDao.areServicesSupportedByVpcOffering(vpc.getVpcOfferingId(), services); @@ -147,13 +140,8 @@ public class NsxPublicNetworkGuru extends PublicNetworkGuru { } boolean hasNatSupport = false; - if (vpc == null) { - NetworkOffering offering = offeringDao.findById(network.getNetworkOfferingId()); - hasNatSupport = NetworkOffering.NsxMode.NATTED.name().equals(offering.getNsxMode()); - } else { - VpcOffering vpcOffering = vpcOfferingDao.findById(vpc.getVpcOfferingId()); - hasNatSupport = NetworkOffering.NsxMode.NATTED.name().equals(vpcOffering.getNsxMode()); - } + VpcOffering vpcOffering = vpcOfferingDao.findById(vpc.getVpcOfferingId()); + hasNatSupport = NetworkOffering.NsxMode.NATTED.name().equals(vpcOffering.getNsxMode()); if (!hasNatSupport) { return nic; diff --git a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java index 1d1dd033928..92e089e4849 100644 --- a/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/utils/NsxHelper.java @@ -31,6 +31,9 @@ import java.util.List; public class NsxHelper { + private NsxHelper() { + } + public static CreateNsxDhcpRelayConfigCommand createNsxDhcpRelayConfigCommand(DomainVO domain, Account account, DataCenter zone, VpcVO vpc, Network network, List<String> addresses) { Long vpcId = vpc != null ? vpc.getId() : null; String vpcName = vpc != null ? vpc.getName() : null; diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java index 603f5fd628d..a0fde08ade8 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxApiClientTest.java @@ -61,9 +61,9 @@ public class NsxApiClientTest { ) { String segmentName = "segment1"; client.createGroupForSegment(segmentName); - Mockito.verify(groupService).patch(Mockito.eq(NsxApiClient.DEFAULT_DOMAIN), Mockito.eq(segmentName), Mockito.eq(groups[0])); + Mockito.verify(groupService).patch(NsxApiClient.DEFAULT_DOMAIN, segmentName, groups[0]); String segmentPath = String.format("%s/%s", NsxApiClient.SEGMENTS_PATH, segmentName); - Mockito.verify(groups[0]).setExpression(Mockito.eq(List.of(pathExpressions[0]))); + Mockito.verify(groups[0]).setExpression(List.of(pathExpressions[0])); Mockito.verify(pathExpressions[0]).setPaths(List.of(segmentPath)); } } diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java index 6a18592885d..cb6f6511d24 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxProviderServiceImplTest.java @@ -125,10 +125,10 @@ public class NsxProviderServiceImplTest { NsxControllerResponse response = nsxProviderService.createNsxControllerResponse(nsxProvider); - assertEquals(response.getEdgeCluster(), "EdgeCluster"); - assertEquals(response.getTier0Gateway(), "Tier0Gw"); - assertEquals(response.getTransportZone(), "Overlay"); - assertEquals(response.getZoneName(), "ZoneNSX"); + assertEquals("EdgeCluster", response.getEdgeCluster()); + assertEquals("Tier0Gw", response.getTier0Gateway()); + assertEquals("Overlay", response.getTransportZone()); + assertEquals("ZoneNSX", response.getZoneName()); } @Test diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 37df5c2de1d..09e7e96bd88 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -4880,7 +4880,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati if (isSharedNetworkWithoutSpecifyVlan) { bypassVlanOverlapCheck = true; } - if (!bypassVlanOverlapCheck && !forNsx && _zoneDao.findVnet(zoneId, physicalNetworkId, BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId))).size() > 0) { + if (!bypassVlanOverlapCheck && !forNsx && !_zoneDao.findVnet(zoneId, physicalNetworkId, BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId))).isEmpty()) { throw new InvalidParameterValueException("The VLAN tag " + vlanId + " is already being used for dynamic vlan allocation for the guest network in zone " + zone.getName()); } @@ -6418,7 +6418,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati final Long serviceOfferingId, final boolean conserveMode, final Map<Service, Map<Capability, String>> serviceCapabilityMap, final boolean specifyIpRanges, final boolean isPersistent, final Map<Detail, String> details, final boolean egressDefaultPolicy, final Integer maxconn, final boolean enableKeepAlive, Boolean forVpc, - Boolean forTungsten, Boolean forNsx, String mode, final List<Long> domainIds, final List<Long> zoneIds, final boolean enableOffering, final NetUtils.InternetProtocol internetProtocol) { + Boolean forTungsten, boolean forNsx, String mode, final List<Long> domainIds, final List<Long> zoneIds, final boolean enableOffering, final NetUtils.InternetProtocol internetProtocol) { String servicePackageUuid; String spDescription = null; diff --git a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java index e0a7a4a18f4..55b98f82163 100644 --- a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java +++ b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java @@ -182,7 +182,7 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis throw new CloudRuntimeException(String.format("Failed to find domain with ID: %s", network.getDomainId())); } VpcVO vpc = null; - if (Objects.nonNull(network) && Objects.nonNull(network.getVpcId())) { + if (Objects.nonNull(network.getVpcId())) { vpc = vpcDao.findById(network.getVpcId()); } to.setNetworkSegmentName(getNetworkName(zone.getId(), domain.getId(), account.getId(), vpc, network.getId()));