Don't return or accept node_version in the Desired Nodes API (#119049)

Re-submission of #114580

>  node_version was deprecated in #104209 (8.13) and shouldn't be set or returned in 9.0

Resolve ES-9443

Author: arteam

qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/DesiredNodesUpgradeIT.java

@@ -82,8 +82,7 @@ private void addClusterNodesToDesiredNodesWithProcessorsOrProcessorRanges(int ve
                         Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeName).build(),
                         randomDoubleProcessorCount(),
                         ByteSizeValue.ofGb(randomIntBetween(10, 24)),
-                        ByteSizeValue.ofGb(randomIntBetween(128, 256)),
-                        null
+                        ByteSizeValue.ofGb(randomIntBetween(128, 256))
                     )
                 )
                 .toList();
@@ -94,8 +93,7 @@ private void addClusterNodesToDesiredNodesWithProcessorsOrProcessorRanges(int ve
                     Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeName).build(),
                     new DesiredNode.ProcessorsRange(minProcessors, minProcessors + randomIntBetween(10, 20)),
                     ByteSizeValue.ofGb(randomIntBetween(10, 24)),
-                    ByteSizeValue.ofGb(randomIntBetween(128, 256)),
-                    null
+                    ByteSizeValue.ofGb(randomIntBetween(128, 256))
                 );
             }).toList();
         }

rest-api-spec/build.gradle

@@ -74,4 +74,12 @@ tasks.named("yamlRestCompatTestTransform").configure ({ task ->
   task.skipTest("index/91_metrics_no_subobjects/Metrics object indexing with synthetic source", "_source.mode mapping attribute is no-op since 9.0.0")
   task.skipTest("index/91_metrics_no_subobjects/Root without subobjects with synthetic source", "_source.mode mapping attribute is no-op since 9.0.0")
   task.skipTest("indices.create/20_synthetic_source/synthetic_source with copy_to inside nested object", "temporary until backported")
+  task.skipTest(
+    "cluster.desired_nodes/10_basic/Test delete desired nodes with node_version generates a warning",
+    "node_version warning is removed in 9.0"
+  )
+  task.skipTest(
+    "cluster.desired_nodes/10_basic/Test update desired nodes with node_version generates a warning",
+    "node_version warning is removed in 9.0"
+  )
 })

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.desired_nodes/10_basic.yml

@@ -59,61 +59,6 @@ teardown:
   - contains: { nodes: { settings: { node: { name: "instance-000187" } }, processors: 8.5, memory: "64gb", storage: "128gb" } }
   - contains: { nodes: { settings: { node: { name: "instance-000188" } }, processors: 16.0, memory: "128gb", storage: "1tb" } }
 ---
-"Test update desired nodes with node_version generates a warning":
-  - skip:
-      reason: "contains is a newly added assertion"
-      features: ["contains", "allowed_warnings"]
-  - do:
-      cluster.state: {}
-
-  # Get master node id
-  - set: { master_node: master }
-
-  - do:
-      nodes.info: {}
-  - set: { nodes.$master.version: es_version }
-
-  - do:
-      _internal.update_desired_nodes:
-        history_id: "test"
-        version: 1
-        body:
-            nodes:
-              - { settings: { "node.name": "instance-000187" }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
-      allowed_warnings:
-        - "[version removal] Specifying node_version in desired nodes requests is deprecated."
-  - match: { replaced_existing_history_id: false }
-
-  - do:
-      _internal.get_desired_nodes: {}
-  - match:
-      $body:
-        history_id: "test"
-        version: 1
-        nodes:
-          - { settings: { node: { name: "instance-000187" } }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
-
-  - do:
-      _internal.update_desired_nodes:
-        history_id: "test"
-        version: 2
-        body:
-          nodes:
-            - { settings: { "node.name": "instance-000187" }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version }
-            - { settings: { "node.name": "instance-000188" }, processors: 16.0, memory: "128gb", storage: "1tb", node_version: $es_version }
-      allowed_warnings:
-        - "[version removal] Specifying node_version in desired nodes requests is deprecated."
-  - match: { replaced_existing_history_id: false }
-
-  - do:
-      _internal.get_desired_nodes: {}
-
-  - match: { history_id: "test" }
-  - match: { version: 2 }
-  - length: { nodes: 2 }
-  - contains: { nodes: { settings: { node: { name: "instance-000187" } }, processors: 8.5, memory: "64gb", storage: "128gb", node_version: $es_version } }
-  - contains: { nodes: { settings: { node: { name: "instance-000188" } }, processors: 16.0, memory: "128gb", storage: "1tb", node_version: $es_version } }
----
 "Test update move to a new history id":
   - skip:
       reason: "contains is a newly added assertion"
@@ -199,46 +144,6 @@ teardown:
       _internal.get_desired_nodes: {}
   - match: { status: 404 }
 ---
-"Test delete desired nodes with node_version generates a warning":
-  - skip:
-      features: allowed_warnings
-  - do:
-      cluster.state: {}
-
-  - set: { master_node: master }
-
-  - do:
-      nodes.info: {}
-  - set: { nodes.$master.version: es_version }
-
-  - do:
-      _internal.update_desired_nodes:
-        history_id: "test"
-        version: 1
-        body:
-          nodes:
-            - { settings: { "node.external_id": "instance-000187" }, processors: 8.0, memory: "64gb", storage: "128gb", node_version: $es_version }
-      allowed_warnings:
-        - "[version removal] Specifying node_version in desired nodes requests is deprecated."
-  - match: { replaced_existing_history_id: false }
-
-  - do:
-      _internal.get_desired_nodes: {}
-  - match:
-      $body:
-        history_id: "test"
-        version: 1
-        nodes:
-          - { settings: { node: { external_id: "instance-000187" } }, processors: 8.0, memory: "64gb", storage: "128gb", node_version: $es_version }
-
-  - do:
-      _internal.delete_desired_nodes: {}
-
-  - do:
-      catch: missing
-      _internal.get_desired_nodes: {}
-  - match: { status: 404 }
----
 "Test update desired nodes is idempotent":
   - skip:
       reason: "contains is a newly added assertion"

server/src/main/java/org/elasticsearch/TransportVersions.java

@@ -178,6 +178,8 @@ static TransportVersion def(int id) {
     public static final TransportVersion REMOVE_SNAPSHOT_FAILURES = def(9_002_0_00);
     public static final TransportVersion TRANSPORT_STATS_HANDLING_TIME_REQUIRED = def(9_003_0_00);
 
+    public static final TransportVersion REMOVE_DESIRED_NODE_VERSION = def(9_004_0_00);
+
     /*
      * STOP! READ THIS FIRST! No, really,
      *        ____ _____ ___  ____  _        ____  _____    _    ____    _____ _   _ ___ ____    _____ ___ ____  ____ _____ _

server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNode.java

@@ -11,18 +11,15 @@
 
 import org.elasticsearch.TransportVersion;
 import org.elasticsearch.TransportVersions;
-import org.elasticsearch.Version;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodeRole;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.ByteSizeValue;
 import org.elasticsearch.common.unit.Processors;
 import org.elasticsearch.core.Nullable;
-import org.elasticsearch.core.UpdateForV9;
 import org.elasticsearch.xcontent.ConstructingObjectParser;
 import org.elasticsearch.xcontent.ObjectParser;
 import org.elasticsearch.xcontent.ParseField;
@@ -36,7 +33,6 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.regex.Pattern;
 
 import static java.lang.String.format;
 import static org.elasticsearch.node.Node.NODE_EXTERNAL_ID_SETTING;
@@ -52,8 +48,6 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
     private static final ParseField PROCESSORS_RANGE_FIELD = new ParseField("processors_range");
     private static final ParseField MEMORY_FIELD = new ParseField("memory");
     private static final ParseField STORAGE_FIELD = new ParseField("storage");
-    @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) // Remove deprecated field
-    private static final ParseField VERSION_FIELD = new ParseField("node_version");
 
     public static final ConstructingObjectParser<DesiredNode, Void> PARSER = new ConstructingObjectParser<>(
         "desired_node",
@@ -63,8 +57,7 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
             (Processors) args[1],
             (ProcessorsRange) args[2],
             (ByteSizeValue) args[3],
-            (ByteSizeValue) args[4],
-            (String) args[5]
+            (ByteSizeValue) args[4]
         )
     );
 
@@ -98,12 +91,6 @@ static <T> void configureParser(ConstructingObjectParser<T, Void> parser) {
             STORAGE_FIELD,
             ObjectParser.ValueType.STRING
         );
-        parser.declareField(
-            ConstructingObjectParser.optionalConstructorArg(),
-            (p, c) -> p.text(),
-            VERSION_FIELD,
-            ObjectParser.ValueType.STRING
-        );
     }
 
     private final Settings settings;
@@ -112,21 +99,9 @@ static <T> void configureParser(ConstructingObjectParser<T, Void> parser) {
     private final ByteSizeValue memory;
     private final ByteSizeValue storage;
 
-    @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) // Remove deprecated version field
-    private final String version;
     private final String externalId;
     private final Set<DiscoveryNodeRole> roles;
 
-    @Deprecated
-    public DesiredNode(Settings settings, ProcessorsRange processorsRange, ByteSizeValue memory, ByteSizeValue storage, String version) {
-        this(settings, null, processorsRange, memory, storage, version);
-    }
-
-    @Deprecated
-    public DesiredNode(Settings settings, double processors, ByteSizeValue memory, ByteSizeValue storage, String version) {
-        this(settings, Processors.of(processors), null, memory, storage, version);
-    }
-
     public DesiredNode(Settings settings, ProcessorsRange processorsRange, ByteSizeValue memory, ByteSizeValue storage) {
         this(settings, null, processorsRange, memory, storage);
     }
@@ -136,17 +111,6 @@ public DesiredNode(Settings settings, double processors, ByteSizeValue memory, B
     }
 
     DesiredNode(Settings settings, Processors processors, ProcessorsRange processorsRange, ByteSizeValue memory, ByteSizeValue storage) {
-        this(settings, processors, processorsRange, memory, storage, null);
-    }
-
-    DesiredNode(
-        Settings settings,
-        Processors processors,
-        ProcessorsRange processorsRange,
-        ByteSizeValue memory,
-        ByteSizeValue storage,
-        @Deprecated String version
-    ) {
         assert settings != null;
         assert memory != null;
         assert storage != null;
@@ -180,7 +144,6 @@ public DesiredNode(Settings settings, double processors, ByteSizeValue memory, B
         this.processorsRange = processorsRange;
         this.memory = memory;
         this.storage = storage;
-        this.version = version;
         this.externalId = NODE_EXTERNAL_ID_SETTING.get(settings);
         this.roles = Collections.unmodifiableSortedSet(new TreeSet<>(DiscoveryNode.getRolesFromSettings(settings)));
     }
@@ -198,25 +161,10 @@ public static DesiredNode readFrom(StreamInput in) throws IOException {
         }
         final var memory = ByteSizeValue.readFrom(in);
         final var storage = ByteSizeValue.readFrom(in);
-        final String version;
-        if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_13_0)) {
-            version = in.readOptionalString();
-        } else {
-            version = Version.readVersion(in).toString();
+        if (in.getTransportVersion().before(TransportVersions.REMOVE_DESIRED_NODE_VERSION)) {
+            in.readOptionalString();
         }
-        return new DesiredNode(settings, processors, processorsRange, memory, storage, version);
-    }
-
-    private static final Pattern SEMANTIC_VERSION_PATTERN = Pattern.compile("^(\\d+\\.\\d+\\.\\d+)\\D?.*");
-
-    private static Version parseLegacyVersion(String version) {
-        if (version != null) {
-            var semanticVersionMatcher = SEMANTIC_VERSION_PATTERN.matcher(version);
-            if (semanticVersionMatcher.matches()) {
-                return Version.fromString(semanticVersionMatcher.group(1));
-            }
-        }
-        return null;
+        return new DesiredNode(settings, processors, processorsRange, memory, storage);
     }
 
     @Override
@@ -232,16 +180,8 @@ public void writeTo(StreamOutput out) throws IOException {
         }
         memory.writeTo(out);
         storage.writeTo(out);
-        if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_13_0)) {
-            out.writeOptionalString(version);
-        } else {
-            Version parsedVersion = parseLegacyVersion(version);
-            if (version == null) {
-                // Some node is from before we made the version field not required. If so, fill in with the current node version.
-                Version.writeVersion(Version.CURRENT, out);
-            } else {
-                Version.writeVersion(parsedVersion, out);
-            }
+        if (out.getTransportVersion().before(TransportVersions.REMOVE_DESIRED_NODE_VERSION)) {
+            out.writeOptionalString(null);
         }
     }
 
@@ -269,14 +209,6 @@ public void toInnerXContent(XContentBuilder builder, Params params) throws IOExc
         }
         builder.field(MEMORY_FIELD.getPreferredName(), memory);
         builder.field(STORAGE_FIELD.getPreferredName(), storage);
-        addDeprecatedVersionField(builder);
-    }
-
-    @UpdateForV9(owner = UpdateForV9.Owner.DISTRIBUTED_COORDINATION) // Remove deprecated field from response
-    private void addDeprecatedVersionField(XContentBuilder builder) throws IOException {
-        if (version != null) {
-            builder.field(VERSION_FIELD.getPreferredName(), version);
-        }
     }
 
     public boolean hasMasterRole() {
@@ -356,7 +288,6 @@ private boolean equalsWithoutProcessorsSpecification(DesiredNode that) {
         return Objects.equals(settings, that.settings)
             && Objects.equals(memory, that.memory)
             && Objects.equals(storage, that.storage)
-            && Objects.equals(version, that.version)
             && Objects.equals(externalId, that.externalId)
             && Objects.equals(roles, that.roles);
     }
@@ -369,7 +300,7 @@ public boolean equalsWithProcessorsCloseTo(DesiredNode that) {
 
     @Override
     public int hashCode() {
-        return Objects.hash(settings, processors, processorsRange, memory, storage, version, externalId, roles);
+        return Objects.hash(settings, processors, processorsRange, memory, storage, externalId, roles);
     }
 
     @Override
@@ -398,10 +329,6 @@ public String toString() {
             + '}';
     }
 
-    public boolean hasVersion() {
-        return Strings.isNullOrBlank(version) == false;
-    }
-
     public record ProcessorsRange(Processors min, @Nullable Processors max) implements Writeable, ToXContentObject {
 
         private static final ParseField MIN_FIELD = new ParseField("min");

server/src/main/java/org/elasticsearch/cluster/metadata/DesiredNodeWithStatus.java

@@ -44,13 +44,12 @@ public record DesiredNodeWithStatus(DesiredNode desiredNode, Status status)
                 (Processors) args[1],
                 (DesiredNode.ProcessorsRange) args[2],
                 (ByteSizeValue) args[3],
-                (ByteSizeValue) args[4],
-                (String) args[5]
+                (ByteSizeValue) args[4]
             ),
             // An unknown status is expected during upgrades to versions >= STATUS_TRACKING_SUPPORT_VERSION
             // the desired node status would be populated when a node in the newer version is elected as
             // master, the desired nodes status update happens in NodeJoinExecutor.
-            args[6] == null ? Status.PENDING : (Status) args[6]
+            args[5] == null ? Status.PENDING : (Status) args[5]
         )
     );
 

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestUpdateDesiredNodesAction.java

@@ -12,8 +12,6 @@
 import org.elasticsearch.action.admin.cluster.desirednodes.UpdateDesiredNodesAction;
 import org.elasticsearch.action.admin.cluster.desirednodes.UpdateDesiredNodesRequest;
 import org.elasticsearch.client.internal.node.NodeClient;
-import org.elasticsearch.cluster.metadata.DesiredNode;
-import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.action.RestToXContentListener;
@@ -27,10 +25,6 @@
 
 public class RestUpdateDesiredNodesAction extends BaseRestHandler {
 
-    private final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestUpdateDesiredNodesAction.class);
-    private static final String VERSION_DEPRECATION_MESSAGE =
-        "[version removal] Specifying node_version in desired nodes requests is deprecated.";
-
     @Override
     public String getName() {
         return "update_desired_nodes";
@@ -59,10 +53,6 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
             );
         }
 
-        if (updateDesiredNodesRequest.getNodes().stream().anyMatch(DesiredNode::hasVersion)) {
-            deprecationLogger.compatibleCritical("desired_nodes_version", VERSION_DEPRECATION_MESSAGE);
-        }
-
         return restChannel -> client.execute(
             UpdateDesiredNodesAction.INSTANCE,
             updateDesiredNodesRequest,