delete legacy acls when upgrading to v1.13.x (kubeovn#4742)

the acls in v1.13.x are in tier 2 rather than tier 0 in v1.12.x, the legacy acls may cause some unexpected behaviors because acls in tier 0 have the higest priority. we should delete legacy acls and recreate them when upgrading to v1.13.x. Signed-off-by: suo <[email protected]>
hackerain · Dec 4, 2024 · 5aa10cc · 5aa10cc
1 parent 86ad84b
commit 5aa10cc
Show file tree

Hide file tree

Showing 14 changed files with 295 additions and 58 deletions.
diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go
diff --git a/pkg/controller/admin_network_policy.go b/pkg/controller/admin_network_policy.go
@@ -190,7 +190,7 @@ func (c *Controller) handleAddAnp(key string) (err error) {
 		return err
 	}
 
-	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil)
+	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("failed to generate clear operations for anp %s ingress acls: %v", key, err)
 		return err
@@ -266,7 +266,7 @@ func (c *Controller) handleAddAnp(key string) (err error) {
 		return fmt.Errorf("failed to delete unused ingress address set for anp %s: %w", key, err)
 	}
 
-	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil)
+	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("failed to generate clear operations for anp %s egress acls: %v", key, err)
 		return err

diff --git a/pkg/controller/baseline_admin_network_policy.go b/pkg/controller/baseline_admin_network_policy.go
@@ -148,7 +148,7 @@ func (c *Controller) handleAddBanp(key string) (err error) {
 		return err
 	}
 
-	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil)
+	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("failed to generate clear operations for banp %s ingress acls: %v", key, err)
 		return err
@@ -225,7 +225,7 @@ func (c *Controller) handleAddBanp(key string) (err error) {
 		return fmt.Errorf("failed to delete unused ingress address set for banp %s: %w", key, err)
 	}
 
-	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil)
+	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("failed to generate clear operations for banp %s egress acls: %v", key, err)
 		return err

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -995,6 +995,8 @@ func (c *Controller) Run(ctx context.Context) {
 		}
 	}
 
+	c.handleUpgrading()
+
 	// start workers to do all the network operations
 	c.startWorkers(ctx)
 
@@ -1131,6 +1133,21 @@ func (c *Controller) shutdown() {
 	}
 }
 
+func (c *Controller) handleUpgrading() {
+	if err := c.upgradeSecurityGroups(); err != nil {
+		util.LogFatalAndExit(err, "failed to upgrade security groups")
+	}
+	if err := c.upgradeSubnets(); err != nil {
+		util.LogFatalAndExit(err, "failed to upgrade subnets")
+	}
+	if err := c.upgradeNetworkPolicies(); err != nil {
+		util.LogFatalAndExit(err, "failed to upgrade network policies")
+	}
+	if err := c.upgradeNodes(); err != nil {
+		util.LogFatalAndExit(err, "failed to upgrade nodes")
+	}
+}
+
 func (c *Controller) startWorkers(ctx context.Context) {
 	klog.Info("Starting workers")
 

diff --git a/pkg/controller/network_policy.go b/pkg/controller/network_policy.go
@@ -62,6 +62,40 @@ func (c *Controller) enqueueUpdateNp(oldObj, newObj interface{}) {
 	}
 }
 
+// for upgrading from v1.12.x to v1.13.x
+func (c *Controller) upgradeNetworkPoliciesForV1_13() error {
+	// clear legacy acls in tier 0 for all network policies
+	// including ingress, egress and subnet gateway acls
+	nps, err := c.npsLister.NetworkPolicies(corev1.NamespaceAll).List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list network policies %v", err)
+		return err
+	}
+
+	for _, np := range nps {
+		npName := np.Name
+		nameArray := []rune(np.Name)
+		if !unicode.IsLetter(nameArray[0]) {
+			npName = "np" + np.Name
+		}
+		pgName := strings.ReplaceAll(fmt.Sprintf("%s.%s", npName, np.Namespace), "-", ".")
+
+		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil {
+			klog.Errorf("clear legacy network policy %s acls: %v", pgName, err)
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (c *Controller) upgradeNetworkPolicies() error {
+	if err := c.upgradeNetworkPoliciesForV1_13(); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) createAsForNetpol(ns, name, direction, asName string, addresses []string) error {
 	if err := c.OVNNbClient.CreateAddressSet(asName, map[string]string{
 		networkPolicyKey: fmt.Sprintf("%s/%s/%s", ns, name, direction),
@@ -165,7 +199,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 		return err
 	}
 
-	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil)
+	ingressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "to-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("generate operations that clear np %s ingress acls: %v", key, err)
 		return err
@@ -281,7 +315,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 			}
 		}
 	} else {
-		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "to-lport", nil); err != nil {
+		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "to-lport", nil, -1); err != nil {
 			klog.Errorf("delete np %s ingress acls: %v", key, err)
 			return err
 		}
@@ -294,7 +328,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 		}
 	}
 
-	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil)
+	egressACLOps, err := c.OVNNbClient.DeleteAclsOps(pgName, portGroupKey, "from-lport", nil, -1)
 	if err != nil {
 		klog.Errorf("generate operations that clear np %s egress acls: %v", key, err)
 		return err
@@ -408,7 +442,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 			}
 		}
 	} else {
-		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "from-lport", nil); err != nil {
+		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "from-lport", nil, -1); err != nil {
 			klog.Errorf("delete np %s egress acls: %v", key, err)
 			return err
 		}

diff --git a/pkg/controller/node.go b/pkg/controller/node.go
@@ -94,6 +94,38 @@ func nodeUnderlayAddressSetName(node string, af int) string {
 	return fmt.Sprintf("node_%s_underlay_v%d", strings.ReplaceAll(node, "-", "_"), af)
 }
 
+// for upgrading from v1.12.x to v1.13.x
+func (c *Controller) upgradeNodesForV1_13() error {
+	// clear legacy acls in tier 0 for node port group
+	nodes, err := c.nodesLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list nodes: %v", err)
+		return err
+	}
+
+	for _, node := range nodes {
+		pgName := strings.ReplaceAll(node.Annotations[util.PortNameAnnotation], "-", ".")
+
+		if pgName == "" {
+			continue
+		}
+
+		if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil {
+			klog.Errorf("delete legacy node acl for node pg %s: %v", pgName, err)
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (c *Controller) upgradeNodes() error {
+	if err := c.upgradeNodesForV1_13(); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) handleAddNode(key string) error {
 	c.nodeKeyMutex.LockKey(key)
 	defer func() { _ = c.nodeKeyMutex.UnlockKey(key) }()
@@ -786,7 +818,7 @@ func (c *Controller) checkAndUpdateNodePortGroup() error {
 			}
 		} else {
 			// clear all acl
-			if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil); err != nil {
+			if err = c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, -1); err != nil {
 				klog.Errorf("delete node acl for node pg %s: %v", pgName, err)
 			}
 		}

diff --git a/pkg/controller/security_group.go b/pkg/controller/security_group.go
@@ -62,6 +62,39 @@ func (c *Controller) enqueueDeleteSg(obj interface{}) {
 	c.delSgQueue.Add(key)
 }
 
+// for upgrading from v1.12.x to v1.13.x
+func (c *Controller) upgradeSecurityGroupsForV1_13() error {
+	// clear legacy acls in tier 0 for deny all sg
+	pgName := ovs.GetSgPortGroupName(util.DenyAllSecurityGroup)
+	if err := c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil {
+		klog.Error(err)
+		return fmt.Errorf("delete legacy acls from port group %s: %w", pgName, err)
+	}
+
+	// clear legacy acls in tier 0 for all sg port groups
+	sgs, err := c.sgsLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list security groups: %v", err)
+		return err
+	}
+	for _, sg := range sgs {
+		pgName := ovs.GetSgPortGroupName(sg.Name)
+		if err := c.OVNNbClient.DeleteAcls(pgName, portGroupKey, "", nil, util.DefaultACLTier); err != nil {
+			klog.Error(err)
+			return fmt.Errorf("delete legacy acls from port group %s: %w", pgName, err)
+		}
+	}
+
+	return nil
+}
+
+func (c *Controller) upgradeSecurityGroups() error {
+	if err := c.upgradeSecurityGroupsForV1_13(); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) initDefaultDenyAllSecurityGroup() error {
 	pgName := ovs.GetSgPortGroupName(util.DenyAllSecurityGroup)
 	if err := c.OVNNbClient.CreatePortGroup(pgName, map[string]string{

diff --git a/pkg/controller/subnet.go b/pkg/controller/subnet.go
@@ -152,6 +152,31 @@ func (c *Controller) enqueueUpdateSubnet(oldObj, newObj interface{}) {
 	}
 }
 
+// for upgrading from v1.12.x to v1.13.x
+func (c *Controller) upgradeSubnetsForV1_13() error {
+	// clear legacy acls in tier 0 for all subnets
+	subnets, err := c.subnetsLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("failed to list subnets %v", err)
+		return err
+	}
+
+	for _, subnet := range subnets {
+		if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil, util.DefaultACLTier); err != nil {
+			klog.Errorf("clear legacy logical switch %s acls: %v", subnet.Name, err)
+			return err
+		}
+	}
+	return nil
+}
+
+func (c *Controller) upgradeSubnets() error {
+	if err := c.upgradeSubnetsForV1_13(); err != nil {
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) formatSubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) {
 	var (
 		changed bool
@@ -790,7 +815,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
 		}
 	} else {
 		// clear acl when direction is ""
-		if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil); err != nil {
+		if err = c.OVNNbClient.DeleteAcls(subnet.Name, logicalSwitchKey, "", nil, -1); err != nil {
 			if err = c.patchSubnetStatus(subnet, "ResetLogicalSwitchAclFailed", err.Error()); err != nil {
 				klog.Error(err)
 				return err
@@ -890,7 +915,7 @@ func (c *Controller) handleDeleteLogicalSwitch(key string) (err error) {
 	}
 
 	// clear acl when direction is ""
-	if err = c.OVNNbClient.DeleteAcls(key, logicalSwitchKey, "", nil); err != nil {
+	if err = c.OVNNbClient.DeleteAcls(key, logicalSwitchKey, "", nil, -1); err != nil {
 		klog.Errorf("clear logical switch %s acls: %v", key, err)
 		return err
 	}

diff --git a/pkg/ovs/interface.go b/pkg/ovs/interface.go
@@ -162,8 +162,8 @@ type ACL interface {
 	SetACLLog(pgName string, logEnable, isIngress bool) error
 	SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error
 	SGLostACL(sg *kubeovnv1.SecurityGroup) (bool, error)
-	DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error
-	DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error)
+	DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error
+	DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error)
 	UpdateAnpRuleACLOps(pgName, asName, protocol, aclName string, priority int, aclAction ovnnb.ACLAction, logACLActions []ovnnb.ACLAction, rulePorts []v1alpha1.AdminNetworkPolicyPort, isIngress, isBanp bool) ([]ovsdb.Operation, error)
 }