Don't allow downscales if some shards are unassigned

pkg/controller/elasticsearch/driver/downscale.go

@@ -187,7 +187,7 @@ func calculatePerformableDownscale(
 	}
 	// iterate on all leaving nodes (ordered by highest ordinal first)
 	for _, node := range downscale.leavingNodeNames() {
-		migrating, err := migration.NodeHasShard(ctx.parentCtx, ctx.shardLister, node)
+		migrating, err := migration.NodeMayHaveShard(ctx.parentCtx, ctx.es, ctx.shardLister, node)
 		if err != nil {
 			return performableDownscale, err
 		}

pkg/controller/elasticsearch/migration/migrate_data.go

@@ -15,17 +15,27 @@ import (
 
 var log = logf.Log.WithName("migrate-data")
 
-// NodeHasShard returns true if the given ES Pod is holding at least one shard (primary or replica).
-func NodeHasShard(ctx context.Context, shardLister esclient.ShardLister, podName string) (bool, error) {
+// NodeMayHaveShard returns true if one of those condition is met:
+// - the given ES Pod is holding at least one shard (primary or replica)
+// - some shards in the cluster don't have a node assigned, in which case we can't make sure about the 1st condition
+//   this may happen if the node was just restarted: the shards it is holding appear unassigned
+func NodeMayHaveShard(ctx context.Context, es esv1.Elasticsearch, shardLister esclient.ShardLister, podName string) (bool, error) {
 	shards, err := shardLister.GetShards(ctx)
 	if err != nil {
 		return false, err
 	}
-	// filter shards affected by node removal
 	for _, shard := range shards {
+		// shard still on the node
 		if shard.NodeName == podName {
 			return true, nil
 		}
+		// shard node undefined (likely unassigned)
+		if shard.NodeName == "" {
+			log.Info("Found orphan shard, preventing data migration",
+				"namespace", es.Namespace, "es_name", es.Name,
+				"index", shard.Index, "shard", shard.Shard, "shard_state", shard.State)
+			return true, nil
+		}
 	}
 	return false, nil
 }

pkg/controller/elasticsearch/migration/migrate_data_test.go

@@ -16,7 +16,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestNodeHasShard(t *testing.T) {
+func TestNodeMayHaveShard(t *testing.T) {
 	type args struct {
 		shardLister client.ShardLister
 		podName     string
@@ -61,16 +61,27 @@ func TestNodeHasShard(t *testing.T) {
 			},
 			want: false,
 		},
+		{
+			name: "Some shards have no node assigned",
+			args: args{
+				podName: "A",
+				shardLister: NewFakeShardLister([]client.Shard{
+					{Index: "index-1", Shard: "0", NodeName: ""},
+					{Index: "index-1", Shard: "0", NodeName: "C"},
+				}),
+			},
+			want: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := NodeHasShard(context.Background(), tt.args.shardLister, tt.args.podName)
+			got, err := NodeMayHaveShard(context.Background(), esv1.Elasticsearch{}, tt.args.shardLister, tt.args.podName)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("NodeHasShard() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("NodeMayHaveShard() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if got != tt.want {
-				t.Errorf("NodeHasShard() = %v, want %v", got, tt.want)
+				t.Errorf("NodeMayHaveShard() = %v, want %v", got, tt.want)
 			}
 		})
 	}