Skip to content

Commit

Permalink
remove dup func
Browse files Browse the repository at this point in the history
Signed-off-by: bobz965 <[email protected]>
  • Loading branch information
bobz965 committed Dec 5, 2024
1 parent c73b46e commit 3949cdf
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 118 deletions.
10 changes: 4 additions & 6 deletions pkg/controller/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -666,13 +666,11 @@ 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
Expand Down
1 change: 0 additions & 1 deletion pkg/ovs/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 0 additions & 4 deletions pkg/ovs/ovn-nb-suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
30 changes: 5 additions & 25 deletions pkg/ovs/ovn-sb-chassis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")
Expand All @@ -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
Expand Down
102 changes: 20 additions & 82 deletions pkg/ovs/ovn-sb-chassis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -520,22 +456,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"])
})
}

0 comments on commit 3949cdf

Please sign in to comment.