fix network policy err

pkg/controller/network_policy.go

@@ -242,6 +242,12 @@ func (c *Controller) handleUpdateNp(key string) error {
 		}
 	}
 
+	// we should first delete acl and address_set before update
+	if err = c.ovnClient.DeleteAcls(pgName, portGroupKey, "", nil); err != nil {
+		klog.Errorf("delete np %s acls: %v", key, err)
+		return err
+	}
+
 	if err := c.ovnClient.DeleteAddressSets(map[string]string{
 		networkPolicyKey: fmt.Sprintf("%s/%s/%s", np.Namespace, np.Name, "ingress"),
 	}); err != nil {
@@ -377,7 +383,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 			}
 		}
 	} else {
-		if err = c.ovnClient.DeleteAcls(key, portGroupKey, "to-lport"); err != nil {
+		if err = c.ovnClient.DeleteAcls(key, portGroupKey, "to-lport", nil); err != nil {
 			klog.Errorf("delete np %s ingress acls: %v", key, err)
 			return err
 		}
@@ -509,7 +515,7 @@ func (c *Controller) handleUpdateNp(key string) error {
 			}
 		}
 	} else {
-		if err = c.ovnClient.DeleteAcls(key, portGroupKey, "from-lport"); err != nil {
+		if err = c.ovnClient.DeleteAcls(key, portGroupKey, "from-lport", nil); err != nil {
 			klog.Errorf("delete np %s egress acls: %v", key, err)
 			return err
 		}

pkg/controller/node.go

@@ -977,7 +977,7 @@ func (c *Controller) checkAndUpdateNodePortGroup() error {
 			}
 		} else {
 			// clear all acl
-			if err = c.ovnClient.DeleteAcls(pgName, portGroupKey, ""); err != nil {
+			if err = c.ovnClient.DeleteAcls(pgName, portGroupKey, "", nil); err != nil {
 				klog.Errorf("delete node acl for node pg %s: %v", pgName, err)
 			}
 		}

pkg/controller/subnet.go

@@ -757,7 +757,7 @@ func (c *Controller) handleAddOrUpdateSubnet(key string) error {
 		c.patchSubnetStatus(subnet, "SetPrivateLogicalSwitchSuccess", "")
 	} else {
 		// clear acl when direction is ""
-		if err = c.ovnClient.DeleteAcls(key, logicalSwitchKey, ""); err != nil {
+		if err = c.ovnClient.DeleteAcls(key, logicalSwitchKey, "", nil); err != nil {
 			c.patchSubnetStatus(subnet, "ResetLogicalSwitchAclFailed", err.Error())
 			return err
 		}
@@ -824,7 +824,7 @@ func (c *Controller) handleDeleteLogicalSwitch(key string) (err error) {
 	}
 
 	// clear acl when direction is ""
-	if err = c.ovnClient.DeleteAcls(key, logicalSwitchKey, ""); err != nil {
+	if err = c.ovnClient.DeleteAcls(key, logicalSwitchKey, "", nil); err != nil {
 		klog.Errorf("clear logical switch %s acls: %v", key, err)
 		return err
 	}

pkg/ovs/interface.go

@@ -59,7 +59,6 @@ type LogicalSwitchPort interface {
 	SetLogicalSwitchPortExternalIds(lspName string, externalIds map[string]string) error
 	SetLogicalSwitchPortVlanTag(lspName string, vlanID int) error
 	SetLogicalSwitchPortsSecurityGroup(sgName string, op string) error
-	UpdateLogicalSwitchAcl(lsName string, subnetAcls []kubeovnv1.Acl) error
 	EnablePortLayer2forward(lspName string) error
 	DeleteLogicalSwitchPort(lspName string) error
 	ListLogicalSwitchPorts(needVendorFilter bool, externalIDs map[string]string, filter func(lsp *ovnnb.LogicalSwitchPort) bool) ([]ovnnb.LogicalSwitchPort, error)
@@ -99,9 +98,10 @@ type ACL interface {
 	CreateSgDenyAllAcl(sgName string) error
 	CreateSgBaseACL(sgName string, direction string) error
 	UpdateSgAcl(sg *kubeovnv1.SecurityGroup, direction string) error
+	UpdateLogicalSwitchAcl(lsName string, subnetAcls []kubeovnv1.Acl) error
 	SetAclLog(pgName string, logEnable, isIngress bool) error
 	SetLogicalSwitchPrivate(lsName, cidrBlock string, allowSubnets []string) error
-	DeleteAcls(parentName, parentType string, direction string) error
+	DeleteAcls(parentName, parentType string, direction string, externalIDs map[string]string) error
 }
 
 type AddressSet interface {

pkg/ovs/ovn-nb-acl.go

@@ -136,27 +136,27 @@ func (c *ovnClient) CreateGatewayAcl(lsName, pgName, gateway string) error {
 			return fmt.Errorf("new allow ingress acl for %s: %v", parentName, err)
 		}
 
-		allowEgressAcl, err := c.newAcl(parentName, ovnnb.ACLDirectionFromLport, util.EgressAllowPriority, fmt.Sprintf("%s.dst == %s", ipSuffix, gw), ovnnb.ACLActionAllowRelated)
+		options := func(acl *ovnnb.ACL) {
+			if acl.Options == nil {
+				acl.Options = make(map[string]string)
+			}
+			acl.Options["apply-after-lb"] = "true"
+		}
+
+		allowEgressAcl, err := c.newAcl(parentName, ovnnb.ACLDirectionFromLport, util.EgressAllowPriority, fmt.Sprintf("%s.dst == %s", ipSuffix, gw), ovnnb.ACLActionAllowRelated, options)
 		if err != nil {
 			return fmt.Errorf("new allow egress acl for %s: %v", parentName, err)
 		}
 
 		acls = append(acls, allowIngressAcl, allowEgressAcl)
 
 		if ipSuffix == "ip6" {
-			options := func(acl *ovnnb.ACL) {
-				if acl.Options == nil {
-					acl.Options = make(map[string]string)
-				}
-				acl.Options["apply-after-lb"] = "true"
-			}
-
 			ndAcl, err := c.newAcl(parentName, ovnnb.ACLDirectionFromLport, util.EgressAllowPriority, "nd || nd_ra || nd_rs", ovnnb.ACLActionAllowRelated, options)
-			acls = append(acls, ndAcl)
-
 			if err != nil {
 				return fmt.Errorf("new nd acl for %s: %v", parentName, err)
 			}
+
+			acls = append(acls, ndAcl)
 		}
 
 	}
@@ -320,7 +320,7 @@ func (c *ovnClient) UpdateSgAcl(sg *kubeovnv1.SecurityGroup, direction string) e
 	pgName := GetSgPortGroupName(sg.Name)
 
 	// clear acl
-	if err := c.DeleteAcls(pgName, portGroupKey, direction); err != nil {
+	if err := c.DeleteAcls(pgName, portGroupKey, direction, nil); err != nil {
 		return fmt.Errorf("delete direction '%s' acls from port group %s: %v", direction, pgName, err)
 	}
 
@@ -373,18 +373,25 @@ func (c *ovnClient) UpdateSgAcl(sg *kubeovnv1.SecurityGroup, direction string) e
 }
 
 func (c *ovnClient) UpdateLogicalSwitchAcl(lsName string, subnetAcls []kubeovnv1.Acl) error {
-	if err := c.DeleteAcls(lsName, logicalSwitchKey, ""); err != nil {
-		return fmt.Errorf("clear logical switch %s acls: %v", lsName, err)
+	if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}); err != nil {
+		return fmt.Errorf("delete subnet acls from %s: %v", lsName, err)
 	}
 
 	if len(subnetAcls) == 0 {
 		return nil
 	}
 	acls := make([]*ovnnb.ACL, 0, len(subnetAcls))
 
+	options := func(acl *ovnnb.ACL) {
+		if acl.ExternalIDs == nil {
+			acl.ExternalIDs = make(map[string]string)
+		}
+		acl.ExternalIDs["subnet"] = lsName
+	}
+
 	/* recreate logical switch acl */
 	for _, subnetAcl := range subnetAcls {
-		acl, err := c.newAcl(lsName, subnetAcl.Direction, strconv.Itoa(subnetAcl.Priority), subnetAcl.Match, subnetAcl.Action)
+		acl, err := c.newAcl(lsName, subnetAcl.Direction, strconv.Itoa(subnetAcl.Priority), subnetAcl.Match, subnetAcl.Action, options)
 		if err != nil {
 			return fmt.Errorf("new acl for logical switch %s: %v", lsName, err)
 		}
@@ -419,7 +426,7 @@ func (c *ovnClient) UpdateAcl(acl *ovnnb.ACL, fields ...interface{}) error {
 // SetLogicalSwitchPrivate will drop all ingress traffic except allow subnets, same subnet and node subnet
 func (c *ovnClient) SetLogicalSwitchPrivate(lsName, cidrBlock string, allowSubnets []string) error {
 	// clear acls
-	if err := c.DeleteAcls(lsName, logicalSwitchKey, ""); err != nil {
+	if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil); err != nil {
 		return fmt.Errorf("clear logical switch %s acls: %v", lsName, err)
 	}
 
@@ -633,8 +640,12 @@ func (c *ovnClient) CreateBareAcl(parentName, direction, priority, match, action
 // DeleteAcls delete several acl once,
 // delete to-lport and from-lport direction acl when direction is empty, otherwise one-way
 // parentType is 'ls' or 'pg'
-func (c *ovnClient) DeleteAcls(parentName, parentType string, direction string) error {
-	externalIDs := map[string]string{aclParentKey: parentName}
+func (c *ovnClient) DeleteAcls(parentName, parentType string, direction string, externalIDs map[string]string) error {
+	if externalIDs == nil {
+		externalIDs = make(map[string]string)
+	}
+
+	externalIDs[aclParentKey] = parentName
 
 	/* delete acls from port group or logical switch */
 	acls, err := c.ListAcls(direction, externalIDs)

pkg/ovs/ovn-nb-acl_test.go

@@ -346,7 +346,9 @@ func (suite *OvnClientTestSuite) testCreateGatewayAcl() {
 			checkAcl(parent, ovnnb.ACLDirectionToLport, util.IngressAllowPriority, match, nil)
 
 			match = fmt.Sprintf("%s.dst == %s", ipSuffix, gw)
-			checkAcl(parent, ovnnb.ACLDirectionFromLport, util.EgressAllowPriority, match, nil)
+			checkAcl(parent, ovnnb.ACLDirectionFromLport, util.EgressAllowPriority, match, map[string]string{
+				"apply-after-lb": "true",
+			})
 
 			if ipSuffix == "ip6" {
 				match = "nd || nd_ra || nd_rs"
@@ -777,6 +779,7 @@ func (suite *OvnClientTestSuite) testUpdateLogicalSwitchAcl() {
 		require.NoError(t, err)
 		expect := newAcl(lsName, subnetAcl.Direction, strconv.Itoa(subnetAcl.Priority), subnetAcl.Match, subnetAcl.Action)
 		expect.UUID = acl.UUID
+		expect.ExternalIDs["subnet"] = lsName
 		require.Equal(t, expect, acl)
 		require.Contains(t, ls.ACLs, acl.UUID)
 	}
@@ -1242,7 +1245,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
 		require.NoError(t, err)
 		require.Len(t, pg.ACLs, 5)
 
-		err = ovnClient.DeleteAcls(pgName, portGroupKey, "")
+		err = ovnClient.DeleteAcls(pgName, portGroupKey, "", nil)
 		require.NoError(t, err)
 
 		pg, err = ovnClient.GetPortGroup(pgName, false)
@@ -1279,15 +1282,15 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
 		require.Len(t, pg.ACLs, 5)
 
 		/* delete to-lport direction acl */
-		err = ovnClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionToLport)
+		err = ovnClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionToLport, nil)
 		require.NoError(t, err)
 
 		pg, err = ovnClient.GetPortGroup(pgName, false)
 		require.NoError(t, err)
 		require.Len(t, pg.ACLs, 3)
 
 		/* delete from-lport direction acl */
-		err = ovnClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionFromLport)
+		err = ovnClient.DeleteAcls(pgName, portGroupKey, ovnnb.ACLDirectionFromLport, nil)
 		require.NoError(t, err)
 
 		pg, err = ovnClient.GetPortGroup(pgName, false)
@@ -1323,7 +1326,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
 		require.NoError(t, err)
 		require.Len(t, ls.ACLs, 5)
 
-		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, "")
+		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, "", nil)
 		require.NoError(t, err)
 
 		ls, err = ovnClient.GetLogicalSwitch(lsName, false)
@@ -1360,15 +1363,52 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
 		require.Len(t, ls.ACLs, 5)
 
 		/* delete to-lport direction acl */
-		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport)
+		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, nil)
 		require.NoError(t, err)
 
 		ls, err = ovnClient.GetLogicalSwitch(lsName, false)
 		require.NoError(t, err)
 		require.Len(t, ls.ACLs, 3)
 
 		/* delete from-lport direction acl */
-		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionFromLport)
+		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionFromLport, nil)
+		require.NoError(t, err)
+
+		ls, err = ovnClient.GetLogicalSwitch(lsName, false)
+		require.NoError(t, err)
+		require.Empty(t, ls.ACLs)
+	})
+
+	t.Run("delete acls with additional external ids", func(t *testing.T) {
+		priority := "5801"
+		basePort := 5801
+		acls := make([]*ovnnb.ACL, 0, 5)
+
+		// to-lport
+
+		match := fmt.Sprintf("%s && udp.dst == %d", matchPrefix, basePort)
+		acl, err := ovnClient.newAcl(lsName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, func(acl *ovnnb.ACL) {
+			if acl.ExternalIDs == nil {
+				acl.ExternalIDs = make(map[string]string)
+			}
+			acl.ExternalIDs["subnet"] = lsName
+		})
+		require.NoError(t, err)
+		acls = append(acls, acl)
+
+		err = ovnClient.CreateAcls(lsName, logicalSwitchKey, acls...)
+		require.NoError(t, err)
+
+		ls, err := ovnClient.GetLogicalSwitch(lsName, false)
+		require.NoError(t, err)
+		require.Len(t, ls.ACLs, 1)
+
+		newAcl := &ovnnb.ACL{UUID: ls.ACLs[0]}
+		err = ovnClient.GetEntityInfo(newAcl)
+		require.NoError(t, err)
+
+		/* delete to-lport direction acl */
+		err = ovnClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
 		require.NoError(t, err)
 
 		ls, err = ovnClient.GetLogicalSwitch(lsName, false)

pkg/ovs/ovn-nb-suite_test.go

@@ -632,8 +632,7 @@ func Test_scratch(t *testing.T) {
 	ovnClient, err := newOvnClient(t, endpoint, 10, "")
 	require.NoError(t, err)
 
-	addresses := []string{"10.104.25.159", "10.244.0.78"}
-	err = ovnClient.AddressSetUpdateAddress("test.as", addresses...)
+	err = ovnClient.CreateGatewayAcl("ovn-default", "", "10.16.0.1")
 	require.NoError(t, err)
 }
 

pkg/ovs/ovn-nb.go

@@ -101,7 +101,7 @@ func (c *ovnClient) DeleteSecurityGroup(sgName string) error {
 	pgName := GetSgPortGroupName(sgName)
 
 	// clear acl
-	if err := c.DeleteAcls(pgName, portGroupKey, ""); err != nil {
+	if err := c.DeleteAcls(pgName, portGroupKey, "", nil); err != nil {
 		return fmt.Errorf("delete acls from port group %s: %v", pgName, err)
 	}