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 · a5568f8 · a5568f8
1 parent 86ad84b
commit a5568f8
Show file tree

Hide file tree

Showing 20 changed files with 435 additions and 59 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, util.NilACLTier)
 	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, util.NilACLTier)
 	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, util.NilACLTier)
 	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, util.NilACLTier)
 	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/controller_test.go b/pkg/controller/controller_test.go
@@ -7,6 +7,7 @@ import (
 	"go.uber.org/mock/gomock"
 	"k8s.io/client-go/informers"
 	coreinformers "k8s.io/client-go/informers/core/v1"
+	networkinformers "k8s.io/client-go/informers/networking/v1"
 	"k8s.io/client-go/kubernetes/fake"
 
 	mockovs "github.com/kubeovn/kube-ovn/mocks/pkg/ovs"
@@ -18,7 +19,10 @@ import (
 type fakeControllerInformers struct {
 	vpcInformer     kubeovninformer.VpcInformer
 	subnetInformer  kubeovninformer.SubnetInformer
+	sgInformer      kubeovninformer.SecurityGroupInformer
 	serviceInformer coreinformers.ServiceInformer
+	npInformer      networkinformers.NetworkPolicyInformer
+	nodeInformer    coreinformers.NodeInformer
 }
 
 type fakeController struct {
@@ -34,25 +38,34 @@ func newFakeController(t *testing.T) *fakeController {
 	kubeClient := fake.NewSimpleClientset()
 	kubeInformerFactory := informers.NewSharedInformerFactory(kubeClient, 0)
 	serviceInformer := kubeInformerFactory.Core().V1().Services()
+	npInformer := kubeInformerFactory.Networking().V1().NetworkPolicies()
+	nodeInformer := kubeInformerFactory.Core().V1().Nodes()
 
 	/* fake kube ovn client */
 	kubeovnClient := kubeovnfake.NewSimpleClientset()
 	kubeovnInformerFactory := kubeovninformerfactory.NewSharedInformerFactory(kubeovnClient, 0)
 	vpcInformer := kubeovnInformerFactory.Kubeovn().V1().Vpcs()
 	subnetInformer := kubeovnInformerFactory.Kubeovn().V1().Subnets()
+	sgInformer := kubeovnInformerFactory.Kubeovn().V1().SecurityGroups()
 
 	fakeInformers := &fakeControllerInformers{
 		vpcInformer:     vpcInformer,
 		subnetInformer:  subnetInformer,
+		sgInformer:      sgInformer,
 		serviceInformer: serviceInformer,
+		npInformer:      npInformer,
+		nodeInformer:    nodeInformer,
 	}
 
 	/* ovn fake client */
 	mockOvnClient := mockovs.NewMockNbClient(gomock.NewController(t))
 
 	ctrl := &Controller{
 		servicesLister:        serviceInformer.Lister(),
+		npsLister:             npInformer.Lister(),
+		nodesLister:           nodeInformer.Lister(),
 		vpcsLister:            vpcInformer.Lister(),
+		sgsLister:             sgInformer.Lister(),
 		vpcSynced:             alwaysReady,
 		subnetsLister:         subnetInformer.Lister(),
 		subnetSynced:          alwaysReady,

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) upgradeNetworkPoliciesToV1_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.upgradeNetworkPoliciesToV1_13(); err != nil {
+		klog.Errorf("failed to upgrade network policies to v1.13.x, err: %v", err)
+		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, util.NilACLTier)
 	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, util.NilACLTier); 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, util.NilACLTier)
 	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, util.NilACLTier); err != nil {
 			klog.Errorf("delete np %s egress acls: %v", key, err)
 			return err
 		}

diff --git a/pkg/controller/network_policy_test.go b/pkg/controller/network_policy_test.go
@@ -0,0 +1,35 @@
+package controller
+
+import (
+	"testing"
+
+	"github.com/kubeovn/kube-ovn/pkg/util"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/mock/gomock"
+	netv1 "k8s.io/api/networking/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func Test_upgradeNetworkPolicies(t *testing.T) {
+	t.Parallel()
+
+	fakeController := newFakeController(t)
+	ctrl := fakeController.fakeController
+	fakeinformers := fakeController.fakeInformers
+	mockOvnClient := fakeController.mockOvnClient
+
+	np := &netv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "np1",
+			Namespace: "default",
+		},
+	}
+
+	err := fakeinformers.npInformer.Informer().GetStore().Add(np)
+	require.NoError(t, err)
+
+	mockOvnClient.EXPECT().DeleteAcls(gomock.Any(), portGroupKey, "", nil, util.DefaultACLTier).Return(nil)
+
+	err = ctrl.upgradeNetworkPolicies()
+	require.NoError(t, err)
+}
diff --git a/pkg/controller/node.go b/pkg/controller/node.go
@@ -94,6 +94,37 @@ 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) upgradeNodesToV1_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.upgradeNodesToV1_13(); err != nil {
+		klog.Errorf("failed to upgrade nodes to v1.13.x, err: %v", err)
+		return err
+	}
+	return nil
+}
+
 func (c *Controller) handleAddNode(key string) error {
 	c.nodeKeyMutex.LockKey(key)
 	defer func() { _ = c.nodeKeyMutex.UnlockKey(key) }()
@@ -786,7 +817,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, util.NilACLTier); err != nil {
 				klog.Errorf("delete node acl for node pg %s: %v", pgName, err)
 			}
 		}