Handle zen1 minimum_master_nodes special case for 2->1 masters

operators/pkg/controller/elasticsearch/driver/downscale.go

@@ -5,6 +5,9 @@
 package driver
 
 import (
+	v1 "k8s.io/api/core/v1"
+
+	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/events"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/reconciler"
 	esclient "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/client"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label"
@@ -205,17 +208,44 @@ func updateZenSettingsForDownscale(ctx downscaleContext, downscale ssetDownscale
 		return nil
 	}
 
-	// TODO: only update in case 2->1 masters.
-	// Update Zen1 minimum master nodes API, accounting for the updated downscaled replicas.
-	_, err := zen1.UpdateMinimumMasterNodes(ctx.k8sClient, ctx.es, ctx.esClient, actualStatefulSets, ctx.reconcileState)
-	if err != nil {
+	// Maybe update zen1 minimum_master_nodes.
+	if err := maybeUpdateZen1ForDownscale(ctx, actualStatefulSets); err != nil {
 		return err
 	}
 
-	// Update zen2 settings to exclude leaving master nodes from voting.
+	// Maybe update zen2 settings to exclude leaving master nodes from voting.
 	if err := zen2.AddToVotingConfigExclusions(ctx.esClient, downscale.statefulSet, downscale.leavingNodeNames()); err != nil {
 		return err
 	}
 
 	return nil
 }
+
+// maybeUpdateZen1ForDownscale updates zen1 minimum master nodes if we are downscaling from 2 to 1 master node.
+func maybeUpdateZen1ForDownscale(ctx downscaleContext, actualStatefulSets sset.StatefulSetList) error {
+	if !zen1.AtLeastOneNodeCompatibleWithZen1(actualStatefulSets) {
+		return nil
+	}
+
+	actualPods, err := sset.GetActualPodsForCluster(ctx.k8sClient, ctx.es)
+	if err != nil {
+		return err
+	}
+	masters := label.FilterMasterNodePods(actualPods)
+	if len(masters) != 2 {
+		// not in the 2->1 situation
+		return nil
+	}
+
+	// We are moving from 2 to 1 master nodes, we need to update minimum_master_nodes before removing
+	// the 2nd node, otherwise the cluster won't be able to form anymore.
+	// This is inherently unsafe (can cause split brains), but there's no alternative.
+	// For other situations (eg. 3 -> 2), it's fine to update minimum_master_nodes after the node is removed
+	// (will be done at next reconciliation, before nodes removal).
+	ctx.reconcileState.AddEvent(
+		v1.EventTypeWarning, events.EventReasonUnhealthy,
+		"Downscaling from 2 to 1 master nodes: unsafe operation",
+	)
+	minimumMasterNodes := 1
+	return zen1.UpdateMinimumMasterNodesTo(ctx.es, ctx.esClient, actualStatefulSets, ctx.reconcileState, minimumMasterNodes)
+}

operators/pkg/controller/elasticsearch/driver/downscale_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/reconciler"
 	esclient "github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/client"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/label"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/nodespec"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/observer"
 	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/reconcile"
@@ -30,7 +31,7 @@ import (
 
 // Sample StatefulSets to use in tests
 var (
-	ssetMaster3Replicas     = nodespec.CreateTestSset("ssetMaster3Replicas", "7.2.0", 3, true, false)
+	ssetMaster3Replicas     = nodespec.TestSset{Name: "ssetMaster3Replicas", Version: "7.2.0", Replicas: 3, Master: true, Data: false}.Build()
 	podsSsetMaster3Replicas = []corev1.Pod{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -51,7 +52,7 @@ var (
 			},
 		},
 	}
-	ssetData4Replicas     = nodespec.CreateTestSset("ssetData4Replicas", "7.2.0", 4, false, true)
+	ssetData4Replicas     = nodespec.TestSset{Name: "ssetData4Replicas", Version: "7.2.0", Replicas: 4, Master: false, Data: true}.Build()
 	podsSsetData4Replicas = []corev1.Pod{
 		{
 			ObjectMeta: metav1.ObjectMeta{
@@ -591,62 +592,62 @@ func Test_attemptDownscale(t *testing.T) {
 		{
 			name: "1 statefulset should be removed",
 			downscale: ssetDownscale{
-				statefulSet:     nodespec.CreateTestSset("should-be-removed", "7.1.0", 0, true, true),
+				statefulSet:     nodespec.TestSset{Name: "should-be-removed", Version: "7.1.0", Replicas: 0, Master: true, Data: true}.Build(),
 				initialReplicas: 0,
 				targetReplicas:  0,
 			},
 			state: &downscaleState{runningMasters: 2, masterRemovalInProgress: false},
 			statefulSets: sset.StatefulSetList{
-				nodespec.CreateTestSset("should-be-removed", "7.1.0", 0, true, true),
-				nodespec.CreateTestSset("should-stay", "7.1.0", 2, true, true),
+				nodespec.TestSset{Name: "should-be-removed", Version: "7.1.0", Replicas: 0, Master: true, Data: true}.Build(),
+				nodespec.TestSset{Name: "should-stay", Version: "7.1.0", Replicas: 2, Master: true, Data: true}.Build(),
 			},
 			expectedStatefulSets: []appsv1.StatefulSet{
-				nodespec.CreateTestSset("should-stay", "7.1.0", 2, true, true),
+				nodespec.TestSset{Name: "should-stay", Version: "7.1.0", Replicas: 2, Master: true, Data: true}.Build(),
 			},
 		},
 		{
 			name: "target replicas == initial replicas",
 			downscale: ssetDownscale{
-				statefulSet:     nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				statefulSet:     nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 				initialReplicas: 3,
 				targetReplicas:  3,
 			},
 			state: &downscaleState{runningMasters: 2, masterRemovalInProgress: false},
 			statefulSets: sset.StatefulSetList{
-				nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 			},
 			expectedStatefulSets: []appsv1.StatefulSet{
-				nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 			},
 		},
 		{
 			name: "upscale case",
 			downscale: ssetDownscale{
-				statefulSet:     nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				statefulSet:     nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 				initialReplicas: 3,
 				targetReplicas:  4,
 			},
 			state: &downscaleState{runningMasters: 2, masterRemovalInProgress: false},
 			statefulSets: sset.StatefulSetList{
-				nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 			},
 			expectedStatefulSets: []appsv1.StatefulSet{
-				nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 			},
 		},
 		{
 			name: "perform 3 -> 2 downscale",
 			downscale: ssetDownscale{
-				statefulSet:     nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				statefulSet:     nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 				initialReplicas: 3,
 				targetReplicas:  2,
 			},
 			state: &downscaleState{runningMasters: 2, masterRemovalInProgress: false},
 			statefulSets: sset.StatefulSetList{
-				nodespec.CreateTestSset("default", "7.1.0", 3, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 3, Master: true, Data: true}.Build(),
 			},
 			expectedStatefulSets: []appsv1.StatefulSet{
-				nodespec.CreateTestSset("default", "7.1.0", 2, true, true),
+				nodespec.TestSset{Name: "default", Version: "7.1.0", Replicas: 2, Master: true, Data: true}.Build(),
 			},
 		},
 	}
@@ -726,8 +727,8 @@ func Test_doDownscale_updateReplicasAndExpectations(t *testing.T) {
 }
 
 func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) {
-	ssetMasters := nodespec.CreateTestSset("masters", "7.1.0", 3, true, false)
-	ssetData := nodespec.CreateTestSset("datas", "7.1.0", 3, false, true)
+	ssetMasters := nodespec.TestSset{Name: "masters", Version: "7.1.0", Replicas: 3, Master: true, Data: false}.Build()
+	ssetData := nodespec.TestSset{Name: "datas", Version: "7.1.0", Replicas: 3, Master: false, Data: true}.Build()
 	tests := []struct {
 		name               string
 		downscale          ssetDownscale
@@ -771,11 +772,113 @@ func Test_doDownscale_zen2VotingConfigExclusions(t *testing.T) {
 			// check call to zen2 is the expected one
 			require.Equal(t, tt.wantZen2Called, esClient.AddVotingConfigExclusionsCalled)
 			require.Equal(t, tt.wantZen2CalledWith, esClient.AddVotingConfigExclusionsCalledWith)
+			// check zen1 was not called
+			require.False(t, esClient.SetMinimumMasterNodesCalled)
 		})
 	}
 }
 
-func Test_doDownscale_callsZen1ForMasterNodes(t *testing.T) {
-	// TODO: implement with https://github.com/elastic/cloud-on-k8s/issues/1281
-	//  to handle the 2->1 masters case
+func Test_doDownscale_zen1MinimumMasterNodes(t *testing.T) {
+	es := v1alpha1.Elasticsearch{ObjectMeta: metav1.ObjectMeta{Namespace: ssetMaster3Replicas.Namespace, Name: "es"}}
+	ssetMasters := nodespec.TestSset{Name: "masters", Version: "6.8.0", Replicas: 3, Master: true, Data: false}.Build()
+	masterPods := []corev1.Pod{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ssetMaster3Replicas.Namespace,
+				Name:      ssetMaster3Replicas.Name + "-0",
+				Labels: map[string]string{
+					label.StatefulSetNameLabelName:         ssetMaster3Replicas.Name,
+					label.ClusterNameLabelName:             es.Name,
+					string(label.NodeTypesMasterLabelName): "true",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ssetMaster3Replicas.Namespace,
+				Name:      ssetMaster3Replicas.Name + "-1",
+				Labels: map[string]string{
+					label.StatefulSetNameLabelName:         ssetMaster3Replicas.Name,
+					label.ClusterNameLabelName:             es.Name,
+					string(label.NodeTypesMasterLabelName): "true",
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: ssetMaster3Replicas.Namespace,
+				Name:      ssetMaster3Replicas.Name + "-2",
+				Labels: map[string]string{
+					label.StatefulSetNameLabelName:         ssetMaster3Replicas.Name,
+					label.ClusterNameLabelName:             es.Name,
+					string(label.NodeTypesMasterLabelName): "true",
+				},
+			},
+		},
+	}
+	ssetData := nodespec.TestSset{Name: "datas", Version: "6.8.0", Replicas: 3, Master: false, Data: true}.Build()
+	tests := []struct {
+		name               string
+		downscale          ssetDownscale
+		statefulSets       sset.StatefulSetList
+		apiserverResources []runtime.Object
+		wantZen1Called     bool
+		wantZen1CalledWith int
+	}{
+		{
+			name: "3 -> 2 master nodes",
+			downscale: ssetDownscale{
+				statefulSet:     ssetMasters,
+				initialReplicas: 3,
+				targetReplicas:  2,
+			},
+			statefulSets:       sset.StatefulSetList{ssetMasters},
+			apiserverResources: []runtime.Object{&ssetMasters, &masterPods[0], &masterPods[1], &masterPods[2]},
+			wantZen1Called:     false,
+		},
+		{
+			name: "3 -> 2 data nodes",
+			downscale: ssetDownscale{
+				statefulSet:     ssetData,
+				initialReplicas: 3,
+				targetReplicas:  2,
+			},
+			statefulSets:       sset.StatefulSetList{ssetMasters, ssetData},
+			apiserverResources: []runtime.Object{&ssetMasters, &ssetData, &masterPods[0], &masterPods[1], &masterPods[2]},
+			wantZen1Called:     false,
+		},
+		{
+			name: "2 -> 1 master nodes",
+			downscale: ssetDownscale{
+				statefulSet:     ssetMasters,
+				initialReplicas: 2,
+				targetReplicas:  1,
+			},
+			statefulSets: sset.StatefulSetList{ssetMasters},
+			// 2 master nodes in the apiserver
+			apiserverResources: []runtime.Object{&ssetMasters, &masterPods[0], &masterPods[1]},
+			wantZen1Called:     true,
+			wantZen1CalledWith: 1,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			k8sClient := k8s.WrapClient(fake.NewFakeClient(tt.apiserverResources...))
+			esClient := &fakeESClient{}
+			downscaleCtx := downscaleContext{
+				k8sClient:      k8sClient,
+				expectations:   reconciler.NewExpectations(),
+				reconcileState: reconcile.NewState(v1alpha1.Elasticsearch{}),
+				esClient:       esClient,
+			}
+			// do the downscale
+			err := doDownscale(downscaleCtx, tt.downscale, tt.statefulSets)
+			require.NoError(t, err)
+			// check call to zen1 is the expected one
+			require.Equal(t, tt.wantZen1Called, esClient.SetMinimumMasterNodesCalled)
+			require.Equal(t, tt.wantZen1CalledWith, esClient.SetMinimumMasterNodesCalledWith)
+			// check zen2 was not called
+			require.False(t, esClient.AddVotingConfigExclusionsCalled)
+		})
+	}
 }

operators/pkg/controller/elasticsearch/nodespec/fixtures.go

@@ -11,18 +11,28 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func CreateTestSset(name string, esversion string, replicas int32, master bool, data bool) appsv1.StatefulSet {
+type TestSset struct {
+	Name        string
+	ClusterName string
+	Version     string
+	Replicas    int32
+	Master      bool
+	Data        bool
+}
+
+func (t TestSset) Build() appsv1.StatefulSet {
 	labels := map[string]string{
-		label.VersionLabelName: esversion,
+		label.VersionLabelName:     t.Version,
+		label.ClusterNameLabelName: t.ClusterName,
 	}
-	label.NodeTypesMasterLabelName.Set(master, labels)
-	label.NodeTypesDataLabelName.Set(data, labels)
+	label.NodeTypesMasterLabelName.Set(t.Master, labels)
+	label.NodeTypesDataLabelName.Set(t.Data, labels)
 	return appsv1.StatefulSet{
 		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
+			Name: t.Name,
 		},
 		Spec: appsv1.StatefulSetSpec{
-			Replicas: &replicas,
+			Replicas: &t.Replicas,
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: labels,

operators/pkg/controller/elasticsearch/nodespec/resources_test.go

@@ -23,9 +23,9 @@ func TestResourcesList_MasterNodesNames(t *testing.T) {
 		{
 			name: "3 master-only nodes, 3 master-data nodes, 3 data nodes",
 			l: ResourcesList{
-				{StatefulSet: CreateTestSset("master-only", "7.2.0", 3, true, false)},
-				{StatefulSet: CreateTestSset("master-data", "7.2.0", 3, true, true)},
-				{StatefulSet: CreateTestSset("data-only", "7.2.0", 3, false, true)},
+				{StatefulSet: TestSset{Name: "master-only", Version: "7.2.0", Replicas: 3, Master: true, Data: false}.Build()},
+				{StatefulSet: TestSset{Name: "master-data", Version: "7.2.0", Replicas: 3, Master: true, Data: true}.Build()},
+				{StatefulSet: TestSset{Name: "data-only", Version: "7.2.0", Replicas: 3, Master: false, Data: true}.Build()},
 			},
 			want: []string{
 				"master-only-0", "master-only-1", "master-only-2",