Don't set intial_master_nodes if already bootstrapped

This is a port of elastic#1272 in
the new code structure.

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

@@ -295,7 +295,7 @@ func (d *defaultDriver) reconcileNodeSpecs(
 	if err := zen1.SetupMinimumMasterNodesConfig(nodeSpecResources); err != nil {
 		return results.WithError(err)
 	}
-	if err := zen2.SetupInitialMasterNodes(nodeSpecResources); err != nil {
+	if err := zen2.SetupInitialMasterNodes(es, observedState, d.Client, nodeSpecResources); err != nil {
 		return results.WithError(err)
 	}
 

operators/pkg/controller/elasticsearch/version/zen2/initial_master_nodes.go

@@ -5,17 +5,66 @@
 package zen2
 
 import (
+	"github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1"
 	"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/settings"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s"
 )
 
+const (
+	// ClusterUUIDAnnotationName used to store the cluster UUID as an annotation when cluster has been bootstrapped.
+	ClusterUUIDAnnotationName = "elasticsearch.k8s.elastic.co/cluster-uuid"
+)
+
+// annotatedForBootstrap returns true if the cluster has been annotated with the UUID already.
+func annotatedForBootstrap(cluster v1alpha1.Elasticsearch) bool {
+	_, bootstrapped := cluster.Annotations[ClusterUUIDAnnotationName]
+	return bootstrapped
+}
+
+// clusterIsBootstrapped returns true if the cluster has formed and has a UUID.
+func clusterIsBootstrapped(observedState observer.State) bool {
+	return observedState.ClusterState != nil && len(observedState.ClusterState.ClusterUUID) > 0
+}
+
+// annotateWithUUID annotates the cluster with its UUID, to mark it as "bootstrapped".
+func annotateWithUUID(cluster v1alpha1.Elasticsearch, observedState observer.State, c k8s.Client) error {
+	log.Info("Annotating bootstrapped cluster with its UUID", "namespace", cluster.Namespace, "es_name", cluster.Name)
+	if cluster.Annotations == nil {
+		cluster.Annotations = make(map[string]string)
+	}
+	cluster.Annotations[ClusterUUIDAnnotationName] = observedState.ClusterState.ClusterUUID
+	if err := c.Update(&cluster); err != nil {
+		return err
+	}
+	return nil
+}
+
 // SetupInitialMasterNodes modifies the ES config of the given resources to setup
 // cluster initial master nodes.
-func SetupInitialMasterNodes(nodeSpecResources nodespec.ResourcesList) error {
-	// TODO: handle zen2 initial master nodes more cleanly
-	//  should be empty once cluster is bootstraped
-	// TODO: see https://github.com/elastic/cloud-on-k8s/issues/1201 to rely on an annotation
-	//  set in the cluster
+// It also saves the cluster UUID as an annotation to ensure that it's not set
+// if the cluster has already been bootstrapped.
+func SetupInitialMasterNodes(
+	cluster v1alpha1.Elasticsearch,
+	observedState observer.State,
+	c k8s.Client,
+	nodeSpecResources nodespec.ResourcesList,
+) error {
+	if annotatedForBootstrap(cluster) {
+		// Cluster already bootstrapped, nothing to do.
+		return nil
+	}
+
+	if clusterIsBootstrapped(observedState) {
+		// Cluster is not annotated for bootstrap, but should be.
+		if err := annotateWithUUID(cluster, observedState, c); err != nil {
+			return err
+		}
+		return nil
+	}
+
+	// Cluster is not bootstrapped yet, set initial_master_nodes setting in each node config.
 	masters := nodeSpecResources.MasterNodesNames()
 	if len(masters) == 0 {
 		return nil

operators/pkg/controller/elasticsearch/version/zen2/initial_master_nodes_test.go

@@ -7,14 +7,110 @@ package zen2
 import (
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/kubernetes/scheme"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
+	"github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1"
 	settings2 "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/settings"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/controller/elasticsearch/client"
 	"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/settings"
+	"github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s"
 )
 
-func TestSetupInitialMasterNodes(t *testing.T) {
+const (
+	defaultClusterUUID = "jiMyMA1hQ-WMPK3vEStZuw"
+)
+
+func setupScheme(t *testing.T) *runtime.Scheme {
+	sc := scheme.Scheme
+	if err := v1alpha1.SchemeBuilder.AddToScheme(sc); err != nil {
+		assert.Fail(t, "failed to add Es types")
+	}
+	return sc
+}
+
+var esNN = types.NamespacedName{
+	Namespace: "ns1",
+	Name:      "foo",
+}
+
+func newElasticsearch() v1alpha1.Elasticsearch {
+	return v1alpha1.Elasticsearch{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: esNN.Namespace,
+			Name:      esNN.Name,
+		},
+	}
+}
+
+func withAnnotation(es v1alpha1.Elasticsearch, name, value string) v1alpha1.Elasticsearch {
+	if es.Annotations == nil {
+		es.Annotations = make(map[string]string)
+	}
+	es.Annotations[name] = value
+	return es
+}
+
+func TestSetupInitialMasterNodes_AlreadyBootstrapped(t *testing.T) {
+	s := setupScheme(t)
+	tests := []struct {
+		name              string
+		es                v1alpha1.Elasticsearch
+		observedState     observer.State
+		nodeSpecResources nodespec.ResourcesList
+		expected          []settings.CanonicalConfig
+		expectedEs        v1alpha1.Elasticsearch
+	}{
+		{
+			name: "cluster already annotated for bootstrap: no changes",
+			es:   withAnnotation(newElasticsearch(), ClusterUUIDAnnotationName, defaultClusterUUID),
+			nodeSpecResources: nodespec.ResourcesList{
+				{StatefulSet: createStatefulSet("data", "7.1.0", 3, false, true), Config: settings.NewCanonicalConfig()},
+			},
+			expected:   []settings.CanonicalConfig{settings.NewCanonicalConfig()},
+			expectedEs: withAnnotation(newElasticsearch(), ClusterUUIDAnnotationName, defaultClusterUUID),
+		},
+		{
+			name:          "cluster bootstrapped but not annotated: should be annotated",
+			es:            newElasticsearch(),
+			observedState: observer.State{ClusterState: &client.ClusterState{ClusterUUID: defaultClusterUUID}},
+			nodeSpecResources: nodespec.ResourcesList{
+				{StatefulSet: createStatefulSet("data", "7.1.0", 3, false, true), Config: settings.NewCanonicalConfig()},
+			},
+			expected:   []settings.CanonicalConfig{settings.NewCanonicalConfig()},
+			expectedEs: withAnnotation(newElasticsearch(), ClusterUUIDAnnotationName, defaultClusterUUID),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			client := k8s.WrapClient(fake.NewFakeClientWithScheme(s, &tt.es))
+			err := SetupInitialMasterNodes(tt.es, tt.observedState, client, tt.nodeSpecResources)
+			require.NoError(t, err)
+			// check if the ES resource was annotated
+			var es v1alpha1.Elasticsearch
+			err = client.Get(esNN, &es)
+			assert.NoError(t, err)
+			require.Equal(t, tt.expectedEs, es)
+			// check if nodespec config were modified
+			for i := 0; i < len(tt.nodeSpecResources); i++ {
+				expected, err := tt.expected[i].Render()
+				require.NoError(t, err)
+				actual, err := tt.nodeSpecResources[i].Config.Render()
+				require.NoError(t, err)
+				require.Equal(t, expected, actual)
+			}
+		})
+	}
+}
+
+func TestSetupInitialMasterNodes_NotBootstrapped(t *testing.T) {
 	tests := []struct {
 		name              string
 		nodeSpecResources nodespec.ResourcesList
@@ -69,7 +165,7 @@ func TestSetupInitialMasterNodes(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			err := SetupInitialMasterNodes(tt.nodeSpecResources)
+			err := SetupInitialMasterNodes(v1alpha1.Elasticsearch{}, observer.State{}, k8s.WrapClient(fake.NewFakeClient()), tt.nodeSpecResources)
 			require.NoError(t, err)
 			for i := 0; i < len(tt.nodeSpecResources); i++ {
 				expected, err := tt.expected[i].Render()