Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
}
Expand Down
8 changes: 8 additions & 0 deletions rest-api-spec/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
})
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/java/org/elasticsearch/TransportVersions.java
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ static TransportVersion def(int id) {
public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED_BACKPORT_8_X = def(8_840_0_01);
public static final TransportVersion ELASTICSEARCH_9_0 = def(9_000_0_00);
public static final TransportVersion COHERE_BIT_EMBEDDING_TYPE_SUPPORT_ADDED = def(9_001_0_00);
public static final TransportVersion REMOVE_DESIRED_NODE_VERSION = def(9_002_0_00);

/*
* STOP! READ THIS FIRST! No, really,
* ____ _____ ___ ____ _ ____ _____ _ ____ _____ _ _ ___ ____ _____ ___ ____ ____ _____ _
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@
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;
Expand All @@ -36,7 +34,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;
Expand All @@ -52,8 +49,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",
Expand All @@ -63,8 +58,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]
)
);

Expand Down Expand Up @@ -98,12 +92,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;
Expand All @@ -112,21 +100,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);
}
Expand All @@ -136,17 +112,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;
Expand Down Expand Up @@ -180,7 +145,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)));
}
Expand All @@ -198,25 +162,15 @@ 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();
}
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));
if (in.getTransportVersion().before(TransportVersions.REMOVE_DESIRED_NODE_VERSION)) {
final String version;
if (in.getTransportVersion().onOrAfter(TransportVersions.V_8_13_0)) {
version = in.readOptionalString();
} else {
version = Version.readVersion(in).toString();
}
}
return null;
return new DesiredNode(settings, processors, processorsRange, memory, storage);
}

@Override
Expand All @@ -232,15 +186,11 @@ 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);
if (out.getTransportVersion().before(TransportVersions.REMOVE_DESIRED_NODE_VERSION)) {
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_13_0)) {
out.writeOptionalString(null);
} else {
Version.writeVersion(parsedVersion, out);
Version.writeVersion(Version.CURRENT, out);
}
}
}
Expand Down Expand Up @@ -269,14 +219,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() {
Expand Down Expand Up @@ -356,7 +298,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);
}
Expand All @@ -369,7 +310,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
Expand Down Expand Up @@ -398,10 +339,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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
)
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand Down Expand Up @@ -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,
Expand Down