From 24f9dcfb6798e1c7eedc40355dea5a40963f1eb2 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 30 Dec 2020 13:57:38 +0530 Subject: [PATCH 01/13] Support for persistence mode in L2 networks --- .../user/network/CreateNetworkCmd.java | 15 +++- .../com/cloud/agent/api/MigrateCommand.java | 9 ++ .../api/SetupPersistentNetworkAnswer.java | 26 ++++++ .../api/SetupPersistentNetworkCommand.java | 41 +++++++++ .../java/com/cloud/agent/api/StopCommand.java | 9 ++ .../cloud/vm/VirtualMachineManagerImpl.java | 58 +++++++++++++ .../orchestration/NetworkOrchestrator.java | 85 ++++++++++++++++++- .../kvm/resource/BridgeVifDriver.java | 9 +- .../kvm/resource/DirectVifDriver.java | 2 +- .../hypervisor/kvm/resource/IvsVifDriver.java | 2 +- .../resource/LibvirtComputingResource.java | 4 +- .../hypervisor/kvm/resource/OvsVifDriver.java | 2 +- .../hypervisor/kvm/resource/VifDriver.java | 2 +- .../kvm/resource/VifDriverBase.java | 2 +- .../wrapper/LibvirtMigrateCommandWrapper.java | 18 +++- .../LibvirtReplugNicCommandWrapper.java | 2 +- ...tSetupPersistentNetworkCommandWrapper.java | 50 +++++++++++ .../wrapper/LibvirtStopCommandWrapper.java | 22 ++++- .../LibvirtUnPlugNicCommandWrapper.java | 2 +- .../LibvirtComputingResourceTest.java | 2 +- .../vmware/resource/VmwareResource.java | 19 +++++ ...xSetupPersistentNetworkCommandWrapper.java | 54 ++++++++++++ scripts/vm/network/vnet/modifyvlan.sh | 47 +++++----- .../com/cloud/network/NetworkServiceImpl.java | 4 - .../cloud/network/guru/GuestNetworkGuru.java | 4 +- 25 files changed, 442 insertions(+), 48 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupPersistentNetworkCommandWrapper.java create mode 100644 plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupPersistentNetworkCommandWrapper.java diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java index 5dff7c9bf2bb..87e092917b5f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java @@ -16,13 +16,13 @@ // under the License. package org.apache.cloudstack.api.command.user.network; +import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; -import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; @@ -37,6 +37,7 @@ import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; +import com.cloud.event.EventTypes; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; @@ -48,7 +49,7 @@ @APICommand(name = "createNetwork", description = "Creates a network", responseObject = NetworkResponse.class, responseView = ResponseView.Restricted, entityType = {Network.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class CreateNetworkCmd extends BaseCmd implements UserCmd { +public class CreateNetworkCmd extends BaseAsyncCmd implements UserCmd { public static final Logger s_logger = Logger.getLogger(CreateNetworkCmd.class.getName()); private static final String s_name = "createnetworkresponse"; @@ -326,4 +327,14 @@ void execute() throws InsufficientCapacityException, ConcurrentOperationExceptio throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create network"); } } + + @Override + public String getEventType() { + return EventTypes.EVENT_NETWORK_CREATE; + } + + @Override + public String getEventDescription() { + return "Creating network"; + } } diff --git a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java index c7aeb69cff00..48328b1eba4e 100644 --- a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java +++ b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java @@ -40,6 +40,7 @@ public class MigrateCommand extends Command { private boolean executeInSequence = false; private List migrateDiskInfoList = new ArrayList<>(); private Map dpdkInterfaceMapping = new HashMap<>(); + Map vlanToPersistenceMap; public Map getDpdkInterfaceMapping() { return dpdkInterfaceMapping; @@ -49,6 +50,14 @@ public void setDpdkInterfaceMapping(Map dpdkInterfaceMapping) { this.dpdkInterfaceMapping = dpdkInterfaceMapping; } + public Map getVlanToPersistenceMap() { + return vlanToPersistenceMap; + } + + public void setVlanToPersistenceMap(Map vlanToPersistenceMap) { + this.vlanToPersistenceMap = vlanToPersistenceMap; + } + protected MigrateCommand() { } diff --git a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java new file mode 100644 index 000000000000..07867c7f93d4 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java @@ -0,0 +1,26 @@ +// 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 com.cloud.agent.api; + +public class SetupPersistentNetworkAnswer extends Answer{ + public SetupPersistentNetworkAnswer(){} + + public SetupPersistentNetworkAnswer(SetupPersistentNetworkCommand cmd, boolean success, String result) { + super(cmd, success, result); + } +} diff --git a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java new file mode 100644 index 000000000000..541eddd6b798 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java @@ -0,0 +1,41 @@ +// 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 com.cloud.agent.api; + +import com.cloud.agent.api.to.NicTO; + +public class SetupPersistentNetworkCommand extends Command{ + + NicTO nic; + + public SetupPersistentNetworkCommand(NicTO nic) { + this.nic = nic; + } + + public NicTO getNic() { + return nic; + } + + protected SetupPersistentNetworkCommand() { + } + + @Override + public boolean executeInSequence() { + return false; + } +} diff --git a/core/src/main/java/com/cloud/agent/api/StopCommand.java b/core/src/main/java/com/cloud/agent/api/StopCommand.java index ad6833f1fea5..3923a35bd0a7 100644 --- a/core/src/main/java/com/cloud/agent/api/StopCommand.java +++ b/core/src/main/java/com/cloud/agent/api/StopCommand.java @@ -36,6 +36,7 @@ public class StopCommand extends RebootCommand { String controlIp = null; boolean forceStop = false; private Map dpdkInterfaceMapping; + Map vlanToPersistenceMap; public Map getDpdkInterfaceMapping() { return dpdkInterfaceMapping; @@ -129,4 +130,12 @@ public void setVolumesToDisconnect(List> volumesToDisconnect public List> getVolumesToDisconnect() { return volumesToDisconnect; } + + public Map getVlanToPersistenceMap() { + return vlanToPersistenceMap; + } + + public void setVlanToPersistenceMap(Map vlanToPersistenceMap) { + this.vlanToPersistenceMap = vlanToPersistenceMap; + } } 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 f1ab9cd1ff95..0547c952a648 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -40,6 +40,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.api.query.dao.UserVmJoinDao; +import com.cloud.api.query.vo.UserVmJoinVO; import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.api.ApiConstants; @@ -166,6 +168,7 @@ import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.network.Network; import com.cloud.network.NetworkModel; +import com.cloud.network.Networks; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkDetailVO; import com.cloud.network.dao.NetworkDetailsDao; @@ -176,6 +179,8 @@ import com.cloud.offering.DiskOfferingInfo; import com.cloud.offering.NetworkOffering; import com.cloud.offering.ServiceOffering; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import com.cloud.org.Cluster; import com.cloud.resource.ResourceManager; @@ -347,6 +352,10 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac private SecurityGroupManager _securityGroupManager; @Inject private UserVmDeployAsIsDetailsDao userVmDeployAsIsDetailsDao; + @Inject + private UserVmJoinDao userVmJoinDao; + @Inject + private NetworkOfferingDao networkOfferingDao; VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this); @@ -1244,6 +1253,10 @@ public void orchestrateStart(final String vmUuid, final Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + stopCmd.setVlanToPersistenceMap(vlanToPersistenceMap); + } final StopCommand cmd = stopCmd; final Answer answer = _agentMgr.easySend(destHostId, cmd); if (answer != null && answer instanceof StopAnswer) { @@ -1640,7 +1653,11 @@ private List> getVolumesToDisconnect(VirtualMachine vm) { protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final boolean force, final boolean checkBeforeCleanup) { final VirtualMachine vm = profile.getVirtualMachine(); + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); StopCommand stpCmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), checkBeforeCleanup); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + stpCmd.setVlanToPersistenceMap(vlanToPersistenceMap); + } stpCmd.setControlIp(getControlNicIpForVM(vm)); stpCmd.setVolumesToDisconnect(getVolumesToDisconnect(vm)); final StopCommand stop = stpCmd; @@ -1827,6 +1844,26 @@ private void orchestrateStop(final String vmUuid, final boolean cleanUpEvenIfUna advanceStop(vm, cleanUpEvenIfUnableToStop); } + private Map getVlanToPersistenceMapForVM(long vmId) { + List userVmJoinVOS = userVmJoinDao.searchByIds(vmId); + Map vlanToPersistenceMap = new HashMap<>(); + for (UserVmJoinVO userVmJoinVO : userVmJoinVOS) { + NetworkVO networkVO = _networkDao.findById(userVmJoinVO.getNetworkId()); + NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId()); + Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); + vlanToPersistenceMap.put(data.first(), data.second()); + } + return vlanToPersistenceMap; + } + + private Pair getVMNetworkDetails(NetworkVO networkVO, boolean isPersistent) { + URI broadcastUri = networkVO.getBroadcastUri(); + String scheme = broadcastUri.getScheme(); + String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); + Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && scheme.equalsIgnoreCase("vlan") && isPersistent); + return new Pair<>(vlanId, shouldDelete); + } + private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnableToStop) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException { final State state = vm.getState(); @@ -1925,8 +1962,13 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl vmGuru.prepareStop(profile); + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); + final StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false, cleanUpEvenIfUnableToStop); stop.setControlIp(getControlNicIpForVM(vm)); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + stop.setVlanToPersistenceMap(vlanToPersistenceMap); + } boolean stopped = false; Answer answer = null; @@ -2582,7 +2624,11 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy Map dpdkInterfaceMapping = null; try { final boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); final MigrateCommand mc = new MigrateCommand(vm.getInstanceName(), dest.getHost().getPrivateIpAddress(), isWindows, to, getExecuteInSequence(vm.getHypervisorType())); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + mc.setVlanToPersistenceMap(vlanToPersistenceMap); + } boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value(); mc.setAutoConvergence(kvmAutoConvergence); @@ -3388,6 +3434,10 @@ public Command cleanup(final VirtualMachine vm, Map dpdkInterfac if (MapUtils.isNotEmpty(dpdkInterfaceMapping)) { cmd.setDpdkInterfaceMapping(dpdkInterfaceMapping); } + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + cmd.setVlanToPersistenceMap(vlanToPersistenceMap); + } return cmd; } @@ -3406,6 +3456,10 @@ public Command cleanup(final String vmName) { StopCommand cmd = new StopCommand(vmName, getExecuteInSequence(null), false); cmd.setControlIp(getControlNicIpForVM(vm)); + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + cmd.setVlanToPersistenceMap(vlanToPersistenceMap); + } return cmd; } @@ -4192,8 +4246,12 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI boolean migrated = false; try { + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); final boolean isWindows = _guestOsCategoryDao.findById(_guestOsDao.findById(vm.getGuestOSId()).getCategoryId()).getName().equalsIgnoreCase("Windows"); final MigrateCommand mc = new MigrateCommand(vm.getInstanceName(), dest.getHost().getPrivateIpAddress(), isWindows, to, getExecuteInSequence(vm.getHypervisorType())); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + mc.setVlanToPersistenceMap(vlanToPersistenceMap); + } boolean kvmAutoConvergence = StorageManager.KvmAutoConvergence.value(); mc.setAutoConvergence(kvmAutoConvergence); 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 8a3c07238c39..26f1b66f8f2b 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 @@ -40,6 +40,10 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.agent.api.SetupPersistentNetworkAnswer; +import com.cloud.agent.api.SetupPersistentNetworkCommand; +import com.cloud.dc.ClusterVO; +import com.cloud.dc.dao.ClusterDao; import com.cloud.deployasis.dao.TemplateDeployAsIsDetailsDao; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.api.ApiConstants; @@ -90,16 +94,19 @@ import com.cloud.domain.Domain; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; +import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.ConnectionException; import com.cloud.exception.InsufficientAddressCapacityException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InsufficientVirtualNetworkCapacityException; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.UnsupportedServiceException; import com.cloud.host.Host; +import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; @@ -182,6 +189,7 @@ import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingDetailsDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; +import com.cloud.resource.ResourceManager; import com.cloud.user.Account; import com.cloud.user.ResourceLimitService; import com.cloud.user.User; @@ -305,6 +313,8 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra UserVmManager _userVmMgr; @Inject TemplateDeployAsIsDetailsDao templateDeployAsIsDetailsDao; + @Inject + ResourceManager resourceManager; List networkGurus; @@ -385,6 +395,8 @@ public void setDhcpProviders(final List dhcpProviders) { NetworkModel _networkModel; @Inject NicSecondaryIpDao _nicSecondaryIpDao; + @Inject + ClusterDao clusterDao; protected StateMachine2 _stateMachine; ScheduledExecutorService _executor; @@ -1158,11 +1170,12 @@ protected NicTO toNicTO(final NicVO nic, final NicProfile profile, final Network boolean isNetworkImplemented(final NetworkVO network) { final Network.State state = network.getState(); + final NetworkOfferingVO offeringVO = _networkOfferingDao.findById(network.getNetworkOfferingId()); if (state == Network.State.Implemented) { return true; } else if (state == Network.State.Setup) { final DataCenterVO zone = _dcDao.findById(network.getDataCenterId()); - if (!isSharedNetworkOfferingWithServices(network.getNetworkOfferingId()) || zone.getNetworkType() == NetworkType.Basic) { + if ((!isSharedNetworkOfferingWithServices(network.getNetworkOfferingId()) && !offeringVO.isPersistent()) || zone.getNetworkType() == NetworkType.Basic) { return true; } } @@ -1187,6 +1200,68 @@ Pair implementNetwork(final long networkId, final Deploy return implemented; } + private NicTO getNicTO(NetworkVO networkVO, NetworkOfferingVO networkOfferingVO) { + NicTO to = new NicTO(); + to.setBroadcastType(networkVO.getBroadcastDomainType()); + to.setType(networkVO.getTrafficType()); + to.setBroadcastUri(networkVO.getBroadcastUri()); + to.setIsolationuri(networkVO.getBroadcastUri()); + to.setNetworkRateMbps(_configMgr.getNetworkOfferingNetworkRate(networkOfferingVO.getId(), networkVO.getDataCenterId())); + to.setSecurityGroupEnabled(_networkModel.isSecurityGroupSupportedInNetwork(networkVO)); + return to; + } + + private boolean isNtwConfiguredInCluster(HostVO hostVO, Map> clusterToHostsMap) { + Long clusterId = hostVO.getClusterId(); + List hosts = clusterToHostsMap.get(clusterId); + if (hosts == null) { + hosts = new ArrayList<>(); + } + if (hostVO.getHypervisorType() == HypervisorType.KVM) { + hosts.add(hostVO.getId()); + clusterToHostsMap.put(clusterId, hosts); + return false; + } + if (hosts != null && !hosts.isEmpty()) { + return true; + } + hosts.add(hostVO.getId()); + clusterToHostsMap.put(clusterId, hosts); + return false; + } + + private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offering, Long dcId) throws AgentUnavailableException, OperationTimedoutException { + NicTO to = getNicTO(network, offering); + List clusterVOS = clusterDao.listClustersByDcId(dcId); + List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, dcId); + Collections.reverse(hosts); + Map> clusterToHostsMap = new HashMap<>(); + SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); + for (HostVO host : hosts) { + if (isNtwConfiguredInCluster(host, clusterToHostsMap)) { + continue; + } + final SetupPersistentNetworkAnswer answer = (SetupPersistentNetworkAnswer)_agentMgr.send(host.getId(), cmd); + + if (answer == null) { + s_logger.warn("Unable to get an answer to the SetupPersistentNetworkCommand from agent:" + host.getId()); + clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); + continue; + // throw new CloudRuntimeException("Unable to get an answer to the SetupPersistentNetworkCommand from agent: " + host.getId()); + } + + if (!answer.getResult()) { + s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails() ); +// final String msg = "Incorrect Network setup on agent, Reinitialize agent after network names are setup, details : " + answer.getDetails(); +// throw new CloudRuntimeException(msg); + continue; + } + } + if (clusterToHostsMap.keySet().size() != clusterVOS.size()) { + throw new CloudRuntimeException("Failed to setup persistent network across all hosts"); + } + } + @Override @DB public Pair implementNetwork(final long networkId, final DeployDestination dest, final ReservationContext context) throws ConcurrentOperationException, @@ -1245,6 +1320,10 @@ public Pair implementNetwork(final long networkId, final // implement network elements and re-apply all the network rules implementNetworkElementsAndResources(dest, context, network, offering); + long dcId = dest.getDataCenter().getId(); + if (network.getGuestType() == GuestType.L2 && offering.isPersistent()) { + setupPersistentNetwork(network, offering, dcId); + } if (isSharedNetworkWithServices(network)) { network.setState(Network.State.Implemented); } else { @@ -1258,7 +1337,7 @@ public Pair implementNetwork(final long networkId, final } catch (final NoTransitionException e) { s_logger.error(e.getMessage()); return null; - } catch (final CloudRuntimeException e) { + } catch (final CloudRuntimeException | OperationTimedoutException e) { s_logger.error("Caught exception: " + e.getMessage()); return null; } finally { @@ -2627,7 +2706,6 @@ public Network doInTransaction(final TransactionStatus status) { userNetwork.setPvlanType(isolatedPvlanType); } } - final List networks = setupNetwork(owner, ntwkOff, userNetwork, plan, name, displayText, true, domainId, aclType, subdomainAccessFinal, vpcId, isDisplayNetworkEnabled); @@ -3937,7 +4015,6 @@ protected NicProfile getNicProfileForVm(final Network network, final NicProfile public NicProfile createNicForVm(final Network network, final NicProfile requested, final ReservationContext context, final VirtualMachineProfile vmProfile, final boolean prepare) throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException, InsufficientCapacityException, ResourceUnavailableException { - final VirtualMachine vm = vmProfile.getVirtualMachine(); final DataCenter dc = _entityMgr.findById(DataCenter.class, network.getDataCenterId()); final Host host = _hostDao.findById(vm.getHostId()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index 8ff265e9b541..34dc0dff17b4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -267,8 +267,8 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA } @Override - public void unplug(LibvirtVMDef.InterfaceDef iface) { - deleteVnetBr(iface.getBrName()); + public void unplug(LibvirtVMDef.InterfaceDef iface, boolean deleteBr) { + deleteVnetBr(iface.getBrName(), deleteBr); } @Override @@ -289,7 +289,7 @@ private String generateVxnetBrName(String pifName, String vnetId) { return "brvx-" + vnetId; } - private String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException { + public String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException { String nic = _pifs.get(pifKey); if (nic == null) { // if not found in bridge map, maybe traffic label refers to pif already? @@ -327,7 +327,7 @@ private void createVnet(String vnetId, String pif, String brName, String protoco } } - private void deleteVnetBr(String brName) { + private void deleteVnetBr(String brName, boolean deleteBr) { synchronized (_vnetBridgeMonitor) { String cmdout = Script.runSimpleBashScript("ls /sys/class/net/" + brName); if (cmdout == null) @@ -376,6 +376,7 @@ private void deleteVnetBr(String brName) { command.add("-v", vNetId); command.add("-p", pName); command.add("-b", brName); + command.add("-d", String.valueOf(deleteBr)); final String result = command.execute(); if (result != null) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java index de65a379f92d..9a2119b6773d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java @@ -62,7 +62,7 @@ public LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicA } @Override - public void unplug(LibvirtVMDef.InterfaceDef iface) { + public void unplug(LibvirtVMDef.InterfaceDef iface, boolean deleteBr) { // not needed, libvirt will cleanup } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java index 5f7066a7a40d..b3dfc11349d2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java @@ -141,7 +141,7 @@ public InterfaceDef plug(NicTO nic, String guestOsType, String nicAdapter, Map nics if (nics != null) { for (final InterfaceDef nic : nics) { for (final VifDriver vifDriver : getAllVifDrivers()) { - vifDriver.unplug(nic); + vifDriver.unplug(nic, true); } } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java index 7bf35e6f0217..71b3c5703f0b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java @@ -194,7 +194,7 @@ public InterfaceDef plug(NicTO nic, String guestOsType, String nicAdapter, Map extraConfig) throws InternalErrorException, LibvirtException; - public void unplug(LibvirtVMDef.InterfaceDef iface); + public void unplug(LibvirtVMDef.InterfaceDef iface, boolean delete); void attach(LibvirtVMDef.InterfaceDef iface); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java index 95e38f007645..77aeec554593 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java @@ -45,7 +45,7 @@ public void configure(Map params) throws ConfigurationException public abstract LibvirtVMDef.InterfaceDef plug(NicTO nic, String guestOsType, String nicAdapter, Map extraConfig) throws InternalErrorException, LibvirtException; @Override - public abstract void unplug(LibvirtVMDef.InterfaceDef iface); + public abstract void unplug(LibvirtVMDef.InterfaceDef iface, boolean deleteBr); protected LibvirtVMDef.InterfaceDef.NicModel getGuestNicModel(String platformEmulator, String nicAdapter) { // if nicAdapter is found in ENUM, use it. Otherwise, match guest OS type as before diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 0524be796174..1e753c25e648 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -96,6 +96,7 @@ protected String createMigrationURI(final String destinationIp, final LibvirtCom @Override public Answer execute(final MigrateCommand command, final LibvirtComputingResource libvirtComputingResource) { final String vmName = command.getVmName(); + final Map vlanToPersistenceMap = command.getVlanToPersistenceMap(); final String destinationUri = createMigrationURI(command.getDestinationIp(), libvirtComputingResource); final List migrateDiskInfoList = command.getMigrateDiskInfoList(); @@ -276,11 +277,12 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. } else { libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName); for (final InterfaceDef iface : ifaces) { + String vlanId = getVlanIdFromBridgeName(iface.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers final List allVifDrivers = libvirtComputingResource.getAllVifDrivers(); for (final VifDriver vifDriver : allVifDrivers) { - vifDriver.unplug(iface); + vifDriver.unplug(iface, deleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -288,6 +290,20 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. return new MigrateAnswer(command, result == null, result, null); } + private String getVlanIdFromBridgeName(String brName) { + if (brName != null) { + return brName.split("-")[1]; + } + return null; + } + + private boolean deleteBridge(Map vlanToPersistenceMap, String vlanId) { + if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { + return vlanToPersistenceMap.get(vlanId); + } + return false; + } + /** * Replace DPDK source path and target before migrations */ diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReplugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReplugNicCommandWrapper.java index 7ee9171928ea..65f1ddf33090 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReplugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReplugNicCommandWrapper.java @@ -90,7 +90,7 @@ public Answer execute(final ReplugNicCommand command, final LibvirtComputingReso // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(oldPluggedNic); + vifDriver.unplug(oldPluggedNic, true); } return new ReplugNicAnswer(command, true, "success"); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupPersistentNetworkCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupPersistentNetworkCommandWrapper.java new file mode 100644 index 000000000000..a2ec64451a7c --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtSetupPersistentNetworkCommandWrapper.java @@ -0,0 +1,50 @@ +// 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 com.cloud.hypervisor.kvm.resource.wrapper; + +import org.apache.log4j.Logger; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.SetupPersistentNetworkAnswer; +import com.cloud.agent.api.SetupPersistentNetworkCommand; +import com.cloud.agent.api.to.NicTO; +import com.cloud.exception.InternalErrorException; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.VifDriver; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; + + +@ResourceWrapper(handles = SetupPersistentNetworkCommand.class) +public class LibvirtSetupPersistentNetworkCommandWrapper extends CommandWrapper { + private static final Logger s_logger = Logger.getLogger(LibvirtSetupPersistentNetworkCommandWrapper.class); + + @Override + public Answer execute(SetupPersistentNetworkCommand command, LibvirtComputingResource serverResource) { + NicTO nic = command.getNic(); + VifDriver driver = serverResource.getVifDriver(nic.getType()); + try { + driver.plug(nic, null, "", null); + } catch (InternalErrorException | LibvirtException e) { + return new SetupPersistentNetworkAnswer(command, false, e.getLocalizedMessage()); + } + + return new SetupPersistentNetworkAnswer(command, true, "Successfully setup persistent network"); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index cb57dbc0ec29..a8126c8f6211 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -56,7 +56,7 @@ public final class LibvirtStopCommandWrapper extends CommandWrapper vlanToPersistenceMap = command.getVlanToPersistenceMap(); final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); if (command.checkBeforeCleanup()) { @@ -125,10 +125,11 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource } } else { for (final InterfaceDef iface : ifaces) { + String vlanId = getVlanIdFromBridgeName(iface.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(iface); + vifDriver.unplug(iface, deleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -160,4 +161,21 @@ private void performAgentStopHook(String vmName, final LibvirtComputingResource } } + private String getVlanIdFromBridgeName(String brName) { + if (brName != null) { + String[] s = brName.split("-"); + if (s.length > 1) { + return s[1]; + } + return null; + } + return null; + } + + private boolean deleteBridge(Map vlanToPersistenceMap, String vlanId) { + if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { + return vlanToPersistenceMap.get(vlanId); + } + return false; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java index 071352c5c9a0..7af9699df163 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java @@ -62,7 +62,7 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(pluggedNic); + vifDriver.unplug(pluggedNic, true); } return new UnPlugNicAnswer(command, true, "success"); } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index bd651f4c02c2..84d08531c2bf 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -3364,7 +3364,7 @@ public void testUnPlugNicCommandMatchMack() { when(libvirtComputingResource.getAllVifDrivers()).thenReturn(drivers); - doNothing().when(vifDriver).unplug(intDef); + doNothing().when(vifDriver).unplug(intDef, true); } catch (final LibvirtException e) { fail(e.getMessage()); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 56d08a4e088e..e2cca5be428c 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -45,6 +45,8 @@ import javax.naming.ConfigurationException; import javax.xml.datatype.XMLGregorianCalendar; +import com.cloud.agent.api.SetupPersistentNetworkAnswer; +import com.cloud.agent.api.SetupPersistentNetworkCommand; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DeployAsIsInfoTO; import com.cloud.agent.api.ValidateVcenterDetailsCommand; @@ -577,6 +579,8 @@ public Answer executeRequest(Command cmd) { answer = execute((PrepareUnmanageVMInstanceCommand) cmd); } else if (clz == ValidateVcenterDetailsCommand.class) { answer = execute((ValidateVcenterDetailsCommand) cmd); + } else if (clz == SetupPersistentNetworkCommand.class) { + answer = execute((SetupPersistentNetworkCommand) cmd); } else { answer = Answer.createUnsupportedCommandAnswer(cmd); } @@ -617,6 +621,21 @@ public Answer executeRequest(Command cmd) { return answer; } + private Answer execute(SetupPersistentNetworkCommand cmd) { + VmwareHypervisorHost host = getHyperHost(getServiceContext()); + String hostname = null; + VmwareContext context = getServiceContext(); + HostMO hostMO = new HostMO(context, host.getMor()); + + try { + prepareNetworkFromNicInfo(hostMO, cmd.getNic(), false, null); + hostname = host.getHyperHostName(); + } catch (Exception e) { + return new SetupPersistentNetworkAnswer(cmd, false, "failed to get response"); + } + return new SetupPersistentNetworkAnswer(cmd, true, hostname); + } + /** * Check if storage NFS version is already set or needs to be reconfigured.
* If _storageNfsVersion is not null -> nothing to do, version already set.
diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupPersistentNetworkCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupPersistentNetworkCommandWrapper.java new file mode 100644 index 000000000000..cab5a080949c --- /dev/null +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupPersistentNetworkCommandWrapper.java @@ -0,0 +1,54 @@ +// 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 com.cloud.hypervisor.xenserver.resource.wrapper.xenbase; + + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.SetupPersistentNetworkAnswer; +import com.cloud.agent.api.SetupPersistentNetworkCommand; +import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase; +import com.cloud.hypervisor.xenserver.resource.XsHost; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Network; + +@ResourceWrapper(handles = SetupPersistentNetworkCommand.class) +public class CitrixSetupPersistentNetworkCommandWrapper extends CommandWrapper { + + private static final Logger s_logger = Logger.getLogger(CitrixSetupPersistentNetworkCommandWrapper.class); + + @Override + public Answer execute(SetupPersistentNetworkCommand command, CitrixResourceBase citrixResourceBase) { + final Connection conn = citrixResourceBase.getConnection(); + final XsHost host = citrixResourceBase.getHost(); + try { + Network network = citrixResourceBase.getNetwork(conn, command.getNic()); + if (network == null) { + return new SetupPersistentNetworkAnswer(command, false, "Failed to setup network on host: "+ host.getIp()); + } + return new SetupPersistentNetworkAnswer(command, true, "Successfully setup network on host: "+ host.getIp()); + } catch (final Exception e) { + final String msg = " Failed to setup network on host: " + host.getIp() + " due to: " + e.toString(); + s_logger.error(msg, e); + return new SetupPersistentNetworkAnswer(command, false, msg); + } + } +} diff --git a/scripts/vm/network/vnet/modifyvlan.sh b/scripts/vm/network/vnet/modifyvlan.sh index 8ee2e392be83..a7ec2568dc91 100755 --- a/scripts/vm/network/vnet/modifyvlan.sh +++ b/scripts/vm/network/vnet/modifyvlan.sh @@ -22,7 +22,7 @@ # set -x usage() { - printf "Usage: %s: -o (add | delete) -v -p -b \n" + printf "Usage: %s: -o (add | delete) -v -p -b -d (true | false)\n" } addVlan() { @@ -90,7 +90,8 @@ deleteVlan() { local vlanId=$1 local pif=$2 local vlanDev=$pif.$vlanId - local vlanBr=$3 + local vlanBr=$3 + local deleteBr=$4 ip link delete $vlanDev type vlan > /dev/null @@ -100,30 +101,33 @@ deleteVlan() { return 1 fi - ip link set $vlanBr down - - if [ $? -gt 0 ] - then - return 1 - fi - - ip link delete $vlanBr type bridge - - if [ $? -gt 0 ] - then - printf "Failed to del bridge $vlanBr" - return 1 - fi + if [ $deleteBr == "true" ] + then + ip link set $vlanBr down + + if [ $? -gt 0 ] + then + return 1 + fi + + ip link delete $vlanBr type bridge + if [ $? -gt 0 ] + then + printf "Failed to del bridge $vlanBr" + return 1 + fi + fi return 0 } op= vlanId= +deleteBr="true" option=$@ -while getopts 'o:v:p:b:' OPTION +while getopts 'o:v:p:b:d:' OPTION do case $OPTION in o) oflag=1 @@ -136,8 +140,11 @@ do pif="$OPTARG" ;; b) bflag=1 - brName="$OPTARG" - ;; + brName="$OPTARG" + ;; + d) dflag=1 + deleteBr="$OPTARG" + ;; ?) usage exit 2 ;; @@ -177,7 +184,7 @@ else if [ "$op" == "delete" ] then # Delete the vlan - deleteVlan $vlanId $pif $brName + deleteVlan $vlanId $pif $brName $deleteBr # Always exit with success exit 0 diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 4a4603c7baad..2cf690adcb3b 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -1374,10 +1374,6 @@ public Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapac // if the network offering has persistent set to true, implement the network if (ntwkOff.isPersistent()) { try { - if (network.getState() == Network.State.Setup) { - s_logger.debug("Network id=" + network.getId() + " is already provisioned"); - return network; - } DeployDestination dest = new DeployDestination(zone, null, null, null); UserVO callerUser = _userDao.findById(CallContext.current().getCallingUserId()); Journal journal = new Journal.LogJournal("Implementing " + network, s_logger); diff --git a/server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java b/server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java index 0ce00315ba2c..c1ce99c0606e 100644 --- a/server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java +++ b/server/src/main/java/com/cloud/network/guru/GuestNetworkGuru.java @@ -214,7 +214,9 @@ public Network design(final NetworkOffering offering, final DeploymentPlan plan, if (offering.isSpecifyVlan()) { network.setBroadcastUri(userSpecified.getBroadcastUri()); - network.setState(State.Setup); + if (!offering.isPersistent()) { + network.setState(State.Setup); + } if (userSpecified.getPvlanType() != null) { network.setBroadcastDomainType(BroadcastDomainType.Pvlan); network.setPvlanType(userSpecified.getPvlanType()); From 04bb7a8b6aea250a027539acb1db6c16b0cc9e4a Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 5 Jan 2021 12:16:09 +0530 Subject: [PATCH 02/13] address comments --- .../com/cloud/vm/VirtualMachineManagerImpl.java | 13 ++++++++++++- .../orchestration/NetworkOrchestrator.java | 16 +++++++++------- .../wrapper/LibvirtMigrateCommandWrapper.java | 12 ++++++++---- .../wrapper/LibvirtStopCommandWrapper.java | 7 ++++--- 4 files changed, 33 insertions(+), 15 deletions(-) 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 0547c952a648..09b5c6932ab7 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1856,11 +1856,22 @@ private Map getVlanToPersistenceMapForVM(long vmId) { return vlanToPersistenceMap; } + /** + * + * @param networkVO - the network object used to determine the vlanId from the broadcast URI + * @param isPersistent - indicates if the corresponding network's network offering is Persistent + * + * @return - basically returns the vlan ID which is used to determine the + * bridge name for KVM hypervisor and based on the network and isolation type and persistent setting of the offering + * we decide whether the bridge is to be deleted (KVM) if the last VM in that host is destroyed / migrated + */ private Pair getVMNetworkDetails(NetworkVO networkVO, boolean isPersistent) { URI broadcastUri = networkVO.getBroadcastUri(); String scheme = broadcastUri.getScheme(); String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); - Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && scheme.equalsIgnoreCase("vlan") && isPersistent); + Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && + (scheme.equalsIgnoreCase("vlan") || scheme.equalsIgnoreCase("vxlan")) + && isPersistent); return new Pair<>(vlanId, shouldDelete); } 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 26f1b66f8f2b..048829d88b66 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 @@ -1200,7 +1200,12 @@ Pair implementNetwork(final long networkId, final Deploy return implemented; } - private NicTO getNicTO(NetworkVO networkVO, NetworkOfferingVO networkOfferingVO) { + /** + * + * Creates a dummy NicTO object which is used by the respective hypervisors to setup network elements / resources + * - bridges(KVM), VLANs(Xen) and portgroups(VMWare) for L2 network + */ + private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOfferingVO networkOfferingVO) { NicTO to = new NicTO(); to.setBroadcastType(networkVO.getBroadcastDomainType()); to.setType(networkVO.getTrafficType()); @@ -1231,10 +1236,9 @@ private boolean isNtwConfiguredInCluster(HostVO hostVO, Map> cl } private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offering, Long dcId) throws AgentUnavailableException, OperationTimedoutException { - NicTO to = getNicTO(network, offering); + NicTO to = createNicTOFromNetworkAndOffering(network, offering); List clusterVOS = clusterDao.listClustersByDcId(dcId); List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, dcId); - Collections.reverse(hosts); Map> clusterToHostsMap = new HashMap<>(); SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); for (HostVO host : hosts) { @@ -1247,13 +1251,11 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin s_logger.warn("Unable to get an answer to the SetupPersistentNetworkCommand from agent:" + host.getId()); clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); continue; - // throw new CloudRuntimeException("Unable to get an answer to the SetupPersistentNetworkCommand from agent: " + host.getId()); } if (!answer.getResult()) { - s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails() ); -// final String msg = "Incorrect Network setup on agent, Reinitialize agent after network names are setup, details : " + answer.getDetails(); -// throw new CloudRuntimeException(msg); + s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); + clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); continue; } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 1e753c25e648..7d2626c5e76a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -282,7 +282,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. // each interface at this point, so inform all vif drivers final List allVifDrivers = libvirtComputingResource.getAllVifDrivers(); for (final VifDriver vifDriver : allVifDrivers) { - vifDriver.unplug(iface, deleteBridge(vlanToPersistenceMap, vlanId)); + vifDriver.unplug(iface, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -291,13 +291,17 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. } private String getVlanIdFromBridgeName(String brName) { - if (brName != null) { - return brName.split("-")[1]; + if (StringUtils.isNotBlank(brName)) { + String[] s = brName.split("-"); + if (s.length > 1) { + return s[1]; + } + return null; } return null; } - private boolean deleteBridge(Map vlanToPersistenceMap, String vlanId) { + private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { return vlanToPersistenceMap.get(vlanId); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index a8126c8f6211..509595f869e0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -30,6 +30,7 @@ import com.cloud.utils.ssh.SshHelper; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -129,7 +130,7 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(iface, deleteBridge(vlanToPersistenceMap, vlanId)); + vifDriver.unplug(iface, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -162,7 +163,7 @@ private void performAgentStopHook(String vmName, final LibvirtComputingResource } private String getVlanIdFromBridgeName(String brName) { - if (brName != null) { + if (StringUtils.isNotBlank(brName)) { String[] s = brName.split("-"); if (s.length > 1) { return s[1]; @@ -172,7 +173,7 @@ private String getVlanIdFromBridgeName(String brName) { return null; } - private boolean deleteBridge(Map vlanToPersistenceMap, String vlanId) { + private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { return vlanToPersistenceMap.get(vlanId); } From e57b34521789f7a940a96e16f13ab1efd2d3cb18 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 6 Jan 2021 18:21:29 +0530 Subject: [PATCH 03/13] Add network tag to nic info - required for xen --- .../orchestration/NetworkOrchestrator.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) 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 048829d88b66..95d425341b4d 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 @@ -1205,8 +1205,9 @@ Pair implementNetwork(final long networkId, final Deploy * Creates a dummy NicTO object which is used by the respective hypervisors to setup network elements / resources * - bridges(KVM), VLANs(Xen) and portgroups(VMWare) for L2 network */ - private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOfferingVO networkOfferingVO) { + private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOfferingVO networkOfferingVO, HostVO hostVO) { NicTO to = new NicTO(); + to.setName(_networkModel.getNetworkTag(hostVO.getHypervisorType(), networkVO)); to.setBroadcastType(networkVO.getBroadcastDomainType()); to.setType(networkVO.getTrafficType()); to.setBroadcastUri(networkVO.getBroadcastUri()); @@ -1216,7 +1217,7 @@ private NicTO createNicTOFromNetworkAndOffering(NetworkVO networkVO, NetworkOffe return to; } - private boolean isNtwConfiguredInCluster(HostVO hostVO, Map> clusterToHostsMap) { + private Pair isNtwConfiguredInCluster(HostVO hostVO, Map> clusterToHostsMap, NetworkVO networkVO, NetworkOfferingVO networkOfferingVO) { Long clusterId = hostVO.getClusterId(); List hosts = clusterToHostsMap.get(clusterId); if (hosts == null) { @@ -1225,26 +1226,28 @@ private boolean isNtwConfiguredInCluster(HostVO hostVO, Map> cl if (hostVO.getHypervisorType() == HypervisorType.KVM) { hosts.add(hostVO.getId()); clusterToHostsMap.put(clusterId, hosts); - return false; + return new Pair<>(false, createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, hostVO)); } if (hosts != null && !hosts.isEmpty()) { - return true; + return new Pair<>(true, createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, hostVO)); } hosts.add(hostVO.getId()); clusterToHostsMap.put(clusterId, hosts); - return false; + return new Pair<>(false, createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, hostVO)); } private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offering, Long dcId) throws AgentUnavailableException, OperationTimedoutException { - NicTO to = createNicTOFromNetworkAndOffering(network, offering); List clusterVOS = clusterDao.listClustersByDcId(dcId); List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, dcId); Map> clusterToHostsMap = new HashMap<>(); - SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); + for (HostVO host : hosts) { - if (isNtwConfiguredInCluster(host, clusterToHostsMap)) { + Pair networkCfgStateAndDetails = isNtwConfiguredInCluster(host, clusterToHostsMap, network, offering); + if (networkCfgStateAndDetails.first()) { continue; } + NicTO to = networkCfgStateAndDetails.second(); + SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); final SetupPersistentNetworkAnswer answer = (SetupPersistentNetworkAnswer)_agentMgr.send(host.getId(), cmd); if (answer == null) { @@ -3145,7 +3148,6 @@ public void reallyRun() { s_logger.info("NetworkGarbageCollector uses '" + netGcWait + "' seconds for GC interval."); for (final Long networkId : networkIds) { - if (!_networkModel.isNetworkReadyForGc(networkId)) { continue; } From af12e27561adc727e398f3c342510a8093f6974c Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 13 Jan 2021 11:08:25 +0530 Subject: [PATCH 04/13] Marvin tests --- .../cloud/vm/VirtualMachineManagerImpl.java | 2 +- .../orchestration/NetworkOrchestrator.java | 3 +- .../smoke/test_persistent_network.py | 342 ++++++++++++++++++ tools/marvin/marvin/config/test_data.py | 14 +- tools/marvin/marvin/lib/vcenter.py | 2 + 5 files changed, 360 insertions(+), 3 deletions(-) create mode 100644 test/integration/smoke/test_persistent_network.py 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 09b5c6932ab7..7ce5f512eb20 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1870,7 +1870,7 @@ private Pair getVMNetworkDetails(NetworkVO networkVO, boolean i String scheme = broadcastUri.getScheme(); String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && - (scheme.equalsIgnoreCase("vlan") || scheme.equalsIgnoreCase("vxlan")) + (scheme.equalsIgnoreCase("vlan")) && isPersistent); return new Pair<>(vlanId, shouldDelete); } 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 95d425341b4d..05d8952b3cee 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 @@ -1326,7 +1326,8 @@ public Pair implementNetwork(final long networkId, final implementNetworkElementsAndResources(dest, context, network, offering); long dcId = dest.getDataCenter().getId(); - if (network.getGuestType() == GuestType.L2 && offering.isPersistent()) { + if (network.getGuestType() == GuestType.L2 && offering.isPersistent() && + (network.getBroadcastUri() != null && BroadcastDomainType.getSchemeValue(network.getBroadcastUri()) == BroadcastDomainType.Vlan)) { setupPersistentNetwork(network, offering, dcId); } if (isSharedNetworkWithServices(network)) { diff --git a/test/integration/smoke/test_persistent_network.py b/test/integration/smoke/test_persistent_network.py new file mode 100644 index 000000000000..a5639bae7169 --- /dev/null +++ b/test/integration/smoke/test_persistent_network.py @@ -0,0 +1,342 @@ +# 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. + +from marvin.cloudstackTestCase import cloudstackTestCase, unittest +from marvin.lib.utils import (cleanup_resources, + validateList, + get_hypervisor_type, get_process_status) +from marvin.lib.base import (Account, + Cluster, + Configurations, + Host, + VPC, + VirtualMachine, + Network, + Router, + ServiceOffering, + NetworkOffering) +from marvin.lib.common import (get_zone, + get_template, + verifyNetworkState, + wait_for_cleanup, list_routers, list_hosts) +from nose.plugins.attrib import attr +from marvin.sshClient import SshClient +from distutils.util import strtobool +from pyVmomi import vim, vmodl +from marvin.lib.vcenter import Vcenter +import logging + +logger = logging.getLogger('TestPesistentNetwork') +stream_handler = logging.StreamHandler() +logger.setLevel(logging.DEBUG) +logger.addHandler(stream_handler) + + +class TestL2PersistentNetworks(cloudstackTestCase): + @classmethod + def setUpClass(cls): + cls.testClient = super(TestL2PersistentNetworks, cls).getClsTestClient() + cls.api_client = cls.testClient.getApiClient() + cls.hypervisor = cls.testClient.getHypervisorInfo() + # Fill services from the external config file + cls.services = cls.testClient.getParsedTestDataConfig() + cls.hostConfig = cls.config.__dict__["zones"][0].__dict__["pods"][0].__dict__["clusters"][0].__dict__["hosts"][ + 0].__dict__ + # Get Zone and templates + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.services["ostype"] + ) + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + cls.services["virtual_machine"]["template"] = cls.template.id + cls.service_offering = ServiceOffering.create( + cls.api_client, + cls.services["service_offering"] + ) + cls.l2_persistent_network_offering = cls.create_network_offering("nw_off_L2_persistent") + cls.isolated_persistent_network_offering = cls.create_network_offering("nw_off_isolated_persistent") + + # network will be deleted as part of account cleanup + cls._cleanup = [ + cls.service_offering, + cls.isolated_persistent_network_offering, + cls.l2_persistent_network_offering] + return + + @classmethod + def tearDownClass(cls): + try: + # Cleanup resources used + cleanup_resources(cls.api_client, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.hypervisor = self.testClient.getHypervisorInfo() + self.cleanup = [] + return + + def tearDown(self): + try: + # Clean up, terminate the resources created + cleanup_resources(self.apiclient, self.cleanup) + self.cleanup[:] = [] + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + @classmethod + def is_ssh_enabled(cls): + conf = Configurations.list(cls.apiclient, name="kvm.ssh.to.agent") + if not conf: + return False + else: + return bool(strtobool(conf[0].value)) if conf[0].value else False + + @classmethod + def create_network_offering(cls, network_offering_type): + network_offering = NetworkOffering.create( + cls.api_client, + cls.services[network_offering_type], + conservemode=False + ) + + # Update network offering state from disabled to enabled. + NetworkOffering.update( + network_offering, + cls.api_client, + id=network_offering.id, + state="enabled") + return network_offering + + def get_ssh_client(self, ip, username, password, retries=10): + """ Setup ssh client connection and return connection """ + try: + ssh_client = SshClient(ip, 22, username, password, retries) + except Exception as e: + raise unittest.SkipTest("Unable to create ssh connection: " % e) + + self.assertIsNotNone( + ssh_client, "Failed to setup ssh connection to ip=%s" % ip) + + return ssh_client + + def list_all_hosts_in_zone(self, zone_id): + hosts = Host.list( + self.apiclient, + type='Routing', + resourcestate='Enabled', + state='Up', + zoneid=zone_id + ) + return hosts + + ''' + Verifies creation of bridge on KVM host + ''' + def verify_bridge_creation(self, host, vlan_id): + username = self.hostConfig["username"] + password = self.hostConfig["password"] + try: + ssh_client = self.get_ssh_client(host.ipaddress, username, password) + res = ssh_client.execute("ip addr | grep breth1-" + str(vlan_id) + " > /dev/null 2>&1; echo $?") + return res[0] + except Exception as e: + self.fail(e) + + ''' + Gets all port groups on the host + ''' + def capture_host_portgroups(self, host): + host_portgroups = [] + for portgroup in host.config.network.portgroup: + host_portgroups.append(portgroup.spec.name) + return host_portgroups + + ''' + Fetches port group names based on VMware switch type - Distributed Virtual Switch(DVS) and + Standard Virtual Switch(SVS) + ''' + def get_port_group_name(self, switch_type, vlan_id, network_rate): + if switch_type == 'DVS': + return 'cloud.guest.' + str(vlan_id) + '.' + str(network_rate) + '.1-dvSwitch1' + elif switch_type == 'SVS': + return 'cloud.guest.' + str(vlan_id) + '.' + str(network_rate) + '.1-vSwitch1' + else: + return None + + ''' + Verifies creation of port group on the Distributed vSwitch or a host in a cluster connected to + a Standard vSwitch + ''' + def verify_port_group_creation(self, vlan_id): + config = self.get_vmware_dc_config(self.zone.id) + vc_object = Vcenter(config[0][0], config[0][1], 'P@ssword123') + dvs = vc_object.get_dvswitches() + port_group_present = False + if dvs is not None: + port_group_name = self.get_port_group_name('DVS', vlan_id, + self.isolated_persistent_network_offering.networkrate) + port_group_present = port_group_name in dvs[0]['dvswitch']['portgroupNameList'] + + else: + port_group_name = self.get_port_group_name('SVS', vlan_id, + self.isolated_persistent_network_offering.networkrate) + hosts = vc_object._get_obj([vim.HostSystem]) + host = hosts[0]['host'] + host_pg = self.capture_host_portgroups(host) + port_group_present = port_group_name in host_pg + return port_group_present + + ''' + Fetch vmware datacenter login details + ''' + def get_vmware_dc_config(self, zone_id): + zid = self.dbclient.execute("select id from data_center where uuid='%s';" % + zone_id) + vmware_dc_id = self.dbclient.execute( + "select vmware_data_center_id from vmware_data_center_zone_map where zone_id='%s';" % + zid[0]) + vmware_dc_config = self.dbclient.execute( + "select vcenter_host, username, password from vmware_data_center where id = '%s';" % vmware_dc_id[0]) + + return vmware_dc_config + + ''' + Verify VLAN creation on specific host in a cluster + ''' + def verify_vlan_network_creation(self, host, vlan_id): + username = self.hostConfig["username"] + password = self.hostConfig["password"] + try: + ssh_client = self.get_ssh_client(host.ipaddress, username, password) + res = ssh_client.execute( + "xe vlan-list | grep -x \"^\s*tag ( RO): \"" + str(vlan_id) + "> /dev/null 2>&1; echo $?") + return res[0] + except Exception as e: + self.fail(e) + + def verify_network_setup_on_host_per_cluster(self, hypervisor, vlan_id): + clusters = Cluster.list( + self.apiclient, + zoneid=self.zone.id, + allocationstate="Enabled", + listall=True + ) + for cluster in clusters: + hosts = Host.list(self.apiclient, + clusterid=cluster.id, + type="Routing", + state="Up", + resourcestate="Enabled") + host = hosts[0] + if hypervisor == "xenserver": + result = self.verify_vlan_network_creation(host, vlan_id) + self.assertEqual( + int(result), + 0, + "Failed to find vlan on host: " + host.name + " in cluster: " + cluster.name) + if hypervisor == "vmware": + result = self.verify_port_group_creation(vlan_id) + self.assertEqual( + result, + True, + "Failed to find port group on hosts of cluster: " + cluster.name) + + ''' + This test verifies that on creation of an Isolated network with network offering with isPersistent flag + set to true the corresponding network resources are created without having to deploy a VM - VR created + ''' + @attr(tags=["advanced", "isolated", "persistent", "network"], required_hardware="false") + def test_01_isolated_persistent_network(self): + network = Network.create( + self.apiclient, + self.services["isolated_network"], + networkofferingid=self.isolated_persistent_network_offering.id, + zoneid=self.zone.id) + self.cleanup.append(network) + networkVlan = network.vlan + response = verifyNetworkState( + self.apiclient, + network.id, + "implemented") + exceptionOccured = response[0] + isNetworkInDesiredState = response[1] + exceptionMessage = response[2] + + if (exceptionOccured or (not isNetworkInDesiredState)): + self.fail(exceptionMessage) + self.assertIsNotNone( + networkVlan, + "vlan must not be null for persistent network") + + router = Router.list(self.apiclient, networkid=network.id)[0] + router_host_id = router.hostid + host = Host.list(self.apiclient, id=router_host_id)[0] + if host.hypervisor.lower() in "kvm": + result = self.verify_bridge_creation(host, networkVlan) + self.assertEqual( + int(result), + 0, + "Failed to find bridge on the breth1$-{networkVlan}") + elif host.hypervisor.lower() in ["xenserver", "vmware"]: + self.verify_network_setup_on_host_per_cluster(host.hypervisor.lower(), networkVlan) + + ''' + This test verifies that on creation of an L2 network with network offering with isPersistent flag + set to true the corresponding network resources are created without having to deploy a VM - VR created + ''' + @attr(tags=["advanced", "l2", "persistent", "network"], required_hardware="false") + def test_02_L2_persistent_network(self): + network_vlan = 90 + network = Network.create( + self.apiclient, + self.services["l2_network"], + networkofferingid=self.l2_persistent_network_offering.id, + zoneid=self.zone.id, + vlan=network_vlan) + self.cleanup.append(network) + response = verifyNetworkState( + self.apiclient, + network.id, + "implemented") + exceptionOccured = response[0] + isNetworkInDesiredState = response[1] + exceptionMessage = response[2] + + if (exceptionOccured or (not isNetworkInDesiredState)): + self.fail(exceptionMessage) + self.assertIsNotNone( + network_vlan, + "vlan must not be null for persistent network") + + hosts = self.list_all_hosts_in_zone(self.zone.id) + if self.hypervisor.lower() in "kvm": + for host in hosts: + result = self.verify_bridge_creation(host, network_vlan) + self.assertEqual( + int(result), + 0, + "Failed to find bridge on the breth1-" + str(network_vlan)) + elif self.hypervisor.lower() in ["xenserver", "vmware"]: + self.verify_network_setup_on_host_per_cluster(self.hypervisor.lower(), network_vlan) \ No newline at end of file diff --git a/tools/marvin/marvin/config/test_data.py b/tools/marvin/marvin/config/test_data.py index 436c656509d9..116ec1f0d70d 100644 --- a/tools/marvin/marvin/config/test_data.py +++ b/tools/marvin/marvin/config/test_data.py @@ -306,13 +306,21 @@ "StaticNat": "VirtualRouter" } }, + "nw_off_L2_persistent": { + "name": 'Test L2 Network Offering persistent', + "displaytext": 'Test L2 Network Offering persistent', + "guestiptype": 'L2', + "traffictype": 'GUEST', + "ispersistent": 'True', + "specifyVlan": 'True' + }, "network_offering_vlan": { "name": 'Test Network offering', "displaytext": 'Test Network offering', "guestiptype": 'Isolated', "supportedservices": 'Dhcp,Dns,SourceNat,PortForwarding', "traffictype": 'GUEST', - "specifyvlan": 'False', + "specifyVlan": 'False', "availability": 'Optional', "serviceProviderList": { "Dhcp": 'VirtualRouter', @@ -338,6 +346,10 @@ "name": "Isolated Network", "displaytext": "Isolated Network" }, + "l2_network": { + "name": "L2 Network", + "displaytext": "L2 Network" + }, "netscaler_VPX": { "ipaddress": "10.223.240.174", "username": "nsroot", diff --git a/tools/marvin/marvin/lib/vcenter.py b/tools/marvin/marvin/lib/vcenter.py index 78ca50e19d87..916fb7e2d039 100644 --- a/tools/marvin/marvin/lib/vcenter.py +++ b/tools/marvin/marvin/lib/vcenter.py @@ -137,6 +137,8 @@ def parse_details(self, obj, vimtype): parsedObject['dvportgroup'] = Vcenter._parse_dvportgroup(obj) elif vim.VirtualMachine in vimtype: parsedObject['vm'] = Vcenter._parse_vm(obj) + elif vim.HostSystem in vimtype: + parsedObject['host'] = obj else: parsedObject['name'] = obj.name return parsedObject From 762df8c170efe945da293cfce4de93f0028a7453 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 19 Jan 2021 15:50:07 +0530 Subject: [PATCH 05/13] Include cleanup of n/w resources on n/w deletion + prevent n/w resource deletion on detroy/stop/unplug of VM from persistent n/w --- ...leanupPersistentNetworkResourceAnswer.java | 27 +++++++++ ...eanupPersistentNetworkResourceCommand.java | 43 +++++++++++++++ .../com/cloud/agent/api/UnPlugNicCommand.java | 11 ++++ .../cloud/vm/VirtualMachineManagerImpl.java | 5 +- .../orchestration/NetworkOrchestrator.java | 44 ++++++++++++++- .../discovery/ApiDiscoveryServiceImpl.java | 3 +- .../kvm/resource/BridgeVifDriver.java | 16 ++++++ .../kvm/resource/DirectVifDriver.java | 3 + .../hypervisor/kvm/resource/IvsVifDriver.java | 4 ++ .../hypervisor/kvm/resource/OvsVifDriver.java | 4 ++ .../hypervisor/kvm/resource/VifDriver.java | 2 + ...rsistentNetworkResourceCommandWrapper.java | 44 +++++++++++++++ .../wrapper/LibvirtStopCommandWrapper.java | 2 +- .../LibvirtUnPlugNicCommandWrapper.java | 25 ++++++++- .../resource/CitrixResourceBase.java | 6 +- .../resource/XenServer56Resource.java | 50 +++++++++-------- ...rsistentNetworkResourceCommandWrapper.java | 55 +++++++++++++++++++ .../xenbase/CitrixStopCommandWrapper.java | 20 ++++++- .../CitrixUnPlugNicCommandWrapper.java | 17 +++++- scripts/vm/network/vnet/modifyvlan.sh | 15 +++-- 20 files changed, 352 insertions(+), 44 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupPersistentNetworkResourceCommandWrapper.java create mode 100644 plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCleanupPersistentNetworkResourceCommandWrapper.java diff --git a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java new file mode 100644 index 000000000000..cf4ab618e644 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java @@ -0,0 +1,27 @@ +// 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 com.cloud.agent.api; + +public class CleanupPersistentNetworkResourceAnswer extends Answer{ + public CleanupPersistentNetworkResourceAnswer() { + } + + public CleanupPersistentNetworkResourceAnswer(CleanupPersistentNetworkResourceCommand cmd, boolean success, String result) { + super(cmd, success, result); + } +} \ No newline at end of file diff --git a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java new file mode 100644 index 000000000000..5daeae0fd8a0 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java @@ -0,0 +1,43 @@ +// 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 com.cloud.agent.api; + +import com.cloud.agent.api.to.NicTO; + +public class CleanupPersistentNetworkResourceCommand extends Command{ + NicTO nicTO; + + protected CleanupPersistentNetworkResourceCommand() {} + + public CleanupPersistentNetworkResourceCommand(NicTO nicTO) { + this.nicTO = nicTO; + } + + public NicTO getNicTO() { + return nicTO; + } + + public void setNicTO(NicTO nicTO) { + this.nicTO = nicTO; + } + + @Override + public boolean executeInSequence() { + return false; + } +} \ No newline at end of file diff --git a/core/src/main/java/com/cloud/agent/api/UnPlugNicCommand.java b/core/src/main/java/com/cloud/agent/api/UnPlugNicCommand.java index 984a0a4a44e9..febed0c01678 100644 --- a/core/src/main/java/com/cloud/agent/api/UnPlugNicCommand.java +++ b/core/src/main/java/com/cloud/agent/api/UnPlugNicCommand.java @@ -19,11 +19,14 @@ package com.cloud.agent.api; +import java.util.Map; + import com.cloud.agent.api.to.NicTO; public class UnPlugNicCommand extends Command { NicTO nic; String instanceName; + Map vlanToPersistenceMap; public NicTO getNic() { return nic; @@ -45,4 +48,12 @@ public UnPlugNicCommand(NicTO nic, String instanceName) { public String getVmName() { return instanceName; } + + public Map getVlanToPersistenceMap() { + return vlanToPersistenceMap; + } + + public void setVlanToPersistenceMap(Map vlanToPersistenceMap) { + this.vlanToPersistenceMap = vlanToPersistenceMap; + } } 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 7ce5f512eb20..175534d1c771 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1974,7 +1974,6 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl vmGuru.prepareStop(profile); Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); - final StopCommand stop = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), false, cleanUpEvenIfUnableToStop); stop.setControlIp(getControlNicIpForVM(vm)); if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { @@ -4419,6 +4418,10 @@ public boolean unplugNic(final Network network, final NicTO nic, final VirtualMa try { final Commands cmds = new Commands(Command.OnError.Stop); final UnPlugNicCommand unplugNicCmd = new UnPlugNicCommand(nic, vm.getName()); + Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); + if (MapUtils.isNotEmpty(vlanToPersistenceMap)) { + unplugNicCmd.setVlanToPersistenceMap(vlanToPersistenceMap); + } cmds.addCommand("unplugnic", unplugNicCmd); _agentMgr.send(dest.getHost().getId(), cmds); 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 05d8952b3cee..fa826dac8d06 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 @@ -40,6 +40,8 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.agent.api.CleanupPersistentNetworkResourceAnswer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand; import com.cloud.agent.api.SetupPersistentNetworkAnswer; import com.cloud.agent.api.SetupPersistentNetworkCommand; import com.cloud.dc.ClusterVO; @@ -1223,7 +1225,7 @@ private Pair isNtwConfiguredInCluster(HostVO hostVO, Map(); } - if (hostVO.getHypervisorType() == HypervisorType.KVM) { + if (hostVO.getHypervisorType() == HypervisorType.KVM || hostVO.getHypervisorType() == HypervisorType.XenServer ) { hosts.add(hostVO.getId()); clusterToHostsMap.put(clusterId, hosts); return new Pair<>(false, createNicTOFromNetworkAndOffering(networkVO, networkOfferingVO, hostVO)); @@ -1267,6 +1269,15 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin } } + private boolean networkMeetsPersistenceCriteria(NetworkVO network, NetworkOfferingVO offering, boolean cleanup) { + boolean criteriaMet = offering.isPersistent() && + (network.getBroadcastUri() != null && BroadcastDomainType.getSchemeValue(network.getBroadcastUri()) == BroadcastDomainType.Vlan); + if (!cleanup) { + return criteriaMet && network.getGuestType() == GuestType.L2; + } else { + return criteriaMet && (network.getGuestType() == GuestType.L2 || network.getGuestType() == GuestType.Isolated); + } + } @Override @DB public Pair implementNetwork(final long networkId, final DeployDestination dest, final ReservationContext context) throws ConcurrentOperationException, @@ -1326,8 +1337,7 @@ public Pair implementNetwork(final long networkId, final implementNetworkElementsAndResources(dest, context, network, offering); long dcId = dest.getDataCenter().getId(); - if (network.getGuestType() == GuestType.L2 && offering.isPersistent() && - (network.getBroadcastUri() != null && BroadcastDomainType.getSchemeValue(network.getBroadcastUri()) == BroadcastDomainType.Vlan)) { + if (networkMeetsPersistenceCriteria(network,offering, false)) { setupPersistentNetwork(network, offering, dcId); } if (isSharedNetworkWithServices(network)) { @@ -2948,6 +2958,32 @@ public boolean shutdownNetworkElementsAndResources(final ReservationContext cont return success; } + private void cleanupPersistentnNetworkResources(NetworkVO network) { + long networkOfferingId = network.getNetworkOfferingId(); + NetworkOfferingVO offering = _networkOfferingDao.findById(networkOfferingId); + if (offering != null) { + if (networkMeetsPersistenceCriteria(network, offering, true)) { + List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, network.getDataCenterId()); + for (HostVO host : hosts) { + try { + NicTO to = createNicTOFromNetworkAndOffering(network, offering, host); + CleanupPersistentNetworkResourceCommand cmd = new CleanupPersistentNetworkResourceCommand(to); + CleanupPersistentNetworkResourceAnswer answer = (CleanupPersistentNetworkResourceAnswer) _agentMgr.send(host.getId(), cmd); + if (answer == null) { + s_logger.warn("Unable to get an answer to the CleanupPersistentNetworkResourceCommand from agent:" + host.getId()); + } + + if (!answer.getResult()) { + s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); + } + } catch (Exception e) { + s_logger.warn("Failed to cleanup network resources"); + } + } + } + } + } + @Override @DB public boolean destroyNetwork(final long networkId, final ReservationContext context, final boolean forced) { @@ -2987,6 +3023,8 @@ public boolean destroyNetwork(final long networkId, final ReservationContext con } } + cleanupPersistentnNetworkResources(network); + // Shutdown network first shutdownNetwork(networkId, context, false); diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index c16d1b233244..5d2285629d65 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -67,7 +68,7 @@ public boolean start() { if (s_apiNameDiscoveryResponseMap == null) { long startTime = System.nanoTime(); s_apiNameDiscoveryResponseMap = new HashMap(); - Set> cmdClasses = new HashSet>(); + Set> cmdClasses = new LinkedHashSet>(); for (PluggableService service : _services) { s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName())); cmdClasses.addAll(service.getCommands()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index 34dc0dff17b4..acdbbb8a170f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -437,4 +437,20 @@ public boolean isExistingBridge(String bridgeName) { return false; } } + + @Override + public void deleteBr(NicTO nic) { + String vlanId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri()); + String trafficLabel = nic.getName(); + String pifName = _pifs.get(trafficLabel); + if (pifName == null) { + // if not found in bridge map, maybe traffic label refers to pif already? + File pif = new File("/sys/class/net/" + trafficLabel); + if (pif.isDirectory()) { + pifName = trafficLabel; + } + } + String brName = generateVnetBrName(pifName, vlanId); + deleteVnetBr(brName, true); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java index 9a2119b6773d..86a45ecc8aa2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/DirectVifDriver.java @@ -80,4 +80,7 @@ public void detach(LibvirtVMDef.InterfaceDef iface) { public void createControlNetwork(String privBrName) { } + @Override + public void deleteBr(NicTO nic) { + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java index b3dfc11349d2..306d70fc1601 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/IvsVifDriver.java @@ -287,6 +287,10 @@ public void createControlNetwork(String privBrName) { } } + @Override + public void deleteBr(NicTO nic) { + } + private boolean isBridgeExists(String bridgeName) { File f = new File("/sys/devices/virtual/net/" + bridgeName); if (f.exists()) { diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java index 71b3c5703f0b..d39ee0d4a995 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java @@ -260,4 +260,8 @@ public boolean isExistingBridge(String bridgeName) { return false; } } + + @Override + public void deleteBr(NicTO nic) { + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriver.java index 41fd0ad56600..72fb82967814 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriver.java @@ -44,4 +44,6 @@ public interface VifDriver { boolean isExistingBridge(String bridgeName); + void deleteBr(NicTO nic); + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupPersistentNetworkResourceCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupPersistentNetworkResourceCommandWrapper.java new file mode 100644 index 000000000000..ebc147a73a32 --- /dev/null +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupPersistentNetworkResourceCommandWrapper.java @@ -0,0 +1,44 @@ +// 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 com.cloud.hypervisor.kvm.resource.wrapper; + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceAnswer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand; +import com.cloud.agent.api.to.NicTO; +import com.cloud.hypervisor.kvm.resource.BridgeVifDriver; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.VifDriver; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; + +@ResourceWrapper(handles = CleanupPersistentNetworkResourceCommand.class) +public class LibvirtCleanupPersistentNetworkResourceCommandWrapper extends CommandWrapper { + private static final Logger s_logger = Logger.getLogger(LibvirtCleanupPersistentNetworkResourceCommandWrapper.class); + @Override + public Answer execute(CleanupPersistentNetworkResourceCommand command, LibvirtComputingResource serverResource) { + NicTO nic = command.getNicTO(); + VifDriver driver = serverResource.getVifDriver(nic.getType()); + if (driver instanceof BridgeVifDriver) { + driver.deleteBr(nic); + } + return new CleanupPersistentNetworkResourceAnswer(command, true, "Successfully deleted bridge"); + } +} diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index 509595f869e0..09e17c98d3cb 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -177,6 +177,6 @@ private boolean shouldDeleteBridge(Map vlanToPersistenceMap, St if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { return vlanToPersistenceMap.get(vlanId); } - return false; + return true; } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java index 7af9699df163..81b73731cdc2 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java @@ -20,7 +20,10 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import java.util.List; +import java.util.Map; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -45,6 +48,7 @@ public final class LibvirtUnPlugNicCommandWrapper extends CommandWrapper vlanToPersistenceMap = command.getVlanToPersistenceMap(); Domain vm = null; try { final LibvirtUtilitiesHelper libvirtUtilitiesHelper = libvirtComputingResource.getLibvirtUtilitiesHelper(); @@ -59,10 +63,11 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso libvirtComputingResource.destroyNetworkRulesForNic(conn, vmName, nic); } vm.detachDevice(pluggedNic.toString()); + String vlanId = getVlanIdFromBridgeName(pluggedNic.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(pluggedNic, true); + vifDriver.unplug(pluggedNic, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } return new UnPlugNicAnswer(command, true, "success"); } @@ -82,4 +87,22 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso } } } + + private String getVlanIdFromBridgeName(String brName) { + if (StringUtils.isNotBlank(brName)) { + String[] s = brName.split("-"); + if (s.length > 1) { + return s[1]; + } + return null; + } + return null; + } + + private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { + if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { + return vlanToPersistenceMap.get(vlanId); + } + return false; + } } diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index a26bcc486d08..ed6f7939dc82 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -678,7 +678,7 @@ protected ExecutionResult cleanupNetworkElementCommand(final IpAssocCommand cmd) correctVif.destroy(conn); // Disable the VLAN network if necessary - disableVlanNetwork(conn, network); + disableVlanNetwork(conn, network, true); } } } @@ -1574,7 +1574,7 @@ void destroyVDIbyNameLabel(final Connection conn, final String nameLabel) { } } - public void disableVlanNetwork(final Connection conn, final Network network) { + public void disableVlanNetwork(final Connection conn, final Network network, boolean deleteVlan) { } @Override @@ -3612,7 +3612,7 @@ public String handleVmStartFailure(final Connection conn, final String vmName, f } for (final Network network : networks) { if (network.getNameLabel(conn).startsWith("VLAN")) { - disableVlanNetwork(conn, network); + disableVlanNetwork(conn, network, true); } } } catch (final Exception e) { diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java index b7d0273675c6..0c2b2c1e6f26 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java @@ -37,38 +37,40 @@ protected String getPatchFilePath() { } @Override - public void disableVlanNetwork(final Connection conn, final Network network) { + public void disableVlanNetwork(final Connection conn, final Network network, boolean deleteVlan) { try { final Network.Record networkr = network.getRecord(conn); if (!networkr.nameLabel.startsWith("VLAN")) { return; } - final String bridge = networkr.bridge.trim(); - for (final PIF pif : networkr.PIFs) { - final PIF.Record pifr = pif.getRecord(conn); - if (!pifr.host.getUuid(conn).equalsIgnoreCase(_host.getUuid())) { - continue; - } - - final VLAN vlan = pifr.VLANMasterOf; - if (vlan != null) { - final String vlannum = pifr.VLAN.toString(); - final String device = pifr.device.trim(); - if (vlannum.equals("-1")) { - return; + if (deleteVlan) { + final String bridge = networkr.bridge.trim(); + for (final PIF pif : networkr.PIFs) { + final PIF.Record pifr = pif.getRecord(conn); + if (!pifr.host.getUuid(conn).equalsIgnoreCase(_host.getUuid())) { + continue; } - try { - vlan.destroy(conn); - final Host host = Host.getByUuid(conn, _host.getUuid()); - host.forgetDataSourceArchives(conn, "pif_" + bridge + "_tx"); - host.forgetDataSourceArchives(conn, "pif_" + bridge + "_rx"); - host.forgetDataSourceArchives(conn, "pif_" + device + "." + vlannum + "_tx"); - host.forgetDataSourceArchives(conn, "pif_" + device + "." + vlannum + "_rx"); - } catch (final XenAPIException e) { - s_logger.trace("Catch " + e.getClass().getName() + ": failed to destory VLAN " + device + " on host " + _host.getUuid() + " due to " + e.toString()); + + final VLAN vlan = pifr.VLANMasterOf; + if (vlan != null) { + final String vlannum = pifr.VLAN.toString(); + final String device = pifr.device.trim(); + if (vlannum.equals("-1")) { + return; + } + try { + vlan.destroy(conn); + final Host host = Host.getByUuid(conn, _host.getUuid()); + host.forgetDataSourceArchives(conn, "pif_" + bridge + "_tx"); + host.forgetDataSourceArchives(conn, "pif_" + bridge + "_rx"); + host.forgetDataSourceArchives(conn, "pif_" + device + "." + vlannum + "_tx"); + host.forgetDataSourceArchives(conn, "pif_" + device + "." + vlannum + "_rx"); + } catch (final XenAPIException e) { + s_logger.trace("Catch " + e.getClass().getName() + ": failed to destroy VLAN " + device + " on host " + _host.getUuid() + " due to " + e.toString()); + } } + return; } - return; } } catch (final XenAPIException e) { final String msg = "Unable to disable VLAN network due to " + e.toString(); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCleanupPersistentNetworkResourceCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCleanupPersistentNetworkResourceCommandWrapper.java new file mode 100644 index 000000000000..3be321cf6b8b --- /dev/null +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixCleanupPersistentNetworkResourceCommandWrapper.java @@ -0,0 +1,55 @@ +// 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 com.cloud.hypervisor.xenserver.resource.wrapper.xenbase; + +import org.apache.log4j.Logger; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceAnswer; +import com.cloud.agent.api.CleanupPersistentNetworkResourceCommand; +import com.cloud.agent.api.to.NicTO; +import com.cloud.hypervisor.xenserver.resource.CitrixResourceBase; +import com.cloud.hypervisor.xenserver.resource.XsHost; +import com.cloud.resource.CommandWrapper; +import com.cloud.resource.ResourceWrapper; +import com.xensource.xenapi.Connection; +import com.xensource.xenapi.Network; + +@ResourceWrapper(handles = CleanupPersistentNetworkResourceCommand.class) +public class CitrixCleanupPersistentNetworkResourceCommandWrapper extends CommandWrapper { + private static final Logger s_logger = Logger.getLogger(CitrixCleanupPersistentNetworkResourceCommandWrapper.class); + + @Override + public Answer execute(CleanupPersistentNetworkResourceCommand command, CitrixResourceBase citrixResourceBase) { + final Connection conn = citrixResourceBase.getConnection(); + final XsHost host = citrixResourceBase.getHost(); + NicTO nic = command.getNicTO(); + try { + Network network = citrixResourceBase.getNetwork(conn, nic); + if (network == null) { + return new CleanupPersistentNetworkResourceAnswer(command, false, "Failed to find network on host " + host.getIp() + " to cleanup"); + } + citrixResourceBase.disableVlanNetwork(conn, network, true); + return new CleanupPersistentNetworkResourceAnswer(command, true, "Successfully deleted network VLAN on host: "+ host.getIp()); + } catch (final Exception e) { + final String msg = " Failed to cleanup network VLAN on host: " + host.getIp() + " due to: " + e.toString(); + s_logger.error(msg, e); + return new CleanupPersistentNetworkResourceAnswer(command, false, msg); + } + } +} diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java index 42d7d3bcb01a..1a74ff4385bf 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java @@ -23,8 +23,10 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; import com.cloud.agent.api.Answer; @@ -53,6 +55,7 @@ public final class CitrixStopCommandWrapper extends CommandWrapper vlanToPersistenceMap = command.getVlanToPersistenceMap(); String platformstring = null; try { final Connection conn = citrixResourceBase.getConnection(); @@ -148,7 +151,8 @@ public Answer execute(final StopCommand command, final CitrixResourceBase citrix for (final Network network : networks) { try { if (network.getNameLabel(conn).startsWith("VLAN")) { - citrixResourceBase.disableVlanNetwork(conn, network); + String networkLabel = network.getNameLabel(conn); + citrixResourceBase.disableVlanNetwork(conn, network, shouldDeleteVlan(networkLabel, vlanToPersistenceMap)); } } catch (final Exception e) { // network might be destroyed by other host @@ -172,4 +176,18 @@ public Answer execute(final StopCommand command, final CitrixResourceBase citrix } return new StopAnswer(command, "Stop VM failed", platformstring, false); } + + private boolean shouldDeleteVlan(String networkLabel, Map vlanToPersistenceMap) { + String[] networkNameParts = null; + if (networkLabel.contains("-")) { + networkNameParts = networkLabel.split("-"); + } else { + networkNameParts = networkLabel.split("VLAN"); + } + String networkVlan = networkNameParts.length > 0 ? networkNameParts[networkNameParts.length - 1] : null; + if (networkVlan != null && MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanToPersistenceMap.containsKey(networkVlan)) { + return vlanToPersistenceMap.get(networkVlan); + } + return true; + } } \ No newline at end of file diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUnPlugNicCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUnPlugNicCommandWrapper.java index 2d6000b342a1..9dce0d109b86 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixUnPlugNicCommandWrapper.java @@ -19,9 +19,12 @@ package com.cloud.hypervisor.xenserver.resource.wrapper.xenbase; +import java.util.Map; import java.util.Set; +import org.apache.commons.collections.MapUtils; import org.apache.log4j.Logger; +import org.apache.xmlrpc.XmlRpcException; import com.cloud.agent.api.Answer; import com.cloud.agent.api.UnPlugNicAnswer; @@ -32,6 +35,7 @@ import com.cloud.resource.ResourceWrapper; import com.xensource.xenapi.Connection; import com.xensource.xenapi.Network; +import com.xensource.xenapi.Types; import com.xensource.xenapi.VIF; import com.xensource.xenapi.VM; @@ -44,6 +48,7 @@ public final class CitrixUnPlugNicCommandWrapper extends CommandWrapper vlanToPersistenceMap = command.getVlanToPersistenceMap(); try { final Set vms = VM.getByNameLabel(conn, vmName); if (vms == null || vms.isEmpty()) { @@ -59,7 +64,8 @@ public Answer execute(final UnPlugNicCommand command, final CitrixResourceBase c vif.destroy(conn); try { if (network.getNameLabel(conn).startsWith("VLAN")) { - citrixResourceBase.disableVlanNetwork(conn, network); + String networkLabel = network.getNameLabel(conn); + citrixResourceBase.disableVlanNetwork(conn, network, shouldDeleteVlan(networkLabel, vlanToPersistenceMap)); } } catch (final Exception e) { } @@ -71,4 +77,13 @@ public Answer execute(final UnPlugNicCommand command, final CitrixResourceBase c return new UnPlugNicAnswer(command, false, msg); } } + + private boolean shouldDeleteVlan(String networkLabel, Map vlanToPersistenceMap) throws XmlRpcException, Types.XenAPIException { + String[] networkNameParts = networkLabel.split("-"); + String networkVlan = networkNameParts[networkNameParts.length -1]; + if (MapUtils.isNotEmpty(vlanToPersistenceMap) && networkVlan != null && vlanToPersistenceMap.containsKey(networkVlan)) { + return vlanToPersistenceMap.get(networkVlan); + } + return true; + } } \ No newline at end of file diff --git a/scripts/vm/network/vnet/modifyvlan.sh b/scripts/vm/network/vnet/modifyvlan.sh index a7ec2568dc91..25008b154290 100755 --- a/scripts/vm/network/vnet/modifyvlan.sh +++ b/scripts/vm/network/vnet/modifyvlan.sh @@ -93,16 +93,15 @@ deleteVlan() { local vlanBr=$3 local deleteBr=$4 - ip link delete $vlanDev type vlan > /dev/null - - if [ $? -gt 0 ] - then - printf "Failed to del vlan: $vlanId" - return 1 - fi - if [ $deleteBr == "true" ] then + ip link delete $vlanDev type vlan > /dev/null + + if [ $? -gt 0 ] + then + printf "Failed to del vlan: $vlanId" + return 1 + fi ip link set $vlanBr down if [ $? -gt 0 ] From be0723201a8bd7284aa173fb3227fa8ce9abd729 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 20 Jan 2021 18:16:07 +0530 Subject: [PATCH 06/13] Add check to prevent deletion of n/w resources when last VM/non-persistent network is deleted but there exists another n/w with overlapping vlan which is persistent --- .../cloud/vm/VirtualMachineManagerImpl.java | 33 ++++++++++++++++--- .../orchestration/NetworkOrchestrator.java | 3 +- .../com/cloud/network/dao/NetworkDao.java | 2 ++ .../com/cloud/network/dao/NetworkDaoImpl.java | 25 ++++++++++++++ .../discovery/ApiDiscoveryServiceImpl.java | 3 +- .../kvm/resource/BridgeVifDriver.java | 2 +- .../wrapper/LibvirtMigrateCommandWrapper.java | 2 +- .../LibvirtUnPlugNicCommandWrapper.java | 2 +- .../com/cloud/vpc/dao/MockNetworkDaoImpl.java | 5 +++ 9 files changed, 67 insertions(+), 10 deletions(-) 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 175534d1c771..a33d253929a3 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -40,7 +40,9 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.api.query.dao.DomainRouterJoinDao; import com.cloud.api.query.dao.UserVmJoinDao; +import com.cloud.api.query.vo.DomainRouterJoinVO; import com.cloud.api.query.vo.UserVmJoinVO; import com.cloud.deployasis.dao.UserVmDeployAsIsDetailsDao; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; @@ -356,6 +358,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac private UserVmJoinDao userVmJoinDao; @Inject private NetworkOfferingDao networkOfferingDao; + @Inject + private DomainRouterJoinDao domainRouterJoinDao; VmWorkJobHandlerProxy _jobHandlerProxy = new VmWorkJobHandlerProxy(this); @@ -1844,14 +1848,29 @@ private void orchestrateStop(final String vmUuid, final boolean cleanUpEvenIfUna advanceStop(vm, cleanUpEvenIfUnableToStop); } + private void getPersistenceMap(Map vlanToPersistenceMap, NetworkVO networkVO) { + NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId()); + Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); + Boolean shouldDeleteNwResource = MapUtils.isNotEmpty(vlanToPersistenceMap) ? vlanToPersistenceMap.get(data.first()) : null; + if (shouldDeleteNwResource == null || shouldDeleteNwResource){ + vlanToPersistenceMap.put(data.first(), data.second()); + } + } + private Map getVlanToPersistenceMapForVM(long vmId) { List userVmJoinVOS = userVmJoinDao.searchByIds(vmId); Map vlanToPersistenceMap = new HashMap<>(); for (UserVmJoinVO userVmJoinVO : userVmJoinVOS) { NetworkVO networkVO = _networkDao.findById(userVmJoinVO.getNetworkId()); - NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId()); - Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); - vlanToPersistenceMap.put(data.first(), data.second()); + getPersistenceMap(vlanToPersistenceMap, networkVO); + } + if (userVmJoinVOS.isEmpty()) { + VMInstanceVO vmInstanceVO = _vmDao.findById(vmId); + if (vmInstanceVO.getType() == VirtualMachine.Type.DomainRouter) { + DomainRouterJoinVO routerVO = domainRouterJoinDao.findById(vmId); + NetworkVO networkVO = _networkDao.findById(routerVO.getNetworkId()); + getPersistenceMap(vlanToPersistenceMap, networkVO); + } } return vlanToPersistenceMap; } @@ -1869,9 +1888,15 @@ private Pair getVMNetworkDetails(NetworkVO networkVO, boolean i URI broadcastUri = networkVO.getBroadcastUri(); String scheme = broadcastUri.getScheme(); String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); - Boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && + boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && (scheme.equalsIgnoreCase("vlan")) && isPersistent); + if (shouldDelete) { + int persistentNetworksCount = _networkDao.getOtherPersistentNetworksCount(networkVO.getId(), networkVO.getBroadcastUri().toString(), true); + if (persistentNetworksCount > 0) { + shouldDelete = false; + } + } return new Pair<>(vlanId, shouldDelete); } 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 fa826dac8d06..4e74cf925bfa 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 @@ -2962,7 +2962,8 @@ private void cleanupPersistentnNetworkResources(NetworkVO network) { long networkOfferingId = network.getNetworkOfferingId(); NetworkOfferingVO offering = _networkOfferingDao.findById(networkOfferingId); if (offering != null) { - if (networkMeetsPersistenceCriteria(network, offering, true)) { + if (networkMeetsPersistenceCriteria(network, offering, true) && + _networksDao.getOtherPersistentNetworksCount(network.getId(), network.getBroadcastUri().toString(), offering.isPersistent()) == 0) { List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, network.getDataCenterId()); for (HostVO host : hosts) { try { diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java index c82e4e745999..f51e3f8775db 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDao.java @@ -45,6 +45,8 @@ public interface NetworkDao extends GenericDao, StateDao getNetworksForOffering(long offeringId, long dataCenterId, long accountId); + int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent); + @Override @Deprecated NetworkVO persist(NetworkVO vo); diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java index 2578a14fc70a..a6f9b5df12dc 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java @@ -79,6 +79,7 @@ public class NetworkDaoImpl extends GenericDaoBaseimplements Ne SearchBuilder SourceNATSearch; GenericSearchBuilder VpcNetworksCount; SearchBuilder OfferingAccountNetworkSearch; + SearchBuilder PersistentNetworkSearch; GenericSearchBuilder GarbageCollectedSearch; SearchBuilder PrivateNetworkSearch; @@ -181,6 +182,16 @@ protected void init() { CountBy.join("offerings", ntwkOffJoin, CountBy.entity().getNetworkOfferingId(), ntwkOffJoin.entity().getId(), JoinBuilder.JoinType.INNER); CountBy.done(); + PersistentNetworkSearch = createSearchBuilder(); + PersistentNetworkSearch.and("id", PersistentNetworkSearch.entity().getId(), Op.NEQ); + PersistentNetworkSearch.and("guestType", PersistentNetworkSearch.entity().getGuestType(), Op.IN); + PersistentNetworkSearch.and("broadcastUri", PersistentNetworkSearch.entity().getBroadcastUri(), Op.EQ); + PersistentNetworkSearch.and("removed", PersistentNetworkSearch.entity().getRemoved(), Op.NULL); + final SearchBuilder persistentNtwkOffJoin = _ntwkOffDao.createSearchBuilder(); + persistentNtwkOffJoin.and("persistent", persistentNtwkOffJoin.entity().isPersistent(), Op.EQ); + PersistentNetworkSearch.join("persistent", persistentNtwkOffJoin, PersistentNetworkSearch.entity().getNetworkOfferingId(), persistentNtwkOffJoin.entity().getId(), JoinType.INNER); + PersistentNetworkSearch.done(); + PhysicalNetworkSearch = createSearchBuilder(); PhysicalNetworkSearch.and("physicalNetworkId", PhysicalNetworkSearch.entity().getPhysicalNetworkId(), Op.EQ); PhysicalNetworkSearch.done(); @@ -265,6 +276,8 @@ protected void init() { join10.and("vpc", join10.entity().getVpcId(), Op.EQ); PrivateNetworkSearch.join("vpcgateways", join10, PrivateNetworkSearch.entity().getId(), join10.entity().getNetworkId(), JoinBuilder.JoinType.INNER); PrivateNetworkSearch.done(); + + } @Override @@ -390,6 +403,18 @@ public List getNetworksForOffering(final long offeringId, final long return search(sc, null); } + @Override + public int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent) { + Object[] guestTypes = {"Isolated", "L2"}; + final SearchCriteria sc = PersistentNetworkSearch.create(); + sc.setParameters("id", id); + sc.setParameters("broadcastUri", broadcastURI); + sc.setParameters("guestType", guestTypes); + sc.setJoinParameters("persistent", "persistent", isPersistent); + List persistentNetworks = search(sc, null); + return persistentNetworks.size(); + } + @Override public String getNextAvailableMacAddress(final long networkConfigId, Integer zoneMacIdentifier) { final SequenceFetcher fetch = SequenceFetcher.getInstance(); diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index 5d2285629d65..c16d1b233244 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -68,7 +67,7 @@ public boolean start() { if (s_apiNameDiscoveryResponseMap == null) { long startTime = System.nanoTime(); s_apiNameDiscoveryResponseMap = new HashMap(); - Set> cmdClasses = new LinkedHashSet>(); + Set> cmdClasses = new HashSet>(); for (PluggableService service : _services) { s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName())); cmdClasses.addAll(service.getCommands()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index acdbbb8a170f..e3db308599cf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -289,7 +289,7 @@ private String generateVxnetBrName(String pifName, String vnetId) { return "brvx-" + vnetId; } - public String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException { + private String createVnetBr(String vNetId, String pifKey, String protocol) throws InternalErrorException { String nic = _pifs.get(pifKey); if (nic == null) { // if not found in bridge map, maybe traffic label refers to pif already? diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index 7d2626c5e76a..efc702c36be4 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -305,7 +305,7 @@ private boolean shouldDeleteBridge(Map vlanToPersistenceMap, St if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { return vlanToPersistenceMap.get(vlanId); } - return false; + return true; } /** diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java index 81b73731cdc2..2b760d7bd045 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java @@ -103,6 +103,6 @@ private boolean shouldDeleteBridge(Map vlanToPersistenceMap, St if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { return vlanToPersistenceMap.get(vlanId); } - return false; + return true; } } diff --git a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java index a20ac6f5b706..405fd7419de1 100644 --- a/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java +++ b/server/src/test/java/com/cloud/vpc/dao/MockNetworkDaoImpl.java @@ -74,6 +74,11 @@ public List getNetworksForOffering(final long offeringId, final long return null; } + @Override + public int getOtherPersistentNetworksCount(long id, String broadcastURI, boolean isPersistent) { + return 0; + } + @Override public String getNextAvailableMacAddress(final long networkConfigId, Integer zoneMacIdentifier) { return null; From 4faa06538424880aae55700878e5993b447697af Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 21 Jan 2021 13:15:49 +0530 Subject: [PATCH 07/13] Added additional marvin tests + defensive checks --- .../cloud/vm/VirtualMachineManagerImpl.java | 39 +++++++++-------- .../smoke/test_persistent_network.py | 43 +++++++++++++++++++ 2 files changed, 65 insertions(+), 17 deletions(-) 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 a33d253929a3..01f8c536697e 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1850,10 +1850,12 @@ private void orchestrateStop(final String vmUuid, final boolean cleanUpEvenIfUna private void getPersistenceMap(Map vlanToPersistenceMap, NetworkVO networkVO) { NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId()); - Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); - Boolean shouldDeleteNwResource = MapUtils.isNotEmpty(vlanToPersistenceMap) ? vlanToPersistenceMap.get(data.first()) : null; - if (shouldDeleteNwResource == null || shouldDeleteNwResource){ - vlanToPersistenceMap.put(data.first(), data.second()); + if (offeringVO != null) { + Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); + Boolean shouldDeleteNwResource = (MapUtils.isNotEmpty(vlanToPersistenceMap) && data != null) ? vlanToPersistenceMap.get(data.first()) : null; + if (data != null && (shouldDeleteNwResource == null || shouldDeleteNwResource)) { + vlanToPersistenceMap.put(data.first(), data.second()); + } } } @@ -1866,7 +1868,7 @@ private Map getVlanToPersistenceMapForVM(long vmId) { } if (userVmJoinVOS.isEmpty()) { VMInstanceVO vmInstanceVO = _vmDao.findById(vmId); - if (vmInstanceVO.getType() == VirtualMachine.Type.DomainRouter) { + if (vmInstanceVO != null && vmInstanceVO.getType() == VirtualMachine.Type.DomainRouter) { DomainRouterJoinVO routerVO = domainRouterJoinDao.findById(vmId); NetworkVO networkVO = _networkDao.findById(routerVO.getNetworkId()); getPersistenceMap(vlanToPersistenceMap, networkVO); @@ -1886,18 +1888,21 @@ private Map getVlanToPersistenceMapForVM(long vmId) { */ private Pair getVMNetworkDetails(NetworkVO networkVO, boolean isPersistent) { URI broadcastUri = networkVO.getBroadcastUri(); - String scheme = broadcastUri.getScheme(); - String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); - boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && - (scheme.equalsIgnoreCase("vlan")) - && isPersistent); - if (shouldDelete) { - int persistentNetworksCount = _networkDao.getOtherPersistentNetworksCount(networkVO.getId(), networkVO.getBroadcastUri().toString(), true); - if (persistentNetworksCount > 0) { - shouldDelete = false; - } - } - return new Pair<>(vlanId, shouldDelete); + if (broadcastUri != null) { + String scheme = broadcastUri.getScheme(); + String vlanId = Networks.BroadcastDomainType.getValue(broadcastUri); + boolean shouldDelete = !((networkVO.getGuestType() == Network.GuestType.L2 || networkVO.getGuestType() == Network.GuestType.Isolated) && + (scheme != null && scheme.equalsIgnoreCase("vlan")) + && isPersistent); + if (shouldDelete) { + int persistentNetworksCount = _networkDao.getOtherPersistentNetworksCount(networkVO.getId(), networkVO.getBroadcastUri().toString(), true); + if (persistentNetworksCount > 0) { + shouldDelete = false; + } + } + return new Pair<>(vlanId, shouldDelete); + } + return null; } private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnableToStop) throws AgentUnavailableException, OperationTimedoutException, diff --git a/test/integration/smoke/test_persistent_network.py b/test/integration/smoke/test_persistent_network.py index a5639bae7169..cf14b04b5635 100644 --- a/test/integration/smoke/test_persistent_network.py +++ b/test/integration/smoke/test_persistent_network.py @@ -330,6 +330,49 @@ def test_02_L2_persistent_network(self): network_vlan, "vlan must not be null for persistent network") + self.validate_persistent_network_resources_created_on_host(network_vlan) + + @attr(tags=["advanced", "l2", "persistent", "network"], required_hardware="false") + def test_03_deploy_and_destroy_VM_and_verify_network_resources_persist(self): + network_vlan = 99 + network = Network.create( + self.apiclient, + self.services["l2_network"], + networkofferingid=self.l2_persistent_network_offering.id, + zoneid=self.zone.id, + vlan=network_vlan) + self.cleanup.append(network) + response = verifyNetworkState( + self.apiclient, + network.id, + "implemented") + logger.debug(response) + exceptionOccured = response[0] + isNetworkInDesiredState = response[1] + exceptionMessage = response[2] + + if (exceptionOccured or (not isNetworkInDesiredState)): + self.fail(exceptionMessage) + self.assertIsNotNone( + network_vlan, + "vlan must not be null for persistent network") + try: + virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["virtual_machine"], + networkids=[ + network.id], + serviceofferingid=self.service_offering.id) + + VirtualMachine.delete(virtual_machine, self.apiclient, expunge=True) + + self.validate_persistent_network_resources_created_on_host(network_vlan) + except Exception as e: + self.fail("Exception occurred: %s" % e) + return + + + def validate_persistent_network_resources_created_on_host(self, network_vlan): hosts = self.list_all_hosts_in_zone(self.zone.id) if self.hypervisor.lower() in "kvm": for host in hosts: From b5e8cd16e27e994f7aac93f03ca781566bae9665 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 18 Jan 2021 17:57:58 +0530 Subject: [PATCH 08/13] API discovery: Prevent overwrite of API parameters in case the API names are the same --- .../apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index c16d1b233244..5d2285629d65 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -67,7 +68,7 @@ public boolean start() { if (s_apiNameDiscoveryResponseMap == null) { long startTime = System.nanoTime(); s_apiNameDiscoveryResponseMap = new HashMap(); - Set> cmdClasses = new HashSet>(); + Set> cmdClasses = new LinkedHashSet>(); for (PluggableService service : _services) { s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName())); cmdClasses.addAll(service.getCommands()); From d4c74df80b0dea2b9172de45295c230be3863b56 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 27 Jan 2021 11:17:18 +0530 Subject: [PATCH 09/13] Revert changes - async cmd --- .../user/network/CreateNetworkCmd.java | 15 +------ .../orchestration/NetworkOrchestrator.java | 42 ++++++++++--------- ui/public/locales/en.json | 1 + ui/src/views/network/VpcTiersTab.vue | 34 +++++++++++++-- 4 files changed, 57 insertions(+), 35 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java index 87e092917b5f..5ba6c9e44eef 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java @@ -16,7 +16,7 @@ // under the License. package org.apache.cloudstack.api.command.user.network; -import org.apache.cloudstack.api.BaseAsyncCmd; +import org.apache.cloudstack.api.BaseCmd; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.RoleType; @@ -37,7 +37,6 @@ import org.apache.cloudstack.api.response.ZoneResponse; import org.apache.cloudstack.context.CallContext; -import com.cloud.event.EventTypes; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; @@ -49,7 +48,7 @@ @APICommand(name = "createNetwork", description = "Creates a network", responseObject = NetworkResponse.class, responseView = ResponseView.Restricted, entityType = {Network.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) -public class CreateNetworkCmd extends BaseAsyncCmd implements UserCmd { +public class CreateNetworkCmd extends BaseCmd implements UserCmd { public static final Logger s_logger = Logger.getLogger(CreateNetworkCmd.class.getName()); private static final String s_name = "createnetworkresponse"; @@ -327,14 +326,4 @@ void execute() throws InsufficientCapacityException, ConcurrentOperationExceptio throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to create network"); } } - - @Override - public String getEventType() { - return EventTypes.EVENT_NETWORK_CREATE; - } - - @Override - public String getEventDescription() { - return "Creating network"; - } } 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 4e74cf925bfa..da09a2251352 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 @@ -1244,28 +1244,32 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin Map> clusterToHostsMap = new HashMap<>(); for (HostVO host : hosts) { - Pair networkCfgStateAndDetails = isNtwConfiguredInCluster(host, clusterToHostsMap, network, offering); - if (networkCfgStateAndDetails.first()) { - continue; - } - NicTO to = networkCfgStateAndDetails.second(); - SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); - final SetupPersistentNetworkAnswer answer = (SetupPersistentNetworkAnswer)_agentMgr.send(host.getId(), cmd); + try { + Pair networkCfgStateAndDetails = isNtwConfiguredInCluster(host, clusterToHostsMap, network, offering); + if (networkCfgStateAndDetails.first()) { + continue; + } + NicTO to = networkCfgStateAndDetails.second(); + SetupPersistentNetworkCommand cmd = new SetupPersistentNetworkCommand(to); + final SetupPersistentNetworkAnswer answer = (SetupPersistentNetworkAnswer) _agentMgr.send(host.getId(), cmd); - if (answer == null) { - s_logger.warn("Unable to get an answer to the SetupPersistentNetworkCommand from agent:" + host.getId()); - clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); - continue; - } + if (answer == null) { + s_logger.warn("Unable to get an answer to the SetupPersistentNetworkCommand from agent:" + host.getId()); + clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); + continue; + } - if (!answer.getResult()) { - s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); - clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); - continue; + if (!answer.getResult()) { + s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); + clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); + continue; + } + } catch (Exception e) { + s_logger.warn("Failed to connect to host: "+ host.getName()); } } if (clusterToHostsMap.keySet().size() != clusterVOS.size()) { - throw new CloudRuntimeException("Failed to setup persistent network across all hosts"); + s_logger.warn("Not all hosts have been configured with network devices."); } } @@ -1352,10 +1356,10 @@ public Pair implementNetwork(final long networkId, final return implemented; } catch (final NoTransitionException e) { s_logger.error(e.getMessage()); - return null; + return new Pair(null, null); } catch (final CloudRuntimeException | OperationTimedoutException e) { s_logger.error("Caught exception: " + e.getMessage()); - return null; + return new Pair(null, null); } finally { if (implemented.first() == null) { s_logger.debug("Cleaning up because we're unable to implement the network " + network); diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index 5b1cc6a95178..30fd06fe1c3c 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -625,6 +625,7 @@ "label.create.ssh.key.pair": "Create a SSH Key Pair", "label.create.template": "Create template", "label.create.user": "Create user", +"label.create.vpc.tier": "Create VPC tier", "label.create.vpn.connection": "Create VPN Connection", "label.created": "Created", "label.created.by.system": "Created by system", diff --git a/ui/src/views/network/VpcTiersTab.vue b/ui/src/views/network/VpcTiersTab.vue index bfadf7581ecb..27b6b2d72651 100644 --- a/ui/src/views/network/VpcTiersTab.vue +++ b/ui/src/views/network/VpcTiersTab.vue @@ -168,12 +168,26 @@ + v-decorator="['networkOffering',{rules: [{ required: true, message: `${$t('label.required')}` }]}]" + @change="val => { this.handleNetworkOfferingChange(val) }"> {{ item.name }} + + + {{ $t('label.vlan') }} + + + + + + 0 && obj.constructor === Object) + }, showIlb (network) { return network.service.filter(s => (s.name === 'Lb') && (s.capability.filter(c => c.name === 'LbSchemes' && c.value === 'Internal').length > 0)).length > 0 || false }, @@ -427,6 +445,7 @@ export default { networkOffering: this.networkOfferings[0].id }) }) + this.selectedNetworkOffering = this.networkOfferings[0] }).catch(error => { this.$notifyError(error) }).finally(() => { @@ -464,6 +483,9 @@ export default { this.fetchLoading = false }) }, + handleNetworkOfferingChange (networkOfferingId) { + this.selectedNetworkOffering = this.networkOfferings.filter(offering => offering.id === networkOfferingId)[0] + }, closeModal () { this.$emit('close-action') }, @@ -488,7 +510,7 @@ export default { } this.showCreateNetworkModal = false - api('createNetwork', { + var params = { vpcid: this.resource.id, domainid: this.resource.domainid, account: this.resource.account, @@ -500,7 +522,13 @@ export default { zoneId: this.resource.zoneid, externalid: values.externalId, aclid: values.acl - }).then(() => { + } + + if (values.vlan) { + params.vlan = values.vlan + } + + api('createNetwork', params).then(() => { this.$notification.success({ message: this.$t('message.success.add.vpc.network') }) From 56716a3f1f0fadf7dab3f101d3c59d29cd1de199 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Wed, 27 Jan 2021 15:03:02 +0530 Subject: [PATCH 10/13] changed log message --- .../cloudstack/engine/orchestration/NetworkOrchestrator.java | 2 +- ui/src/views/network/VpcTiersTab.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 da09a2251352..79dcb6afb769 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 @@ -1269,7 +1269,7 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin } } if (clusterToHostsMap.keySet().size() != clusterVOS.size()) { - s_logger.warn("Not all hosts have been configured with network devices."); + s_logger.warn("Hosts on all clusters may not have been configured with network devices."); } } diff --git a/ui/src/views/network/VpcTiersTab.vue b/ui/src/views/network/VpcTiersTab.vue index 27b6b2d72651..07f4c8494422 100644 --- a/ui/src/views/network/VpcTiersTab.vue +++ b/ui/src/views/network/VpcTiersTab.vue @@ -178,7 +178,7 @@ {{ $t('label.vlan') }} - + From 792dda1cc4ddb03b870f2acf51b238618c53c44d Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 28 Jan 2021 15:17:41 +0530 Subject: [PATCH 11/13] Address review comments --- .../api/command/user/network/CreateNetworkCmd.java | 2 +- .../agent/api/CleanupPersistentNetworkResourceAnswer.java | 2 +- .../agent/api/CleanupPersistentNetworkResourceCommand.java | 2 +- core/src/main/java/com/cloud/agent/api/MigrateCommand.java | 2 +- .../com/cloud/agent/api/SetupPersistentNetworkAnswer.java | 2 +- .../com/cloud/agent/api/SetupPersistentNetworkCommand.java | 2 +- .../engine/orchestration/NetworkOrchestrator.java | 2 +- .../cloudstack/discovery/ApiDiscoveryServiceImpl.java | 3 +-- .../com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java | 6 ++++-- 9 files changed, 12 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java index 5ba6c9e44eef..5dff7c9bf2bb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/network/CreateNetworkCmd.java @@ -16,13 +16,13 @@ // under the License. package org.apache.cloudstack.api.command.user.network; -import org.apache.cloudstack.api.BaseCmd; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.ApiErrorCode; +import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ResponseObject.ResponseView; import org.apache.cloudstack.api.ServerApiException; diff --git a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java index cf4ab618e644..f17ab910db5a 100644 --- a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceAnswer.java @@ -17,7 +17,7 @@ package com.cloud.agent.api; -public class CleanupPersistentNetworkResourceAnswer extends Answer{ +public class CleanupPersistentNetworkResourceAnswer extends Answer { public CleanupPersistentNetworkResourceAnswer() { } diff --git a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java index 5daeae0fd8a0..354a9c497892 100644 --- a/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CleanupPersistentNetworkResourceCommand.java @@ -19,7 +19,7 @@ import com.cloud.agent.api.to.NicTO; -public class CleanupPersistentNetworkResourceCommand extends Command{ +public class CleanupPersistentNetworkResourceCommand extends Command { NicTO nicTO; protected CleanupPersistentNetworkResourceCommand() {} diff --git a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java index 48328b1eba4e..a6ab45973e2e 100644 --- a/core/src/main/java/com/cloud/agent/api/MigrateCommand.java +++ b/core/src/main/java/com/cloud/agent/api/MigrateCommand.java @@ -40,7 +40,7 @@ public class MigrateCommand extends Command { private boolean executeInSequence = false; private List migrateDiskInfoList = new ArrayList<>(); private Map dpdkInterfaceMapping = new HashMap<>(); - Map vlanToPersistenceMap; + Map vlanToPersistenceMap = new HashMap<>(); public Map getDpdkInterfaceMapping() { return dpdkInterfaceMapping; diff --git a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java index 07867c7f93d4..8f13d715f8c4 100644 --- a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkAnswer.java @@ -17,7 +17,7 @@ package com.cloud.agent.api; -public class SetupPersistentNetworkAnswer extends Answer{ +public class SetupPersistentNetworkAnswer extends Answer { public SetupPersistentNetworkAnswer(){} public SetupPersistentNetworkAnswer(SetupPersistentNetworkCommand cmd, boolean success, String result) { diff --git a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java index 541eddd6b798..26e51b2d9635 100644 --- a/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java +++ b/core/src/main/java/com/cloud/agent/api/SetupPersistentNetworkCommand.java @@ -19,7 +19,7 @@ import com.cloud.agent.api.to.NicTO; -public class SetupPersistentNetworkCommand extends Command{ +public class SetupPersistentNetworkCommand extends Command { NicTO nic; 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 79dcb6afb769..8382a1214e63 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 @@ -2982,7 +2982,7 @@ private void cleanupPersistentnNetworkResources(NetworkVO network) { s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); } } catch (Exception e) { - s_logger.warn("Failed to cleanup network resources"); + s_logger.warn("Failed to cleanup network resources on host: "+ host.getName()); } } } diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index 5d2285629d65..c16d1b233244 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -20,7 +20,6 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -68,7 +67,7 @@ public boolean start() { if (s_apiNameDiscoveryResponseMap == null) { long startTime = System.nanoTime(); s_apiNameDiscoveryResponseMap = new HashMap(); - Set> cmdClasses = new LinkedHashSet>(); + Set> cmdClasses = new HashSet>(); for (PluggableService service : _services) { s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName())); cmdClasses.addAll(service.getCommands()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index e3db308599cf..f298670492f7 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -450,7 +450,9 @@ public void deleteBr(NicTO nic) { pifName = trafficLabel; } } - String brName = generateVnetBrName(pifName, vlanId); - deleteVnetBr(brName, true); + if (vlanId != null && pifName != null) { + String brName = generateVnetBrName(pifName, vlanId); + deleteVnetBr(brName, true); + } } } From 4500a2dc02ac73e4828d42adbf9724d128dbcf61 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 1 Feb 2021 10:32:34 +0530 Subject: [PATCH 12/13] Address review comments --- .../cloud/vm/VirtualMachineManagerImpl.java | 17 ++++++------- .../orchestration/NetworkOrchestrator.java | 6 ++--- .../com/cloud/network/dao/NetworkDaoImpl.java | 2 -- .../resource/LibvirtComputingResource.java | 18 ++++++++++++++ .../wrapper/LibvirtMigrateCommandWrapper.java | 22 ++--------------- .../wrapper/LibvirtStopCommandWrapper.java | 23 ++---------------- .../LibvirtUnPlugNicCommandWrapper.java | 24 ++----------------- .../vmware/resource/VmwareResource.java | 2 +- 8 files changed, 37 insertions(+), 77 deletions(-) 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 01f8c536697e..57a4b205d646 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1848,7 +1848,7 @@ private void orchestrateStop(final String vmUuid, final boolean cleanUpEvenIfUna advanceStop(vm, cleanUpEvenIfUnableToStop); } - private void getPersistenceMap(Map vlanToPersistenceMap, NetworkVO networkVO) { + private void updatePersistenceMap(Map vlanToPersistenceMap, NetworkVO networkVO) { NetworkOfferingVO offeringVO = networkOfferingDao.findById(networkVO.getNetworkOfferingId()); if (offeringVO != null) { Pair data = getVMNetworkDetails(networkVO, offeringVO.isPersistent()); @@ -1860,18 +1860,19 @@ private void getPersistenceMap(Map vlanToPersistenceMap, Networ } private Map getVlanToPersistenceMapForVM(long vmId) { - List userVmJoinVOS = userVmJoinDao.searchByIds(vmId); + List userVmJoinVOs = userVmJoinDao.searchByIds(vmId); Map vlanToPersistenceMap = new HashMap<>(); - for (UserVmJoinVO userVmJoinVO : userVmJoinVOS) { - NetworkVO networkVO = _networkDao.findById(userVmJoinVO.getNetworkId()); - getPersistenceMap(vlanToPersistenceMap, networkVO); - } - if (userVmJoinVOS.isEmpty()) { + if (userVmJoinVOs != null && !userVmJoinVOs.isEmpty()) { + for (UserVmJoinVO userVmJoinVO : userVmJoinVOs) { + NetworkVO networkVO = _networkDao.findById(userVmJoinVO.getNetworkId()); + updatePersistenceMap(vlanToPersistenceMap, networkVO); + } + } else { VMInstanceVO vmInstanceVO = _vmDao.findById(vmId); if (vmInstanceVO != null && vmInstanceVO.getType() == VirtualMachine.Type.DomainRouter) { DomainRouterJoinVO routerVO = domainRouterJoinDao.findById(vmId); NetworkVO networkVO = _networkDao.findById(routerVO.getNetworkId()); - getPersistenceMap(vlanToPersistenceMap, networkVO); + updatePersistenceMap(vlanToPersistenceMap, networkVO); } } return vlanToPersistenceMap; 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 8382a1214e63..c85e60f85ccc 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 @@ -1239,7 +1239,7 @@ private Pair isNtwConfiguredInCluster(HostVO hostVO, Map clusterVOS = clusterDao.listClustersByDcId(dcId); + List clusterVOs = clusterDao.listClustersByDcId(dcId); List hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByType(Host.Type.Routing, dcId); Map> clusterToHostsMap = new HashMap<>(); @@ -1262,13 +1262,12 @@ private void setupPersistentNetwork(NetworkVO network, NetworkOfferingVO offerin if (!answer.getResult()) { s_logger.warn("Unable to setup agent " + host.getId() + " due to " + answer.getDetails()); clusterToHostsMap.get(host.getClusterId()).remove(host.getId()); - continue; } } catch (Exception e) { s_logger.warn("Failed to connect to host: "+ host.getName()); } } - if (clusterToHostsMap.keySet().size() != clusterVOS.size()) { + if (clusterToHostsMap.keySet().size() != clusterVOs.size()) { s_logger.warn("Hosts on all clusters may not have been configured with network devices."); } } @@ -2976,6 +2975,7 @@ private void cleanupPersistentnNetworkResources(NetworkVO network) { CleanupPersistentNetworkResourceAnswer answer = (CleanupPersistentNetworkResourceAnswer) _agentMgr.send(host.getId(), cmd); if (answer == null) { s_logger.warn("Unable to get an answer to the CleanupPersistentNetworkResourceCommand from agent:" + host.getId()); + continue; } if (!answer.getResult()) { diff --git a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java index a6f9b5df12dc..a27a22dc147a 100644 --- a/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/network/dao/NetworkDaoImpl.java @@ -276,8 +276,6 @@ protected void init() { join10.and("vpc", join10.entity().getVpcId(), Op.EQ); PrivateNetworkSearch.join("vpcgateways", join10, PrivateNetworkSearch.entity().getId(), join10.entity().getNetworkId(), JoinBuilder.JoinType.INNER); PrivateNetworkSearch.done(); - - } @Override diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index 210bac1d0395..00c67c965668 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -4189,6 +4189,24 @@ public List> cleanVMSnapshotMetadata(Domain dm) return vmsnapshots; } + public String getVlanIdFromBridgeName(String brName) { + if (org.apache.commons.lang.StringUtils.isNotBlank(brName)) { + String[] s = brName.split("-"); + if (s.length > 1) { + return s[1]; + } + return null; + } + return null; + } + + public boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { + if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { + return vlanToPersistenceMap.get(vlanId); + } + return true; + } + private static String getTagValue(String tag, Element eElement) { NodeList nlList = eElement.getElementsByTagName(tag).item(0).getChildNodes(); Node nValue = nlList.item(0); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index efc702c36be4..a16836283a91 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -277,12 +277,12 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. } else { libvirtComputingResource.destroyNetworkRulesForVM(conn, vmName); for (final InterfaceDef iface : ifaces) { - String vlanId = getVlanIdFromBridgeName(iface.getBrName()); + String vlanId = libvirtComputingResource.getVlanIdFromBridgeName(iface.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers final List allVifDrivers = libvirtComputingResource.getAllVifDrivers(); for (final VifDriver vifDriver : allVifDrivers) { - vifDriver.unplug(iface, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); + vifDriver.unplug(iface, libvirtComputingResource.shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -290,24 +290,6 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. return new MigrateAnswer(command, result == null, result, null); } - private String getVlanIdFromBridgeName(String brName) { - if (StringUtils.isNotBlank(brName)) { - String[] s = brName.split("-"); - if (s.length > 1) { - return s[1]; - } - return null; - } - return null; - } - - private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { - if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { - return vlanToPersistenceMap.get(vlanId); - } - return true; - } - /** * Replace DPDK source path and target before migrations */ diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java index 09e17c98d3cb..ec243475a200 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStopCommandWrapper.java @@ -30,7 +30,6 @@ import com.cloud.utils.ssh.SshHelper; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -126,11 +125,11 @@ public Answer execute(final StopCommand command, final LibvirtComputingResource } } else { for (final InterfaceDef iface : ifaces) { - String vlanId = getVlanIdFromBridgeName(iface.getBrName()); + String vlanId = libvirtComputingResource.getVlanIdFromBridgeName(iface.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(iface, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); + vifDriver.unplug(iface, libvirtComputingResource.shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } } } @@ -161,22 +160,4 @@ private void performAgentStopHook(String vmName, final LibvirtComputingResource s_logger.warn("Exception occurred when handling LibVirt VM onStop hook: {}", e); } } - - private String getVlanIdFromBridgeName(String brName) { - if (StringUtils.isNotBlank(brName)) { - String[] s = brName.split("-"); - if (s.length > 1) { - return s[1]; - } - return null; - } - return null; - } - - private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { - if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { - return vlanToPersistenceMap.get(vlanId); - } - return true; - } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java index 2b760d7bd045..e40563b291bf 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUnPlugNicCommandWrapper.java @@ -22,8 +22,6 @@ import java.util.List; import java.util.Map; -import org.apache.commons.collections.MapUtils; -import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; @@ -63,11 +61,11 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso libvirtComputingResource.destroyNetworkRulesForNic(conn, vmName, nic); } vm.detachDevice(pluggedNic.toString()); - String vlanId = getVlanIdFromBridgeName(pluggedNic.getBrName()); + String vlanId = libvirtComputingResource.getVlanIdFromBridgeName(pluggedNic.getBrName()); // We don't know which "traffic type" is associated with // each interface at this point, so inform all vif drivers for (final VifDriver vifDriver : libvirtComputingResource.getAllVifDrivers()) { - vifDriver.unplug(pluggedNic, shouldDeleteBridge(vlanToPersistenceMap, vlanId)); + vifDriver.unplug(pluggedNic, libvirtComputingResource.shouldDeleteBridge(vlanToPersistenceMap, vlanId)); } return new UnPlugNicAnswer(command, true, "success"); } @@ -87,22 +85,4 @@ public Answer execute(final UnPlugNicCommand command, final LibvirtComputingReso } } } - - private String getVlanIdFromBridgeName(String brName) { - if (StringUtils.isNotBlank(brName)) { - String[] s = brName.split("-"); - if (s.length > 1) { - return s[1]; - } - return null; - } - return null; - } - - private boolean shouldDeleteBridge(Map vlanToPersistenceMap, String vlanId) { - if (MapUtils.isNotEmpty(vlanToPersistenceMap) && vlanId != null && vlanToPersistenceMap.containsKey(vlanId)) { - return vlanToPersistenceMap.get(vlanId); - } - return true; - } } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index e2cca5be428c..454771d93c8b 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -631,7 +631,7 @@ private Answer execute(SetupPersistentNetworkCommand cmd) { prepareNetworkFromNicInfo(hostMO, cmd.getNic(), false, null); hostname = host.getHyperHostName(); } catch (Exception e) { - return new SetupPersistentNetworkAnswer(cmd, false, "failed to get response"); + return new SetupPersistentNetworkAnswer(cmd, false, "failed to setup port-group due to: "+ e.getLocalizedMessage()); } return new SetupPersistentNetworkAnswer(cmd, true, hostname); } From 54da999182db4f6a4e655541ee32acb77f5aea0e Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 18 Jan 2021 17:57:58 +0530 Subject: [PATCH 13/13] API discovery: Prevent overwrite of API parameters in case the API names are the same --- .../apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java index c16d1b233244..5d2285629d65 100644 --- a/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java +++ b/plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -67,7 +68,7 @@ public boolean start() { if (s_apiNameDiscoveryResponseMap == null) { long startTime = System.nanoTime(); s_apiNameDiscoveryResponseMap = new HashMap(); - Set> cmdClasses = new HashSet>(); + Set> cmdClasses = new LinkedHashSet>(); for (PluggableService service : _services) { s_logger.debug(String.format("getting api commands of service: %s", service.getClass().getName())); cmdClasses.addAll(service.getCommands());