Skip to content

Commit efd240f

Browse files
zhangzujianzhangzujian
authored
controller: fix lsp not deleted in gc (#5735)
Signed-off-by: zhangzujian <zhangzujian.7@gmail.com>
1 parent 6765134 commit efd240f

File tree

13 files changed

+160
-117
lines changed

13 files changed

+160
-117
lines changed

‎go.mod‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ require (
1919
github.com/evanphx/json-patch/v5 v5.9.11
2020
github.com/go-logr/logr v1.4.3
2121
github.com/go-logr/stdr v1.2.2
22+
github.com/go-logr/zapr v1.3.0
2223
github.com/google/gopacket v1.1.19
2324
github.com/google/uuid v1.6.0
2425
github.com/httprunner/httprunner/v5 v5.0.0-20250818135001-47996ed22fb1
@@ -48,6 +49,7 @@ require (
4849
github.com/stretchr/testify v1.11.1
4950
github.com/vishvananda/netlink v1.3.1
5051
go.uber.org/mock v0.6.0
52+
go.uber.org/zap v1.27.0
5153
go.universe.tf/metallb v0.15.2
5254
golang.org/x/mod v0.28.0
5355
golang.org/x/net v0.44.0
@@ -325,7 +327,6 @@ require (
325327
go.opentelemetry.io/proto/otlp v1.7.1 // indirect
326328
go.uber.org/automaxprocs v1.6.0 // indirect
327329
go.uber.org/multierr v1.11.0 // indirect
328-
go.uber.org/zap v1.27.0 // indirect
329330
go.yaml.in/yaml/v2 v2.4.3 // indirect
330331
go.yaml.in/yaml/v3 v3.0.4 // indirect
331332
golang.org/x/arch v0.21.0 // indirect

‎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
@@ -489,8 +489,8 @@ func (c *Controller) markAndCleanLSP() error {
489489
continue
490490
}
491491

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

‎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
},

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ func (suite *OvnClientTestSuite) testDeleteDHCPOptionsByUUIDs() {
320320
require.NoError(t, err)
321321
}
322322

323-
out, err := nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName})
323+
out, err := nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName})
324324
require.NoError(t, err)
325325
require.Len(t, out, 3)
326326
for _, o := range out {
@@ -330,7 +330,7 @@ func (suite *OvnClientTestSuite) testDeleteDHCPOptionsByUUIDs() {
330330
err = nbClient.DeleteDHCPOptionsByUUIDs(uuidList...)
331331
require.NoError(t, err)
332332

333-
out, err = nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName})
333+
out, err = nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName})
334334
require.NoError(t, err)
335335
require.Empty(t, out)
336336
}
@@ -365,19 +365,19 @@ func (suite *OvnClientTestSuite) testDeleteDHCPOptions() {
365365
err := nbClient.DeleteDHCPOptions(lsName, "IPv4")
366366
require.NoError(t, err)
367367

368-
out, err := nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName, "protocol": "IPv4"})
368+
out, err := nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName, "protocol": "IPv4"})
369369
require.NoError(t, err)
370370
require.Empty(t, out)
371371

372-
out, err = nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName, "protocol": "IPv6"})
372+
out, err = nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName, "protocol": "IPv6"})
373373
require.NoError(t, err)
374374
require.Len(t, out, 2)
375375

376376
/* delete ipv6 dhcp options */
377377
err = nbClient.DeleteDHCPOptions(lsName, "IPv6")
378378
require.NoError(t, err)
379379

380-
out, err = nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName, "protocol": "IPv6"})
380+
out, err = nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName, "protocol": "IPv6"})
381381
require.NoError(t, err)
382382
require.Empty(t, out)
383383
})
@@ -388,7 +388,7 @@ func (suite *OvnClientTestSuite) testDeleteDHCPOptions() {
388388
err := nbClient.DeleteDHCPOptions(lsName, kubeovnv1.ProtocolDual)
389389
require.NoError(t, err)
390390

391-
out, err := nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName, "protocol": "IPv6"})
391+
out, err := nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName, "protocol": "IPv6"})
392392
require.NoError(t, err)
393393
require.Empty(t, out)
394394
})
@@ -483,7 +483,7 @@ func (suite *OvnClientTestSuite) testListDHCPOptions() {
483483
}
484484

485485
/* list all direction acl */
486-
out, err := nbClient.ListDHCPOptions(true, map[string]string{logicalSwitchKey: lsName})
486+
out, err := nbClient.ListDHCPOptions(true, map[string]string{LogicalSwitchKey: lsName})
487487
require.NoError(t, err)
488488
require.Len(t, out, 3)
489489
}
@@ -517,7 +517,7 @@ func (suite *OvnClientTestSuite) testDhcpOptionsFilter() {
517517
// create three ipv4 dhcp options with other logical switch name
518518
for _, cidr := range v4CidrBlock {
519519
dhcpOpt, err := newDHCPOptions(lsName, cidr, "")
520-
dhcpOpt.ExternalIDs[logicalSwitchKey] = lsName + "-test"
520+
dhcpOpt.ExternalIDs[LogicalSwitchKey] = lsName + "-test"
521521
require.NoError(t, err)
522522
dhcpOpts = append(dhcpOpts, dhcpOpt)
523523
}
@@ -551,7 +551,7 @@ func (suite *OvnClientTestSuite) testDhcpOptionsFilter() {
551551
require.Equal(t, count, 8)
552552

553553
/* include same ls dhcp options */
554-
filterFunc = dhcpOptionsFilter(true, map[string]string{logicalSwitchKey: lsName})
554+
filterFunc = dhcpOptionsFilter(true, map[string]string{LogicalSwitchKey: lsName})
555555
count = 0
556556
for _, dhcpOpt := range dhcpOpts {
557557
if filterFunc(dhcpOpt) {
@@ -561,7 +561,7 @@ func (suite *OvnClientTestSuite) testDhcpOptionsFilter() {
561561
require.Equal(t, count, 5)
562562

563563
/* include same protocol dhcp options */
564-
filterFunc = dhcpOptionsFilter(true, map[string]string{logicalSwitchKey: lsName, "protocol": "IPv4"})
564+
filterFunc = dhcpOptionsFilter(true, map[string]string{LogicalSwitchKey: lsName, "protocol": "IPv4"})
565565
count = 0
566566
for _, dhcpOpt := range dhcpOpts {
567567
if filterFunc(dhcpOpt) {
@@ -571,7 +571,7 @@ func (suite *OvnClientTestSuite) testDhcpOptionsFilter() {
571571
require.Equal(t, count, 3)
572572

573573
/* include all protocol dhcp options */
574-
filterFunc = dhcpOptionsFilter(true, map[string]string{logicalSwitchKey: lsName, "protocol": ""})
574+
filterFunc = dhcpOptionsFilter(true, map[string]string{LogicalSwitchKey: lsName, "protocol": ""})
575575
count = 0
576576
for _, dhcpOpt := range dhcpOpts {
577577
if filterFunc(dhcpOpt) {
@@ -588,7 +588,7 @@ func (suite *OvnClientTestSuite) testDhcpOptionsFilter() {
588588
require.NoError(t, err)
589589

590590
filterFunc := dhcpOptionsFilter(true, map[string]string{
591-
logicalSwitchKey: lsName,
591+
LogicalSwitchKey: lsName,
592592
"key": "value",
593593
})
594594

0 commit comments

Comments
 (0)