diff --git a/mocks/pkg/ovs/interface.go b/mocks/pkg/ovs/interface.go index 98cbc0553c9..30921d3efb2 100644 --- a/mocks/pkg/ovs/interface.go +++ b/mocks/pkg/ovs/interface.go @@ -5111,21 +5111,6 @@ func (mr *MockSbClientMockRecorder) DeleteChassisByHost(node any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteChassisByHost", reflect.TypeOf((*MockSbClient)(nil).DeleteChassisByHost), node) } -// GetAllChassisByHost mocks base method. -func (m *MockSbClient) GetAllChassisByHost(nodeName string) (*[]ovnsb.Chassis, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAllChassisByHost", nodeName) - ret0, _ := ret[0].(*[]ovnsb.Chassis) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetAllChassisByHost indicates an expected call of GetAllChassisByHost. -func (mr *MockSbClientMockRecorder) GetAllChassisByHost(nodeName any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllChassisByHost", reflect.TypeOf((*MockSbClient)(nil).GetAllChassisByHost), nodeName) -} - // GetChassis mocks base method. func (m *MockSbClient) GetChassis(chassisName string, ignoreNotFound bool) (*ovnsb.Chassis, error) { m.ctrl.T.Helper() @@ -5351,21 +5336,6 @@ func (mr *MockChassisMockRecorder) DeleteChassisByHost(node any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteChassisByHost", reflect.TypeOf((*MockChassis)(nil).DeleteChassisByHost), node) } -// GetAllChassisByHost mocks base method. -func (m *MockChassis) GetAllChassisByHost(nodeName string) (*[]ovnsb.Chassis, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetAllChassisByHost", nodeName) - ret0, _ := ret[0].(*[]ovnsb.Chassis) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetAllChassisByHost indicates an expected call of GetAllChassisByHost. -func (mr *MockChassisMockRecorder) GetAllChassisByHost(nodeName any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllChassisByHost", reflect.TypeOf((*MockChassis)(nil).GetAllChassisByHost), nodeName) -} - // GetChassis mocks base method. func (m *MockChassis) GetChassis(chassisName string, ignoreNotFound bool) (*ovnsb.Chassis, error) { m.ctrl.T.Helper() diff --git a/pkg/controller/node.go b/pkg/controller/node.go index 343d6731685..9a75d58f2eb 100644 --- a/pkg/controller/node.go +++ b/pkg/controller/node.go @@ -666,19 +666,18 @@ func (c *Controller) checkGatewayReady() error { func (c *Controller) cleanDuplicatedChassis(node *v1.Node) error { // if multi chassis has the same node name, delete all of them - chassises, err := c.OVNSbClient.GetAllChassisByHost(node.Name) - if err != nil { - klog.Errorf("failed to list chassis %v", err) - return err + var err error + if _, err := c.OVNSbClient.GetChassisByHost(node.Name); err == nil { + return nil } - if len(*chassises) > 1 { - klog.Warningf("node %s has multiple chassis", node.Name) + klog.Errorf("failed to get chassis for node %s: %v", node.Name, err) + if errors.Is(err, ovs.ErrOneNodeMultiChassis) { if err := c.OVNSbClient.DeleteChassisByHost(node.Name); err != nil { klog.Errorf("failed to delete chassis for node %s: %v", node.Name, err) return err } } - return nil + return err } func (c *Controller) retryDelDupChassis(attempts, sleep int, f func(node *v1.Node) error, node *v1.Node) (err error) { diff --git a/pkg/ovs/interface.go b/pkg/ovs/interface.go index 67a230b9a16..6f5710ab6c9 100644 --- a/pkg/ovs/interface.go +++ b/pkg/ovs/interface.go @@ -257,7 +257,6 @@ type Common interface { type Chassis interface { DeleteChassis(chassisName string) error DeleteChassisByHost(node string) error - GetAllChassisByHost(nodeName string) (*[]ovnsb.Chassis, error) GetChassisByHost(nodeName string) (*ovnsb.Chassis, error) GetChassis(chassisName string, ignoreNotFound bool) (*ovnsb.Chassis, error) GetKubeOvnChassisses() (*[]ovnsb.Chassis, error) diff --git a/pkg/ovs/ovn-nb-suite_test.go b/pkg/ovs/ovn-nb-suite_test.go index 054d39f8607..29597044c34 100644 --- a/pkg/ovs/ovn-nb-suite_test.go +++ b/pkg/ovs/ovn-nb-suite_test.go @@ -1024,10 +1024,6 @@ func (suite *OvnClientTestSuite) Test_ListChassis() { suite.testListChassis() } -func (suite *OvnClientTestSuite) Test_GetAllChassisByHost() { - suite.testGetAllChassisByHost() -} - func (suite *OvnClientTestSuite) Test_GetChassisByHost() { suite.testGetChassisByHost() } diff --git a/pkg/ovs/ovn-sb-chassis.go b/pkg/ovs/ovn-sb-chassis.go index 137d1474781..a5958cf768f 100644 --- a/pkg/ovs/ovn-sb-chassis.go +++ b/pkg/ovs/ovn-sb-chassis.go @@ -12,6 +12,10 @@ import ( "github.com/kubeovn/kube-ovn/pkg/util" ) +var ( + ErrOneNodeMultiChassis = errors.New("OneNodeMultiChassis") +) + func (c *OVNSbClient) UpdateChassis(chassis *ovnsb.Chassis, fields ...interface{}) error { op, err := c.ovsDbClient.Where(chassis).Update(chassis, fields...) if err != nil { @@ -83,30 +87,6 @@ func (c *OVNSbClient) ListChassis() (*[]ovnsb.Chassis, error) { return &css, nil } -func (c *OVNSbClient) GetAllChassisByHost(nodeName string) (*[]ovnsb.Chassis, error) { - ctx, cancel := context.WithTimeout(context.Background(), c.Timeout) - defer cancel() - - chassisList := make([]ovnsb.Chassis, 0) - if err := c.ovsDbClient.WhereCache(func(chassis *ovnsb.Chassis) bool { - return chassis.Hostname == nodeName - }).List(ctx, &chassisList); err != nil { - klog.Error(err) - return nil, fmt.Errorf("failed to list Chassis with host name=%s: %w", nodeName, err) - } - if len(chassisList) == 0 { - err := fmt.Errorf("failed to get Chassis with with host name=%s", nodeName) - klog.Error(err) - return nil, err - } - if len(chassisList) != 1 { - err := fmt.Errorf("found more than one Chassis with with host name=%s", nodeName) - klog.Error(err) - return nil, err - } - return &chassisList, nil -} - func (c *OVNSbClient) GetChassisByHost(nodeName string) (*ovnsb.Chassis, error) { if nodeName == "" { err := errors.New("failed to get Chassis with empty hostname") @@ -131,7 +111,7 @@ func (c *OVNSbClient) GetChassisByHost(nodeName string) (*ovnsb.Chassis, error) if len(chassisList) != 1 { err := fmt.Errorf("found more than one Chassis with host name=%s", nodeName) klog.Error(err) - return nil, err + return nil, ErrOneNodeMultiChassis } // #nosec G602 diff --git a/pkg/ovs/ovn-sb-chassis_test.go b/pkg/ovs/ovn-sb-chassis_test.go index 64b150dee86..26f9f90eb54 100644 --- a/pkg/ovs/ovn-sb-chassis_test.go +++ b/pkg/ovs/ovn-sb-chassis_test.go @@ -199,102 +199,69 @@ func (suite *OvnClientTestSuite) testListChassis() { }) } -func (suite *OvnClientTestSuite) testGetAllChassisByHost() { +func (suite *OvnClientTestSuite) testGetChassisByHost() { t := suite.T() t.Parallel() sbClient := suite.ovnSBClient t.Cleanup(func() { - err := sbClient.DeleteChassis("chassis-3") + err := sbClient.DeleteChassis("chassis-3x") + require.NoError(t, err) + err = sbClient.DeleteChassis("chassis-4x") + require.NoError(t, err) + err = sbClient.DeleteChassis("chassis-5x") require.NoError(t, err) - err = sbClient.DeleteChassis("chassis-4") + err = sbClient.DeleteChassis("chassis-6x") require.NoError(t, err) - err = sbClient.DeleteChassis("chassis-5") + err = sbClient.DeleteChassis("chassis-7x") require.NoError(t, err) }) - chassis1 := newChassis(0, "host-3", "chassis-3", nil, nil, nil, nil, nil) - chassis2 := newChassis(0, "host-4", "chassis-4", nil, nil, nil, nil, nil) - chassis3 := newChassis(0, "host-4", "chassis-5", nil, nil, nil, nil, nil) + chassis3x := newChassis(0, "host-3x", "chassis-3x", nil, nil, nil, nil, nil) + chassis4x := newChassis(0, "host-4x", "chassis-4x", nil, nil, nil, nil, nil) + chassis5x := newChassis(0, "host-5x", "chassis-5x", nil, nil, nil, nil, nil) + chassis6x := newChassis(0, "host-6x", "chassis-6x", nil, nil, nil, nil, nil) + chassis7x := newChassis(0, "host-7x", "chassis-7x", nil, nil, nil, nil, nil) - ops1, err := sbClient.ovsDbClient.Create(chassis1) + ops3x, err := sbClient.ovsDbClient.Create(chassis3x) require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops1) + err = sbClient.Transact("chassis-add", ops3x) require.NoError(t, err) - ops2, err := sbClient.ovsDbClient.Create(chassis2) + ops4x, err := sbClient.ovsDbClient.Create(chassis4x) require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops2) + err = sbClient.Transact("chassis-add", ops4x) require.NoError(t, err) - ops3, err := sbClient.ovsDbClient.Create(chassis3) + ops5x, err := sbClient.ovsDbClient.Create(chassis5x) require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops3) + err = sbClient.Transact("chassis-add", ops5x) require.NoError(t, err) - t.Run("test get all chassis by host with single chassis", func(t *testing.T) { - chassisList, err := sbClient.GetAllChassisByHost("host-3") - require.NoError(t, err) - require.NotNil(t, chassisList) - require.Len(t, *chassisList, 1) - require.Equal(t, "chassis-3", (*chassisList)[0].Name) - }) - - t.Run("test get all chassis by host with multiple chassis", func(t *testing.T) { - chassisList, err := sbClient.GetAllChassisByHost("host-4") - require.Error(t, err) - require.Nil(t, chassisList) - require.Contains(t, err.Error(), "found more than one Chassis") - }) - - t.Run("test get all chassis by non-existent host", func(t *testing.T) { - chassisList, err := sbClient.GetAllChassisByHost("non-existent-host") - require.Error(t, err) - require.Nil(t, chassisList) - require.Contains(t, err.Error(), "failed to get Chassis") - }) - - t.Run("test get all chassis by host with empty hostname", func(t *testing.T) { - chassisList, err := sbClient.GetAllChassisByHost("") - require.Error(t, err) - require.Nil(t, chassisList) - require.Contains(t, err.Error(), "failed to get Chassis") - }) -} - -func (suite *OvnClientTestSuite) testGetChassisByHost() { - t := suite.T() - t.Parallel() - - sbClient := suite.ovnSBClient - - t.Cleanup(func() { - err := sbClient.DeleteChassis("chassis-6") - require.NoError(t, err) - err = sbClient.DeleteChassis("chassis-7") - require.NoError(t, err) - }) - - chassis1 := newChassis(0, "host-6", "chassis-6", nil, nil, nil, nil, nil) - chassis2 := newChassis(0, "host-7", "chassis-7", nil, nil, nil, nil, nil) - - ops1, err := sbClient.ovsDbClient.Create(chassis1) + ops6x, err := sbClient.ovsDbClient.Create(chassis6x) require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops1) + err = sbClient.Transact("chassis-add", ops6x) require.NoError(t, err) - ops2, err := sbClient.ovsDbClient.Create(chassis2) + ops7x, err := sbClient.ovsDbClient.Create(chassis7x) require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops2) + err = sbClient.Transact("chassis-add", ops7x) require.NoError(t, err) + t.Run("test get all chassis by host with single chassis", func(t *testing.T) { + chassis, err := sbClient.GetChassisByHost("host-3x") + require.NoError(t, err) + require.NotNil(t, chassis) + require.Equal(t, "chassis-3", chassis.Name) + }) + t.Run("test get chassis by host with valid hostname", func(t *testing.T) { - chassis, err := sbClient.GetChassisByHost("host-6") + chassis, err := sbClient.GetChassisByHost("host-6x") require.NoError(t, err) require.NotNil(t, chassis) - require.Equal(t, "chassis-6", chassis.Name) - require.Equal(t, "host-6", chassis.Hostname) + require.Equal(t, "chassis-6x", chassis.Name) + require.Equal(t, "host-6x", chassis.Hostname) }) t.Run("test get chassis by host with non-existent hostname", func(t *testing.T) { @@ -312,18 +279,18 @@ func (suite *OvnClientTestSuite) testGetChassisByHost() { }) t.Run("test get chassis by host with multiple chassis", func(t *testing.T) { - chassis3 := newChassis(0, "host-6", "chassis-8", nil, nil, nil, nil, nil) - ops3, err := sbClient.ovsDbClient.Create(chassis3) + chassis66 := newChassis(0, "host-6x", "chassis-6x-fake", nil, nil, nil, nil, nil) + ops3, err := sbClient.ovsDbClient.Create(chassis66) require.NoError(t, err) err = sbClient.Transact("chassis-add", ops3) require.NoError(t, err) - chassis, err := sbClient.GetChassisByHost("host-6") + chassis, err := sbClient.GetChassisByHost("host-6x") require.Error(t, err) require.Nil(t, chassis) - require.Contains(t, err.Error(), "found more than one Chassis") + require.Contains(t, err.Error(), "OneNodeMultiChassis") - err = sbClient.DeleteChassis("chassis-8") + err = sbClient.DeleteChassis("chassis-6x-fake") require.NoError(t, err) }) } @@ -374,24 +341,6 @@ func (suite *OvnClientTestSuite) testDeleteChassisByHost() { err = sbClient.Transact("chassis-add", ops4) require.NoError(t, err) - t.Run("test delete chassis by host with multiple chassis", func(t *testing.T) { - err := sbClient.DeleteChassisByHost("node1") - require.NoError(t, err) - - chassisList, err := sbClient.GetAllChassisByHost("node1") - require.ErrorContains(t, err, "failed to get Chassis with with host name") - require.Nil(t, chassisList) - }) - - t.Run("test delete chassis by host with ExternalIDs", func(t *testing.T) { - err := sbClient.DeleteChassisByHost("node2") - require.NoError(t, err) - - chassisList, err := sbClient.GetAllChassisByHost("node2") - require.ErrorContains(t, err, "failed to get Chassis with with host name") - require.Nil(t, chassisList) - }) - t.Run("test delete chassis by non-existent host", func(t *testing.T) { err := sbClient.DeleteChassisByHost("non-existent-node") require.NoError(t, err) @@ -520,22 +469,24 @@ func (suite *OvnClientTestSuite) testGetKubeOvnChassisses() { require.False(t, names["non-kube-ovn-chassis"]) require.True(t, names["mixed-chassis"]) - err = sbClient.DeleteChassis("kube-ovn-chassis-1") - require.NoError(t, err) - err = sbClient.DeleteChassis("kube-ovn-chassis-2") - require.NoError(t, err) - err = sbClient.DeleteChassis("non-kube-ovn-chassis") - require.NoError(t, err) - err = sbClient.DeleteChassis("mixed-chassis") - require.NoError(t, err) - chassisList, err = sbClient.GetKubeOvnChassisses() - require.NoError(t, err) - names = make(map[string]bool) - for _, chassis := range *chassisList { - names[chassis.Name] = true - } - require.False(t, names["kube-ovn-chassis-1"]) - require.False(t, names["kube-ovn-chassis-2"]) - require.False(t, names["non-kube-ovn-chassis"]) - require.False(t, names["mixed-chassis"]) + t.Cleanup(func() { + err := sbClient.DeleteChassis("kube-ovn-chassis-1") + require.NoError(t, err) + err = sbClient.DeleteChassis("kube-ovn-chassis-2") + require.NoError(t, err) + err = sbClient.DeleteChassis("non-kube-ovn-chassis") + require.NoError(t, err) + err = sbClient.DeleteChassis("mixed-chassis") + require.NoError(t, err) + chassisList, err := sbClient.GetKubeOvnChassisses() + require.NoError(t, err) + names := make(map[string]bool) + for _, chassis := range *chassisList { + names[chassis.Name] = true + } + require.False(t, names["kube-ovn-chassis-1"]) + require.False(t, names["kube-ovn-chassis-2"]) + require.False(t, names["non-kube-ovn-chassis"]) + require.False(t, names["mixed-chassis"]) + }) }