Remove one master at a time upscale restriction

[pebrc]

Fixes #8939

#8939 describes how requests to Elasticsearch are made during upscales. When scaling an overloaded ES cluster this can be problematic as the cluster won't be able to serve these requests in time.

This change removes the source of those requests: the on master at a time scale up restriction. This functionality stems for a time when the ECK operator had to manually reconfigure the quorum during scale up (Elasticsearch 6 and earlier). We no longer support these early versions where a quick scale up could lead to potential split brain scenarios if a partition was formed during scale up.

By removing the one-at-at-time invariant the call to the _nodes API during scale up becomes unnecessary. The call is now only made when calculating the scale downs.

[prodsecmachine]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[pebrc]

buildkite test this -f p=kind,t="TestMutation.*"

[Copilot]

Pull request overview

This PR removes the restriction that limited master node upscaling to one node at a time, which was necessary for Elasticsearch 6 and earlier versions to prevent split-brain scenarios during quorum reconfiguration. Since ECK no longer supports these early versions, this restriction is unnecessary and causes problems when scaling overloaded clusters, as it requires calls to the Elasticsearch _nodes API that may fail.

Key Changes:

• Removed one-at-a-time master node creation logic and associated ES API calls during upscale
• Exported InitialMasterNodesAnnotation constant for broader use
• Added test coverage for upscale scenarios with unavailable Elasticsearch API

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
pkg/controller/elasticsearch/driver/upscale.go	Removed comment about limiting master node creation to one at a time
pkg/controller/elasticsearch/driver/upscale_state.go	Removed isBootstrapped, allowMasterCreation fields and related logic including isMasterNodeJoining, recordMasterNodeCreation, canCreateMasterNode, and limitMasterNodesCreation functions
pkg/controller/elasticsearch/driver/upscale_state_test.go	Updated tests to remove bootstrap and master-specific logic, simplified test cases
pkg/controller/elasticsearch/driver/upscale_test.go	Updated test expectations to allow multiple master nodes to be created at once, added test for upscale with unavailable ES API
pkg/controller/elasticsearch/version/zen2/initial_master_nodes.go	Changed initialMasterNodesAnnotation from private to exported InitialMasterNodesAnnotation
pkg/controller/elasticsearch/version/zen2/initial_master_nodes_test.go	Updated references to use exported constant name
test/e2e/test/elasticsearch/checks_budget.go	Removed NewMasterChangeBudgetWatcher function and unused math import
test/e2e/test/elasticsearch/steps_mutation.go	Removed NewMasterChangeBudgetWatcher from mutation test watchers
pkg/controller/common/expectations/expectations.go	Removed comment about preventing creation of more than one master node at a time

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pebrc]

buildkite test this -f p=gke,t=TestMutation

[barkbay]

I'm trying to understand in which cases we still need this. Could we just remove it?

diff --git a/pkg/controller/elasticsearch/driver/nodes.go b/pkg/controller/elasticsearch/driver/nodes.go
index 65199fc48..7beb9f0e4 100644
--- a/pkg/controller/elasticsearch/driver/nodes.go
+++ b/pkg/controller/elasticsearch/driver/nodes.go
@@ -96,13 +96,11 @@ func (d *defaultDriver) reconcileNodeSpecs(
                }
        }
 
-       esState := NewMemoizingESState(ctx, esClient)
        // Phase 1: apply expected StatefulSets resources and scale up.
        upscaleCtx := upscaleCtx{
                parentCtx:            ctx,
                k8sClient:            d.K8sClient(),
                es:                   d.ES,
-               esState:              esState,
                expectations:         d.Expectations,
                validateStorageClass: d.OperatorParameters.ValidateStorageClass,
                upscaleReporter:      reconcileState.UpscaleReporter,
@@ -188,6 +186,7 @@ func (d *defaultDriver) reconcileNodeSpecs(
                results.WithReconciliationState(defaultRequeue.WithReason("Cannot clear voting exclusions yet"))
        }
        // shutdown logic is dependent on Elasticsearch version
+       esState := NewMemoizingESState(ctx, esClient)
        nodeShutdowns, err := newShutdownInterface(ctx, d.ES, esClient, esState, reconcileState.StatusReporter)
        if err != nil {
                return results.WithError(err)
diff --git a/pkg/controller/elasticsearch/driver/upscale.go b/pkg/controller/elasticsearch/driver/upscale.go
index bf2047aaf..df7ee75cc 100644
--- a/pkg/controller/elasticsearch/driver/upscale.go
+++ b/pkg/controller/elasticsearch/driver/upscale.go
@@ -25,10 +25,10 @@ import (
 )
 
 type upscaleCtx struct {
-       parentCtx            context.Context
-       k8sClient            k8s.Client
-       es                   esv1.Elasticsearch
-       esState              ESState
+       parentCtx context.Context
+       k8sClient k8s.Client
+       es        esv1.Elasticsearch
+
        expectations         *expectations.Expectations
        validateStorageClass bool
        upscaleReporter      *reconcile.UpscaleReporter

[pebrc]

Addressed in 42ee6eb I think we still want ESState for the predicates and the shutdown handling but it is no longer required for the upscale and spec changes part.

[barkbay]

LGTM 👍