diff --git a/pkg/ipam/ip_range_list_test.go b/pkg/ipam/ip_range_list_test.go index 61eef0ca297..2bca1e6a828 100644 --- a/pkg/ipam/ip_range_list_test.go +++ b/pkg/ipam/ip_range_list_test.go @@ -26,7 +26,11 @@ func TestNewIPRangeList(t *testing.T) { v4RangeEnd2, err := NewIP("10.0.0.18") require.NoError(t, err) - v4, err := NewIPRangeList(v4RangeStart1, v4RangeEnd1, v4RangeStart2, v4RangeEnd2) + v4, err := NewIPRangeList(v4RangeStart1) + require.ErrorContains(t, err, "length of ips must be an even number") + require.Nil(t, v4) + + v4, err = NewIPRangeList(v4RangeStart1, v4RangeEnd1, v4RangeStart2, v4RangeEnd2) require.NoError(t, err) fakeV4RangeItem1, err := NewIP("10.0.0.4") @@ -837,6 +841,22 @@ func TestNewIPRangeListFrom(t *testing.T) { require.True(t, ipList.Contains(end.Sub(1))) } } + + ipList, err = NewIPRangeListFrom("192.168.1.2..192.168.1.1") + require.ErrorContains(t, err, "invalid ip range \"192.168.1.2..192.168.1.1\": 192.168.1.2 is greater than 192.168.1.1") + require.Nil(t, ipList) + + ipList, err = NewIPRangeListFrom("invalidIP..192.168.1.1") + require.ErrorContains(t, err, "invalid IP address") + require.Nil(t, ipList) + + ipList, err = NewIPRangeListFrom("192.168.1.2..invalidIP") + require.ErrorContains(t, err, "invalid IP address") + require.Nil(t, ipList) + + ipList, err = NewIPRangeListFrom("invalidCIDR/24") + require.ErrorContains(t, err, "invalid CIDR address: invalidCIDR/24") + require.Nil(t, ipList) } func TestRemove(t *testing.T) { @@ -1140,3 +1160,68 @@ func TestEqual(t *testing.T) { v6RL5 := NewEmptyIPRangeList() require.False(t, v6RangeList1.Equal(v6RL5)) } + +func TestAllocate(t *testing.T) { + v4RangeStart, err := NewIP("10.0.0.1") + require.NoError(t, err) + require.NotNil(t, v4RangeStart) + v4RangeEnd, err := NewIP("10.0.0.4") + require.NoError(t, err) + require.NotNil(t, v4RangeEnd) + v4Range := NewIPRange(v4RangeStart, v4RangeEnd) + v4RangeList := NewEmptyIPRangeList().MergeRange(v4Range) + + t.Run("Allocate from empty range", func(t *testing.T) { + emptyRange := NewEmptyIPRangeList() + allocated := emptyRange.Allocate(nil) + require.Nil(t, allocated) + }) + + t.Run("Allocate without skipped IPs", func(t *testing.T) { + allocated := v4RangeList.Allocate(nil) + require.Equal(t, "10.0.0.1", allocated.String()) + require.False(t, v4RangeList.Contains(allocated)) + }) + + t.Run("Allocate with skipped IPs", func(t *testing.T) { + skipped1, err := NewIP("10.0.0.2") + require.NoError(t, err) + skipped2, err := NewIP("10.0.0.3") + require.NoError(t, err) + allocated := v4RangeList.Allocate([]IP{skipped1, skipped2}) + require.Equal(t, "10.0.0.4", allocated.String()) + require.False(t, v4RangeList.Contains(allocated)) + }) + + t.Run("Allocate all IPs", func(t *testing.T) { + skipped1, err := NewIP("10.0.0.1") + require.NoError(t, err) + skipped2, err := NewIP("10.0.0.2") + require.NoError(t, err) + skipped3, err := NewIP("10.0.0.3") + require.NoError(t, err) + skipped4, err := NewIP("10.0.0.4") + require.NoError(t, err) + allocated := v4RangeList.Allocate([]IP{skipped1, skipped2, skipped3, skipped4}) + require.Nil(t, allocated) + }) + + t.Run("Allocate from IPv6 range", func(t *testing.T) { + v6RangeStart, err := NewIP("2001:db8::1") + require.NoError(t, err) + v6RangeEnd, err := NewIP("2001:db8::10") + require.NoError(t, err) + v6Range := NewIPRange(v6RangeStart, v6RangeEnd) + v6RangeList := NewEmptyIPRangeList().MergeRange(v6Range) + + allocated := v6RangeList.Allocate(nil) + require.Equal(t, "2001:db8::1", allocated.String()) + require.False(t, v6RangeList.Contains(allocated)) + + skipped, err := NewIP("2001:db8::2") + require.NoError(t, err) + allocated = v6RangeList.Allocate([]IP{skipped}) + require.Equal(t, "2001:db8::3", allocated.String()) + require.False(t, v6RangeList.Contains(allocated)) + }) +} diff --git a/pkg/ipam/ip_range_test.go b/pkg/ipam/ip_range_test.go index c0cd87c7bcb..9bb2da550e3 100644 --- a/pkg/ipam/ip_range_test.go +++ b/pkg/ipam/ip_range_test.go @@ -207,6 +207,12 @@ func TestIPRangeRemove(t *testing.T) { wantRanges: []*IPRange{NewIPRange(start, IP(net.ParseIP("192.168.1.9").To4()))}, wantOk: true, }, + { + name: "Remove IP that is not included", + ip: IP(net.ParseIP("192.168.1.254").To4()), + wantRanges: nil, + wantOk: false, + }, } for _, tt := range tests { diff --git a/pkg/ovs/ovn-nb-load_balancer_health_check_test.go b/pkg/ovs/ovn-nb-load_balancer_health_check_test.go index 9d91e7f33ed..26118e66d3c 100644 --- a/pkg/ovs/ovn-nb-load_balancer_health_check_test.go +++ b/pkg/ovs/ovn-nb-load_balancer_health_check_test.go @@ -9,6 +9,7 @@ import ( "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/require" + ovsclient "github.com/kubeovn/kube-ovn/pkg/ovsdb/client" "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" ) @@ -45,6 +46,14 @@ func (suite *OvnClientTestSuite) testAddLoadBalancerHealthCheck() { require.NotEmpty(t, lbhcRepeat.UUID) require.Equal(t, lbhc.UUID, lbhcRepeat.UUID) + + // create lbhc with empty lbName + err = nbClient.AddLoadBalancerHealthCheck("", vip, map[string]string{}) + require.ErrorContains(t, err, "the lb name is required") + + // create lbhc with empty vip + err = nbClient.AddLoadBalancerHealthCheck(lbName, "", map[string]string{}) + require.ErrorContains(t, err, "the vip endpoint is required") } func (suite *OvnClientTestSuite) testUpdateLoadBalancerHealthCheck() { @@ -95,6 +104,10 @@ func (suite *OvnClientTestSuite) testDeleteLoadBalancerHealthCheck() { err error ) + // delete lbhc for non-exist lb + err = nbClient.DeleteLoadBalancerHealthCheck(lbName, vip) + require.ErrorContains(t, err, "not found load balancer") + err = nbClient.CreateLoadBalancer(lbName, "tcp", "ip_dst") require.NoError(t, err) @@ -202,6 +215,43 @@ func (suite *OvnClientTestSuite) testGetLoadBalancerHealthCheck() { require.NoError(t, err) }, ) + + t.Run("should return err when health checks have the same vipEndpoint", + func(t *testing.T) { + t.Parallel() + err = nbClient.CreateLoadBalancer(lbName+"1", "tcp", "") + require.NoError(t, err) + lbhc1 := &ovnnb.LoadBalancerHealthCheck{ + UUID: ovsclient.NamedUUID(), + ExternalIDs: nil, + Options: map[string]string{ + "timeout": "20", + "interval": "5", + "success_count": "3", + "failure_count": "3", + }, + Vip: vip, + } + lbhc2 := &ovnnb.LoadBalancerHealthCheck{ + UUID: ovsclient.NamedUUID(), + ExternalIDs: nil, + Options: map[string]string{ + "timeout": "20", + "interval": "5", + "success_count": "3", + "failure_count": "3", + }, + Vip: vip, + } + err = nbClient.CreateLoadBalancerHealthCheck(lbName+"1", vip, lbhc1) + require.NoError(t, err) + err = nbClient.CreateLoadBalancerHealthCheck(lbName+"1", vip, lbhc2) + require.NoError(t, err) + + _, _, err := nbClient.GetLoadBalancerHealthCheck(lbName+"1", vip, true) + require.ErrorContains(t, err, "lb test-get-lb-hc1 has more than one health check with the same vip 1.1.1.22:80") + }, + ) } func (suite *OvnClientTestSuite) testListLoadBalancerHealthChecks() { @@ -368,3 +418,60 @@ func (suite *OvnClientTestSuite) testDeleteLoadBalancerHealthCheckOp() { }, ) } + +func (suite *OvnClientTestSuite) testNewLoadBalancerHealthCheck() { + t := suite.T() + t.Parallel() + + var ( + nbClient = suite.ovnNBClient + lbName = "test-new-lb-hc" + vip = "4.4.4.4:80" + ) + + t.Run("create new load balancer health check", func(t *testing.T) { + externals := map[string]string{"key1": "value1", "key2": "value2"} + lbhc, err := nbClient.newLoadBalancerHealthCheck(lbName, vip, externals) + require.ErrorContains(t, err, "not found load balancer") + require.Nil(t, lbhc) + + err = nbClient.CreateLoadBalancer(lbName, "tcp", "") + require.NoError(t, err) + + lbhc, err = nbClient.newLoadBalancerHealthCheck(lbName, vip, externals) + require.NoError(t, err) + require.NotNil(t, lbhc) + require.Equal(t, vip, lbhc.Vip) + require.Equal(t, externals, lbhc.ExternalIDs) + require.Equal(t, "20", lbhc.Options["timeout"]) + require.Equal(t, "5", lbhc.Options["interval"]) + require.Equal(t, "3", lbhc.Options["success_count"]) + require.Equal(t, "3", lbhc.Options["failure_count"]) + }) + + t.Run("create with empty lb name", func(t *testing.T) { + lbhc, err := nbClient.newLoadBalancerHealthCheck("", vip, nil) + require.Error(t, err) + require.Nil(t, lbhc) + require.Contains(t, err.Error(), "the lb name is required") + }) + + t.Run("create with empty vip", func(t *testing.T) { + lbhc, err := nbClient.newLoadBalancerHealthCheck(lbName, "", nil) + require.Error(t, err) + require.Nil(t, lbhc) + require.Contains(t, err.Error(), "the vip endpoint is required") + }) + + t.Run("create existing health check", func(t *testing.T) { + err := nbClient.CreateLoadBalancer(lbName, "tcp", "ip_dst") + require.NoError(t, err) + + err = nbClient.AddLoadBalancerHealthCheck(lbName, vip, nil) + require.NoError(t, err) + + lbhc, err := nbClient.newLoadBalancerHealthCheck(lbName, vip, nil) + require.NoError(t, err) + require.Nil(t, lbhc) + }) +} diff --git a/pkg/ovs/ovn-nb-load_balancer_test.go b/pkg/ovs/ovn-nb-load_balancer_test.go index c9e0d586f65..069f18c2dbf 100644 --- a/pkg/ovs/ovn-nb-load_balancer_test.go +++ b/pkg/ovs/ovn-nb-load_balancer_test.go @@ -8,9 +8,11 @@ import ( "strings" "testing" + "github.com/ovn-org/libovsdb/model" "github.com/ovn-org/libovsdb/ovsdb" "github.com/stretchr/testify/require" + ovsclient "github.com/kubeovn/kube-ovn/pkg/ovsdb/client" "github.com/kubeovn/kube-ovn/pkg/ovsdb/ovnnb" ) @@ -283,6 +285,38 @@ func (suite *OvnClientTestSuite) testDeleteLoadBalancerOp() { require.NoError(t, err) require.Len(t, ops, 0) }) + + t.Run("Create load balancer when multiple load balancer exist", func(t *testing.T) { + t.Parallel() + + lbName := "test-delete-lb-op-duplicate" + // create load balancer + lb1 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err := nbClient.ovsDbClient.Create(lb1) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + lb2 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err = nbClient.ovsDbClient.Create(lb2) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + ops, err = nbClient.DeleteLoadBalancerOp(lbName) + require.ErrorContains(t, err, "more than one load balancer with same name") + require.Nil(t, ops) + }) } func (suite *OvnClientTestSuite) testSetLoadBalancerAffinityTimeout() { @@ -324,6 +358,37 @@ func (suite *OvnClientTestSuite) testSetLoadBalancerAffinityTimeout() { require.Equal(t, lb.Options["affinity_timeout"], strconv.Itoa(expectedTimeout)) }) + + t.Run("set loadbalancer affinity timeout when multiple load balancer exist", + func(t *testing.T) { + lbName := "test-set-lb-affinity" + // create load balancer + lb1 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err := nbClient.ovsDbClient.Create(lb1) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + lb2 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err = nbClient.ovsDbClient.Create(lb2) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + err = nbClient.SetLoadBalancerAffinityTimeout(lbName, expectedTimeout) + require.ErrorContains(t, err, "more than one load balancer with same name") + }, + ) } func (suite *OvnClientTestSuite) testLoadBalancerAddVip() { @@ -387,6 +452,13 @@ func (suite *OvnClientTestSuite) testLoadBalancerAddVip() { require.Equal(t, lb.Vips, expectedVips) }, ) + + t.Run("add new vips to non-exist load balancer", + func(t *testing.T) { + err := nbClient.LoadBalancerAddVip("non-exist-lb", "10.96.0.2:443", "192.168.20.3:6443") + require.ErrorContains(t, err, "not found load balancer") + }, + ) } func (suite *OvnClientTestSuite) testLoadBalancerAddHealthCheck() { @@ -394,14 +466,15 @@ func (suite *OvnClientTestSuite) testLoadBalancerAddHealthCheck() { t.Parallel() nbClient := suite.ovnNBClient - lbName := "test-add-hc-lb" - vips := map[string]string{ - "10.96.0.5:443": "192.168.20.3:6443", - "10.107.43.241:8080": "10.244.0.15:8080,10.244.0.16:8080,10.244.0.17:8080", - "[fd00:10:96::e86f]:8080": "[fc00::af4:a]:8080,[fc00::af4:b]:8080,[fc00::af4:c]:8080", - } + t.Run("add health check to load balancer", func(t *testing.T) { + lbName := "test-add-hc-lb" + vips := map[string]string{ + "10.96.0.5:443": "192.168.20.3:6443", + "10.107.43.241:8080": "10.244.0.15:8080,10.244.0.16:8080,10.244.0.17:8080", + "[fd00:10:96::e86f]:8080": "[fc00::af4:a]:8080,[fc00::af4:b]:8080,[fc00::af4:c]:8080", + } // create load balancer err := nbClient.CreateLoadBalancer(lbName, "tcp", "") require.NoError(t, err) @@ -427,6 +500,37 @@ func (suite *OvnClientTestSuite) testLoadBalancerAddHealthCheck() { } }, ) + + t.Run("create load balancer when multiple load balancer exist", + func(t *testing.T) { + lbName := "test-create-lb-duplicate" + // create load balancer + lb1 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err := nbClient.ovsDbClient.Create(lb1) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + lb2 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err = nbClient.ovsDbClient.Create(lb2) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + err = nbClient.CreateLoadBalancer(lbName, "tcp", "") + require.ErrorContains(t, err, "more than one load balancer with same name") + }, + ) } func (suite *OvnClientTestSuite) testLoadBalancerDeleteVip() { @@ -474,6 +578,43 @@ func (suite *OvnClientTestSuite) testLoadBalancerDeleteVip() { lb, err = nbClient.GetLoadBalancer(lbName, false) require.NoError(t, err) require.Equal(t, vips, lb.Vips) + + err = nbClient.LoadBalancerAddHealthCheck(lbName, "10.107.43.239:8080", false, nil, nil) + require.NoError(t, err) + + err = nbClient.LoadBalancerDeleteVip(lbName, "10.107.43.239:8080", false) + require.NoError(t, err) + + // delete vip when lb.Vips is empty + err = nbClient.LoadBalancerDeleteVip(lbName, "10.107.43.239:8080", false) + require.NoError(t, err) + + // delete vip when multiple load balancer exist + lbName = "test-delete-lb-vip" + lb1 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err := nbClient.ovsDbClient.Create(lb1) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + lb2 := &ovnnb.LoadBalancer{ + UUID: ovsclient.NamedUUID(), + Name: lbName, + Protocol: &ovnnb.LoadBalancerProtocolTCP, + } + ops, err = nbClient.ovsDbClient.Create(lb2) + require.NoError(t, err) + require.NotNil(t, ops) + err = nbClient.Transact("lb-add", ops) + require.NoError(t, err) + + err = nbClient.LoadBalancerDeleteVip(lbName, "10.107.43.239:8080", ignoreHealthCheck) + require.ErrorContains(t, err, "more than one load balancer with same name") } func (suite *OvnClientTestSuite) testLoadBalancerAddIPPortMapping() { @@ -572,6 +713,13 @@ func (suite *OvnClientTestSuite) testLoadBalancerAddIPPortMapping() { } }, ) + + t.Run("add nil port mappings to load balancer", + func(t *testing.T) { + err = nbClient.LoadBalancerAddIPPortMapping(lbName, "", nil) + require.NoError(t, err) + }, + ) } func (suite *OvnClientTestSuite) testLoadBalancerDeleteIPPortMapping() { @@ -837,4 +985,115 @@ func (suite *OvnClientTestSuite) testLoadBalancerWithHealthCheck() { require.NotContains(t, lb.HealthCheck, lbhcID) }, ) + + t.Run("delete health check from non-exist load balancer", + func(t *testing.T) { + err = nbClient.LoadBalancerDeleteHealthCheck("non-exist-lbName", lbhcID) + require.ErrorContains(t, err, "not found load balancer") + }, + ) +} + +func (suite *OvnClientTestSuite) testLoadBalancerOp() { + t := suite.T() + t.Parallel() + + nbClient := suite.ovnNBClient + lbName := "test-lb-op" + + err := nbClient.CreateLoadBalancer(lbName, "tcp", "") + require.NoError(t, err) + + t.Run("no mutations", func(t *testing.T) { + ops, err := nbClient.LoadBalancerOp(lbName) + require.NoError(t, err) + require.Empty(t, ops) + }) + + t.Run("single mutation", func(t *testing.T) { + mutationFunc := func(lb *ovnnb.LoadBalancer) []model.Mutation { + return []model.Mutation{ + { + Field: &lb.HealthCheck, + Value: []string{}, + Mutator: ovsdb.MutateOperationDelete, + }, + } + } + + ops, err := nbClient.LoadBalancerOp(lbName, mutationFunc) + require.NoError(t, err) + require.Len(t, ops, 1) + require.Equal(t, ovsdb.OperationMutate, ops[0].Op) + }) + + t.Run("multiple mutations", func(t *testing.T) { + mutationFunc1 := func(lb *ovnnb.LoadBalancer) []model.Mutation { + return []model.Mutation{ + { + Field: &lb.HealthCheck, + Value: []string{}, + Mutator: ovsdb.MutateOperationDelete, + }, + } + } + mutationFunc2 := func(lb *ovnnb.LoadBalancer) []model.Mutation { + return []model.Mutation{ + { + Field: &lb.Options, + Value: map[string]string{"skip_snat": "true"}, + Mutator: ovsdb.MutateOperationInsert, + }, + } + } + + ops, err := nbClient.LoadBalancerOp(lbName, mutationFunc1, mutationFunc2) + require.NoError(t, err) + require.Len(t, ops, 1) + require.Equal(t, ovsdb.OperationMutate, ops[0].Op) + require.Len(t, ops[0].Mutations, 2) + }) + + t.Run("empty mutation", func(t *testing.T) { + mutationFunc := func(_ *ovnnb.LoadBalancer) []model.Mutation { + return []model.Mutation{} + } + + ops, err := nbClient.LoadBalancerOp(lbName, mutationFunc) + require.NoError(t, err) + require.Empty(t, ops) + }) + + t.Run("non-existent load balancer", func(t *testing.T) { + mutationFunc := func(lb *ovnnb.LoadBalancer) []model.Mutation { + return []model.Mutation{ + { + Field: &lb.HealthCheck, + Value: []string{}, + Mutator: ovsdb.MutateOperationDelete, + }, + } + } + + ops, err := nbClient.LoadBalancerOp("non-existent-lb", mutationFunc) + require.Error(t, err) + require.Nil(t, ops) + }) +} + +func (suite *OvnClientTestSuite) testLoadBalancerUpdateHealthCheckOp() { + t := suite.T() + t.Parallel() + + nbClient := suite.ovnNBClient + lbName := "test-lb-update-hc-op" + + err := nbClient.CreateLoadBalancer(lbName, "tcp", "") + require.NoError(t, err) + + t.Run("empty lbhcUUIDs", func(t *testing.T) { + ops, err := nbClient.LoadBalancerUpdateHealthCheckOp(lbName, []string{}, ovsdb.MutateOperationInsert) + require.NoError(t, err) + require.Nil(t, ops) + }) } diff --git a/pkg/ovs/ovn-nb-suite_test.go b/pkg/ovs/ovn-nb-suite_test.go index e2a64a20f1a..f5567c7b066 100644 --- a/pkg/ovs/ovn-nb-suite_test.go +++ b/pkg/ovs/ovn-nb-suite_test.go @@ -521,11 +521,23 @@ func (suite *OvnClientTestSuite) Test_LoadBalancerWithHealthCheck() { suite.testLoadBalancerWithHealthCheck() } +func (suite *OvnClientTestSuite) Test_LoadBalancerOp() { + suite.testLoadBalancerOp() +} + +func (suite *OvnClientTestSuite) Test_LoadBalancerUpdateHealthCheckOp() { + suite.testLoadBalancerUpdateHealthCheckOp() +} + /* load_balancer health check unit test */ func (suite *OvnClientTestSuite) Test_CreateLoadBalancerHealthCheck() { suite.testAddLoadBalancerHealthCheck() } +func (suite *OvnClientTestSuite) Test_NewLoadBalancerHealthCheck() { + suite.testNewLoadBalancerHealthCheck() +} + func (suite *OvnClientTestSuite) Test_UpdateLoadBalancerHealthCheck() { suite.testUpdateLoadBalancerHealthCheck() }