Skip to content

Commit 75a1eac

Browse files
zhangzujianzhangzujian
committed
controller: fix lsp not deleted in gc (#5735)
Signed-off-by: zhangzujian <[email protected]>
1 parent e33b6d3 commit 75a1eac

File tree

14 files changed

+166
-120
lines changed

14 files changed

+166
-120
lines changed

‎go.mod‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ require (
1818
github.com/evanphx/json-patch/v5 v5.9.11
1919
github.com/go-logr/logr v1.4.3
2020
github.com/go-logr/stdr v1.2.2
21+
github.com/go-logr/zapr v1.3.0
2122
github.com/google/gopacket v1.1.19
2223
github.com/google/uuid v1.6.0
2324
github.com/k8snetworkplumbingwg/network-attachment-definition-client v1.7.6
@@ -45,6 +46,7 @@ require (
4546
github.com/stretchr/testify v1.10.0
4647
github.com/vishvananda/netlink v1.3.1
4748
go.uber.org/mock v0.5.2
49+
go.uber.org/zap v1.27.0
4850
golang.org/x/mod v0.28.0
4951
golang.org/x/net v0.44.0
5052
golang.org/x/sys v0.36.0
@@ -178,7 +180,7 @@ require (
178180
go.opentelemetry.io/proto/otlp v1.7.0 // indirect
179181
go.uber.org/automaxprocs v1.6.0 // indirect
180182
go.uber.org/multierr v1.11.0 // indirect
181-
go.yaml.in/yaml/v2 v2.4.2 // indirect
183+
go.yaml.in/yaml/v2 v2.4.3 // indirect
182184
go.yaml.in/yaml/v3 v3.0.4 // indirect
183185
golang.org/x/exp v0.0.0-20250911091902-df9299821621 // indirect
184186
golang.org/x/oauth2 v0.31.0 // indirect
@@ -200,6 +202,7 @@ require (
200202
kubevirt.io/controller-lifecycle-operator-sdk/api v0.2.4 // indirect
201203
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.32.1 // indirect
202204
sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect
205+
sigs.k8s.io/randfill v1.0.0 // indirect
203206
sigs.k8s.io/structured-merge-diff/v4 v4.7.0 // indirect
204207
sigs.k8s.io/yaml v1.5.0 // indirect
205208
)

‎go.sum‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -560,8 +560,8 @@ go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0=
560560
go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y=
561561
go.uber.org/zap v1.27.0 h1:aJMhYGrd5QSmlpLMr2MftRKl7t8J8PTZPA732ud/XR8=
562562
go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
563-
go.yaml.in/yaml/v2 v2.4.2 h1:DzmwEr2rDGHl7lsFgAHxmNz/1NlQ7xLIrlN2h5d1eGI=
564-
go.yaml.in/yaml/v2 v2.4.2/go.mod h1:081UH+NErpNdqlCXm3TtEran0rJZGxAYx9hb/ELlsPU=
563+
go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0=
564+
go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8=
565565
go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc=
566566
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
567567
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
@@ -959,8 +959,9 @@ sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 h1:gBQPwqORJ8d8/YNZWEjoZs7np
959959
sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg=
960960
sigs.k8s.io/network-policy-api v0.1.5 h1:xyS7VAaM9EfyB428oFk7WjWaCK6B129i+ILUF4C8l6E=
961961
sigs.k8s.io/network-policy-api v0.1.5/go.mod h1:D7Nkr43VLNd7iYryemnj8qf0N/WjBzTZDxYA+g4u1/Y=
962-
sigs.k8s.io/randfill v0.0.0-20250304075658-069ef1bbf016 h1:kXv6kKdoEtedwuqMmkqhbkgvYKeycVbC8+iPCP9j5kQ=
963962
sigs.k8s.io/randfill v0.0.0-20250304075658-069ef1bbf016/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY=
963+
sigs.k8s.io/randfill v1.0.0 h1:JfjMILfT8A6RbawdsK2JXGBR5AQVfd+9TbzrlneTyrU=
964+
sigs.k8s.io/randfill v1.0.0/go.mod h1:XeLlZ/jmk4i1HRopwe7/aU3H5n1zNUcX6TM94b3QxOY=
964965
sigs.k8s.io/structured-merge-diff/v4 v4.4.2/go.mod h1:N8f93tFZh9U6vpxwRArLiikrE5/2tiu1w1AGfACIGE4=
965966
sigs.k8s.io/structured-merge-diff/v4 v4.7.0 h1:qPeWmscJcXP0snki5IYF79Z8xrl8ETFxgMd7wez1XkI=
966967
sigs.k8s.io/structured-merge-diff/v4 v4.7.0/go.mod h1:dDy58f92j70zLsuZVuUX5Wp9vtxXpaZnkPGWeqDfCps=

‎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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,8 +490,8 @@ func (c *Controller) markAndCleanLSP() error {
490490
continue
491491
}
492492

493-
klog.Infof("gc logical switch port %s", lsp.Name)
494-
if err := c.OVNNbClient.DeleteLogicalSwitchPort(lsp.Name); err != nil {
493+
klog.Infof("gc logical switch port %s with uuid %s", lsp.Name, lsp.UUID)
494+
if err := c.OVNNbClient.DeleteLogicalSwitchPortByUUID(lsp.ExternalIDs[ovs.LogicalSwitchKey], lsp.UUID); err != nil {
495495
klog.Errorf("failed to delete lsp %s: %v", lsp.Name, err)
496496
return err
497497
}

‎pkg/ovs/interface.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ type LogicalSwitchPort interface {
104104
SetLogicalSwitchPortsSecurityGroup(sgName, op string) error
105105
EnablePortLayer2forward(lspName string) error
106106
DeleteLogicalSwitchPort(lspName string) error
107+
DeleteLogicalSwitchPortByUUID(lsName, lspUUID string) error
107108
DeleteLogicalSwitchPorts(externalIDs map[string]string, filter func(lsp *ovnnb.LogicalSwitchPort) bool) error
108109
ListLogicalSwitchPorts(needVendorFilter bool, externalIDs map[string]string, filter func(lsp *ovnnb.LogicalSwitchPort) bool) ([]ovnnb.LogicalSwitchPort, error)
109110
ListNormalLogicalSwitchPorts(needVendorFilter bool, externalIDs map[string]string) ([]ovnnb.LogicalSwitchPort, error)

‎pkg/ovs/ovn-nb-acl.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func (c *OVNNbClient) CreateGatewayACL(lsName, pgName, gateway, u2oInterconnecti
181181
case len(pgName) != 0:
182182
parentName, parentType = pgName, portGroupKey
183183
case len(lsName) != 0:
184-
parentName, parentType = lsName, logicalSwitchKey
184+
parentName, parentType = lsName, LogicalSwitchKey
185185
default:
186186
return errors.New("one of port group name and logical switch name must be specified")
187187
}
@@ -472,7 +472,7 @@ func (c *OVNNbClient) UpdateSgACL(sg *kubeovnv1.SecurityGroup, direction string)
472472

473473
func (c *OVNNbClient) UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcls []kubeovnv1.ACL, allowEWTraffic bool) error {
474474
if len(subnetAcls) == 0 {
475-
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName}); err != nil {
475+
if err := c.DeleteAcls(lsName, LogicalSwitchKey, "", map[string]string{"subnet": lsName}); err != nil {
476476
klog.Error(err)
477477
return fmt.Errorf("delete subnet acls from %s: %w", lsName, err)
478478
}
@@ -529,13 +529,13 @@ func (c *OVNNbClient) UpdateLogicalSwitchACL(lsName, cidrBlock string, subnetAcl
529529
acls = append(acls, acl)
530530
}
531531

532-
delOps, err := c.DeleteAclsOps(lsName, logicalSwitchKey, "", map[string]string{"subnet": lsName})
532+
delOps, err := c.DeleteAclsOps(lsName, LogicalSwitchKey, "", map[string]string{"subnet": lsName})
533533
if err != nil {
534534
klog.Error(err)
535535
return err
536536
}
537537

538-
addOps, err := c.CreateAclsOps(lsName, logicalSwitchKey, acls...)
538+
addOps, err := c.CreateAclsOps(lsName, LogicalSwitchKey, acls...)
539539
if err != nil {
540540
klog.Error(err)
541541
return err
@@ -572,7 +572,7 @@ func (c *OVNNbClient) UpdateACL(acl *ovnnb.ACL, fields ...any) error {
572572
// SetLogicalSwitchPrivate will drop all ingress traffic except allow subnets, same subnet and node subnet
573573
func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR string, allowSubnets []string) error {
574574
// clear acls
575-
if err := c.DeleteAcls(lsName, logicalSwitchKey, "", nil); err != nil {
575+
if err := c.DeleteAcls(lsName, LogicalSwitchKey, "", nil); err != nil {
576576
klog.Error(err)
577577
return fmt.Errorf("clear logical switch %s acls: %w", lsName, err)
578578
}
@@ -687,7 +687,7 @@ func (c *OVNNbClient) SetLogicalSwitchPrivate(lsName, cidrBlock, nodeSwitchCIDR
687687
}
688688
}
689689

690-
if err := c.CreateAcls(lsName, logicalSwitchKey, acls...); err != nil {
690+
if err := c.CreateAcls(lsName, LogicalSwitchKey, acls...); err != nil {
691691
klog.Error(err)
692692
return fmt.Errorf("add ingress acls to logical switch %s: %w", lsName, err)
693693
}
@@ -1174,8 +1174,8 @@ func aclFilter(direction string, externalIDs map[string]string) func(acl *ovnnb.
11741174
// CreateAcls return operations which create several acl once
11751175
// parentType is 'ls' or 'pg'
11761176
func (c *OVNNbClient) CreateAclsOps(parentName, parentType string, acls ...*ovnnb.ACL) ([]ovsdb.Operation, error) {
1177-
if parentType != portGroupKey && parentType != logicalSwitchKey {
1178-
return nil, fmt.Errorf("acl parent type must be '%s' or '%s'", portGroupKey, logicalSwitchKey)
1177+
if parentType != portGroupKey && parentType != LogicalSwitchKey {
1178+
return nil, fmt.Errorf("acl parent type must be '%s' or '%s'", portGroupKey, LogicalSwitchKey)
11791179
}
11801180

11811181
if len(acls) == 0 {

‎pkg/ovs/ovn-nb-acl_test.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,7 @@ func (suite *OvnClientTestSuite) testCreateAcls() {
13601360
acls = append(acls, acl)
13611361
}
13621362

1363-
err = nbClient.CreateAcls(lsName, logicalSwitchKey, append(acls, nil)...)
1363+
err = nbClient.CreateAcls(lsName, LogicalSwitchKey, append(acls, nil)...)
13641364
require.NoError(t, err)
13651365

13661366
ls, err := nbClient.GetLogicalSwitch(lsName, false)
@@ -1503,14 +1503,14 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
15031503
acls = append(acls, acl)
15041504
}
15051505

1506-
err = nbClient.CreateAcls(lsName, logicalSwitchKey, acls...)
1506+
err = nbClient.CreateAcls(lsName, LogicalSwitchKey, acls...)
15071507
require.NoError(t, err)
15081508

15091509
ls, err := nbClient.GetLogicalSwitch(lsName, false)
15101510
require.NoError(t, err)
15111511
require.Len(t, ls.ACLs, 5)
15121512

1513-
err = nbClient.DeleteAcls(lsName, logicalSwitchKey, "", nil)
1513+
err = nbClient.DeleteAcls(lsName, LogicalSwitchKey, "", nil)
15141514
require.NoError(t, err)
15151515

15161516
ls, err = nbClient.GetLogicalSwitch(lsName, false)
@@ -1539,23 +1539,23 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
15391539
acls = append(acls, acl)
15401540
}
15411541

1542-
err = nbClient.CreateAcls(lsName, logicalSwitchKey, acls...)
1542+
err = nbClient.CreateAcls(lsName, LogicalSwitchKey, acls...)
15431543
require.NoError(t, err)
15441544

15451545
ls, err := nbClient.GetLogicalSwitch(lsName, false)
15461546
require.NoError(t, err)
15471547
require.Len(t, ls.ACLs, 5)
15481548

15491549
/* delete to-lport direction acl */
1550-
err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, nil)
1550+
err = nbClient.DeleteAcls(lsName, LogicalSwitchKey, ovnnb.ACLDirectionToLport, nil)
15511551
require.NoError(t, err)
15521552

15531553
ls, err = nbClient.GetLogicalSwitch(lsName, false)
15541554
require.NoError(t, err)
15551555
require.Len(t, ls.ACLs, 3)
15561556

15571557
/* delete from-lport direction acl */
1558-
err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionFromLport, nil)
1558+
err = nbClient.DeleteAcls(lsName, LogicalSwitchKey, ovnnb.ACLDirectionFromLport, nil)
15591559
require.NoError(t, err)
15601560

15611561
ls, err = nbClient.GetLogicalSwitch(lsName, false)
@@ -1580,7 +1580,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
15801580
require.NoError(t, err)
15811581
acls = append(acls, acl)
15821582

1583-
err = nbClient.CreateAcls(lsName, logicalSwitchKey, acls...)
1583+
err = nbClient.CreateAcls(lsName, LogicalSwitchKey, acls...)
15841584
require.NoError(t, err)
15851585

15861586
ls, err := nbClient.GetLogicalSwitch(lsName, false)
@@ -1592,7 +1592,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
15921592
require.NoError(t, err)
15931593

15941594
/* delete to-lport direction acl */
1595-
err = nbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
1595+
err = nbClient.DeleteAcls(lsName, LogicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
15961596
require.NoError(t, err)
15971597

15981598
ls, err = nbClient.GetLogicalSwitch(lsName, false)
@@ -1601,7 +1601,7 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
16011601
})
16021602

16031603
t.Run("should no err when acls does not exist", func(t *testing.T) {
1604-
err = nbClient.DeleteAcls("test-nonexist-ls", logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": "test-nonexist-ls"})
1604+
err = nbClient.DeleteAcls("test-nonexist-ls", LogicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": "test-nonexist-ls"})
16051605
require.NoError(t, err)
16061606
})
16071607

@@ -1620,10 +1620,10 @@ func (suite *OvnClientTestSuite) testDeleteAcls() {
16201620
require.NoError(t, err)
16211621
acls = append(acls, acl)
16221622

1623-
err = failedNbClient.CreateAcls(lsName, logicalSwitchKey, acls...)
1623+
err = failedNbClient.CreateAcls(lsName, LogicalSwitchKey, acls...)
16241624
require.Error(t, err)
16251625
// TODO:// should err but not for now
1626-
err = failedNbClient.DeleteAcls(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
1626+
err = failedNbClient.DeleteAcls(lsName, LogicalSwitchKey, ovnnb.ACLDirectionToLport, map[string]string{"subnet": lsName})
16271627
require.NoError(t, err)
16281628
})
16291629
}
@@ -1674,14 +1674,14 @@ func (suite *OvnClientTestSuite) testDeleteACL() {
16741674
acl, err := nbClient.newACL(lsName, ovnnb.ACLDirectionToLport, priority, match, ovnnb.ACLActionAllowRelated, util.NetpolACLTier)
16751675
require.NoError(t, err)
16761676

1677-
err = nbClient.CreateAcls(lsName, logicalSwitchKey, acl)
1677+
err = nbClient.CreateAcls(lsName, LogicalSwitchKey, acl)
16781678
require.NoError(t, err)
16791679

16801680
ls, err := nbClient.GetLogicalSwitch(lsName, false)
16811681
require.NoError(t, err)
16821682
require.Len(t, ls.ACLs, 1)
16831683

1684-
err = nbClient.DeleteACL(lsName, logicalSwitchKey, ovnnb.ACLDirectionToLport, priority, match)
1684+
err = nbClient.DeleteACL(lsName, LogicalSwitchKey, ovnnb.ACLDirectionToLport, priority, match)
16851685
require.NoError(t, err)
16861686

16871687
ls, err = nbClient.GetLogicalSwitch(lsName, false)

‎pkg/ovs/ovn-nb-dhcp_options.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ func (c *OVNNbClient) DeleteDHCPOptions(lsName, protocol string) error {
224224
protocol = ""
225225
}
226226
externalIDs := map[string]string{
227-
logicalSwitchKey: lsName,
227+
LogicalSwitchKey: lsName,
228228
"protocol": protocol, // list all protocol dhcp options when protocol is ""
229229
}
230230

@@ -254,7 +254,7 @@ func (c *OVNNbClient) GetDHCPOptions(lsName, protocol string, ignoreNotFound boo
254254
}
255255

256256
dhcpOptList, err := c.ListDHCPOptions(true, map[string]string{
257-
logicalSwitchKey: lsName,
257+
LogicalSwitchKey: lsName,
258258
"protocol": protocol,
259259
})
260260
if err != nil {
@@ -307,7 +307,7 @@ func newDHCPOptions(lsName, cidr, options string) (*ovnnb.DHCPOptions, error) {
307307
return &ovnnb.DHCPOptions{
308308
Cidr: cidr,
309309
ExternalIDs: map[string]string{
310-
logicalSwitchKey: lsName,
310+
LogicalSwitchKey: lsName,
311311
"protocol": protocol,
312312
"vendor": util.CniTypeName,
313313
},

0 commit comments

Comments
 (0)