diff --git a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java index a209acf6442e..589c0f84f56e 100644 --- a/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java +++ b/framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java @@ -140,8 +140,10 @@ import com.cloud.network.dao.PhysicalNetworkDao; import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.network.vpc.Vpc; +import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.network.vpc.dao.VpcServiceMapDao; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.org.Cluster; import com.cloud.serializer.GsonHelper; @@ -248,6 +250,12 @@ public class ExtensionsManagerImpl extends ManagerBase implements ExtensionsMana @Inject VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + @Inject + NetworkOfferingDao networkOfferingDao; + + @Inject + VpcOfferingDao vpcOfferingDao; + @Inject NetworkModel networkModel; @@ -1306,10 +1314,13 @@ protected void updateNetworkExtensionServicesOnPhysicalNetworks(ExtensionVO exte return; } for (Service removedService : removedServices) { - boolean usedByNetworkOffering = CollectionUtils.isNotEmpty( - networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(removedService, extension.getName())); - boolean usedByVpcOffering = CollectionUtils.isNotEmpty( - vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(removedService, extension.getName())); + // offering service map rows are kept when an offering is deleted, so ignore removed offerings + boolean usedByNetworkOffering = networkOfferingServiceMapDao + .listOfferingIdsByServiceAndProvider(removedService, extension.getName()).stream() + .anyMatch(offeringId -> networkOfferingDao.findById(offeringId) != null); + boolean usedByVpcOffering = vpcOfferingServiceMapDao + .listOfferingIdsByServiceAndProvider(removedService, extension.getName()).stream() + .anyMatch(offeringId -> vpcOfferingDao.findById(offeringId) != null); if (usedByNetworkOffering || usedByVpcOffering) { throw new CloudRuntimeException(String.format( "Cannot remove service %s from extension '%s' as it is used by a network/VPC offering", diff --git a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java index fa8c97742697..9a85840544fa 100644 --- a/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java +++ b/framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java @@ -52,6 +52,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import org.apache.cloudstack.acl.Role; @@ -139,8 +140,12 @@ import com.cloud.network.dao.PhysicalNetworkVO; import com.cloud.network.element.NetworkElement; import com.cloud.network.vpc.Vpc; +import com.cloud.network.vpc.VpcOfferingVO; +import com.cloud.network.vpc.dao.VpcOfferingDao; import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.network.vpc.dao.VpcServiceMapDao; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import org.apache.cloudstack.extension.NetworkCustomActionProvider; import com.cloud.org.Cluster; @@ -223,6 +228,12 @@ public class ExtensionsManagerImplTest { @Mock private VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + @Mock + private NetworkOfferingDao networkOfferingDao; + + @Mock + private VpcOfferingDao vpcOfferingDao; + @Before public void setUp() { MockitoAnnotations.openMocks(this); @@ -961,6 +972,7 @@ public void testUpdateExtension_RemovingUsedNetworkServiceThrows() { when(networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.StaticNat, "MyExt")) .thenReturn(Collections.singletonList(1L)); + when(networkOfferingDao.findById(1L)).thenReturn(mock(NetworkOfferingVO.class)); extensionsManager.updateExtension(cmd); } @@ -992,10 +1004,42 @@ public void testUpdateExtension_RemovingServiceUsedByVpcOfferingThrows() { .thenReturn(Collections.emptyList()); when(vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.StaticNat, "MyExt")) .thenReturn(Collections.singletonList(1L)); + when(vpcOfferingDao.findById(1L)).thenReturn(mock(VpcOfferingVO.class)); extensionsManager.updateExtension(cmd); } + @Test + public void updateNetworkExtensionServicesOnPhysicalNetworks_IgnoresRemovedOfferings() { + ExtensionVO ext = mock(ExtensionVO.class); + when(ext.getId()).thenReturn(9L); + when(ext.getName()).thenReturn("MyExt"); + + // service map rows still reference offerings 1 (network) and 2 (VPC), but both offerings are removed + when(networkOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.CustomAction, "MyExt")) + .thenReturn(Collections.singletonList(1L)); + when(vpcOfferingServiceMapDao.listOfferingIdsByServiceAndProvider(Network.Service.CustomAction, "MyExt")) + .thenReturn(Collections.singletonList(2L)); + when(networkOfferingDao.findById(1L)).thenReturn(null); + when(vpcOfferingDao.findById(2L)).thenReturn(null); + + when(extensionResourceMapDao.listResourceIdsByExtensionIdAndType(9L, ExtensionResourceMap.ResourceType.PhysicalNetwork)) + .thenReturn(Collections.singletonList(100L)); + PhysicalNetworkServiceProviderVO nsp = mock(PhysicalNetworkServiceProviderVO.class); + when(nsp.getId()).thenReturn(500L); + when(nsp.getEnabledServices()).thenReturn(new ArrayList<>(Collections.singletonList(Network.Service.CustomAction))); + when(physicalNetworkServiceProviderDao.findByServiceProvider(100L, "MyExt")).thenReturn(nsp); + + extensionsManager.updateNetworkExtensionServicesOnPhysicalNetworks(ext, + Set.of(Network.Service.CustomAction), Set.of(Network.Service.UserData)); + + ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); + verify(nsp).setEnabledServices(captor.capture()); + assertFalse(captor.getValue().contains(Network.Service.CustomAction)); + assertTrue(captor.getValue().contains(Network.Service.UserData)); + verify(physicalNetworkServiceProviderDao).update(500L, nsp); + } + @Test public void testUpdateExtension_UpdatesPhysicalNetworkServicesWhenNotInUse() { UpdateExtensionCmd cmd = mock(UpdateExtensionCmd.class);