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 24, 2024
1 parent c73b46e commit 85542c2
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 174 deletions.
30 changes: 0 additions & 30 deletions mocks/pkg/ovs/interface.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 6 additions & 7 deletions pkg/controller/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion pkg/ovs/interface.go
View file
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
View file
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
28 changes: 3 additions & 25 deletions pkg/ovs/ovn-sb-chassis.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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 +85,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 +109,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
165 changes: 58 additions & 107 deletions pkg/ovs/ovn-sb-chassis_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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-3x", 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) {
Expand All @@ -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)
})
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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"])
})
}

0 comments on commit 85542c2

Please sign in to comment.