From 433142a66baf7c7dd104d94fb90996f9f3f755c1 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Mon, 5 Nov 2018 21:42:14 -0300 Subject: [PATCH 1/2] Add missing ConfigDrive entries on existing zones after upgrade --- .../com/cloud/network/NetworkServiceImpl.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index 33a817872835..d8fd886c61f5 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -37,6 +37,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -623,11 +624,42 @@ public boolean configure(final String name, final Map params) th _allowSubdomainNetworkAccess = Boolean.valueOf(_configs.get(Config.SubDomainNetworkAccess.key())); + verifyConfigDriveEntriesOnZones(); + s_logger.info("Network Service is configured."); return true; } + /** + * Verifies ConfigDrive entries on a zone. Adds Diabled ConfigDrive provider if missing + */ + private void checkConfigDriveEntriesOnZone(DataCenterVO zone) { + if (zone.getNetworkType() == NetworkType.Advanced) { + List physicalNetworks = _physicalNetworkDao.listByZoneAndTrafficType( + zone.getId(), TrafficType.Guest); + for (PhysicalNetworkVO physicalNetworkVO : physicalNetworks) { + PhysicalNetworkServiceProviderVO provider = _pNSPDao.findByServiceProvider( + physicalNetworkVO.getId(), Provider.ConfigDrive.getName()); + if (provider == null) { + addConfigDriveToPhysicalNetwork(physicalNetworkVO.getId()); + } + } + } + } + + /** + * Verifies ConfigDrive entries on enabled zones. Adds Diabled ConfigDrive provider if missing. + */ + private void verifyConfigDriveEntriesOnZones() { + List zones = _dcDao.listEnabledZones(); + if (CollectionUtils.isNotEmpty(zones)) { + for (DataCenterVO zone : zones) { + checkConfigDriveEntriesOnZone(zone); + } + } + } + @Override public boolean start() { return true; From b3364d6030db5786b263c28da95cc0452a45bf68 Mon Sep 17 00:00:00 2001 From: nvazquez Date: Wed, 7 Nov 2018 19:35:16 -0300 Subject: [PATCH 2/2] Refactor, move the logic to NetworkModel and add unit tests --- .../com/cloud/network/NetworkModelImpl.java | 37 ++++++ .../com/cloud/network/NetworkServiceImpl.java | 32 ----- .../com/cloud/network/NetworkModelTest.java | 119 +++++++++++++++++- 3 files changed, 154 insertions(+), 34 deletions(-) diff --git a/server/src/com/cloud/network/NetworkModelImpl.java b/server/src/com/cloud/network/NetworkModelImpl.java index 31cdf914d5f0..4dae7437ae99 100644 --- a/server/src/com/cloud/network/NetworkModelImpl.java +++ b/server/src/com/cloud/network/NetworkModelImpl.java @@ -34,6 +34,7 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -202,6 +203,8 @@ public void setNetworkElements(List networkElements) { private ProjectAccountDao _projectAccountDao; @Inject NetworkOfferingDetailsDao _ntwkOffDetailsDao; + @Inject + private NetworkService _networkService; private final HashMap _systemNetworks = new HashMap(5); static Long s_privateOfferingId = null; @@ -2113,10 +2116,44 @@ public boolean start() { } } } + + //After network elements are configured correctly, verify ConfigDrive entries on enabled zones + verifyDisabledConfigDriveEntriesOnEnabledZones(); + s_logger.info("Started Network Model"); return true; } + /** + * Verifies ConfigDrive entries on a zone and adds disabled ConfigDrive provider if missing. + */ + protected void addDisabledConfigDriveEntriesOnZone(DataCenterVO zone) { + if (zone.getNetworkType() == DataCenter.NetworkType.Advanced) { + List physicalNetworks = _physicalNetworkDao.listByZoneAndTrafficType( + zone.getId(), TrafficType.Guest); + for (PhysicalNetworkVO physicalNetworkVO : physicalNetworks) { + PhysicalNetworkServiceProviderVO provider = _pNSPDao.findByServiceProvider( + physicalNetworkVO.getId(), Provider.ConfigDrive.getName()); + if (provider == null) { + _networkService.addProviderToPhysicalNetwork( + physicalNetworkVO.getId(), Provider.ConfigDrive.getName(), null, null); + } + } + } + } + + /** + * Verifies ConfigDrive entries on enabled zones, adds disabled ConfigDrive provider if missing. + */ + protected void verifyDisabledConfigDriveEntriesOnEnabledZones() { + List zones = _dcDao.listEnabledZones(); + if (CollectionUtils.isNotEmpty(zones)) { + for (DataCenterVO zone : zones) { + addDisabledConfigDriveEntriesOnZone(zone); + } + } + } + @Override public boolean stop() { return true; diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index d8fd886c61f5..33a817872835 100644 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -37,7 +37,6 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; -import org.apache.commons.collections.CollectionUtils; import org.apache.log4j.Logger; import org.apache.cloudstack.acl.ControlledEntity.ACLType; @@ -624,42 +623,11 @@ public boolean configure(final String name, final Map params) th _allowSubdomainNetworkAccess = Boolean.valueOf(_configs.get(Config.SubDomainNetworkAccess.key())); - verifyConfigDriveEntriesOnZones(); - s_logger.info("Network Service is configured."); return true; } - /** - * Verifies ConfigDrive entries on a zone. Adds Diabled ConfigDrive provider if missing - */ - private void checkConfigDriveEntriesOnZone(DataCenterVO zone) { - if (zone.getNetworkType() == NetworkType.Advanced) { - List physicalNetworks = _physicalNetworkDao.listByZoneAndTrafficType( - zone.getId(), TrafficType.Guest); - for (PhysicalNetworkVO physicalNetworkVO : physicalNetworks) { - PhysicalNetworkServiceProviderVO provider = _pNSPDao.findByServiceProvider( - physicalNetworkVO.getId(), Provider.ConfigDrive.getName()); - if (provider == null) { - addConfigDriveToPhysicalNetwork(physicalNetworkVO.getId()); - } - } - } - } - - /** - * Verifies ConfigDrive entries on enabled zones. Adds Diabled ConfigDrive provider if missing. - */ - private void verifyConfigDriveEntriesOnZones() { - List zones = _dcDao.listEnabledZones(); - if (CollectionUtils.isNotEmpty(zones)) { - for (DataCenterVO zone : zones) { - checkConfigDriveEntriesOnZone(zone); - } - } - } - @Override public boolean start() { return true; diff --git a/server/test/com/cloud/network/NetworkModelTest.java b/server/test/com/cloud/network/NetworkModelTest.java index ba575420ed8f..f707329557a0 100644 --- a/server/test/com/cloud/network/NetworkModelTest.java +++ b/server/test/com/cloud/network/NetworkModelTest.java @@ -19,18 +19,32 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.Mockito.spy; import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import com.cloud.dc.DataCenter; +import com.cloud.dc.DataCenterVO; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.dao.PhysicalNetworkServiceProviderDao; +import com.cloud.network.dao.PhysicalNetworkServiceProviderVO; +import com.cloud.network.dao.PhysicalNetworkVO; import junit.framework.Assert; import org.junit.Before; @@ -48,11 +62,64 @@ import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.net.Ip; import com.cloud.network.Network.Provider; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; public class NetworkModelTest { + + @Mock + private DataCenterDao dataCenterDao; + @Mock + private PhysicalNetworkDao physicalNetworkDao; + @Mock + private PhysicalNetworkServiceProviderDao physicalNetworkServiceProviderDao; + @Mock + private NetworkService networkService; + + @InjectMocks + @Spy + private NetworkModelImpl networkModel = new NetworkModelImpl(); + + @Mock + private DataCenterVO zone1; + @Mock + private DataCenterVO zone2; + @Mock + private PhysicalNetworkVO physicalNetworkZone1; + @Mock + private PhysicalNetworkVO physicalNetworkZone2; + @Mock + private PhysicalNetworkServiceProviderVO providerVO; + + private static final long ZONE_1_ID = 1L; + private static final long ZONE_2_ID = 2L; + private static final long PHYSICAL_NETWORK_1_ID = 1L; + private static final long PHYSICAL_NETWORK_2_ID = 2L; + @Before public void setUp() { - + MockitoAnnotations.initMocks(this); + + when(dataCenterDao.listEnabledZones()).thenReturn(Arrays.asList(zone1, zone2)); + when(physicalNetworkDao.listByZoneAndTrafficType(ZONE_1_ID, Networks.TrafficType.Guest)). + thenReturn(Collections.singletonList(physicalNetworkZone1)); + when(physicalNetworkDao.listByZoneAndTrafficType(ZONE_2_ID, Networks.TrafficType.Guest)). + thenReturn(Collections.singletonList(physicalNetworkZone2)); + when(physicalNetworkServiceProviderDao.findByServiceProvider( + PHYSICAL_NETWORK_1_ID, Network.Provider.ConfigDrive.getName())).thenReturn(null); + when(physicalNetworkServiceProviderDao.findByServiceProvider( + PHYSICAL_NETWORK_2_ID, Network.Provider.ConfigDrive.getName())).thenReturn(null); + + when(zone1.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); + when(zone1.getId()).thenReturn(ZONE_1_ID); + + when(zone2.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); + when(zone2.getId()).thenReturn(ZONE_2_ID); + + when(physicalNetworkZone1.getId()).thenReturn(PHYSICAL_NETWORK_1_ID); + when(physicalNetworkZone2.getId()).thenReturn(PHYSICAL_NETWORK_2_ID); } @Test @@ -69,7 +136,7 @@ public void testGetSourceNatIpAddressForGuestNetwork() { when(fakeVlanDao.findById(anyLong())).thenReturn(mock(VlanVO.class)); modelImpl._vlanDao = fakeVlanDao; when(fakeSearch.create()).thenReturn(mock(SearchCriteria.class)); - when(ipAddressDao.search(any(SearchCriteria.class), (Filter)org.mockito.Matchers.isNull())).thenReturn(fakeList); + when(ipAddressDao.search(any(SearchCriteria.class), (Filter) isNull())).thenReturn(fakeList); when(ipAddressDao.findById(anyLong())).thenReturn(fakeIp); Account fakeAccount = mock(Account.class); when(fakeAccount.getId()).thenReturn(1L); @@ -134,5 +201,53 @@ public void testCapabilityForProvider() { } } + @Test + public void testVerifyDisabledConfigDriveEntriesOnZonesBothEnabledZones() { + networkModel.verifyDisabledConfigDriveEntriesOnEnabledZones(); + verify(networkModel, times(2)).addDisabledConfigDriveEntriesOnZone(any(DataCenterVO.class)); + } + + @Test + public void testVerifyDisabledConfigDriveEntriesOnZonesOneEnabledZone() { + when(dataCenterDao.listEnabledZones()).thenReturn(Collections.singletonList(zone1)); + + networkModel.verifyDisabledConfigDriveEntriesOnEnabledZones(); + verify(networkModel).addDisabledConfigDriveEntriesOnZone(any(DataCenterVO.class)); + } + + @Test + public void testVerifyDisabledConfigDriveEntriesOnZonesNoEnabledZones() { + when(dataCenterDao.listEnabledZones()).thenReturn(null); + + networkModel.verifyDisabledConfigDriveEntriesOnEnabledZones(); + verify(networkModel, never()).addDisabledConfigDriveEntriesOnZone(any(DataCenterVO.class)); + } + + @Test + public void testAddDisabledConfigDriveEntriesOnZoneBasicZone() { + when(zone1.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); + + networkModel.addDisabledConfigDriveEntriesOnZone(zone1); + verify(physicalNetworkDao, never()).listByZoneAndTrafficType(ZONE_1_ID, Networks.TrafficType.Guest); + verify(networkService, never()). + addProviderToPhysicalNetwork(anyLong(), eq(Provider.ConfigDrive.getName()), isNull(Long.class), isNull(List.class)); + } + + @Test + public void testAddDisabledConfigDriveEntriesOnZoneAdvancedZoneExistingConfigDrive() { + when(physicalNetworkServiceProviderDao.findByServiceProvider( + PHYSICAL_NETWORK_1_ID, Network.Provider.ConfigDrive.getName())).thenReturn(providerVO); + + networkModel.addDisabledConfigDriveEntriesOnZone(zone1); + verify(networkService, never()). + addProviderToPhysicalNetwork(anyLong(), eq(Provider.ConfigDrive.getName()), isNull(Long.class), isNull(List.class)); + } + + @Test + public void testAddDisabledConfigDriveEntriesOnZoneAdvancedZoneNonExistingConfigDrive() { + networkModel.addDisabledConfigDriveEntriesOnZone(zone1); + verify(networkService). + addProviderToPhysicalNetwork(anyLong(), eq(Provider.ConfigDrive.getName()), isNull(Long.class), isNull(List.class)); + } }