Revert "Don't return or accept node_version in the Desired Nodes API (#114580)" (#115829)

This reverts commit c64226c350.
This commit is contained in:
Artem Prigoda 2024-10-29 12:03:19 +01:00 committed by GitHub
parent 7feb4d5159
commit ef2cf37a6d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 191 additions and 13 deletions

View file

@ -11,6 +11,7 @@ package org.elasticsearch.upgrades;
import com.carrotsearch.randomizedtesting.annotations.Name;
import org.elasticsearch.Build;
import org.elasticsearch.action.admin.cluster.desirednodes.UpdateDesiredNodesRequest;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.ResponseException;
@ -81,7 +82,8 @@ public class DesiredNodesUpgradeIT extends AbstractRollingUpgradeTestCase {
Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeName).build(),
1238.49922909,
ByteSizeValue.ofGb(32),
ByteSizeValue.ofGb(128)
ByteSizeValue.ofGb(128),
clusterHasFeature(DesiredNode.DESIRED_NODE_VERSION_DEPRECATED) ? null : Build.current().version()
)
)
.toList();
@ -151,7 +153,8 @@ public class DesiredNodesUpgradeIT extends AbstractRollingUpgradeTestCase {
Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeName).build(),
processorsPrecision == ProcessorsPrecision.DOUBLE ? randomDoubleProcessorCount() : 0.5f,
ByteSizeValue.ofGb(randomIntBetween(10, 24)),
ByteSizeValue.ofGb(randomIntBetween(128, 256))
ByteSizeValue.ofGb(randomIntBetween(128, 256)),
clusterHasFeature(DesiredNode.DESIRED_NODE_VERSION_DEPRECATED) ? null : Build.current().version()
)
)
.toList();
@ -164,7 +167,8 @@ public class DesiredNodesUpgradeIT extends AbstractRollingUpgradeTestCase {
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))
ByteSizeValue.ofGb(randomIntBetween(128, 256)),
clusterHasFeature(DesiredNode.DESIRED_NODE_VERSION_DEPRECATED) ? null : Build.current().version()
);
}).toList();
}
@ -178,7 +182,8 @@ public class DesiredNodesUpgradeIT extends AbstractRollingUpgradeTestCase {
Settings.builder().put(NODE_NAME_SETTING.getKey(), nodeName).build(),
randomIntBetween(1, 24),
ByteSizeValue.ofGb(randomIntBetween(10, 24)),
ByteSizeValue.ofGb(randomIntBetween(128, 256))
ByteSizeValue.ofGb(randomIntBetween(128, 256)),
clusterHasFeature(DesiredNode.DESIRED_NODE_VERSION_DEPRECATED) ? null : Build.current().version()
)
)
.toList();

View file

@ -61,6 +61,4 @@ tasks.named("yamlRestCompatTestTransform").configure ({ task ->
task.skipTest("search/330_fetch_fields/Test search rewrite", "warning does not exist for compatibility")
task.skipTest("indices.create/21_synthetic_source_stored/object param - nested object with stored array", "temporary until backported")
task.skipTest("cat.aliases/10_basic/Deprecated local parameter", "CAT APIs not covered by compatibility policy")
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")
})

View file

@ -59,6 +59,61 @@ 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"
@ -144,6 +199,46 @@ 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"

View file

@ -14,6 +14,7 @@ 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;
@ -21,6 +22,7 @@ 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.features.NodeFeature;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ObjectParser;
@ -36,6 +38,7 @@ import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import static java.lang.String.format;
import static org.elasticsearch.node.Node.NODE_EXTERNAL_ID_SETTING;
@ -55,6 +58,8 @@ 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",
@ -64,7 +69,8 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
(Processors) args[1],
(ProcessorsRange) args[2],
(ByteSizeValue) args[3],
(ByteSizeValue) args[4]
(ByteSizeValue) args[4],
(String) args[5]
)
);
@ -98,6 +104,12 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
STORAGE_FIELD,
ObjectParser.ValueType.STRING
);
parser.declareField(
ConstructingObjectParser.optionalConstructorArg(),
(p, c) -> p.text(),
VERSION_FIELD,
ObjectParser.ValueType.STRING
);
}
private final Settings settings;
@ -106,9 +118,21 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
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);
}
@ -118,6 +142,17 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
}
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;
@ -151,6 +186,7 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
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)));
}
@ -174,7 +210,19 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
} else {
version = Version.readVersion(in).toString();
}
return new DesiredNode(settings, processors, processorsRange, memory, storage);
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;
}
@Override
@ -191,9 +239,15 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
memory.writeTo(out);
storage.writeTo(out);
if (out.getTransportVersion().onOrAfter(TransportVersions.V_8_13_0)) {
out.writeOptionalString(null);
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);
}
}
}
@ -221,6 +275,14 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
}
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() {
@ -304,6 +366,7 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
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);
}
@ -316,7 +379,7 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
@Override
public int hashCode() {
return Objects.hash(settings, processors, processorsRange, memory, storage, externalId, roles);
return Objects.hash(settings, processors, processorsRange, memory, storage, version, externalId, roles);
}
@Override
@ -345,6 +408,10 @@ public final class DesiredNode implements Writeable, ToXContentObject, Comparabl
+ '}';
}
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");

View file

@ -44,12 +44,13 @@ public record DesiredNodeWithStatus(DesiredNode desiredNode, Status status)
(Processors) args[1],
(DesiredNode.ProcessorsRange) args[2],
(ByteSizeValue) args[3],
(ByteSizeValue) args[4]
(ByteSizeValue) args[4],
(String) args[5]
),
// 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[5] == null ? Status.PENDING : (Status) args[5]
args[6] == null ? Status.PENDING : (Status) args[6]
)
);

View file

@ -12,11 +12,13 @@ package org.elasticsearch.rest.action.admin.cluster;
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.features.NodeFeature;
import org.elasticsearch.rest.BaseRestHandler;
import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.action.RestToXContentListener;
import org.elasticsearch.xcontent.XContentParseException;
import org.elasticsearch.xcontent.XContentParser;
import java.io.IOException;
@ -65,6 +67,16 @@ public class RestUpdateDesiredNodesAction extends BaseRestHandler {
);
}
if (clusterSupportsFeature.test(DesiredNode.DESIRED_NODE_VERSION_DEPRECATED)) {
if (updateDesiredNodesRequest.getNodes().stream().anyMatch(DesiredNode::hasVersion)) {
deprecationLogger.compatibleCritical("desired_nodes_version", VERSION_DEPRECATION_MESSAGE);
}
} else {
if (updateDesiredNodesRequest.getNodes().stream().anyMatch(n -> n.hasVersion() == false)) {
throw new XContentParseException("[node_version] field is required and must have a valid value");
}
}
return restChannel -> client.execute(
UpdateDesiredNodesAction.INSTANCE,
updateDesiredNodesRequest,