Skip to content

Commit

Permalink
delete legacy acls when upgrading to v1.13.x (kubeovn#4742)
Browse files Browse the repository at this point in the history 
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: Rain Suo <[email protected]>
  • Loading branch information
@hackerain
hackerain committed Dec 2, 2024
1 parent 7b63325 commit 3b23583
Show file tree
Hide file tree
Showing 14 changed files with 173 additions and 46 deletions.
8 changes: 4 additions & 4 deletions mocks/pkg/ovs/interface.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions pkg/controller/admin_network_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/baseline_admin_network_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions pkg/controller/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,10 @@ func (c *Controller) Run(ctx context.Context) {
}
}

// for upgrading from v1.12.x to v1.13.x
// see https://github.com/kubeovn/kube-ovn/issues/4742
c.handleUpgrading()

// start workers to do all the network operations
c.startWorkers(ctx)

Expand Down Expand Up @@ -1078,6 +1082,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")

Expand Down
35 changes: 31 additions & 4 deletions pkg/controller/network_policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ func (c *Controller) enqueueUpdateNp(oldObj, newObj interface{}) {
}
}

func (c *Controller) upgradeNetworkPolicies() error {
// for upgrading from v1.12.x to v1.13.x

// 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) 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),
Expand Down Expand Up @@ -165,7 +192,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
Expand Down Expand Up @@ -281,7 +308,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
}
Expand All @@ -294,7 +321,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
Expand Down Expand Up @@ -408,7 +435,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
}
Expand Down
28 changes: 27 additions & 1 deletion pkg/controller/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,32 @@ func nodeUnderlayAddressSetName(node string, af int) string {
return fmt.Sprintf("node_%s_underlay_v%d", strings.ReplaceAll(node, "-", "_"), af)
}

func (c *Controller) upgradeNodes() error {
// for upgrading from v1.12.x to v1.13.x

// 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) handleAddNode(key string) error {
c.nodeKeyMutex.LockKey(key)
defer func() { _ = c.nodeKeyMutex.UnlockKey(key) }()
Expand Down Expand Up @@ -786,7 +812,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)
}
}
Expand Down
27 changes: 27 additions & 0 deletions pkg/controller/security_group.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,33 @@ func (c *Controller) enqueueDeleteSg(obj interface{}) {
c.delSgQueue.Add(key)
}

func (c *Controller) upgradeSecurityGroups() error {
// for upgrading from v1.12.x to v1.13.x

// clear legacy acls in tier 0 for deny all port group
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) initDefaultDenyAllSecurityGroup() error {
pgName := ovs.GetSgPortGroupName(util.DenyAllSecurityGroup)
if err := c.OVNNbClient.CreatePortGroup(pgName, map[string]string{
Expand Down
23 changes: 21 additions & 2 deletions pkg/controller/subnet.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,25 @@ func (c *Controller) enqueueUpdateSubnet(oldObj, newObj interface{}) {
}
}

func (c *Controller) upgradeSubnets() error {
// for upgrading from v1.12.x to v1.13.x

// 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) formatSubnet(subnet *kubeovnv1.Subnet) (*kubeovnv1.Subnet, error) {
var (
changed bool
Expand Down Expand Up @@ -790,7 +809,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
Expand Down Expand Up @@ -890,7 +909,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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/ovs/interface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,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)
}

Expand Down
28 changes: 18 additions & 10 deletions pkg/ovs/ovn-nb-acl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func (c *OVNNbClient) UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string)
pgName := GetSgPortGroupName(sg.Name)

// clear acl
if err := c.DeleteAcls(pgName, portGroupKey, direction, nil); err != nil {
if err := c.DeleteAcls(pgName, portGroupKey, direction, nil, -1); err != nil {
klog.Error(err)
return fmt.Errorf("delete direction '%s' acls from port group %s: %w", direction, pgName, err)
}
Expand Down Expand Up @@ -441,7 +441,7 @@ func (c *OVNNbClient) UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string)
}

func (c *OVNNbClient) UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcls []kubeovnv1.ACL, allowEWTraffic bool) error {
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}); err != nil {
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}, -1); err != nil {
klog.Error(err)
return fmt.Errorf("delete subnet acls from %s: %w", lsName, err)
}
Expand Down Expand Up @@ -530,7 +530,7 @@ func (c *OVNNbClient) UpdateACL(acl *ovnnb.ACL, fields ...interface{}) error {
// SetLogicalSwitchPrivate will drop all ingress traffic except allow subnets, same subnet and node subnet
func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error {
// clear acls
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil); err != nil {
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil, -1); err != nil {
klog.Error(err)
return fmt.Errorf("clear logical switch %s acls: %w", lsName, err)
}
Expand Down Expand Up @@ -731,8 +731,8 @@ func (c *OVNNbClient) CreateBareACL(parentName, direction, priority, match, acti
// 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 *OVNNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string) error {
ops, err := c.DeleteAclsOps(parentName, parentType, direction, externalIDs)
func (c *OVNNbClient) DeleteAcls(parentName, parentType, direction string, externalIDs map[string]string, tier int) error {
ops, err := c.DeleteAclsOps(parentName, parentType, direction, externalIDs, tier)
if err != nil {
klog.Error(err)
return err
Expand Down Expand Up @@ -822,14 +822,16 @@ func (c *OVNNbClient) GetACL(parent, direction, priority, match string, ignoreNo
// result should include all to-lport and from-lport acls when direction is empty,
// result should include all acls when externalIDs is empty,
// result should include all acls which externalIDs[key] is not empty when externalIDs[key] is ""
// result should include all acls when tier is -1
// result should include all acls in specific tier when tier is not -1
// TODO: maybe add other filter conditions(priority or match)
func (c *OVNNbClient) ListAcls(direction string, externalIDs map[string]string) ([]ovnnb.ACL, error) {
func (c *OVNNbClient) ListAcls(direction string, externalIDs map[string]string, tier int) ([]ovnnb.ACL, error) {
ctx, cancel := context.WithTimeout(context.Background(), c.Timeout)
defer cancel()

aclList := make([]ovnnb.ACL, 0)

if err := c.WhereCache(aclFilter(direction, externalIDs)).List(ctx, &aclList); err != nil {
if err := c.WhereCache(aclFilter(direction, externalIDs, tier)).List(ctx, &aclList); err != nil {
klog.Error(err)
return nil, fmt.Errorf("list acls: %w", err)
}
Expand Down Expand Up @@ -1091,8 +1093,10 @@ func newNetworkPolicyACLMatch(pgName, asAllowName, asExceptName, protocol, direc
// result should include all to-lport and from-lport acls when direction is empty,
// result should include all acls when externalIDs is empty,
// result should include all acls which externalIDs[key] is not empty when externalIDs[key] is ""
// result should include all acls when tier is -1
// result should include all acls in specific tier when tier is not -1
// TODO: maybe add other filter conditions(priority or match)
func aclFilter(direction string, externalIDs map[string]string) func(acl *ovnnb.ACL) bool {
func aclFilter(direction string, externalIDs map[string]string, tier int) func(acl *ovnnb.ACL) bool {
return func(acl *ovnnb.ACL) bool {
if len(acl.ExternalIDs) < len(externalIDs) {
return false
Expand All @@ -1118,6 +1122,10 @@ func aclFilter(direction string, externalIDs map[string]string) func(acl *ovnnb.
return false
}

if tier != -1 && acl.Tier != tier {
return false
}

return true
}
}
Expand Down Expand Up @@ -1173,7 +1181,7 @@ func (c *OVNNbClient) CreateAclsOps(parentName, parentType string, acls ...*ovnn
// DeleteAcls return operation which 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 *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string) ([]ovsdb.Operation, error) {
func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, externalIDs map[string]string, tier int) ([]ovsdb.Operation, error) {
if parentName == "" {
return nil, errors.New("the port group name or logical switch name is required")
}
Expand All @@ -1185,7 +1193,7 @@ func (c *OVNNbClient) DeleteAclsOps(parentName, parentType, direction string, ex
externalIDs[aclParentKey] = parentName

/* delete acls from port group or logical switch */
acls, err := c.ListAcls(direction, externalIDs)
acls, err := c.ListAcls(direction, externalIDs, tier)
if err != nil {
klog.Error(err)
return nil, fmt.Errorf("list type %s %s acls: %w", parentType, parentName, err)
Expand Down
Loading

0 comments on commit 3b23583

Please sign in to comment.