Skip to content

Commit 52a4d23

Browse files
committed
feat: add vendor tagging for OVN resources and migration support
Add vendor=kube-ovn externalID to all OVN resources created by kube-ovn to distinguish them from resources created by external systems like OpenStack Neutron. This prevents kube-ovn from accidentally garbage collecting or modifying resources it doesn't own. Changes: - Add version tracking in NBGlobal.ExternalIDs["kube-ovn-version"] - Add migration that runs only when upgrading from versions < v1.15.0 - Auto-tag new resources: AddressSet, PortGroup, LoadBalancer, LogicalRouterPort, ACL - Update GC functions to only operate on vendor=kube-ovn resources - Update CleanNoParentKeyAcls to skip non-kube-ovn ACLs - Add pattern matching to identify existing kube-ovn resources during migration (security groups, network policies, load balancers) The migration identifies kube-ovn resources using: - Existing externalIDs (lr, ls, sg, parent, subnet) - Naming patterns (ovn.sg.*, cluster-*-loadbalancer, vpc-*-load) - Association with tagged logical routers/switches Resources that cannot be positively identified as kube-ovn owned are left untouched to avoid interfering with external systems. Fixes: #5995 Signed-off-by: Kevin Carter <kevin.carter@rackspace.com>
1 parent 7e936e7 commit 52a4d23

17 files changed

+1111
-35
lines changed

mocks/pkg/ovs/interface.go

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/controller/gc.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,12 @@ func (c *Controller) gcLoadBalancer() error {
599599
}
600600
}
601601
// lbs will remove from logical switch automatically when delete lbs
602+
// Only delete load balancers that belong to kube-ovn (vendor=kube-ovn)
603+
// This prevents deleting load balancers managed by external systems like OpenStack Neutron
602604
if err = c.OVNNbClient.DeleteLoadBalancers(func(lb *ovnnb.LoadBalancer) bool {
605+
if lb.ExternalIDs["vendor"] != util.CniTypeName {
606+
return false
607+
}
603608
return !vpcLbs.Has(lb.Name)
604609
}); err != nil {
605610
klog.Errorf("delete all load balancers: %v", err)
@@ -740,8 +745,9 @@ func (c *Controller) gcLoadBalancer() error {
740745

741746
func (c *Controller) gcAddressSet() error {
742747
klog.Infof("start to gc address set")
743-
// get all address
744-
addressSets, err := c.OVNNbClient.ListAddressSets(nil)
748+
// Only list address sets that belong to kube-ovn (vendor=kube-ovn)
749+
// This prevents deleting address sets managed by external systems like OpenStack Neutron
750+
addressSets, err := c.OVNNbClient.ListAddressSets(map[string]string{"vendor": util.CniTypeName})
745751
if err != nil {
746752
klog.Errorf("failed to list address set,%v", err)
747753
return err
@@ -784,7 +790,9 @@ func (c *Controller) gcSecurityGroup() error {
784790
sgSet.Add(sg.Name)
785791
}
786792

787-
pgs, err := c.OVNNbClient.ListPortGroups(nil)
793+
// Only list port groups that belong to kube-ovn (vendor=kube-ovn)
794+
// This prevents deleting port groups managed by external systems like OpenStack Neutron
795+
pgs, err := c.OVNNbClient.ListPortGroups(map[string]string{"vendor": util.CniTypeName})
788796
if err != nil {
789797
klog.Errorf("failed to list port group,%v", err)
790798
return err
@@ -1220,6 +1228,12 @@ func (c *Controller) gcVPCDNS() error {
12201228

12211229
func logicalRouterPortFilter(exceptPeerPorts *strset.Set) func(lrp *ovnnb.LogicalRouterPort) bool {
12221230
return func(lrp *ovnnb.LogicalRouterPort) bool {
1231+
// Only delete logical router ports that belong to kube-ovn (vendor=kube-ovn)
1232+
// This prevents deleting LRPs managed by external systems like OpenStack Neutron
1233+
if lrp.ExternalIDs["vendor"] != util.CniTypeName {
1234+
return false
1235+
}
1236+
12231237
if exceptPeerPorts.Has(lrp.Name) {
12241238
return false // ignore except lrp
12251239
}

pkg/controller/gc_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/stretchr/testify/require"
88

99
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
10+
"github.com/kubeovn/kube-ovn/pkg/util"
1011
)
1112

1213
func newLogicalRouterPort(lrName, lrpName, mac string, networks []string) *ovnnb.LogicalRouterPort {
@@ -15,7 +16,8 @@ func newLogicalRouterPort(lrName, lrpName, mac string, networks []string) *ovnnb
1516
MAC: mac,
1617
Networks: networks,
1718
ExternalIDs: map[string]string{
18-
"lr": lrName,
19+
"lr": lrName,
20+
"vendor": util.CniTypeName,
1921
},
2022
}
2123
}

pkg/controller/init.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,24 @@ import (
3030
func (c *Controller) InitOVN() error {
3131
var err error
3232

33-
if err = c.migrateACLForVersionCompat(); err != nil {
34-
klog.Errorf("failed to sync the older acl : %v", err)
33+
// migrate vendor externalIDs to kube-ovn resources created in versions prior to v1.15.0
34+
// this must run before ACL cleanup to ensure existing resources are properly tagged
35+
if err = c.OVNNbClient.MigrateVendorExternalIDs(); err != nil {
36+
klog.Errorf("failed to migrate vendor externalIDs: %v", err)
37+
return err
38+
}
39+
40+
// migrate tier field of ACL rules created in versions prior to v1.13.0
41+
// after upgrading, the tier field has a default value of zero, which is not the value used in versions >= v1.13.0
42+
// we need to migrate the tier field to the correct value
43+
if err = c.OVNNbClient.MigrateACLTier(); err != nil {
44+
klog.Errorf("failed to migrate ACL tier: %v", err)
45+
return err
46+
}
47+
48+
// clean all no parent key acls
49+
if err = c.OVNNbClient.CleanNoParentKeyAcls(); err != nil {
50+
klog.Errorf("failed to clean all no parent key acls: %v", err)
3551
return err
3652
}
3753

@@ -70,22 +86,6 @@ func (c *Controller) InitOVN() error {
7086
return nil
7187
}
7288

73-
func (c *Controller) migrateACLForVersionCompat() error {
74-
// migrate tier field of ACL rules created in versions prior to v1.13.0
75-
// after upgrading, the tier field has a default value of zero, which is not the value used in versions >= v1.13.0
76-
// we need to migrate the tier field to the correct value
77-
if err := c.OVNNbClient.MigrateACLTier(); err != nil {
78-
klog.Errorf("failed to migrate ACL tier: %v", err)
79-
return err
80-
}
81-
// clean all no parent key acls
82-
if err := c.OVNNbClient.CleanNoParentKeyAcls(); err != nil {
83-
klog.Errorf("failed to clean all no parent key acls: %v", err)
84-
return err
85-
}
86-
return nil
87-
}
88-
8989
func (c *Controller) InitDefaultVpc() error {
9090
cachedVpc, err := c.vpcsLister.Get(c.config.ClusterRouter)
9191
if err != nil {

pkg/ovs/interface.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ type NBGlobal interface {
2727
SetNodeLocalDNSIP(nodeLocalDNSIP string) error
2828
SetSkipConntrackCidrs(skipConntrackCidrs string) error
2929
GetNbGlobal() (*ovnnb.NBGlobal, error)
30+
MigrateVendorExternalIDs() error
3031
}
3132

3233
type LogicalRouter interface {

pkg/ovs/ovn-nb-acl.go

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,7 @@ func (c *OVNNbClient) newACL(parent, direction, priority, match, action string,
10031003
Priority: intPriority,
10041004
ExternalIDs: map[string]string{
10051005
aclParentKey: parent,
1006+
"vendor": util.CniTypeName,
10061007
},
10071008
Tier: tier,
10081009
}
@@ -1036,6 +1037,7 @@ func (c *OVNNbClient) newACLWithoutCheck(parent, direction, priority, match, act
10361037
Priority: intPriority,
10371038
ExternalIDs: map[string]string{
10381039
aclParentKey: parent,
1040+
"vendor": util.CniTypeName,
10391041
},
10401042
Tier: tier,
10411043
}
@@ -1726,10 +1728,21 @@ func (c *OVNNbClient) CleanNoParentKeyAcls() error {
17261728

17271729
var aclList []ovnnb.ACL
17281730
if err := c.ovsDbClient.WhereCache(func(acl *ovnnb.ACL) bool {
1729-
_, ok := acl.ExternalIDs[aclParentKey]
1730-
return !ok
1731+
// Only clean ACLs that belong to kube-ovn (vendor=kube-ovn) but are missing the parent key.
1732+
// This ensures we never touch ACLs created by external systems like OpenStack Neutron.
1733+
// ACLs without vendor tag or with a different vendor are left untouched.
1734+
if len(acl.ExternalIDs) == 0 {
1735+
return false
1736+
}
1737+
// Skip ACLs that don't belong to kube-ovn
1738+
if acl.ExternalIDs["vendor"] != util.CniTypeName {
1739+
return false
1740+
}
1741+
// Only target kube-ovn ACLs that are missing the parent key
1742+
_, hasParent := acl.ExternalIDs[aclParentKey]
1743+
return !hasParent
17311744
}).List(ctx, &aclList); err != nil {
1732-
err = fmt.Errorf("failed to list acls without parent: %w", err)
1745+
err = fmt.Errorf("failed to list kube-ovn acls without parent: %w", err)
17331746
klog.Error(err)
17341747
return err
17351748
}
@@ -1769,7 +1782,7 @@ func (c *OVNNbClient) CleanNoParentKeyAcls() error {
17691782

17701783
if err := c.Transact("acl-clean-no-parent", ops); err != nil {
17711784
klog.Error(err)
1772-
return fmt.Errorf("failed to clean acls without parent: %w", err)
1785+
return fmt.Errorf("failed to clean kube-ovn acls without parent: %w", err)
17731786
}
17741787

17751788
return nil

pkg/ovs/ovn-nb-acl_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func newACL(parentName, direction, priority, match, action string, tier int, opt
7070
Priority: intPriority,
7171
ExternalIDs: map[string]string{
7272
aclParentKey: parentName,
73+
"vendor": util.CniTypeName,
7374
},
7475
Tier: tier,
7576
}
@@ -1910,6 +1911,7 @@ func (suite *OvnClientTestSuite) testNewACL() {
19101911
Priority: 1000,
19111912
ExternalIDs: map[string]string{
19121913
aclParentKey: pgName,
1914+
"vendor": util.CniTypeName,
19131915
},
19141916
Log: true,
19151917
Severity: ptr.To(ovnnb.ACLSeverityWarning),

pkg/ovs/ovn-nb-address_set.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"maps"
78
"net"
89
"strings"
910

@@ -13,6 +14,7 @@ import (
1314
"k8s.io/klog/v2"
1415

1516
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
17+
"github.com/kubeovn/kube-ovn/pkg/util"
1618
)
1719

1820
// CreateAddressSet create address set with external ids
@@ -22,6 +24,11 @@ func (c *OVNNbClient) CreateAddressSet(asName string, externalIDs map[string]str
2224
return fmt.Errorf("address set %s must match `[a-zA-Z_.][a-zA-Z_.0-9]*`", asName)
2325
}
2426

27+
// Create new map with vendor tag to avoid modifying caller's map
28+
finalExternalIDs := make(map[string]string, len(externalIDs)+1)
29+
maps.Copy(finalExternalIDs, externalIDs)
30+
finalExternalIDs["vendor"] = util.CniTypeName
31+
2532
exists, err := c.AddressSetExists(asName)
2633
if err != nil {
2734
klog.Error(err)
@@ -35,7 +42,7 @@ func (c *OVNNbClient) CreateAddressSet(asName string, externalIDs map[string]str
3542

3643
as := &ovnnb.AddressSet{
3744
Name: asName,
38-
ExternalIDs: externalIDs,
45+
ExternalIDs: finalExternalIDs,
3946
}
4047

4148
ops, err := c.Create(as)

pkg/ovs/ovn-nb-address_set_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/stretchr/testify/require"
88

99
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
10+
"github.com/kubeovn/kube-ovn/pkg/util"
1011
)
1112

1213
func newAddressSet(name string, externalIDs map[string]string) *ovnnb.AddressSet {
@@ -33,8 +34,10 @@ func (suite *OvnClientTestSuite) testCreateAddressSet() {
3334
require.NoError(t, err)
3435
require.NotEmpty(t, as.UUID)
3536
require.Equal(t, asName, as.Name)
37+
// vendor is automatically added by CreateAddressSet
3638
require.Equal(t, map[string]string{
37-
sgKey: "test-sg",
39+
sgKey: "test-sg",
40+
"vendor": util.CniTypeName,
3841
}, as.ExternalIDs)
3942
})
4043

pkg/ovs/ovn-nb-load_balancer.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
ovsclient "github.com/kubeovn/kube-ovn/pkg/ovsdb/client"
1818
"github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb"
19+
"github.com/kubeovn/kube-ovn/pkg/util"
1920
)
2021

2122
// CreateLoadBalancer create loadbalancer
@@ -43,6 +44,9 @@ func (c *OVNNbClient) CreateLoadBalancer(lbName, protocol, selectFields string)
4344
UUID: ovsclient.NamedUUID(),
4445
Name: lbName,
4546
Protocol: &protocol,
47+
ExternalIDs: map[string]string{
48+
"vendor": util.CniTypeName,
49+
},
4650
}
4751

4852
if len(selectFields) != 0 {

0 commit comments

Comments
 (0)