From a6196b0a605c2afbeffdfc1866b3949663c67882 Mon Sep 17 00:00:00 2001 From: Frank Maximus Date: Tue, 9 Oct 2018 12:13:48 +0200 Subject: [PATCH 1/2] Fixes: #2881 Improve Exception message (#2889) Network.Service and Network.Provider were missing a toString() method. Added this so appending (a list of) them will be understandable. --- api/src/com/cloud/network/Network.java | 21 ++++++++++++++++++ .../network/element/NuageVspElementTest.java | 22 ++++++++++++++++--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/api/src/com/cloud/network/Network.java b/api/src/com/cloud/network/Network.java index 75196a469d3..d9830810958 100644 --- a/api/src/com/cloud/network/Network.java +++ b/api/src/com/cloud/network/Network.java @@ -21,6 +21,9 @@ import java.net.URI; import java.util.ArrayList; import java.util.List; +import org.apache.commons.lang.builder.ToStringBuilder; +import org.apache.commons.lang.builder.ToStringStyle; + import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.api.Displayable; import org.apache.cloudstack.api.Identity; @@ -95,6 +98,12 @@ public interface Network extends ControlledEntity, StateObject, I return success; } + @Override public String toString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("name", name) + .toString(); + } + public static Service getService(String serviceName) { for (Service service : supportedServices) { if (service.getName().equalsIgnoreCase(serviceName)) { @@ -186,6 +195,12 @@ public interface Network extends ControlledEntity, StateObject, I } return null; } + + @Override public String toString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("name", name) + .toString(); + } } public static class Capability { @@ -241,6 +256,12 @@ public interface Network extends ControlledEntity, StateObject, I } return null; } + + @Override public String toString() { + return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE) + .append("name", name) + .toString(); + } } enum Event { diff --git a/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java b/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java index e48d9a46ced..9a87dac1997 100644 --- a/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java +++ b/plugins/network-elements/nuage-vsp/test/com/cloud/network/element/NuageVspElementTest.java @@ -26,9 +26,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.Set; -import com.cloud.network.dao.NetworkDao; -import com.cloud.network.dao.NetworkVO; -import com.cloud.tags.dao.ResourceTagDao; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; @@ -50,6 +47,7 @@ import com.cloud.exception.CloudException; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.ResourceUnavailableException; +import com.cloud.exception.UnsupportedServiceException; import com.cloud.host.HostVO; import com.cloud.host.dao.HostDao; import com.cloud.network.Network; @@ -62,7 +60,9 @@ import com.cloud.network.NuageVspDeviceVO; import com.cloud.network.dao.FirewallRulesDao; import com.cloud.network.dao.IPAddressDao; import com.cloud.network.dao.IPAddressVO; +import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkServiceMapDao; +import com.cloud.network.dao.NetworkVO; import com.cloud.network.dao.NuageVspDao; import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; @@ -77,14 +77,18 @@ import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.resource.ResourceManager; +import com.cloud.tags.dao.ResourceTagDao; import com.cloud.user.Account; import com.cloud.util.NuageVspEntityBuilder; import com.cloud.vm.DomainRouterVO; import com.cloud.vm.ReservationContext; import com.cloud.vm.dao.DomainRouterDao; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -233,6 +237,18 @@ public class NuageVspElementTest extends NuageTest { Service.Connectivity, Service.Firewall); assertTrue(_nuageVspElement.verifyServicesCombination(services)); + + + services = Sets.newHashSet( + Service.Dhcp, + Service.StaticNat, + Service.Firewall); + try { + _nuageVspElement.verifyServicesCombination(services); + fail("Expected Exception"); + } catch (UnsupportedServiceException e) { + assertThat(e.getMessage(), is("Provider Network.Provider[name=NuageVsp] requires services: [Network.Service[name=Connectivity]]")); + } } @Test From ea771cfda4839e934cffbf647906f5c0af4993a4 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Wed, 10 Oct 2018 15:20:36 +0530 Subject: [PATCH 2/2] router: Fixes #2719 program VR nics by device id order for VPC (#2888) This fixes #2719 where private gateway IP might be incorrectly programmed on a guest network nic. The VR would now check ipassoc requests by mac addresses than provided nic/device id in case they are wrong. The root cause is that the device id information is lost when aggregated commands are created upon starting of a new VPC VR, without the correct device id in ip_associations json it mis-programs the VR. Signed-off-by: Rohit Yadav --- api/src/com/cloud/network/NetworkProfile.java | 8 +++++--- engine/schema/src/com/cloud/vm/dao/NicDao.java | 2 ++ .../schema/src/com/cloud/vm/dao/NicDaoImpl.java | 7 +++++++ .../VpcVirtualNetworkApplianceManagerImpl.java | 2 +- systemvm/debian/opt/cloud/bin/cs_ip.py | 17 +++++++++++++++++ .../java/com/cloud/utils/net/NetUtilsTest.java | 6 ++++-- 6 files changed, 36 insertions(+), 6 deletions(-) diff --git a/api/src/com/cloud/network/NetworkProfile.java b/api/src/com/cloud/network/NetworkProfile.java index d8733ca6c50..bf21c93c89f 100644 --- a/api/src/com/cloud/network/NetworkProfile.java +++ b/api/src/com/cloud/network/NetworkProfile.java @@ -16,12 +16,12 @@ // under the License. package com.cloud.network; +import java.net.URI; + import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.Mode; import com.cloud.network.Networks.TrafficType; -import java.net.URI; - public class NetworkProfile implements Network { private final long id; private final String uuid; @@ -33,6 +33,7 @@ public class NetworkProfile implements Network { private URI broadcastUri; private final State state; private boolean isRedundant; + private boolean isRollingRestart = false; private final String name; private final Mode mode; private final BroadcastDomainType broadcastDomainType; @@ -92,6 +93,7 @@ public class NetworkProfile implements Network { guruName = network.getGuruName(); strechedL2Subnet = network.isStrechedL2Network(); isRedundant = network.isRedundant(); + isRollingRestart = network.isRollingRestart(); externalId = network.getExternalId(); } @@ -157,7 +159,7 @@ public class NetworkProfile implements Network { @Override public boolean isRollingRestart() { - return false; + return isRollingRestart; } @Override diff --git a/engine/schema/src/com/cloud/vm/dao/NicDao.java b/engine/schema/src/com/cloud/vm/dao/NicDao.java index 0d86964974a..df4fb06325f 100644 --- a/engine/schema/src/com/cloud/vm/dao/NicDao.java +++ b/engine/schema/src/com/cloud/vm/dao/NicDao.java @@ -26,6 +26,8 @@ import com.cloud.vm.VirtualMachine; public interface NicDao extends GenericDao { List listByVmId(long instanceId); + List listByVmIdOrderByDeviceId(long instanceId); + List listIpAddressInNetwork(long networkConfigId); List listByVmIdIncludingRemoved(long instanceId); diff --git a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java index f953a4c1f93..c125d80c534 100644 --- a/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java +++ b/engine/schema/src/com/cloud/vm/dao/NicDaoImpl.java @@ -118,6 +118,13 @@ public class NicDaoImpl extends GenericDaoBase implements NicDao { return listBy(sc); } + @Override + public List listByVmIdOrderByDeviceId(long instanceId) { + SearchCriteria sc = AllFieldsSearch.create(); + sc.setParameters("instance", instanceId); + return customSearch(sc, new Filter(NicVO.class, "deviceId", true, null, null)); + } + @Override public List listByVmIdIncludingRemoved(long instanceId) { SearchCriteria sc = AllFieldsSearch.create(); diff --git a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java index d22dcbafbba..fefd97217af 100644 --- a/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java +++ b/server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java @@ -316,7 +316,7 @@ public class VpcVirtualNetworkApplianceManagerImpl extends VirtualNetworkApplian final List> publicNics = new ArrayList>(); final Map vlanMacAddress = new HashMap(); - final List routerNics = _nicDao.listByVmId(profile.getId()); + final List routerNics = _nicDao.listByVmIdOrderByDeviceId(profile.getId()); for (final Nic routerNic : routerNics) { final Network network = _networkModel.getNetwork(routerNic.getNetworkId()); if (network.getTrafficType() == TrafficType.Guest) { diff --git a/systemvm/debian/opt/cloud/bin/cs_ip.py b/systemvm/debian/opt/cloud/bin/cs_ip.py index 1e7b326a1ec..a4e0c33e798 100755 --- a/systemvm/debian/opt/cloud/bin/cs_ip.py +++ b/systemvm/debian/opt/cloud/bin/cs_ip.py @@ -16,9 +16,21 @@ # specific language governing permissions and limitations # under the License. +import os from netaddr import * +def macdevice_map(): + device_map = {} + for eth in os.listdir('/sys/class/net'): + if not eth.startswith('eth'): + continue + with open('/sys/class/net/%s/address' % eth) as f: + mac_address = f.read().replace('\n', '') + device_map[mac_address] = eth[3:] + return device_map + + def merge(dbag, ip): nic_dev_id = None for dev in dbag: @@ -33,6 +45,11 @@ def merge(dbag, ip): ipo = IPNetwork(ip['public_ip'] + '/' + ip['netmask']) if 'nic_dev_id' in ip: nic_dev_id = ip['nic_dev_id'] + if 'vif_mac_address' in ip: + mac_address = ip['vif_mac_address'] + device_map = macdevice_map() + if mac_address in device_map: + nic_dev_id = device_map[mac_address] ip['device'] = 'eth' + str(nic_dev_id) ip['broadcast'] = str(ipo.broadcast) ip['cidr'] = str(ipo.ip) + '/' + str(ipo.prefixlen) diff --git a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java index 80d25e874a2..262e928b68b 100644 --- a/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java +++ b/utils/src/test/java/com/cloud/utils/net/NetUtilsTest.java @@ -37,13 +37,13 @@ import java.net.UnknownHostException; import java.util.SortedSet; import java.util.TreeSet; -import com.googlecode.ipv6.IPv6Network; import org.apache.log4j.Logger; import org.junit.Test; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils.SupersetOrSubset; import com.googlecode.ipv6.IPv6Address; +import com.googlecode.ipv6.IPv6Network; public class NetUtilsTest { @@ -682,6 +682,8 @@ public class NetUtilsTest { @Test public void testAllIpsOfDefaultNic() { final String defaultHostIp = NetUtils.getDefaultHostIp(); - assertTrue(NetUtils.getAllDefaultNicIps().stream().anyMatch(defaultHostIp::contains)); + if (defaultHostIp != null) { + assertTrue(NetUtils.getAllDefaultNicIps().stream().anyMatch(defaultHostIp::contains)); + } } }