Fix incorrectly modelled node shutdown API in Elasticsearch client (#5308) (#5311)

Co-authored-by: Michael Morello <[email protected]>

Author: pebrc

pkg/controller/elasticsearch/client/model.go

@@ -356,7 +356,7 @@ type LicenseResponse struct {
 	License License `json:"license"`
 }
 
-// StartTrialResponse is the response to the start trial API call.
+// StartBasicResponse is the response to the start trial API call.
 type StartBasicResponse struct {
 	Acknowledged    bool   `json:"acknowledged"`
 	BasicWasStarted bool   `json:"basic_was_started"`
@@ -378,7 +378,7 @@ type RemoteClusters struct {
 	RemoteClusters map[string]RemoteCluster `json:"remote,omitempty"`
 }
 
-// RemoteClusterSeeds is the set of seeds to use in a remote cluster setting.
+// RemoteCluster is the set of seeds to use in a remote cluster setting.
 type RemoteCluster struct {
 	Seeds []string `json:"seeds"`
 }
@@ -421,8 +421,8 @@ var (
 type ShutdownStatus string
 
 var (
-	// ShutdownStarted means a shutdown request has been accepted and is being processed in Elasticsearch.
-	ShutdownStarted ShutdownStatus = "STARTED"
+	// ShutdownInProgress means a shutdown request has been accepted and is being processed in Elasticsearch.
+	ShutdownInProgress ShutdownStatus = "IN_PROGRESS"
 	// ShutdownComplete means a shutdown request has been processed and the node can be either restarted or taken out
 	// of the cluster by an orchestrator.
 	ShutdownComplete ShutdownStatus = "COMPLETE"
@@ -435,9 +435,9 @@ var (
 
 // ShardMigration is the status of shards that are being migrated away from a node that goes through a shutdown.
 type ShardMigration struct {
-	Status          ShutdownStatus `json:"status"`
-	ShardsRemaining int            `json:"shards_remaining"`
-	Explanation     string         `json:"explanation"`
+	Status                   ShutdownStatus `json:"status"`
+	ShardMigrationsRemaining int            `json:"shard_migrations_remaining"`
+	Explanation              string         `json:"explanation"`
 }
 
 // PersistentTasks expresses the status of preparing ongoing persistent tasks for a node shutdown.
@@ -455,7 +455,7 @@ type NodeShutdown struct {
 	NodeID                string          `json:"node_id"`
 	Type                  string          `json:"type"`
 	Reason                string          `json:"reason"`
-	ShutdownStartedMillis int             `json:"shutdown_started_millis"`
+	ShutdownStartedMillis int             `json:"shutdown_startedmillis"` // missing _ is a serialization inconsistency in Elasticsearch
 	Status                ShutdownStatus  `json:"status"`
 	ShardMigration        ShardMigration  `json:"shard_migration"`
 	PersistentTasks       PersistentTasks `json:"persistent_tasks"`

pkg/controller/elasticsearch/client/model_test.go

@@ -161,3 +161,50 @@ func TestLicenseUpdateResponse_IsSuccess(t *testing.T) {
 		})
 	}
 }
+
+func TestShutdownResponse(t *testing.T) {
+	nodeShudownSample := `{
+	"nodes": [{
+		"node_id": "PQKHA4xCQd2xErO2fUK-hg",
+		"type": "REMOVE",
+		"reason": "2331481",
+		"shutdown_startedmillis": 1643648932189,
+		"status": "IN_PROGRESS",
+		"shard_migration": {
+			"status": "IN_PROGRESS",
+			"shard_migrations_remaining": 1
+		},
+		"persistent_tasks": {
+			"status": "COMPLETE"
+		},
+		"plugins": {
+			"status": "COMPLETE"
+		}
+	}]
+}`
+	expected := ShutdownResponse{Nodes: []NodeShutdown{
+		{
+			NodeID:                "PQKHA4xCQd2xErO2fUK-hg",
+			Type:                  "REMOVE",
+			Reason:                "2331481",
+			ShutdownStartedMillis: 1643648932189,
+			Status:                ShutdownInProgress,
+			ShardMigration: ShardMigration{
+				Status:                   ShutdownInProgress,
+				ShardMigrationsRemaining: 1,
+				Explanation:              "",
+			},
+			PersistentTasks: PersistentTasks{
+				Status: ShutdownComplete,
+			},
+			Plugins: Plugins{
+				Status: ShutdownComplete,
+			},
+		},
+	},
+	}
+
+	var actual ShutdownResponse
+	require.NoError(t, json.Unmarshal([]byte(nodeShudownSample), &actual))
+	require.Equal(t, expected, actual)
+}

pkg/controller/elasticsearch/driver/downscale.go

@@ -203,7 +203,7 @@ func calculatePerformableDownscale(
 			ctx.reconcileState.UpdateElasticsearchShutdownStalled(ctx.resourcesState, ctx.observedState, response.Explanation)
 			// no need to check other nodes since we remove them in order and this one isn't ready anyway
 			return performableDownscale, nil
-		case esclient.ShutdownStarted:
+		case esclient.ShutdownInProgress:
 			ctx.reconcileState.UpdateElasticsearchMigrating(ctx.resourcesState, ctx.observedState)
 			// no need to check other nodes since we remove them in order and this one isn't ready anyway
 			return performableDownscale, nil

pkg/controller/elasticsearch/driver/upgrade_predicates_test.go

@@ -821,7 +821,7 @@ func TestUpgradePodsDeletion_Delete(t *testing.T) {
 					newTestPod("masters-0").withRoles(esv1.MasterRole, esv1.DataRole).isHealthy(true).needsUpgrade(true).isInCluster(true),
 				),
 				shutdowns: map[string]client.NodeShutdown{
-					"masters-0": {Status: client.ShutdownStarted},
+					"masters-0": {Status: client.ShutdownInProgress},
 				},
 				maxUnavailable: 1,
 				shardLister:    migration.NewFakeShardLister(client.Shards{}),

pkg/controller/elasticsearch/migration/migrate_data.go

@@ -48,7 +48,7 @@ func (sm *ShardMigration) ShutdownStatus(ctx context.Context, podName string) (s
 		return shutdown.NodeShutdownStatus{}, err
 	}
 	if migrating {
-		return shutdown.NodeShutdownStatus{Status: esclient.ShutdownStarted}, nil
+		return shutdown.NodeShutdownStatus{Status: esclient.ShutdownInProgress}, nil
 	}
 	return shutdown.NodeShutdownStatus{Status: esclient.ShutdownComplete}, nil
 }

pkg/controller/elasticsearch/shutdown/node.go

@@ -131,7 +131,7 @@ func logStatus(logger logr.Logger, podName string, shutdown esclient.NodeShutdow
 	switch shutdown.Status {
 	case esclient.ShutdownComplete:
 		logger.Info("Node shutdown complete, can start node deletion", "type", shutdown.Type, "node", podName)
-	case esclient.ShutdownStarted:
+	case esclient.ShutdownInProgress:
 		logger.V(1).Info("Node shutdown not over yet, hold off with node deletion", "type", shutdown.Type, "node", podName)
 	case esclient.ShutdownStalled:
 		logger.Info("Node shutdown stalled, user intervention maybe required if condition persists", "type", shutdown.Type, "explanation", shutdown.ShardMigration.Explanation, "node", podName)

pkg/controller/elasticsearch/shutdown/node_test.go

@@ -133,7 +133,7 @@ func TestNodeShutdown_Clear(t *testing.T) {
 			fixture: shutdownFixture,
 			args: args{
 				typ:    esclient.Remove,
-				status: &esclient.ShutdownStarted,
+				status: &esclient.ShutdownInProgress,
 			},
 			wantErr:    false,
 			wantDelete: false,