diff --git a/pkg/controller/elasticsearch/driver/downscale_test.go b/pkg/controller/elasticsearch/driver/downscale_test.go index fcdbf595f5..93ca20b474 100644 --- a/pkg/controller/elasticsearch/driver/downscale_test.go +++ b/pkg/controller/elasticsearch/driver/downscale_test.go @@ -34,7 +34,13 @@ import ( // Sample StatefulSets to use in tests var ( - clusterName = "cluster-name" + clusterName = "cluster-name" + es = esv1.Elasticsearch{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Namespace: "ns", + }, + } ssetMaster3Replicas = sset.TestSset{ Name: "ssetMaster3Replicas", Namespace: "ns", @@ -118,7 +124,7 @@ var ( Ready: true, }.Build(), } - runtimeObjs = []runtime.Object{&ssetMaster3Replicas, &ssetData4Replicas, + runtimeObjs = []runtime.Object{&es, &ssetMaster3Replicas, &ssetData4Replicas, &podsSsetMaster3Replicas[0], &podsSsetMaster3Replicas[1], &podsSsetMaster3Replicas[2], &podsSsetData4Replicas[0], &podsSsetData4Replicas[1], &podsSsetData4Replicas[2], &podsSsetData4Replicas[3], } @@ -147,13 +153,8 @@ func TestHandleDownscale(t *testing.T) { {Index: "index-1", Shard: "0", State: esclient.STARTED, NodeName: "ssetData4Replicas-2"}, }, ), - esClient: esClient, - es: esv1.Elasticsearch{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterName, - Namespace: "ns", - }, - }, + esClient: esClient, + es: es, parentCtx: context.Background(), } @@ -802,8 +803,8 @@ func Test_doDownscale_updateReplicasAndExpectations(t *testing.T) { func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) { ssetMasters := sset.TestSset{ Name: "masters", - Namespace: "ns", - ClusterName: "es", + Namespace: es.Namespace, + ClusterName: es.Name, Version: "7.1.0", Replicas: 3, Master: true, @@ -811,8 +812,8 @@ func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) { }.Build() ssetData := sset.TestSset{ Name: "datas", - Namespace: "ns", - ClusterName: "es", + Namespace: es.Namespace, + ClusterName: es.Name, Version: "7.1.0", Replicas: 3, Master: false, @@ -847,12 +848,6 @@ func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - es := esv1.Elasticsearch{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: ssetMasters.Namespace, - Name: "es", - }, - } // simulate an existing v7 master for zen2 to be called v7Pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -865,7 +860,7 @@ func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) { }, }, } - k8sClient := k8s.WrappedFakeClient(&ssetMasters, &ssetData, &v7Pod) + k8sClient := k8s.WrappedFakeClient(es.DeepCopy(), &ssetMasters, &ssetData, &v7Pod) esClient := &fakeESClient{} downscaleCtx := downscaleContext{ k8sClient: k8sClient, diff --git a/pkg/controller/elasticsearch/version/zen2/voting_exclusions.go b/pkg/controller/elasticsearch/version/zen2/voting_exclusions.go index 15da6a3f5f..e5adfd48b5 100644 --- a/pkg/controller/elasticsearch/version/zen2/voting_exclusions.go +++ b/pkg/controller/elasticsearch/version/zen2/voting_exclusions.go @@ -6,6 +6,8 @@ package zen2 import ( "context" + "sort" + "strings" logf "sigs.k8s.io/controller-runtime/pkg/log" @@ -19,6 +21,40 @@ var ( log = logf.Log.WithName("zen2") ) +const ( + // VotingConfigExclusionsAnnotationName is an annotation that stores the last applied voting config exclusions. + // An empty value means no voting config exclusions are set. + VotingConfigExclusionsAnnotationName = "elasticsearch.k8s.elastic.co/voting-config-exclusions" +) + +// serializeExcludedNodesForAnnotation returns a sorted comma-separated representation of the given slice. +func serializeExcludedNodesForAnnotation(excludedNodes []string) string { + // sort a copy to not mutate the given slice + sliceCopy := make([]string, len(excludedNodes)) + copy(sliceCopy, excludedNodes) + sort.Strings(sliceCopy) + return strings.Join(sliceCopy, ",") +} + +// votingConfigAnnotationMatches returns true if the voting config exclusions annotation value +// matches the given excluded nodes. +func votingConfigAnnotationMatches(es esv1.Elasticsearch, excludedNodes []string) bool { + value, exists := es.Annotations[VotingConfigExclusionsAnnotationName] + if !exists { + return false + } + return value == serializeExcludedNodesForAnnotation(excludedNodes) +} + +// setVotingConfigAnnotation sets the value of the voting config exclusions annotation to the given excluded nodes. +func setVotingConfigAnnotation(c k8s.Client, es esv1.Elasticsearch, excludedNodes []string) error { + if es.Annotations == nil { + es.Annotations = map[string]string{} + } + es.Annotations[VotingConfigExclusionsAnnotationName] = serializeExcludedNodesForAnnotation(excludedNodes) + return c.Update(&es) +} + // AddToVotingConfigExclusions adds the given node names to exclude from voting config exclusions. func AddToVotingConfigExclusions(ctx context.Context, c k8s.Client, esClient client.Client, es esv1.Elasticsearch, excludeNodes []string) error { compatible, err := AllMastersCompatibleWithZen2(c, es) @@ -28,13 +64,20 @@ func AddToVotingConfigExclusions(ctx context.Context, c k8s.Client, esClient cli if !compatible { return nil } + + if votingConfigAnnotationMatches(es, excludeNodes) { + // nothing to do, we already applied that setting + return nil + } + log.Info("Setting voting config exclusions", "namespace", es.Namespace, "nodes", excludeNodes) ctx, cancel := context.WithTimeout(ctx, client.DefaultReqTimeout) defer cancel() if err := esClient.AddVotingConfigExclusions(ctx, excludeNodes, ""); err != nil { return err } - return nil + // store the excluded nodes value in an annotation so we don't perform the same API call over and over again + return setVotingConfigAnnotation(c, es, excludeNodes) } // canClearVotingConfigExclusions returns true if it is safe to clear voting config exclusions. @@ -63,6 +106,12 @@ func ClearVotingConfigExclusions(ctx context.Context, es esv1.Elasticsearch, c k return false, nil } + var noExcludedNodes []string = nil + if votingConfigAnnotationMatches(es, noExcludedNodes) { + // nothing to do, we already applied that setting + return false, nil + } + canClear, err := canClearVotingConfigExclusions(c, actualStatefulSets) if err != nil { return false, err @@ -78,5 +127,7 @@ func ClearVotingConfigExclusions(ctx context.Context, es esv1.Elasticsearch, c k if err := esClient.DeleteVotingConfigExclusions(ctx, false); err != nil { return false, err } - return false, nil + + // store the excluded nodes value in an annotation so we don't perform the same API call over and over again + return false, setVotingConfigAnnotation(c, es, noExcludedNodes) } diff --git a/pkg/controller/elasticsearch/version/zen2/voting_exclusions_test.go b/pkg/controller/elasticsearch/version/zen2/voting_exclusions_test.go index 89d5287d42..b24821e104 100644 --- a/pkg/controller/elasticsearch/version/zen2/voting_exclusions_test.go +++ b/pkg/controller/elasticsearch/version/zen2/voting_exclusions_test.go @@ -18,7 +18,8 @@ import ( ) type fakeVotingConfigExclusionsESClient struct { - called bool + called bool + excludedNodes []string client.Client } @@ -27,6 +28,20 @@ func (f *fakeVotingConfigExclusionsESClient) DeleteVotingConfigExclusions(ctx co return nil } +func (f *fakeVotingConfigExclusionsESClient) AddVotingConfigExclusions(ctx context.Context, nodeNames []string, timeout string) error { + f.called = true + f.excludedNodes = nodeNames + return nil +} + +func withVotingConfigAnnotation(es esv1.Elasticsearch, value string) *esv1.Elasticsearch { + clone := es.DeepCopy() + clone.Annotations = map[string]string{ + VotingConfigExclusionsAnnotationName: value, + } + return clone +} + func Test_ClearVotingConfigExclusions(t *testing.T) { // dummy statefulset with 3 pods statefulSet3rep := sset.TestSset{Name: "nodes", Version: "7.2.0", Replicas: 3, Master: true, Data: true}.Build() @@ -45,38 +60,64 @@ func Test_ClearVotingConfigExclusions(t *testing.T) { // simulate 2 pods out of the 3 statefulSet2rep := sset.TestSset{Name: "nodes", Version: "7.2.0", Replicas: 2, Master: true, Data: true}.Build() tests := []struct { - name string - c k8s.Client - actualStatefulSets sset.StatefulSetList - wantCall bool - wantRequeue bool + name string + c k8s.Client + es *esv1.Elasticsearch + actualStatefulSets sset.StatefulSetList + wantCall bool + wantRequeue bool + wantVotingConfigAnnotation string }{ { name: "no v7 nodes", - c: k8s.WrappedFakeClient(), + c: k8s.WrappedFakeClient(&es), + es: &es, actualStatefulSets: sset.StatefulSetList{ createStatefulSetWithESVersion("6.8.0"), }, - wantCall: false, - wantRequeue: false, + wantCall: false, + wantRequeue: false, + wantVotingConfigAnnotation: "", }, { - name: "3/3 nodes there: can clear", - c: k8s.WrappedFakeClient(&statefulSet3rep, &pods[0], &pods[1], &pods[2]), - actualStatefulSets: sset.StatefulSetList{statefulSet3rep}, - wantCall: true, - wantRequeue: false, + name: "3/3 nodes there, no annotation set: should clear", + c: k8s.WrappedFakeClient(&es, &statefulSet3rep, &pods[0], &pods[1], &pods[2]), + es: &es, + actualStatefulSets: sset.StatefulSetList{statefulSet3rep}, + wantCall: true, + wantRequeue: false, + wantVotingConfigAnnotation: "", + }, + { + name: "3/3 nodes there, annotation already set, should do nothing", + c: k8s.WrappedFakeClient(withVotingConfigAnnotation(es, ""), &statefulSet3rep, &pods[0], &pods[1], &pods[2]), + es: withVotingConfigAnnotation(es, ""), + actualStatefulSets: sset.StatefulSetList{statefulSet3rep}, + wantCall: false, + wantRequeue: false, + wantVotingConfigAnnotation: "", + }, + { + name: "3/3 nodes there, annotation set to the wrong value, should clear", + c: k8s.WrappedFakeClient(withVotingConfigAnnotation(es, "node1"), &statefulSet3rep, &pods[0], &pods[1], &pods[2]), + es: withVotingConfigAnnotation(es, "node1"), + actualStatefulSets: sset.StatefulSetList{statefulSet3rep}, + wantCall: true, + wantRequeue: false, + wantVotingConfigAnnotation: "", }, { name: "2/3 nodes there: cannot clear, should requeue", - c: k8s.WrappedFakeClient(&statefulSet3rep, &pods[0], &pods[1]), + c: k8s.WrappedFakeClient(&es, &statefulSet3rep, &pods[0], &pods[1]), + es: &es, actualStatefulSets: sset.StatefulSetList{statefulSet3rep}, wantCall: false, wantRequeue: true, }, { name: "3/2 nodes there: cannot clear, should requeue", - c: k8s.WrappedFakeClient(&statefulSet2rep, &pods[0], &pods[1], &pods[2]), + es: &es, + c: k8s.WrappedFakeClient(&es, &statefulSet2rep, &pods[0], &pods[1], &pods[2]), actualStatefulSets: sset.StatefulSetList{statefulSet2rep}, wantCall: false, wantRequeue: true, @@ -85,10 +126,104 @@ func Test_ClearVotingConfigExclusions(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { clientMock := &fakeVotingConfigExclusionsESClient{} - requeue, err := ClearVotingConfigExclusions(context.Background(), es, tt.c, clientMock, tt.actualStatefulSets) + requeue, err := ClearVotingConfigExclusions(context.Background(), *tt.es, tt.c, clientMock, tt.actualStatefulSets) require.NoError(t, err) require.Equal(t, tt.wantRequeue, requeue) require.Equal(t, tt.wantCall, clientMock.called) + var retrievedES esv1.Elasticsearch + err = tt.c.Get(k8s.ExtractNamespacedName(tt.es), &retrievedES) + require.NoError(t, err) + require.Equal(t, tt.wantVotingConfigAnnotation, retrievedES.Annotations[VotingConfigExclusionsAnnotationName]) }) } } + +func TestAddToVotingConfigExclusions(t *testing.T) { + es := esv1.Elasticsearch{ObjectMeta: metav1.ObjectMeta{Name: "es", Namespace: "ns"}} + masterPod := sset.TestPod{ + Namespace: "ns", + Name: "pod-name", + ClusterName: "es", + Version: "7.2.0", + Master: true, + }.BuildPtr() + tests := []struct { + name string + es *esv1.Elasticsearch + c k8s.Client + excludeNodes []string + wantAPICalled bool + wantAPICalledWith []string + wantVotingConfigAnnotation string + }{ + { + name: "some zen1 masters: do nothing", + es: &es, + c: k8s.WrappedFakeClient(&es, sset.TestPod{ + Namespace: "ns", + Name: "pod-name", + ClusterName: "es", + Version: "6.8.0", + Master: true, + }.BuildPtr()), + excludeNodes: []string{"node1"}, + wantAPICalled: false, + }, + { + name: "setting already applied based on annotation: do nothing", + es: withVotingConfigAnnotation(es, "node1,node2"), + c: k8s.WrappedFakeClient(withVotingConfigAnnotation(es, "node1,node2")), + excludeNodes: []string{"node1", "node2"}, + wantAPICalled: false, + wantVotingConfigAnnotation: "node1,node2", + }, + { + name: "no annotation: set voting config exclusions", + es: &es, + c: k8s.WrappedFakeClient(&es, masterPod), + excludeNodes: []string{"node1", "node2"}, + wantAPICalled: true, + wantAPICalledWith: []string{"node1", "node2"}, + wantVotingConfigAnnotation: "node1,node2", + }, + { + name: "empty annotation: set voting config exclusions", + es: withVotingConfigAnnotation(es, ""), + c: k8s.WrappedFakeClient(withVotingConfigAnnotation(es, ""), masterPod), + excludeNodes: []string{"node1", "node2"}, + wantAPICalled: true, + wantAPICalledWith: []string{"node1", "node2"}, + wantVotingConfigAnnotation: "node1,node2", + }, + { + name: "annotation mismatch: set voting config exclusions", + es: withVotingConfigAnnotation(es, "node1"), + c: k8s.WrappedFakeClient(withVotingConfigAnnotation(es, "node1"), masterPod), + excludeNodes: []string{"node1", "node2"}, + wantAPICalled: true, + wantAPICalledWith: []string{"node1", "node2"}, + wantVotingConfigAnnotation: "node1,node2", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clientMock := &fakeVotingConfigExclusionsESClient{} + err := AddToVotingConfigExclusions(context.Background(), tt.c, clientMock, *tt.es, tt.excludeNodes) + require.NoError(t, err) + require.Equal(t, tt.wantAPICalled, clientMock.called) + require.Equal(t, tt.wantAPICalledWith, clientMock.excludedNodes) + var retrievedES esv1.Elasticsearch + err = tt.c.Get(k8s.ExtractNamespacedName(tt.es), &retrievedES) + require.NoError(t, err) + require.Equal(t, tt.wantVotingConfigAnnotation, retrievedES.Annotations[VotingConfigExclusionsAnnotationName]) + }) + } +} + +func Test_serializeExcludedNodesForAnnotation1(t *testing.T) { + nodes := []string{"nodeA", "nodeC", "nodeB"} + // should be sorted alphabetically in a single comma-separated string + require.Equal(t, "nodeA,nodeB,nodeC", serializeExcludedNodesForAnnotation(nodes)) + // initial slice should not be mutated + require.Equal(t, []string{"nodeA", "nodeC", "nodeB"}, nodes) +}