diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 1b19ffee2b5a..d0b6dbb19d9a 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -611,11 +611,18 @@ private boolean isValidSystemVMType(VirtualMachine vm) { VirtualMachine.Type.ConsoleProxy.equals(vm.getType()); } - protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException { + private boolean isVmDestroyed(VMInstanceVO vm) { if (vm == null || vm.getRemoved() != null) { if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to find vm or vm is destroyed: " + vm); } + return true; + } + return false; + } + + protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException { + if (isVmDestroyed(vm)) { return; } @@ -642,7 +649,7 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti final HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vm.getHypervisorType()); List vmNics = profile.getNics(); - s_logger.debug(String.format("Cleaning up NICS [%s] of %s.", vmNics.stream().map(nic -> nic.toString()).collect(Collectors.joining(", ")),vm.toString())); + s_logger.debug(String.format("Cleaning up NICS [%s] of %s.", vmNics.stream().map(NicProfile::toString).collect(Collectors.joining(", ")),vm.toString())); final List nicExpungeCommands = hvGuru.finalizeExpungeNics(vm, profile.getNics()); _networkMgr.cleanupNics(profile); @@ -686,28 +693,31 @@ protected void advanceExpunge(VMInstanceVO vm) throws ResourceUnavailableExcepti // send hypervisor-dependent commands before removing final List finalizeExpungeCommands = hvGuru.finalizeExpunge(vm); - if (CollectionUtils.isNotEmpty(finalizeExpungeCommands) || CollectionUtils.isNotEmpty(nicExpungeCommands)) { - if (hostId != null) { - final Commands cmds = new Commands(Command.OnError.Stop); - addAllExpungeCommandsFromList(finalizeExpungeCommands, cmds, vm); - addAllExpungeCommandsFromList(nicExpungeCommands, cmds, vm); - _agentMgr.send(hostId, cmds); - if (!cmds.isSuccessful()) { - for (final Answer answer : cmds.getAnswers()) { - if (!answer.getResult()) { - s_logger.warn("Failed to expunge vm due to: " + answer.getDetails()); - throw new CloudRuntimeException("Unable to expunge " + vm + " due to " + answer.getDetails()); - } - } - } - } - } + handleUnsuccessfulExpungeOperation(finalizeExpungeCommands, nicExpungeCommands, vm, hostId); if (s_logger.isDebugEnabled()) { s_logger.debug("Expunged " + vm); } } + private void handleUnsuccessfulExpungeOperation(List finalizeExpungeCommands, List nicExpungeCommands, + VMInstanceVO vm, Long hostId) throws OperationTimedoutException, AgentUnavailableException { + if (CollectionUtils.isNotEmpty(finalizeExpungeCommands) || CollectionUtils.isNotEmpty(nicExpungeCommands) && (hostId != null)) { + final Commands cmds = new Commands(Command.OnError.Stop); + addAllExpungeCommandsFromList(finalizeExpungeCommands, cmds, vm); + addAllExpungeCommandsFromList(nicExpungeCommands, cmds, vm); + _agentMgr.send(hostId, cmds); + if (!cmds.isSuccessful()) { + for (final Answer answer : cmds.getAnswers()) { + if (!answer.getResult()) { + s_logger.warn("Failed to expunge vm due to: " + answer.getDetails()); + throw new CloudRuntimeException(String.format("Unable to expunge %s due to %s", vm, answer.getDetails())); + } + } + } + } + } + protected void handleUnsuccessfulCommands(Commands cmds, VMInstanceVO vm) throws CloudRuntimeException { String cmdsStr = cmds.toString(); String vmToString = vm.toString(); 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 b9873470a530..05fc2e27ae4d 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 @@ -258,6 +258,7 @@ import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.googlecode.ipv6.IPv6Address; +import org.jetbrains.annotations.NotNull; /** * NetworkManagerImpl implements NetworkManager. @@ -749,20 +750,8 @@ public List setupNetwork(final Account owner, final NetworkOf .getBroadcastDomainType() == BroadcastDomainType.Vlan || predefined.getBroadcastDomainType() == BroadcastDomainType.Lswitch || predefined .getBroadcastDomainType() == BroadcastDomainType.Vxlan)) { final List configs = _networksDao.listBy(owner.getId(), offering.getId(), plan.getDataCenterId()); - if (configs.size() > 0) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Found existing network configuration for offering " + offering + ": " + configs.get(0)); - } - - if (errorIfAlreadySetup) { - final InvalidParameterValueException ex = new InvalidParameterValueException( - "Found existing network configuration (with specified id) for offering (with specified id)"); - ex.addProxyObject(offering.getUuid(), "offeringId"); - ex.addProxyObject(configs.get(0).getUuid(), "networkConfigId"); - throw ex; - } else { - return configs; - } + if (!configs.isEmpty()) { + return existingConfiguration(offering, configs, errorIfAlreadySetup); } } @@ -794,11 +783,8 @@ public List setupNetwork(final Account owner, final NetworkOf Transaction.execute(new TransactionCallbackNoReturn() { @Override public void doInTransactionWithoutResult(final TransactionStatus status) { - final NetworkVO vo = new NetworkVO(id, network, offering.getId(), guru.getName(), owner.getDomainId(), owner.getId(), relatedFile, name, displayText, predefined - .getNetworkDomain(), offering.getGuestType(), plan.getDataCenterId(), plan.getPhysicalNetworkId(), aclType, offering.isSpecifyIpRanges(), - vpcId, offering.isRedundantRouter(), predefined.getExternalId()); - vo.setDisplayNetwork(isDisplayNetworkEnabled == null ? true : isDisplayNetworkEnabled); - vo.setStrechedL2Network(offering.isSupportingStrechedL2()); + final NetworkVO vo = getNetworkVO(id, offering, plan, predefined, + network, guru, owner, name, displayText,relatedFile, aclType,vpcId, isDisplayNetworkEnabled); final NetworkVO networkPersisted = _networksDao.persist(vo, vo.getGuestType() == Network.GuestType.Isolated, finalizeServicesAndProvidersForNetwork(offering, plan.getPhysicalNetworkId())); networks.add(networkPersisted); @@ -815,14 +801,14 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { } if (domainId != null && aclType == ACLType.Domain) { - _networksDao.addDomainToNetwork(id, domainId, subdomainAccess == null ? true : subdomainAccess); + _networksDao.addDomainToNetwork(id, domainId, subdomainAccess == null || subdomainAccess); } } }); guru.setup(network, relatedFile); } - if (networks.size() < 1) { + if (networks.isEmpty()) { // see networkOfferingVO.java final CloudRuntimeException ex = new CloudRuntimeException("Unable to convert network offering with specified id to network profile"); ex.addProxyObject(offering.getUuid(), "offeringId"); @@ -836,6 +822,37 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { } } + @NotNull + private static NetworkVO getNetworkVO(long id, final NetworkOffering offering, final DeploymentPlan plan, final Network predefined, + Network network, final NetworkGuru guru, final Account owner, + final String name, final String displayText, long relatedFile, final ACLType aclType, + final Long vpcId, final Boolean isDisplayNetworkEnabled) { + final NetworkVO vo = new NetworkVO(id, network, offering.getId(), guru.getName(), owner.getDomainId(), owner.getId(), + relatedFile, name, displayText, predefined.getNetworkDomain(), offering.getGuestType(), + plan.getDataCenterId(), plan.getPhysicalNetworkId(), aclType, offering.isSpecifyIpRanges(), + vpcId, offering.isRedundantRouter(), predefined.getExternalId()); + vo.setDisplayNetwork(isDisplayNetworkEnabled == null || isDisplayNetworkEnabled); + vo.setStrechedL2Network(offering.isSupportingStrechedL2()); + return vo; + } + + private List existingConfiguration(final NetworkOffering offering, List configs, + final boolean errorIfAlreadySetup) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Found existing network configuration for offering " + offering + ": " + configs.get(0)); + } + + if (errorIfAlreadySetup) { + final InvalidParameterValueException ex = new InvalidParameterValueException( + "Found existing network configuration (with specified id) for offering (with specified id)"); + ex.addProxyObject(offering.getUuid(), "offeringId"); + ex.addProxyObject(configs.get(0).getUuid(), "networkConfigId"); + throw ex; + } else { + return configs; + } + } + @Override @DB public void allocate(final VirtualMachineProfile vm, final LinkedHashMap> networks, final Map> extraDhcpOptions) throws InsufficientCapacityException, @@ -3929,9 +3946,8 @@ private boolean cleanupNetworkResources(final long networkId, final Account call } //revoke all network ACLs for network - // TODO: Change this when ACLs are supported for NSX try { - if (networkOffering.isForNsx() || _networkACLMgr.revokeACLItemsForNetwork(networkId)) { + if (_networkACLMgr.revokeACLItemsForNetwork(networkId)) { s_logger.debug("Successfully cleaned up NetworkACLs for network id=" + networkId); } else { success = false; 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 a9c13afceea8..4356c97d754a 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 @@ -668,10 +668,10 @@ public String getNsxInfraServices(String ruleName, String port, String protocol, List services = serviceList.getResults(); List matchedDefaultSvc = services.parallelStream().filter(svc -> - (svc.getServiceEntries().get(0) instanceof L4PortSetServiceEntry) && - ((L4PortSetServiceEntry) svc.getServiceEntries().get(0)).getDestinationPorts().get(0).equals(port) - && (((L4PortSetServiceEntry) svc.getServiceEntries().get(0)).getL4Protocol().equals(protocol))) - .map(svc -> ((L4PortSetServiceEntry) svc.getServiceEntries().get(0)).getDestinationPorts().get(0)) + (svc.getServiceEntries().get(0)._getDataValue().getField("resource_type").toString().equals("L4PortSetServiceEntry")) && + svc.getServiceEntries().get(0)._getDataValue().getField("destination_ports").toString().equals("["+port+"]") + && svc.getServiceEntries().get(0)._getDataValue().getField("l4_protocol").toString().equals(protocol)) + .map(svc -> svc.getServiceEntries().get(0)._getDataValue().getField("parent_path").toString()) .collect(Collectors.toList()); if (!CollectionUtils.isEmpty(matchedDefaultSvc)) { return matchedDefaultSvc.get(0); diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 4b507c5273df..17277b2f7dd8 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -397,7 +397,7 @@ public void doInTransactionWithoutResult(final TransactionStatus status) { // configure default vpc offering with NSX as network service provider in Route mode if (_vpcOffDao.findByUniqueName(VpcOffering.DEFAULT_VPC_ROUTE_NSX_OFFERING_NAME) == null) { s_logger.debug("Creating default VPC offering with NSX as network service provider" + VpcOffering.DEFAULT_VPC_ROUTE_NSX_OFFERING_NAME); - final Map> svcProviderMap = new HashMap>(); + final Map> svcProviderMap = new HashMap<>(); final Set defaultProviders = Set.of(Provider.Nsx); for (final Service svc : getSupportedServices()) { if (List.of(Service.UserData, Service.Dhcp, Service.Dns).contains(svc)) { @@ -471,22 +471,8 @@ public VpcOffering createVpcOffering(CreateVPCOfferingCmd cmd) { final Boolean forNsx = cmd.isForNsx(); String nsxMode = cmd.getNsxMode(); final boolean enable = cmd.getEnable(); + nsxMode = validateNsxMode(forNsx, nsxMode); - if (Boolean.TRUE.equals(forNsx)) { - if (Objects.isNull(nsxMode)) { - throw new InvalidParameterValueException("Mode for an NSX offering needs to be specified.Valid values: " + Arrays.toString(NetworkOffering.NsxMode.values())); - } - if (!EnumUtils.isValidEnum(NetworkOffering.NsxMode.class, nsxMode)) { - throw new InvalidParameterValueException("Invalid mode passed. Valid values: " + Arrays.toString(NetworkOffering.NsxMode.values())); - } - } else { - if (Objects.nonNull(nsxMode)) { - if (s_logger.isTraceEnabled()) { - s_logger.trace("nsxMode has is ignored for non-NSX enabled zones"); - } - nsxMode = null; - } - } // check if valid domain if (CollectionUtils.isNotEmpty(cmd.getDomainIds())) { for (final Long domainId: cmd.getDomainIds()) { @@ -513,6 +499,25 @@ public VpcOffering createVpcOffering(CreateVPCOfferingCmd cmd) { domainIds, zoneIds, (enable ? State.Enabled : State.Disabled)); } + private String validateNsxMode(Boolean forNsx, String nsxMode) { + if (Boolean.TRUE.equals(forNsx)) { + if (Objects.isNull(nsxMode)) { + throw new InvalidParameterValueException("Mode for an NSX offering needs to be specified.Valid values: " + Arrays.toString(NetworkOffering.NsxMode.values())); + } + if (!EnumUtils.isValidEnum(NetworkOffering.NsxMode.class, nsxMode)) { + throw new InvalidParameterValueException("Invalid mode passed. Valid values: " + Arrays.toString(NetworkOffering.NsxMode.values())); + } + } else { + if (Objects.nonNull(nsxMode)) { + if (s_logger.isTraceEnabled()) { + s_logger.trace("nsxMode has is ignored for non-NSX enabled zones"); + } + nsxMode = null; + } + } + return nsxMode; + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VPC_OFFERING_CREATE, eventDescription = "creating vpc offering", create = true) public VpcOffering createVpcOffering(final String name, final String displayText, final List supportedServices, final Map> serviceProviders, @@ -1198,7 +1203,7 @@ private void allocateSourceNatIp(Vpc vpc, String sourceNatIP) { try { if (isVpcForNsx(vpc) && org.apache.commons.lang3.StringUtils.isBlank(sourceNatIP)) { s_logger.debug(String.format("Reserving a source NAT IP for NSX VPC %s", vpc.getName())); - sourceNatIP = reserveSourceNatIpForNsxVpc(account, zone, vpc); + sourceNatIP = reserveSourceNatIpForNsxVpc(account, zone); } IpAddress ip = _ipAddrMgr.allocateIp(account, false, CallContext.current().getCallingAccount(), CallContext.current().getCallingUserId(), zone, null, sourceNatIP); this.associateIPToVpc(ip.getId(), vpc.getId()); @@ -1207,7 +1212,7 @@ private void allocateSourceNatIp(Vpc vpc, String sourceNatIP) { } } - private String reserveSourceNatIpForNsxVpc(Account account, DataCenter zone, Vpc vpc) throws ResourceAllocationException { + private String reserveSourceNatIpForNsxVpc(Account account, DataCenter zone) throws ResourceAllocationException { IpAddress ipAddress = _ntwkSvc.reserveIpAddressWithVlanDetail(account, zone, true, ApiConstants.NSX_DETAIL_KEY); return ipAddress.getAddress().addr(); } diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 95bad927c63b..966b593542f9 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -837,6 +837,7 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe public static final Logger s_logger = Logger.getLogger(ManagementServerImpl.class.getName()); protected StateMachine2 _stateMachine; + static final String FOR_SYSTEMVMS = "forsystemvms"; static final ConfigKey vmPasswordLength = new ConfigKey("Advanced", Integer.class, "vm.password.length", "6", "Specifies the length of a randomly generated password", false); static final ConfigKey sshKeyLength = new ConfigKey("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global); static final ConfigKey humanReadableSizes = new ConfigKey("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global); @@ -2562,7 +2563,7 @@ private void buildParameters(final SearchBuilder sb, final ListPubl sb.and("vpcId", sb.entity().getVpcId(), SearchCriteria.Op.EQ); sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); sb.and("display", sb.entity().isDisplay(), SearchCriteria.Op.EQ); - sb.and("forsystemvms", sb.entity().isForSystemVms(), SearchCriteria.Op.EQ); + sb.and(FOR_SYSTEMVMS, sb.entity().isForSystemVms(), SearchCriteria.Op.EQ); if (forLoadBalancing != null && forLoadBalancing) { final SearchBuilder lbSearch = _loadbalancerDao.createSearchBuilder(); @@ -2670,9 +2671,9 @@ protected void setParameters(SearchCriteria sc, final ListPublicIpA } if (IpAddressManagerImpl.getSystemvmpublicipreservationmodestrictness().value() && IpAddress.State.Free.name().equalsIgnoreCase(state)) { - sc.setParameters("forsystemvms", false); + sc.setParameters(FOR_SYSTEMVMS, false); } else { - sc.setParameters("forsystemvms", forSystemVms); + sc.setParameters(FOR_SYSTEMVMS, forSystemVms); } } diff --git a/ui/src/views/network/VpcTiersTab.vue b/ui/src/views/network/VpcTiersTab.vue index bda759d8c82a..8bb84392bd3d 100644 --- a/ui/src/views/network/VpcTiersTab.vue +++ b/ui/src/views/network/VpcTiersTab.vue @@ -475,6 +475,7 @@ export default { fetchData () { this.networks = this.resource.network this.fetchMtuForZone() + this.getVpcNetworkOffering() if (!this.networks || this.networks.length === 0) { return } @@ -483,7 +484,6 @@ export default { this.fetchVMs(network.id) } this.publicLBNetworkExists() - this.getVpcNetworkOffering() }, fetchMtuForZone () { api('listZones', { @@ -524,9 +524,9 @@ export default { api('listVPCOfferings', { id: this.resource.vpcofferingid }).then(json => { - var vpcOffering = json?.listvpcofferingsresponse?.vpcoffering[0] - this.isOfferingNatMode = vpcOffering?.nsxmode === 'NATTED' || false + const vpcOffering = json?.listvpcofferingsresponse?.vpcoffering[0] resolve(vpcOffering) + this.isOfferingNatMode = vpcOffering?.nsxmode === 'NATTED' || false }).catch(e => { reject(e) })