keep another cluster's old acl rule when a cks delete is retried

Author: Damans227

plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java

@@ -681,13 +681,23 @@ protected void removeVpcTierAllowPortACLRule(final Network network, int startPor
         List<NetworkACLItemVO> clusterAclItems = aclItems.stream()
                 .filter(networkACLItem -> getVpcTierAclRuleReason().equals(networkACLItem.getReason()))
                 .collect(Collectors.toList());
-        if (clusterAclItems.isEmpty()) {
-            // Rules created before they were marked with the cluster, remove only one of them
-            clusterAclItems = aclItems.stream()
+        // Record the cleanup before revoking, so a retried deletion knows this cluster's rule is already gone
+        String cleanupDetail = getVpcTierAclRuleCleanupDetail(startPort, endPort);
+        if (clusterAclItems.isEmpty() && kubernetesClusterDetailsDao.findDetail(kubernetesCluster.getId(), cleanupDetail) == null) {
+            // Rule created before rules were marked with the cluster, claim only one of them
+            NetworkACLItemVO legacyAclItem = aclItems.stream()
                     .filter(networkACLItem -> StringUtils.isEmpty(networkACLItem.getReason()))
-                    .limit(1)
-                    .collect(Collectors.toList());
+                    .findFirst().orElse(null);
+            if (legacyAclItem != null) {
+                legacyAclItem.setReason(getVpcTierAclRuleReason());
+                networkACLItemDao.update(legacyAclItem.getId(), legacyAclItem);
+                clusterAclItems = List.of(legacyAclItem);
+            }
         }
+        if (clusterAclItems.isEmpty()) {
+            return;
+        }
+        kubernetesClusterDetailsDao.addDetail(kubernetesCluster.getId(), cleanupDetail, String.valueOf(true), false);
 
         for (NetworkACLItemVO aclItem : clusterAclItems) {
             networkACLService.revokeNetworkACLItem(aclItem.getId());
@@ -698,6 +708,10 @@ protected String getVpcTierAclRuleReason() {
         return String.format("Kubernetes cluster %s", kubernetesCluster.getUuid());
     }
 
+    protected String getVpcTierAclRuleCleanupDetail(int startPort, int endPort) {
+        return String.format("vpcTierAclRuleRemoved-%d-%d", startPort, endPort);
+    }
+
     protected void provisionLoadBalancerRule(final IpAddress publicIp, final Network network,
             final Account account, final List<Long> clusterVMIds, final int port) throws NetworkRuleConflictException,
             InsufficientAddressCapacityException {

plugins/integrations/kubernetes-service/src/test/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorkerTest.java

@@ -18,6 +18,7 @@
 package com.cloud.kubernetes.cluster.actionworkers;
 
 import com.cloud.kubernetes.cluster.KubernetesCluster;
+import com.cloud.kubernetes.cluster.KubernetesClusterDetailsVO;
 import com.cloud.kubernetes.cluster.KubernetesClusterManagerImpl;
 import com.cloud.kubernetes.cluster.dao.KubernetesClusterDao;
 import com.cloud.kubernetes.cluster.dao.KubernetesClusterDetailsDao;
@@ -118,10 +119,55 @@ public void removeVpcTierAllowPortACLRuleTestRemovesOneUnmarkedRuleForOlderClust
 
         kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 22, 22);
 
+        Mockito.verify(oldRule1).setReason("Kubernetes cluster cluster-a");
+        Mockito.verify(networkACLItemDaoMock).update(3L, oldRule1);
         Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(3L);
         Mockito.verifyNoMoreInteractions(networkACLServiceMock);
     }
 
+    private void rememberCleanupDetails() {
+        Mockito.doAnswer(invocation -> {
+            Mockito.when(kubernetesClusterDetailsDaoMock.findDetail(1L, invocation.getArgument(1)))
+                    .thenReturn(Mockito.mock(KubernetesClusterDetailsVO.class));
+            return null;
+        }).when(kubernetesClusterDetailsDaoMock).addDetail(Mockito.eq(1L), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean());
+    }
+
+    @Test
+    public void removeVpcTierAllowPortACLRuleTestRetryKeepsOtherClusterUnmarkedRule() throws Exception {
+        Mockito.when(kubernetesClusterMock.getId()).thenReturn(1L);
+        Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a");
+        Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L);
+        rememberCleanupDetails();
+        NetworkACLItemVO clusterARule = mockAclItem(1L, 6443, "Kubernetes cluster cluster-a");
+        NetworkACLItemVO clusterBOldRule = mockAclItem(2L, 6443, null);
+        Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(clusterARule, clusterBOldRule), List.of(clusterBOldRule));
+
+        kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 6443, 6443);
+        kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 6443, 6443);
+
+        Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(1L);
+        Mockito.verify(networkACLServiceMock, Mockito.never()).revokeNetworkACLItem(2L);
+        Mockito.verify(networkACLItemDaoMock, Mockito.never()).update(Mockito.anyLong(), Mockito.any());
+    }
+
+    @Test
+    public void removeVpcTierAllowPortACLRuleTestRetryOfOlderClusterRemovesOnlyOneUnmarkedRule() throws Exception {
+        Mockito.when(kubernetesClusterMock.getId()).thenReturn(1L);
+        Mockito.when(kubernetesClusterMock.getUuid()).thenReturn("cluster-a");
+        Mockito.when(networkMock.getNetworkACLId()).thenReturn(10L);
+        rememberCleanupDetails();
+        NetworkACLItemVO oldRule1 = mockAclItem(3L, 22, null);
+        NetworkACLItemVO oldRule2 = mockAclItem(4L, 22, null);
+        Mockito.when(networkACLItemDaoMock.listByACL(10L)).thenReturn(List.of(oldRule1, oldRule2), List.of(oldRule2));
+
+        kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 22, 22);
+        kubernetesClusterResourceModifierActionWorker.removeVpcTierAllowPortACLRule(networkMock, 22, 22);
+
+        Mockito.verify(networkACLServiceMock).revokeNetworkACLItem(3L);
+        Mockito.verify(networkACLServiceMock, Mockito.never()).revokeNetworkACLItem(4L);
+    }
+
     @Test
     public void getKubernetesClusterNodeNamePrefixTestReturnOriginalPrefixWhenNamingAllRequirementsAreMet() {
         String originalPrefix = "k8s-cluster-01";