From 911d7eda9a8e208f4f9941d34b0df8e949e437b6 Mon Sep 17 00:00:00 2001 From: bobz965 Date: Thu, 5 Dec 2024 16:52:48 +0800 Subject: [PATCH] remove dup func Signed-off-by: bobz965 --- mocks/pkg/ovs/interface.go | 30 -------- pkg/controller/node.go | 12 ++-- pkg/ovs/interface.go | 1 - pkg/ovs/ovn-nb-suite_test.go | 4 -- pkg/ovs/ovn-sb-chassis.go | 30 ++------ pkg/ovs/ovn-sb-chassis_test.go | 122 ++++++--------------------------- 6 files changed, 31 insertions(+), 168 deletions(-) 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..6ea6f372971 100644 --- a/pkg/controller/node.go +++ b/pkg/controller/node.go @@ -666,19 +666,17 @@ 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) + if 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..950e6253676 100644 --- a/pkg/ovs/ovn-sb-chassis_test.go +++ b/pkg/ovs/ovn-sb-chassis_test.go @@ -199,70 +199,6 @@ func (suite *OvnClientTestSuite) testListChassis() { }) } -func (suite *OvnClientTestSuite) testGetAllChassisByHost() { - t := suite.T() - t.Parallel() - - sbClient := suite.ovnSBClient - - t.Cleanup(func() { - err := sbClient.DeleteChassis("chassis-3") - require.NoError(t, err) - err = sbClient.DeleteChassis("chassis-4") - require.NoError(t, err) - err = sbClient.DeleteChassis("chassis-5") - 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) - - ops1, err := sbClient.ovsDbClient.Create(chassis1) - require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops1) - require.NoError(t, err) - - ops2, err := sbClient.ovsDbClient.Create(chassis2) - require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops2) - require.NoError(t, err) - - ops3, err := sbClient.ovsDbClient.Create(chassis3) - require.NoError(t, err) - err = sbClient.Transact("chassis-add", ops3) - 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() @@ -321,7 +257,7 @@ func (suite *OvnClientTestSuite) testGetChassisByHost() { chassis, err := sbClient.GetChassisByHost("host-6") 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") require.NoError(t, err) @@ -374,24 +310,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 +438,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"]) + }) }