Merge pull request #1516 from npinaeva/ocpbugs-7230

[release-4.12] OCPBUGS-7230: Delete IGMP Groups when deleting stale chassis
openshift · Feb 13, 2023 · 4b5172a · 4b5172a
2 parents 38cd36f + 7f0683d
commit 4b5172a
Show file tree

Hide file tree

Showing 19 changed files with 443 additions and 253 deletions.
diff --git a/go-controller/go.mod b/go-controller/go.mod
@@ -22,7 +22,7 @@ require (
 	github.com/onsi/gomega v1.15.0
 	github.com/openshift/api v0.0.0-20220525145417-ee5b62754c68
 	github.com/openshift/client-go v0.0.0-20220525160904-9e1acff93e4a
-	github.com/ovn-org/libovsdb v0.6.1-0.20221101143603-8f21d188c3a5
+	github.com/ovn-org/libovsdb v0.6.1-0.20230203213244-a6a173993830
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.12.1
 	github.com/prometheus/client_model v0.2.0

diff --git a/go-controller/go.sum b/go-controller/go.sum
@@ -616,8 +616,8 @@ github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mo
 github.com/openshift/client-go v0.0.0-20220525160904-9e1acff93e4a h1:ylsEgoC8Dlg4A0C1TLH0A4x/TZao7k1YveLwROhRUdk=
 github.com/openshift/client-go v0.0.0-20220525160904-9e1acff93e4a/go.mod h1:eDO5QeVi2IiXmDwB0e2z1DpAznWroZKe978pzZwFBzg=
 github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
-github.com/ovn-org/libovsdb v0.6.1-0.20221101143603-8f21d188c3a5 h1:Cw0JXIHSGp8etz5/P7zNQ0tCMmTljBGBr8Rn2P1PuzQ=
-github.com/ovn-org/libovsdb v0.6.1-0.20221101143603-8f21d188c3a5/go.mod h1:S/+Hux9//oB7yLaPsUKnXTzZc6S1C4a9HP0UifXfKz0=
+github.com/ovn-org/libovsdb v0.6.1-0.20230203213244-a6a173993830 h1:eV+OMJFLtayfrYTCEBIsxvuMV0HK6KPWCqIUdaxoIwQ=
+github.com/ovn-org/libovsdb v0.6.1-0.20230203213244-a6a173993830/go.mod h1:S/+Hux9//oB7yLaPsUKnXTzZc6S1C4a9HP0UifXfKz0=
 github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=
 github.com/pelletier/go-toml v1.8.1/go.mod h1:T2/BmBdy8dvIRq1a/8aqjN41wvWlN4lrapLU/GW4pbc=
@@ -1037,8 +1037,6 @@ golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab h1:2QkjZIsXupsJbJIdSjjUOgWK3aEtzyuh2mPt3l/CkeU=
-golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
 golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=

diff --git a/go-controller/pkg/libovsdb/libovsdb.go b/go-controller/pkg/libovsdb/libovsdb.go
@@ -102,6 +102,7 @@ func NewSBClientWithConfig(cfg config.OvnAuthConfig, promRegistry prometheus.Reg
 
 	// Only Monitor Required SBDB tables to reduce memory overhead
 	chassisPrivate := sbdb.ChassisPrivate{}
+	igmpGroup := sbdb.IGMPGroup{}
 	_, err = c.Monitor(ctx,
 		c.NewMonitor(
 			// used by unidling controller
@@ -112,6 +113,8 @@ func NewSBClientWithConfig(cfg config.OvnAuthConfig, promRegistry prometheus.Reg
 			client.WithTable(&sbdb.Chassis{}),
 			// used by node sync, only interested in names
 			client.WithTable(&chassisPrivate, &chassisPrivate.Name),
+			// used by node sync, only interested in Chassis reference
+			client.WithTable(&igmpGroup, &igmpGroup.Chassis),
 			// used for metrics
 			client.WithTable(&sbdb.SBGlobal{}),
 			// used for metrics

diff --git a/go-controller/pkg/libovsdbops/chassis.go b/go-controller/pkg/libovsdbops/chassis.go
@@ -36,6 +36,7 @@ func DeleteChassis(sbClient libovsdbclient.Client, chassis ...*sbdb.Chassis) err
 		chassisPrivate := sbdb.ChassisPrivate{
 			Name: chassis[i].Name,
 		}
+		chassisUUID := ""
 		opModel := []operationModel{
 			{
 				Model:          chassis[i],
@@ -45,6 +46,7 @@ func DeleteChassis(sbClient libovsdbclient.Client, chassis ...*sbdb.Chassis) err
 				DoAfter: func() {
 					if len(foundChassis) > 0 {
 						chassisPrivate.Name = foundChassis[0].Name
+						chassisUUID = foundChassis[0].UUID
 					}
 				},
 			},
@@ -53,6 +55,16 @@ func DeleteChassis(sbClient libovsdbclient.Client, chassis ...*sbdb.Chassis) err
 				ErrNotFound: false,
 				BulkOp:      false,
 			},
+			// IGMPGroup has a weak link to chassis, deleting multiple chassis may result in IGMP_Groups
+			// with identical values on columns "address", "datapath", and "chassis", when "chassis" goes empty
+			{
+				Model: &sbdb.IGMPGroup{},
+				ModelPredicate: func(group *sbdb.IGMPGroup) bool {
+					return group.Chassis != nil && chassisUUID != "" && *group.Chassis == chassisUUID
+				},
+				ErrNotFound: false,
+				BulkOp:      true,
+			},
 		}
 		opModels = append(opModels, opModel...)
 	}
@@ -69,6 +81,7 @@ type chassisPredicate func(*sbdb.Chassis) bool
 func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredicate) error {
 	foundChassis := []*sbdb.Chassis{}
 	foundChassisNames := sets.NewString()
+	foundChassisUUIDS := sets.NewString()
 	opModels := []operationModel{
 		{
 			Model:          &sbdb.Chassis{},
@@ -79,6 +92,7 @@ func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredica
 			DoAfter: func() {
 				for _, chassis := range foundChassis {
 					foundChassisNames.Insert(chassis.Name)
+					foundChassisUUIDS.Insert(chassis.UUID)
 				}
 			},
 		},
@@ -88,6 +102,14 @@ func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredica
 			ErrNotFound:    false,
 			BulkOp:         true,
 		},
+		// IGMPGroup has a weak link to chassis, deleting multiple chassis may result in IGMP_Groups
+		// with identical values on columns "address", "datapath", and "chassis", when "chassis" goes empty
+		{
+			Model:          &sbdb.IGMPGroup{},
+			ModelPredicate: func(group *sbdb.IGMPGroup) bool { return group.Chassis != nil && foundChassisUUIDS.Has(*group.Chassis) },
+			ErrNotFound:    false,
+			BulkOp:         true,
+		},
 	}
 	m := newModelClient(sbClient)
 	err := m.Delete(opModels...)

diff --git a/go-controller/pkg/libovsdbops/chassis_test.go b/go-controller/pkg/libovsdbops/chassis_test.go
@@ -10,6 +10,9 @@ import (
 
 func TestDeleteChassis(t *testing.T) {
 	uuid := "b9998337-2498-4d1e-86e6-fc0417abb2f0"
+	uuid2 := "b9998337-2498-4d1e-86e6-fc0417abb2f1"
+	uuid3 := "b9998337-2498-4d1e-86e6-fc0417abb2f2"
+	fakeDatapathUUID := "datapath-uuid"
 	tests := []struct {
 		desc             string
 		chassis          *sbdb.Chassis
@@ -31,6 +34,38 @@ func TestDeleteChassis(t *testing.T) {
 				&sbdb.ChassisPrivate{Name: "test2"},
 			},
 		},
+		{
+			desc:    "delete chassis and igmp group by chassis UUID",
+			chassis: &sbdb.Chassis{UUID: uuid},
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid},
+				&sbdb.IGMPGroup{Address: "1.1.1.1", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+				&sbdb.IGMPGroup{Address: "1.1.1.2", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+
+				&sbdb.Chassis{UUID: uuid2},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid2},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
+			},
+		},
+		{
+			desc:    "delete chassis and igmp group by chassis Name",
+			chassis: &sbdb.Chassis{Name: "test"},
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid, Name: "test"},
+				&sbdb.IGMPGroup{Address: "1.1.1.1", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+				&sbdb.IGMPGroup{Address: "1.1.1.2", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+
+				&sbdb.Chassis{UUID: uuid2, Name: "test2"},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid2, Name: "test2"},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
+			},
+		},
 		{
 			desc:    "delete chassis and chassis private by UUID",
 			chassis: &sbdb.Chassis{UUID: uuid},
@@ -41,16 +76,18 @@ func TestDeleteChassis(t *testing.T) {
 			expectedDB: []libovsdbtest.TestData{},
 		},
 		{
-			desc:    "delete chassis when chassis private does not exist",
+			desc:    "delete chassis when chassis private and igmp group do not exist",
 			chassis: &sbdb.Chassis{Name: "test"},
 			initialDB: []libovsdbtest.TestData{
 				&sbdb.Chassis{Name: "test"},
-				&sbdb.Chassis{Name: "test2"},
+				&sbdb.Chassis{UUID: uuid2, Name: "test2"},
 				&sbdb.ChassisPrivate{Name: "test2"},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
 			},
 			expectedDB: []libovsdbtest.TestData{
-				&sbdb.Chassis{Name: "test2"},
+				&sbdb.Chassis{UUID: uuid2, Name: "test2"},
 				&sbdb.ChassisPrivate{Name: "test2"},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
 			},
 		},
 		{
@@ -82,6 +119,23 @@ func TestDeleteChassis(t *testing.T) {
 				&sbdb.ChassisPrivate{Name: "test3"},
 			},
 		},
+		{
+			desc:             "delete chassis and igmp group by predicate",
+			chassisPredicate: func(c *sbdb.Chassis) bool { return c.Hostname == "testNode" },
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid, Hostname: "testNode"},
+				&sbdb.IGMPGroup{Address: "1.1.1.1", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+				&sbdb.IGMPGroup{Address: "1.1.1.2", Chassis: &uuid, Datapath: &fakeDatapathUUID},
+				&sbdb.Chassis{UUID: uuid2, Hostname: "testNode"},
+				&sbdb.IGMPGroup{Chassis: &uuid2, Datapath: &fakeDatapathUUID},
+				&sbdb.Chassis{UUID: uuid3, Hostname: "testNode3"},
+				&sbdb.IGMPGroup{Chassis: &uuid3, Datapath: &fakeDatapathUUID},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid3, Hostname: "testNode3"},
+				&sbdb.IGMPGroup{Chassis: &uuid3, Datapath: &fakeDatapathUUID},
+			},
+		},
 		{
 			desc:             "delete chassis by predicate when chassis private does not exist",
 			chassisPredicate: func(c *sbdb.Chassis) bool { return c.Hostname == "testNode" },

diff --git a/go-controller/pkg/libovsdbops/model.go b/go-controller/pkg/libovsdbops/model.go
@@ -54,6 +54,8 @@ func getUUID(model model.Model) string {
 		return t.UUID
 	case *sbdb.ChassisPrivate:
 		return t.UUID
+	case *sbdb.IGMPGroup:
+		return t.UUID
 	case *sbdb.MACBinding:
 		return t.UUID
 	case *sbdb.SBGlobal:
@@ -107,6 +109,8 @@ func setUUID(model model.Model, uuid string) {
 		t.UUID = uuid
 	case *sbdb.ChassisPrivate:
 		t.UUID = uuid
+	case *sbdb.IGMPGroup:
+		t.UUID = uuid
 	case *sbdb.MACBinding:
 		t.UUID = uuid
 	case *sbdb.SBGlobal:
@@ -212,6 +216,10 @@ func copyIndexes(model model.Model) model.Model {
 			UUID: t.UUID,
 			Name: t.Name,
 		}
+	case *sbdb.IGMPGroup:
+		return &sbdb.IGMPGroup{
+			UUID: t.UUID,
+		}
 	case *sbdb.MACBinding:
 		return &sbdb.MACBinding{
 			UUID:        t.UUID,
@@ -273,6 +281,8 @@ func getListFromModel(model model.Model) interface{} {
 		return &[]*sbdb.Chassis{}
 	case *sbdb.ChassisPrivate:
 		return &[]*sbdb.ChassisPrivate{}
+	case *sbdb.IGMPGroup:
+		return &[]*sbdb.IGMPGroup{}
 	case *sbdb.MACBinding:
 		return &[]*sbdb.MACBinding{}
 	case *nbdb.QoS:

diff --git a/go-controller/pkg/testing/libovsdb/libovsdb.go b/go-controller/pkg/testing/libovsdb/libovsdb.go
@@ -17,6 +17,7 @@ import (
 	guuid "github.com/google/uuid"
 	"github.com/mitchellh/copystructure"
 	libovsdbclient "github.com/ovn-org/libovsdb/client"
+	"github.com/ovn-org/libovsdb/database"
 	"github.com/ovn-org/libovsdb/mapper"
 	"github.com/ovn-org/libovsdb/model"
 	"github.com/ovn-org/libovsdb/ovsdb"
@@ -188,7 +189,7 @@ func newNBServer(cfg config.OvnAuthConfig, data []TestData) (*server.OvsdbServer
 	return newOVSDBServer(cfg, dbModel, schema, data)
 }
 
-func updateData(db server.Database, dbModel model.ClientDBModel, schema ovsdb.DatabaseSchema, data []TestData) error {
+func updateData(db database.Database, dbModel model.ClientDBModel, schema ovsdb.DatabaseSchema, data []TestData) error {
 	dbName := dbModel.Name()
 	m := mapper.NewMapper(schema)
 	updates := ovsdb.TableUpdates2{}
@@ -264,7 +265,7 @@ func newOVSDBServer(cfg config.OvnAuthConfig, dbModel model.ClientDBModel, schem
 	}
 	serverSchema := serverdb.Schema()
 
-	db := server.NewInMemoryDatabase(map[string]model.ClientDBModel{
+	db := database.NewInMemoryDatabase(map[string]model.ClientDBModel{
 		schema.Name:       dbModel,
 		serverSchema.Name: serverDBModel,
 	})