From 4ce7f64ebdf2e9c09ee27e80e9d984dfee125012 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 2 Jan 2024 12:46:08 -0500 Subject: [PATCH] NSX: Fix code smells and reported bugs (#8409) * NSX: Fix code smells and reported bugs * fox override issue * remove unused imports * fix test * refactor code to reduce complexity * add lisence * cleanup * fix build failure * fix build failure * address comments * test - add config to ignore certain files from test coverage * test exclusion of classes from test cov * rever pom changes --- .../network/CreateNetworkOfferingCmd.java | 4 +- .../admin/vpc/CreateVPCOfferingCmd.java | 4 +- .../configuration/ConfigurationManager.java | 29 ++-- .../orchestration/NetworkOrchestrator.java | 8 +- .../vm/VirtualMachineManagerImplTest.java | 5 +- .../cloud/network/element/NsxProviderVO.java | 1 + plugins/network-elements/nsx/pom.xml | 15 -- .../api/CreateNsxDhcpRelayConfigCommand.java | 18 +++ ...ateNsxDistributedFirewallRulesCommand.java | 18 +++ .../api/CreateNsxLoadBalancerRuleCommand.java | 18 +++ .../api/CreateNsxPortForwardRuleCommand.java | 19 +++ .../CreateOrUpdateNsxTier1NatRuleCommand.java | 19 +++ .../api/DeleteNsxLoadBalancerRuleCommand.java | 18 +++ .../agent/api/DeleteNsxNatRuleCommand.java | 19 +++ .../agent/api/DeleteNsxSegmentCommand.java | 19 +++ .../api/DeleteNsxTier1GatewayCommand.java | 19 +++ .../cloudstack/resource/NsxNetworkRule.java | 1 + .../cloudstack/resource/NsxOpObject.java | 129 ++++++++++++++++++ .../cloudstack/resource/NsxResource.java | 14 +- .../cloudstack/service/NsxApiClient.java | 31 ++--- .../apache/cloudstack/service/NsxElement.java | 97 ++++++------- .../service/NsxGuestNetworkGuru.java | 105 +++++++------- .../service/NsxProviderServiceImpl.java | 16 ++- .../service/NsxPublicNetworkGuru.java | 26 +--- .../apache/cloudstack/utils/NsxHelper.java | 3 + .../cloudstack/service/NsxApiClientTest.java | 4 +- .../cloudstack/service/NsxElementTest.java | 47 ++++++- .../service/NsxGuestNetworkGuruTest.java | 1 - .../service/NsxProviderServiceImplTest.java | 8 +- .../service/NsxPublicNetworkGuruTest.java | 1 + .../ConfigurationManagerImpl.java | 4 +- .../cloud/hypervisor/HypervisorGuruBase.java | 2 +- .../vpc/MockConfigurationManagerImpl.java | 2 +- 33 files changed, 517 insertions(+), 207 deletions(-) create mode 100644 plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxOpObject.java 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> getServiceProviders() { - Map> serviceProviderMap = new HashMap>(); + Map> 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> getServiceProviders() { - Map> serviceProviderMap = new HashMap>();; + Map> serviceProviderMap = new HashMap<>(); if (serviceProviderList != null && !serviceProviderList.isEmpty() && !isForNsx()) { Collection> servicesCollection = serviceProviderList.values(); Iterator> 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 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 AllowNonRFC1918CompliantIPs = new ConfigKey(Boolean.class, + public static final ConfigKey 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> serviceProviderMap, boolean isDefault, Network.GuestType type, boolean systemOnly, Long serviceOfferingId, boolean conserveMode, Map> serviceCapabilityMap, boolean specifyIpRanges, boolean isPersistent, Map details, boolean egressDefaultPolicy, Integer maxconn, boolean enableKeepAlive, Boolean forVpc, - Boolean forTungsten, Boolean forNsx, String mode, List domainIds, List zoneIds, boolean enableOffering, final NetUtils.InternetProtocol internetProtocol); + Boolean forTungsten, boolean forNsx, String mode, List domainIds, List 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 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/pom.xml b/plugins/network-elements/nsx/pom.xml index aaefbe8438d..82dd1511039 100644 --- a/plugins/network-elements/nsx/pom.xml +++ b/plugins/network-elements/nsx/pom.xml @@ -29,21 +29,6 @@ 4.19.0.0-SNAPSHOT ../../pom.xml - - - - org.jacoco - jacoco-maven-plugin - ${cs.jacoco-plugin.version} - - - **/org/apache/cloudstack/agent/api/* - **/org/apache/cloudstack/api/response/* - - - - - com.vmware 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..6ef75b21e30 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,21 @@ public class CreateNsxDhcpRelayConfigCommand extends NsxCommand { public List getAddresses() { return addresses; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass() || !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..f598a201429 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,21 @@ public class CreateNsxDistributedFirewallRulesCommand extends NsxCommand { public List getRules() { return rules; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass() || !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..92acc83064d 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,21 @@ 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() || !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..d72295563c7 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,21 @@ 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() || !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..c14be743ea0 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,21 @@ 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() || !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..72aa61fdca3 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,21 @@ public class DeleteNsxLoadBalancerRuleCommand extends NsxNetworkCommand { } public List getMemberList() { return memberList; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass() || !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..c5231b19ac4 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,21 @@ 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() || !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..882b55388ff 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,21 @@ 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() || !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..d05acc18002 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,21 @@ 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() || !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/NsxOpObject.java b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxOpObject.java new file mode 100644 index 00000000000..bb411249b88 --- /dev/null +++ b/plugins/network-elements/nsx/src/main/java/org/apache/cloudstack/resource/NsxOpObject.java @@ -0,0 +1,129 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.resource; + +import com.cloud.network.dao.NetworkVO; +import com.cloud.network.vpc.VpcVO; + +import java.util.Objects; + +public class NsxOpObject { + VpcVO vpcVO; + NetworkVO networkVO; + long accountId; + long domainId; + long zoneId; + + public VpcVO getVpcVO() { + return vpcVO; + } + + public void setVpcVO(VpcVO vpcVO) { + this.vpcVO = vpcVO; + } + + public NetworkVO getNetworkVO() { + return networkVO; + } + + public void setNetworkVO(NetworkVO networkVO) { + this.networkVO = networkVO; + } + + public long getAccountId() { + return accountId; + } + + public void setAccountId(long accountId) { + this.accountId = accountId; + } + + public long getDomainId() { + return domainId; + } + + public void setDomainId(long domainId) { + this.domainId = domainId; + } + + public long getZoneId() { + return zoneId; + } + + public void setZoneId(long zoneId) { + this.zoneId = zoneId; + } + + public String getNetworkResourceName() { + return Objects.nonNull(vpcVO) ? vpcVO.getName() : networkVO.getName(); + } + + public boolean isVpcResource() { + return Objects.nonNull(vpcVO); + } + + public long getNetworkResourceId() { + return Objects.nonNull(vpcVO) ? vpcVO.getId() : networkVO.getId(); + } + + public static final class Builder { + VpcVO vpcVO; + NetworkVO networkVO; + long accountId; + long domainId; + long zoneId; + + public Builder() { + // Default constructor + } + + public Builder vpcVO(VpcVO vpcVO) { + this.vpcVO = vpcVO; + return this; + } + + public Builder networkVO(NetworkVO networkVO) { + this.networkVO = networkVO; + return this; + } + + public Builder domainId(long domainId) { + this.domainId = domainId; + return this; + } + + public Builder accountId(long accountId) { + this.accountId = accountId; + return this; + } + + public Builder zoneId(long zoneId) { + this.zoneId = zoneId; + return this; + } + + public NsxOpObject build() { + NsxOpObject object = new NsxOpObject(); + object.setVpcVO(this.vpcVO); + object.setNetworkVO(this.networkVO); + object.setDomainId(this.domainId); + object.setAccountId(this.accountId); + object.setZoneId(this.zoneId); + return object; + } + } +} 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 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..59776c4a3a1 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 @@ -81,7 +81,6 @@ import org.apache.log4j.Logger; import java.util.ArrayList; import java.util.List; import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.function.Function; @@ -105,7 +104,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 +114,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 +152,6 @@ public class NsxApiClient { JUMP_TO_APPLICATION } - private Map 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 +265,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 +322,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 +348,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 +357,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 +366,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 +525,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 +554,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 +577,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 +872,14 @@ public class NsxApiClient { List segmentGroup = List.of(String.format("%s/%s", GROUPS_PATH_PREFIX, segmentName)); List sourceCidrList = rule.getSourceCidrList(); List destCidrList = rule.getDestinationCidrList(); + List ingressSource = (rule.getService() == Network.Service.NetworkACL ? segmentGroup : destCidrList); + List 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..4bba3a717ed 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 @@ -83,7 +83,6 @@ import com.cloud.uservm.UserVm; import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.vm.Nic; import com.cloud.vm.NicProfile; import com.cloud.vm.ReservationContext; import com.cloud.vm.VMInstanceVO; @@ -93,6 +92,7 @@ import net.sf.ehcache.config.InvalidConfigurationException; import org.apache.cloudstack.StartupNsxCommand; import org.apache.cloudstack.resource.NsxLoadBalancerMember; import org.apache.cloudstack.resource.NsxNetworkRule; +import org.apache.cloudstack.resource.NsxOpObject; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -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 vpcOrNetwork = getVpcOrNetwork(config.getVpcId(), config.getId()); VpcVO vpc = vpcOrNetwork.first(); NetworkVO network = vpcOrNetwork.second(); @@ -516,26 +511,18 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns if (vm == null || networkModel.getNicInNetwork(vm.getId(), network.getId()) == null) { continue; } - Pair vpcOrNetwork = getVpcOrNetwork(network.getVpcId(), network.getId()); - VpcVO vpc = vpcOrNetwork.first(); - NetworkVO networkVO = vpcOrNetwork.second(); - Long networkResourceId = Objects.nonNull(vpc) ? vpc.getId() : networkVO.getId(); - String networkResourceName = Objects.nonNull(vpc) ? vpc.getName() : networkVO.getName(); - boolean isVpcResource = Objects.nonNull(vpc); - long domainId = Objects.nonNull(vpc) ? vpc.getDomainId() : networkVO.getDomainId(); - long accountId = Objects.nonNull(vpc) ? vpc.getAccountId() : networkVO.getAccountId(); - long zoneId = Objects.nonNull(vpc) ? vpc.getZoneId() : networkVO.getDataCenterId(); + NsxOpObject nsxObject = getNsxOpObject(network); String publicPort = getPublicPortRange(rule); String privatePort = getPrivatePFPortRange(rule); NsxNetworkRule networkRule = new NsxNetworkRule.Builder() - .setDomainId(domainId) - .setAccountId(accountId) - .setZoneId(zoneId) - .setNetworkResourceId(networkResourceId) - .setNetworkResourceName(networkResourceName) - .setVpcResource(isVpcResource) + .setDomainId(nsxObject.getDomainId()) + .setAccountId(nsxObject.getAccountId()) + .setZoneId(nsxObject.getZoneId()) + .setNetworkResourceId(nsxObject.getNetworkResourceId()) + .setNetworkResourceName(nsxObject.getNetworkResourceName()) + .setVpcResource(nsxObject.isVpcResource()) .setVmId(vm.getId()) .setVmIp(vm.getPrivateIpAddress()) .setPublicIp(publicIp.getAddress().addr()) @@ -545,13 +532,9 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns .setProtocol(rule.getProtocol().toUpperCase(Locale.ROOT)) .build(); if (rule.getState() == FirewallRule.State.Add) { - if (!nsxService.createPortForwardRule(networkRule)) { - return false; - } + return nsxService.createPortForwardRule(networkRule); } else if (rule.getState() == FirewallRule.State.Revoke) { - if (!nsxService.deletePortForwardRule(networkRule)) { - return false; - } + return nsxService.deletePortForwardRule(networkRule); } } return true; @@ -598,6 +581,36 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns String.valueOf(rule.getSourcePortStart()).concat("-").concat(String.valueOf(rule.getSourcePortEnd())); } + private long getResourceId(String resource, VpcVO vpc, NetworkVO network) { + switch (resource) { + case "domain": + return Objects.nonNull(vpc) ? vpc.getDomainId() : network.getDomainId(); + case "account": + return Objects.nonNull(vpc) ? vpc.getAccountId() : network.getAccountId(); + case "zone": + return Objects.nonNull(vpc) ? vpc.getZoneId() : network.getDataCenterId(); + default: + return 0; + } + } + + private NsxOpObject getNsxOpObject(Network network) { + Pair vpcOrNetwork = getVpcOrNetwork(network.getVpcId(), network.getId()); + VpcVO vpc = vpcOrNetwork.first(); + NetworkVO networkVO = vpcOrNetwork.second(); + long domainId = getResourceId("domain", vpc, networkVO); + long accountId = getResourceId("account", vpc, networkVO); + long zoneId = getResourceId("zone", vpc, networkVO); + + return new NsxOpObject.Builder() + .vpcVO(vpc) + .networkVO(networkVO) + .domainId(domainId) + .accountId(accountId) + .zoneId(zoneId) + .build(); + } + @Override public boolean applyLBRules(Network network, List rules) throws ResourceUnavailableException { for (LoadBalancingRule loadBalancingRule : rules) { @@ -606,24 +619,16 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns } IPAddressVO publicIp = ipAddressDao.findByIpAndDcId(network.getDataCenterId(), loadBalancingRule.getSourceIp().addr()); + NsxOpObject nsxObject = getNsxOpObject(network); - Pair vpcOrNetwork = getVpcOrNetwork(network.getVpcId(), network.getId()); - VpcVO vpc = vpcOrNetwork.first(); - NetworkVO networkVO = vpcOrNetwork.second(); - Long networkResourceId = Objects.nonNull(vpc) ? vpc.getId() : networkVO.getId(); - String networkResourceName = Objects.nonNull(vpc) ? vpc.getName() : networkVO.getName(); - boolean isVpcResource = Objects.nonNull(vpc); - long domainId = Objects.nonNull(vpc) ? vpc.getDomainId() : networkVO.getDomainId(); - long accountId = Objects.nonNull(vpc) ? vpc.getAccountId() : networkVO.getAccountId(); - long zoneId = Objects.nonNull(vpc) ? vpc.getZoneId() : networkVO.getDataCenterId(); List lbMembers = getLoadBalancerMembers(loadBalancingRule); NsxNetworkRule networkRule = new NsxNetworkRule.Builder() - .setDomainId(domainId) - .setAccountId(accountId) - .setZoneId(zoneId) - .setNetworkResourceId(networkResourceId) - .setNetworkResourceName(networkResourceName) - .setVpcResource(isVpcResource) + .setDomainId(nsxObject.getDomainId()) + .setAccountId(nsxObject.getAccountId()) + .setZoneId(nsxObject.getZoneId()) + .setNetworkResourceId(nsxObject.getNetworkResourceId()) + .setNetworkResourceName(nsxObject.getNetworkResourceName()) + .setVpcResource(nsxObject.isVpcResource()) .setMemberList(lbMembers) .setPublicIp(publicIp.getAddress().addr()) .setPublicPort(String.valueOf(loadBalancingRule.getSourcePortStart())) @@ -633,13 +638,9 @@ public class NsxElement extends AdapterBase implements DhcpServiceProvider, Dns .setAlgorithm(loadBalancingRule.getAlgorithm()) .build(); if (loadBalancingRule.getState() == FirewallRule.State.Add) { - if (!nsxService.createLbRule(networkRule)) { - return false; - } + return nsxService.createLbRule(networkRule); } else if (loadBalancingRule.getState() == FirewallRule.State.Revoke) { - if (!nsxService.deleteLbRule(networkRule)) { - return false; - } + return nsxService.deleteLbRule(networkRule); } } 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,63 +197,64 @@ 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); + 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); + } + + 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); } - 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); - } - - 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 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); - } + // Create the DHCP relay config for the segment + String iPv4Address = nicProfile.getIPv4Address(); + List 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..c59ebfd8755 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 @@ -42,6 +42,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 +192,21 @@ public class NsxProviderServiceImpl implements NsxProviderService { @Override public List> getCommands() { - List> cmdList = new ArrayList>(); - cmdList.add(AddNsxControllerCmd.class); - cmdList.add(ListNsxControllersCmd.class); - cmdList.add(DeleteNsxControllerCmd.class); + List> 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 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 ips = _ipAddressDao.listByAssociatedVpc(vpcId, true); + List 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 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/NsxElementTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java index 252b189a58e..ae3143e32b8 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxElementTest.java @@ -249,13 +249,29 @@ public class NsxElementTest { } @Test - public void testApplyPFRules() throws ResourceUnavailableException { + public void testApplyPFRules_add() throws ResourceUnavailableException { NetworkVO networkVO = new NetworkVO(1L, Networks.TrafficType.Public, Networks.Mode.Static, Networks.BroadcastDomainType.NSX, 12L, 2L, 5L, 1L, "network1", "network1", null, Network.GuestType.Isolated, 2L, 2L, ControlledEntity.ACLType.Domain, false, 1L, false ); - PortForwardingRule rule = new PortForwardingRuleVO("1", 11L, 80, 90, new Ip("172.30.10.11"), 8080, 8090, "tcp", 12L, + PortForwardingRuleVO rule = new PortForwardingRuleVO("1", 11L, 80, 90, new Ip("172.30.10.11"), 8080, 8090, "tcp", 12L, 5L, 2L, 15L); + rule.setState(FirewallRule.State.Add); + Network.Service service = new Network.Service("service1", new Network.Capability("capability")); + + when(nsxElement.canHandle(networkVO, service)).thenReturn(true); + assertTrue(nsxElement.applyPFRules(networkVO, List.of(rule))); + } + + @Test + public void testApplyPFRules_delete() throws ResourceUnavailableException { + NetworkVO networkVO = new NetworkVO(1L, Networks.TrafficType.Public, Networks.Mode.Static, + Networks.BroadcastDomainType.NSX, 12L, 2L, 5L, 1L, "network1", + "network1", null, Network.GuestType.Isolated, 2L, 2L, + ControlledEntity.ACLType.Domain, false, 1L, false ); + PortForwardingRuleVO rule = new PortForwardingRuleVO("1", 11L, 80, 90, new Ip("172.30.10.11"), 8080, 8090, "tcp", 12L, + 5L, 2L, 15L); + rule.setState(FirewallRule.State.Revoke); Network.Service service = new Network.Service("service1", new Network.Capability("capability")); when(nsxElement.canHandle(networkVO, service)).thenReturn(true); @@ -363,12 +379,13 @@ public class NsxElementTest { } @Test - public void testApplyLBRules() throws ResourceUnavailableException { + public void testApplyLBRules_add() throws ResourceUnavailableException { NetworkVO networkVO = new NetworkVO(1L, Networks.TrafficType.Public, Networks.Mode.Static, Networks.BroadcastDomainType.NSX, 12L, 2L, 5L, 1L, "network1", "network1", null, Network.GuestType.Isolated, 2L, 2L, ControlledEntity.ACLType.Domain, false, 1L, false ); LoadBalancerVO lb = new LoadBalancerVO(null, null, null, 0L, 8080, 8081, null, 0L, 0L, 1L, null, null); + lb.setState(FirewallRule.State.Add); LoadBalancingRule.LbDestination destination = new LoadBalancingRule.LbDestination(6443, 6443, "172.30.110.11", false); LoadBalancingRule rule = new LoadBalancingRule(lb, List.of(destination), null, null, new Ip("10.1.13.10")); @@ -379,6 +396,30 @@ public class NsxElementTest { when(vpc.getDomainId()).thenReturn(2L); when(vpc.getAccountId()).thenReturn(5L); when(ipAddressDao.findByIpAndDcId(anyLong(), anyString())).thenReturn(ipAddress); + when(nsxService.createLbRule(any(NsxNetworkRule.class))).thenReturn(true); + + assertTrue(nsxElement.applyLBRules(networkVO, List.of(rule))); + } + + @Test + public void testApplyLBRules_delete() throws ResourceUnavailableException { + NetworkVO networkVO = new NetworkVO(1L, Networks.TrafficType.Public, Networks.Mode.Static, + Networks.BroadcastDomainType.NSX, 12L, 2L, 5L, 1L, "network1", + "network1", null, Network.GuestType.Isolated, 2L, 2L, + ControlledEntity.ACLType.Domain, false, 1L, false ); + LoadBalancerVO lb = new LoadBalancerVO(null, null, null, 0L, 8080, 8081, null, 0L, 0L, 1L, null, null); + lb.setState(FirewallRule.State.Revoke); + LoadBalancingRule.LbDestination destination = new LoadBalancingRule.LbDestination(6443, 6443, "172.30.110.11", false); + LoadBalancingRule rule = new LoadBalancingRule(lb, List.of(destination), null, null, new Ip("10.1.13.10")); + + VpcVO vpc = Mockito.mock(VpcVO.class); + + IPAddressVO ipAddress = new IPAddressVO(new Ip("10.1.13.10"), 1L, 1L, 1L,false); + when(vpcDao.findById(anyLong())).thenReturn(vpc); + when(vpc.getDomainId()).thenReturn(2L); + when(vpc.getAccountId()).thenReturn(5L); + when(ipAddressDao.findByIpAndDcId(anyLong(), anyString())).thenReturn(ipAddress); + when(nsxService.deleteLbRule(any(NsxNetworkRule.class))).thenReturn(true); assertTrue(nsxElement.applyLBRules(networkVO, List.of(rule))); } diff --git a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java index 8343713aaa8..e6efc8f0503 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxGuestNetworkGuruTest.java @@ -121,7 +121,6 @@ public class NsxGuestNetworkGuruTest { NetworkOfferingDao networkOfferingDao; NsxGuestNetworkGuru guru; - GuestNetworkGuru guestNetworkGuru; AutoCloseable closeable; @Before 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/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxPublicNetworkGuruTest.java b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxPublicNetworkGuruTest.java index 03809bd283c..da21bf11b64 100644 --- a/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxPublicNetworkGuruTest.java +++ b/plugins/network-elements/nsx/src/test/java/org/apache/cloudstack/service/NsxPublicNetworkGuruTest.java @@ -160,6 +160,7 @@ public class NsxPublicNetworkGuruTest { when(ipAddressDao.listByAssociatedVpc(12L, true)).thenReturn(sourceNatList); when(vlanDetailsDao.findDetail(anyLong(), anyString())).thenReturn(vlanDetailVO); when(vpcVO.getVpcOfferingId()).thenReturn(12L); + when(vpcVO.getId()).thenReturn(12L); when(vpcVO.getName()).thenReturn("nsxVPCNet"); when(vpcOfferingServiceMapDao.areServicesSupportedByVpcOffering(anyLong(), any())).thenReturn(true); when(nsxService.createVpcNetwork(anyLong(), anyLong(), anyLong(), anyLong(), anyString(), anyBoolean())).thenReturn(true); 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> serviceCapabilityMap, final boolean specifyIpRanges, final boolean isPersistent, final Map details, final boolean egressDefaultPolicy, final Integer maxconn, final boolean enableKeepAlive, Boolean forVpc, - Boolean forTungsten, Boolean forNsx, String mode, final List domainIds, final List zoneIds, final boolean enableOffering, final NetUtils.InternetProtocol internetProtocol) { + Boolean forTungsten, boolean forNsx, String mode, final List domainIds, final List 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())); diff --git a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java index 1ad7223d6c3..f9f66550657 100644 --- a/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockConfigurationManagerImpl.java @@ -546,7 +546,7 @@ public class MockConfigurationManagerImpl extends ManagerBase implements Configu Integer networkRate, Map> serviceProviderMap, boolean isDefault, GuestType type, boolean systemOnly, Long serviceOfferingId, boolean conserveMode, Map> serviceCapabilityMap, boolean specifyIpRanges, boolean isPersistent, Map details, boolean egressDefaultPolicy, Integer maxconn, boolean enableKeepAlive, Boolean forVpc, - Boolean forTungsten, Boolean forNsx, String mode, List domainIds, List zoneIds, boolean enableOffering, NetUtils.InternetProtocol internetProtocol) { + Boolean forTungsten, boolean forNsx, String mode, List domainIds, List zoneIds, boolean enableOffering, NetUtils.InternetProtocol internetProtocol) { // TODO Auto-generated method stub return null; }