Fix target changes on LB

If a target with an identifier was previously registered with IPs and is
now changing its IPs (IP refresh, different target...), the LB can now
handle that case by removing it completly and adding it again.

the internal setTargets is now removing old targets before adding new
ones. It also checks if IPs for an identifier have changed or not.

pkg/loadbalancer/stream/loadbalancer.go

@@ -1,5 +1,5 @@
 /*
-Copyright (c) 2021-2022 Nordix Foundation
+Copyright (c) 2021-2023 Nordix Foundation
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -33,6 +33,7 @@ import (
 	"github.com/nordix/meridio/pkg/log"
 	"github.com/nordix/meridio/pkg/networking"
 	"github.com/nordix/meridio/pkg/retry"
+	"github.com/nordix/meridio/pkg/slice"
 )
 
 // LoadBalancer -
@@ -340,31 +341,39 @@ func (lb *LoadBalancer) setTargets(targets []*nspAPI.Target) error {
 		return nil
 	}
 	var errFinal error
-	toRemoveTargetsMap := make(map[int]struct{})
-	for identifier := range lb.targets {
-		toRemoveTargetsMap[identifier] = struct{}{}
-	}
-	lb.logger.V(2).Info("setTargets", "targets", targets)
-	for _, target := range targets { // targets to add
+	newTargetsMap := make(map[int]types.Target)
+	for _, target := range targets {
 		t, err := NewTarget(target, lb.netUtils)
 		if err != nil {
 			continue
 		}
-		if lb.targetExists(t.GetIdentifier()) {
-			delete(toRemoveTargetsMap, t.GetIdentifier())
-		} else {
-			err = lb.AddTarget(t) // todo: pending targets?
+		newTargetsMap[t.GetIdentifier()] = t
+	}
+	for identifier, target := range lb.targets { // targets to remove
+		newTarget, exists := newTargetsMap[identifier]
+		if !exists {
+			err := lb.RemoveTarget(identifier)
 			if err != nil {
 				errFinal = fmt.Errorf("%w; %v", errFinal, err)
 			}
+			continue
 		}
-	}
-	for identifier := range toRemoveTargetsMap { // targets to remove
+		if slice.ContainsSameStrings(target.GetIps(), newTarget.GetIps()) { // have the same IPs?
+			delete(newTargetsMap, identifier)
+			continue
+		}
+		// Have different IPs, so the target IPs have changed and need to be removed and re-added
 		err := lb.RemoveTarget(identifier)
 		if err != nil {
 			errFinal = fmt.Errorf("%w; %v", errFinal, err)
 		}
 	}
+	for _, target := range newTargetsMap { // targets to add
+		err := lb.AddTarget(target)
+		if err != nil {
+			errFinal = fmt.Errorf("%w; %v", errFinal, err)
+		}
+	}
 	return errFinal
 }
 

pkg/slice/compare.go

@@ -0,0 +1,40 @@
+/*
+Copyright (c) 2023 Nordix Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package slice
+
+// ContainsSameStrings returns if 2 slices are containing the exactly the
+// same strings, No matter the order.
+func ContainsSameStrings(a []string, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	aMap := map[string]int{}
+	for _, e := range a {
+		aMap[e]++
+	}
+	for _, e := range b {
+		_, exists := aMap[e]
+		if !exists {
+			return false
+		}
+		aMap[e]--
+		if aMap[e] == 0 {
+			delete(aMap, e)
+		}
+	}
+	return len(aMap) == 0
+}

pkg/slice/compare_test.go

@@ -0,0 +1,115 @@
+/*
+Copyright (c) 2023 Nordix Foundation
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package slice_test
+
+import (
+	"testing"
+
+	"github.com/nordix/meridio/pkg/slice"
+)
+
+func TestContainsSameStrings(t *testing.T) {
+	type args struct {
+		a []string
+		b []string
+	}
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "empty",
+			args: args{
+				a: []string{},
+				b: []string{},
+			},
+			want: true,
+		},
+		{
+			name: "empty and nil",
+			args: args{
+				a: []string{},
+				b: nil,
+			},
+			want: true,
+		},
+		{
+			name: "a empty",
+			args: args{
+				a: []string{"a"},
+				b: []string{},
+			},
+			want: false,
+		},
+		{
+			name: "b empty",
+			args: args{
+				a: []string{"a"},
+				b: []string{},
+			},
+			want: false,
+		},
+		{
+			name: "equal with 1 element",
+			args: args{
+				a: []string{"a"},
+				b: []string{"a"},
+			},
+			want: true,
+		},
+		{
+			name: "not equal with 1 element",
+			args: args{
+				a: []string{"a"},
+				b: []string{"b"},
+			},
+			want: false,
+		},
+		{
+			name: "equal with 2 element same order",
+			args: args{
+				a: []string{"a", "b"},
+				b: []string{"a", "b"},
+			},
+			want: true,
+		},
+		{
+			name: "equal with 2 element different order",
+			args: args{
+				a: []string{"a", "b"},
+				b: []string{"b", "a"},
+			},
+			want: true,
+		},
+		{
+			name: "duplicates",
+			args: args{
+				a: []string{"hello", "world", "hello"},
+				b: []string{"hello", "hello", "world"},
+			},
+			want: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := slice.ContainsSameStrings(tt.args.a, tt.args.b); got != tt.want {
+				t.Errorf("ContainsSameStrings() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}