From f80aac796c26aff31a2a7af3613f3a8149bd3740 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 8 Dec 2021 15:03:00 +0100 Subject: [PATCH 1/3] network: update ip in lb/pf/dnat tables when update vm nic ip --- .../java/com/cloud/vm/UserVmManagerImpl.java | 51 +++++++++---------- .../java/com/cloud/vm/UserVmManagerTest.java | 4 -- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index d7c90b305c86..3af7d1b0fabb 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -341,6 +341,7 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.exception.ExecutionException; import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.utils.net.Ip; import com.cloud.utils.net.NetUtils; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.DomainRouterDao; @@ -1748,35 +1749,33 @@ public UserVm updateNicIpForVirtualMachine(UpdateVmNicIpCmd cmd) { } if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.StaticNat)) { - IPAddressVO oldIP = _ipAddressDao.findByAssociatedVmId(vm.getId()); - if (oldIP != null) { - oldIP.setVmIp(ipaddr); - _ipAddressDao.persist(oldIP); + List publicIps = _ipAddressDao.listByAssociatedVmId(vm.getId()); + for (IPAddressVO publicIp : publicIps) { + if (nicVO.getIPv4Address().equals(publicIp.getVmIp() ) && publicIp.getAssociatedWithNetworkId() == network.getId()) { + publicIp.setVmIp(ipaddr); + _ipAddressDao.persist(publicIp); + } } } - // implementing the network elements and resources as a part of vm nic ip update if network has services and it is in Implemented state - if (!_networkModel.listNetworkOfferingServices(offering.getId()).isEmpty() && network.getState() == Network.State.Implemented) { - User callerUser = _accountMgr.getActiveUser(CallContext.current().getCallingUserId()); - ReservationContext context = new ReservationContextImpl(null, null, callerUser, caller); - DeployDestination dest = new DeployDestination(_dcDao.findById(network.getDataCenterId()), null, null, null); - - s_logger.debug("Implementing the network " + network + " elements and resources as a part of vm nic ip update"); - try { - // implement the network elements and rules again - _networkMgr.implementNetworkElementsAndResources(dest, context, network, offering); - } catch (Exception ex) { - s_logger.warn("Failed to implement network " + network + " elements and resources as a part of vm nic ip update due to ", ex); - CloudRuntimeException e = new CloudRuntimeException("Failed to implement network (with specified id) elements and resources as a part of vm nic ip update"); - e.addProxyObject(network.getUuid(), "networkId"); - // restore to old ip address - if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.StaticNat)) { - IPAddressVO oldIP = _ipAddressDao.findByAssociatedVmId(vm.getId()); - if (oldIP != null) { - oldIP.setVmIp(nicVO.getIPv4Address()); - _ipAddressDao.persist(oldIP); - } + if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Lb)) { + List loadBalancerVMMaps = _loadBalancerVMMapDao.listByInstanceId(vm.getId()); + for (LoadBalancerVMMapVO map : loadBalancerVMMaps) { + long lbId = map.getLoadBalancerId(); + FirewallRuleVO rule = _rulesDao.findById(lbId); + if (nicVO.getIPv4Address().equals(map.getInstanceIp()) && network.getId() == rule.getNetworkId()) { + map.setInstanceIp(ipaddr); + _loadBalancerVMMapDao.persist(map); + } + } + } + if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.PortForwarding)) { + List firewallRules = _portForwardingDao.listByVm(vm.getId()); + for (PortForwardingRuleVO firewallRule : firewallRules) { + FirewallRuleVO rule = _rulesDao.findById(firewallRule.getId()); + if (nicVO.getIPv4Address().equals(firewallRule.getDestinationIpAddress().toString()) && network.getId() == rule.getNetworkId()) { + firewallRule.setDestinationIpAddress(new Ip(ipaddr)); + _portForwardingDao.persist(firewallRule); } - throw e; } } } else if (dc.getNetworkType() == NetworkType.Basic || network.getGuestType() == Network.GuestType.Shared) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java index 1103fb179611..37cd6e512999 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java @@ -30,7 +30,6 @@ import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; @@ -78,7 +77,6 @@ import com.cloud.dc.VlanVO; import com.cloud.dc.dao.DataCenterDao; import com.cloud.dc.dao.VlanDao; -import com.cloud.deploy.DeployDestination; import com.cloud.event.dao.UsageEventDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; @@ -575,8 +573,6 @@ public void testUpdateVmNicIpSuccess1() throws Exception { when(_dcMock.getNetworkType()).thenReturn(NetworkType.Advanced); when(_ipAddrMgr.allocateGuestIP(Mockito.eq(_networkMock), anyString())).thenReturn("10.10.10.10"); - doNothing().when(_networkMgr).implementNetworkElementsAndResources(Mockito.any(DeployDestination.class), Mockito.any(ReservationContext.class), Mockito.eq(_networkMock), - Mockito.eq(_networkOfferingMock)); when(_nicDao.persist(any(NicVO.class))).thenReturn(nic); Account caller = new AccountVO("testaccount", 1, "networkdomain", (short)0, UUID.randomUUID().toString()); From 7b3bc943a7e03b207c85bace2a130b453761bb2d Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 9 Dec 2021 11:29:50 +0100 Subject: [PATCH 2/3] Update #5757: create 3 separated methods for DNAT/LB/PF update --- .../java/com/cloud/vm/UserVmManagerImpl.java | 73 +++++++++++-------- 1 file changed, 44 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 3af7d1b0fabb..2a66ce634879 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1748,36 +1748,12 @@ public UserVm updateNicIpForVirtualMachine(UpdateVmNicIpCmd cmd) { throw new InvalidParameterValueException("Allocating ip to guest nic " + nicVO.getUuid() + " failed, please choose another ip"); } - if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.StaticNat)) { - List publicIps = _ipAddressDao.listByAssociatedVmId(vm.getId()); - for (IPAddressVO publicIp : publicIps) { - if (nicVO.getIPv4Address().equals(publicIp.getVmIp() ) && publicIp.getAssociatedWithNetworkId() == network.getId()) { - publicIp.setVmIp(ipaddr); - _ipAddressDao.persist(publicIp); - } - } - } - if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Lb)) { - List loadBalancerVMMaps = _loadBalancerVMMapDao.listByInstanceId(vm.getId()); - for (LoadBalancerVMMapVO map : loadBalancerVMMaps) { - long lbId = map.getLoadBalancerId(); - FirewallRuleVO rule = _rulesDao.findById(lbId); - if (nicVO.getIPv4Address().equals(map.getInstanceIp()) && network.getId() == rule.getNetworkId()) { - map.setInstanceIp(ipaddr); - _loadBalancerVMMapDao.persist(map); - } - } - } - if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.PortForwarding)) { - List firewallRules = _portForwardingDao.listByVm(vm.getId()); - for (PortForwardingRuleVO firewallRule : firewallRules) { - FirewallRuleVO rule = _rulesDao.findById(firewallRule.getId()); - if (nicVO.getIPv4Address().equals(firewallRule.getDestinationIpAddress().toString()) && network.getId() == rule.getNetworkId()) { - firewallRule.setDestinationIpAddress(new Ip(ipaddr)); - _portForwardingDao.persist(firewallRule); - } - } + if (nicVO.getIPv4Address() != null) { + updatePublicIpDnatVmIp(vm.getId(), network.getId(), nicVO.getIPv4Address(), ipaddr); + updateLoadBalancerRulesVmIp(vm.getId(), network.getId(), nicVO.getIPv4Address(), ipaddr); + updatePortForwardingRulesVmIp(vm.getId(), network.getId(), nicVO.getIPv4Address(), ipaddr); } + } else if (dc.getNetworkType() == NetworkType.Basic || network.getGuestType() == Network.GuestType.Shared) { //handle the basic networks here //for basic zone, need to provide the podId to ensure proper ip alloation @@ -1825,6 +1801,45 @@ public void doInTransactionWithoutResult(TransactionStatus status) { return vm; } + private void updatePublicIpDnatVmIp(long vmId, long networkId, String oldIp, String newIp) { + if (_networkModel.areServicesSupportedInNetwork(networkId, Service.StaticNat)) { + List publicIps = _ipAddressDao.listByAssociatedVmId(vmId); + for (IPAddressVO publicIp : publicIps) { + if (oldIp.equals(publicIp.getVmIp()) && publicIp.getAssociatedWithNetworkId() == networkId) { + publicIp.setVmIp(newIp); + _ipAddressDao.persist(publicIp); + } + } + } + } + + private void updateLoadBalancerRulesVmIp(long vmId, long networkId, String oldIp, String newIp) { + if (_networkModel.areServicesSupportedInNetwork(networkId, Service.Lb)) { + List loadBalancerVMMaps = _loadBalancerVMMapDao.listByInstanceId(vmId); + for (LoadBalancerVMMapVO map : loadBalancerVMMaps) { + long lbId = map.getLoadBalancerId(); + FirewallRuleVO rule = _rulesDao.findById(lbId); + if (oldIp.equals(map.getInstanceIp()) && networkId == rule.getNetworkId()) { + map.setInstanceIp(newIp); + _loadBalancerVMMapDao.persist(map); + } + } + } + } + + private void updatePortForwardingRulesVmIp(long vmId, long networkId, String oldIp, String newIp) { + if (_networkModel.areServicesSupportedInNetwork(networkId, Service.PortForwarding)) { + List firewallRules = _portForwardingDao.listByVm(vmId); + for (PortForwardingRuleVO firewallRule : firewallRules) { + FirewallRuleVO rule = _rulesDao.findById(firewallRule.getId()); + if (oldIp.equals(firewallRule.getDestinationIpAddress().toString()) && networkId == rule.getNetworkId()) { + firewallRule.setDestinationIpAddress(new Ip(newIp)); + _portForwardingDao.persist(firewallRule); + } + } + } + } + @Override @ActionEvent(eventType = EventTypes.EVENT_VM_UPGRADE, eventDescription = "Upgrading VM", async = true) public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException, From a15d69b7a7ffeb6dc7abd30e2ae99b02c8ba57bc Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 22 Dec 2021 13:38:50 +0100 Subject: [PATCH 3/3] Update #5757: invert if condition to reduce code indentation --- .../java/com/cloud/vm/UserVmManagerImpl.java | 51 ++++++++++--------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 2a66ce634879..d19c67f99706 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1802,40 +1802,43 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } private void updatePublicIpDnatVmIp(long vmId, long networkId, String oldIp, String newIp) { - if (_networkModel.areServicesSupportedInNetwork(networkId, Service.StaticNat)) { - List publicIps = _ipAddressDao.listByAssociatedVmId(vmId); - for (IPAddressVO publicIp : publicIps) { - if (oldIp.equals(publicIp.getVmIp()) && publicIp.getAssociatedWithNetworkId() == networkId) { - publicIp.setVmIp(newIp); - _ipAddressDao.persist(publicIp); - } + if (!_networkModel.areServicesSupportedInNetwork(networkId, Service.StaticNat)) { + return; + } + List publicIps = _ipAddressDao.listByAssociatedVmId(vmId); + for (IPAddressVO publicIp : publicIps) { + if (oldIp.equals(publicIp.getVmIp()) && publicIp.getAssociatedWithNetworkId() == networkId) { + publicIp.setVmIp(newIp); + _ipAddressDao.persist(publicIp); } } } private void updateLoadBalancerRulesVmIp(long vmId, long networkId, String oldIp, String newIp) { - if (_networkModel.areServicesSupportedInNetwork(networkId, Service.Lb)) { - List loadBalancerVMMaps = _loadBalancerVMMapDao.listByInstanceId(vmId); - for (LoadBalancerVMMapVO map : loadBalancerVMMaps) { - long lbId = map.getLoadBalancerId(); - FirewallRuleVO rule = _rulesDao.findById(lbId); - if (oldIp.equals(map.getInstanceIp()) && networkId == rule.getNetworkId()) { - map.setInstanceIp(newIp); - _loadBalancerVMMapDao.persist(map); - } + if (!_networkModel.areServicesSupportedInNetwork(networkId, Service.Lb)) { + return; + } + List loadBalancerVMMaps = _loadBalancerVMMapDao.listByInstanceId(vmId); + for (LoadBalancerVMMapVO map : loadBalancerVMMaps) { + long lbId = map.getLoadBalancerId(); + FirewallRuleVO rule = _rulesDao.findById(lbId); + if (oldIp.equals(map.getInstanceIp()) && networkId == rule.getNetworkId()) { + map.setInstanceIp(newIp); + _loadBalancerVMMapDao.persist(map); } } } private void updatePortForwardingRulesVmIp(long vmId, long networkId, String oldIp, String newIp) { - if (_networkModel.areServicesSupportedInNetwork(networkId, Service.PortForwarding)) { - List firewallRules = _portForwardingDao.listByVm(vmId); - for (PortForwardingRuleVO firewallRule : firewallRules) { - FirewallRuleVO rule = _rulesDao.findById(firewallRule.getId()); - if (oldIp.equals(firewallRule.getDestinationIpAddress().toString()) && networkId == rule.getNetworkId()) { - firewallRule.setDestinationIpAddress(new Ip(newIp)); - _portForwardingDao.persist(firewallRule); - } + if (!_networkModel.areServicesSupportedInNetwork(networkId, Service.PortForwarding)) { + return; + } + List firewallRules = _portForwardingDao.listByVm(vmId); + for (PortForwardingRuleVO firewallRule : firewallRules) { + FirewallRuleVO rule = _rulesDao.findById(firewallRule.getId()); + if (oldIp.equals(firewallRule.getDestinationIpAddress().toString()) && networkId == rule.getNetworkId()) { + firewallRule.setDestinationIpAddress(new Ip(newIp)); + _portForwardingDao.persist(firewallRule); } } }