From df618480de2e9d01cc4a15ddf5e9a557b2309107 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 27 Jun 2022 16:39:27 -0400 Subject: [PATCH 01/14] Implement few operator handlers Implement operator handlers for cluster state and ILM --- server/src/main/java/module-info.java | 2 + .../TransportClusterUpdateSettingsAction.java | 18 +++ .../master/TransportMasterNodeAction.java | 30 +++++ .../OperatorClusterUpdateSettingsAction.java | 90 ++++++++++++++ ...ratorClusterUpdateSettingsActionTests.java | 100 ++++++++++++++++ .../ilm/action/DeleteLifecycleAction.java | 7 ++ .../plugin/ilm/src/main/java/module-info.java | 2 + .../TransportDeleteLifecycleAction.java | 18 +++ .../action/TransportPutLifecycleAction.java | 48 +++++++- .../operator/ILMOperatorHandlerProvider.java | 31 +++++ .../action/OperatorLifecycleAction.java | 113 ++++++++++++++++++ .../TransportDeleteLifecycleActionTests.java | 36 ++++++ .../TransportPutLifecycleActionTests.java | 49 ++++++++ 13 files changed, 538 insertions(+), 6 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsAction.java create mode 100644 server/src/test/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsActionTests.java create mode 100644 x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java create mode 100644 x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java create mode 100644 x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index a7fb33167dc23..19e1cd3780ddb 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -359,4 +359,6 @@ org.elasticsearch.cluster.coordination.NodeToolCliProvider, org.elasticsearch.index.shard.ShardToolCliProvider; provides org.apache.logging.log4j.util.PropertySource with org.elasticsearch.common.logging.ESSystemPropertiesPropertySource; + + uses org.elasticsearch.operator.OperatorHandlerProvider; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index 4f19aad41bc5e..c182862edfd3c 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -29,11 +29,13 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.SuppressForbidden; +import org.elasticsearch.operator.action.OperatorClusterUpdateSettingsAction; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import java.util.HashSet; +import java.util.Optional; import java.util.Set; import static org.elasticsearch.common.settings.AbstractScopedSettings.ARCHIVED_SETTINGS_PREFIX; @@ -128,6 +130,17 @@ private static boolean checkClearedBlockAndArchivedSettings( return true; } + @Override + protected Optional operatorHandlerName() { + return Optional.of(OperatorClusterUpdateSettingsAction.NAME); + } + + @Override + protected Set modifiedKeys(ClusterUpdateSettingsRequest request) { + Settings allSettings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build(); + return allSettings.keySet(); + } + private static final String UPDATE_TASK_SOURCE = "cluster_update_settings"; private static final String REROUTE_TASK_SOURCE = "reroute_after_cluster_update_settings"; @@ -243,6 +256,11 @@ public ClusterUpdateSettingsTask( this.request = request; } + // Used by the operator handler + public ClusterUpdateSettingsTask(final ClusterSettings clusterSettings, ClusterUpdateSettingsRequest request) { + this(clusterSettings, Priority.IMMEDIATE, request, null); + } + @Override public ClusterState execute(final ClusterState currentState) { final ClusterState clusterState = updater.updateSettings( diff --git a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java index d92fb8f2956f9..a13a8d19601dc 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java @@ -42,6 +42,9 @@ import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportService; +import java.util.Collections; +import java.util.Optional; +import java.util.Set; import java.util.function.Predicate; import static org.elasticsearch.core.Strings.format; @@ -142,6 +145,33 @@ private ClusterBlockException checkBlockIfStateRecovered(Request request, Cluste } } + /** + * Override this method if the master node action also has an {@link org.elasticsearch.operator.OperatorHandler} + * interaction. + *

+ * We need to check if certain settings or entities are allowed to be modified by the master node + * action, depending on if they are set already in operator mode. + * + * @return an Optional of the operator handler name + */ + protected Optional operatorHandlerName() { + return Optional.empty(); + } + + /** + * Override this method to return the keys of the cluster state or cluster entities that are modified by + * the Request object. + *

+ * This method is used by the operator handler logic (see {@link org.elasticsearch.operator.OperatorHandler}) + * to verify if the keys don't conflict with an existing key set in operator mode. + * + * @param request the TransportMasterNode request + * @return set of String keys intended to be modified/set/deleted by this request + */ + protected Set modifiedKeys(Request request) { + return Collections.emptySet(); + } + @Override protected void doExecute(Task task, final Request request, ActionListener listener) { ClusterState state = clusterService.state(); diff --git a/server/src/main/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsAction.java new file mode 100644 index 0000000000000..81e210081b751 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsAction.java @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.operator.action; + +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.elasticsearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction; +import org.elasticsearch.client.internal.Requests; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.operator.OperatorHandler; +import org.elasticsearch.operator.TransformState; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.common.util.Maps.asMap; + +/** + * This Action is the Operator version of RestClusterUpdateSettingsAction + *

+ * It is used by the OperatorClusterStateController to update the persistent cluster settings. + * Since transient cluster settings are deprecated, this action doesn't support updating cluster settings. + */ +public class OperatorClusterUpdateSettingsAction implements OperatorHandler { + + public static final String NAME = "cluster_settings"; + + private final ClusterSettings clusterSettings; + + public OperatorClusterUpdateSettingsAction(ClusterSettings clusterSettings) { + this.clusterSettings = clusterSettings; + } + + @Override + public String name() { + return NAME; + } + + @SuppressWarnings("unchecked") + private ClusterUpdateSettingsRequest prepare(Object input, Set previouslySet) { + final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = Requests.clusterUpdateSettingsRequest(); + + Map source = asMap(input); + Map persistentSettings = new HashMap<>(); + Set toDelete = new HashSet<>(previouslySet); + + source.forEach((k, v) -> { + persistentSettings.put(k, v); + toDelete.remove(k); + }); + + toDelete.forEach(k -> persistentSettings.put(k, null)); + + clusterUpdateSettingsRequest.persistentSettings(persistentSettings); + return clusterUpdateSettingsRequest; + } + + @Override + public TransformState transform(Object input, TransformState prevState) { + ClusterUpdateSettingsRequest request = prepare(input, prevState.keys()); + + // allow empty requests, this is how we clean up settings + if (request.persistentSettings().isEmpty() == false) { + validate(request); + } + + ClusterState state = prevState.state(); + + TransportClusterUpdateSettingsAction.ClusterUpdateSettingsTask updateSettingsTask = + new TransportClusterUpdateSettingsAction.ClusterUpdateSettingsTask(clusterSettings, request); + + state = updateSettingsTask.execute(state); + Set currentKeys = request.persistentSettings() + .keySet() + .stream() + .filter(k -> request.persistentSettings().hasValue(k)) + .collect(Collectors.toSet()); + + return new TransformState(state, currentKeys); + } +} diff --git a/server/src/test/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsActionTests.java b/server/src/test/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsActionTests.java new file mode 100644 index 0000000000000..c2e29a10117fa --- /dev/null +++ b/server/src/test/java/org/elasticsearch/operator/action/OperatorClusterUpdateSettingsActionTests.java @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.operator.action; + +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.operator.TransformState; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; + +import java.util.Collections; + +import static org.hamcrest.Matchers.containsInAnyOrder; + +public class OperatorClusterUpdateSettingsActionTests extends ESTestCase { + + private TransformState processJSON(OperatorClusterUpdateSettingsAction action, TransformState prevState, String json) throws Exception { + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { + return action.transform(parser.map(), prevState); + } + } + + public void testValidation() throws Exception { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + TransformState prevState = new TransformState(state, Collections.emptySet()); + OperatorClusterUpdateSettingsAction action = new OperatorClusterUpdateSettingsAction(clusterSettings); + + String badPolicyJSON = """ + { + "indices.recovery.min_bytes_per_sec": "50mb" + }"""; + + assertEquals( + "persistent setting [indices.recovery.min_bytes_per_sec], not recognized", + expectThrows(IllegalArgumentException.class, () -> processJSON(action, prevState, badPolicyJSON)).getMessage() + ); + } + + public void testSetUnsetSettings() throws Exception { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + TransformState prevState = new TransformState(state, Collections.emptySet()); + OperatorClusterUpdateSettingsAction action = new OperatorClusterUpdateSettingsAction(clusterSettings); + + String emptyJSON = ""; + + TransformState updatedState = processJSON(action, prevState, emptyJSON); + assertEquals(0, updatedState.keys().size()); + assertEquals(prevState.state(), updatedState.state()); + + String settingsJSON = """ + { + "indices.recovery.max_bytes_per_sec": "50mb", + "cluster": { + "remote": { + "cluster_one": { + "seeds": [ + "127.0.0.1:9300" + ] + } + } + } + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, settingsJSON); + assertThat(updatedState.keys(), containsInAnyOrder("indices.recovery.max_bytes_per_sec", "cluster.remote.cluster_one.seeds")); + assertEquals("50mb", updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + assertEquals("[127.0.0.1:9300]", updatedState.state().metadata().persistentSettings().get("cluster.remote.cluster_one.seeds")); + + String oneSettingJSON = """ + { + "indices.recovery.max_bytes_per_sec": "25mb" + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, oneSettingJSON); + assertThat(updatedState.keys(), containsInAnyOrder("indices.recovery.max_bytes_per_sec")); + assertEquals("25mb", updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + assertNull(updatedState.state().metadata().persistentSettings().get("cluster.remote.cluster_one.seeds")); + + prevState = updatedState; + updatedState = processJSON(action, prevState, emptyJSON); + assertEquals(0, updatedState.keys().size()); + assertNull(updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + } +} diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/action/DeleteLifecycleAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/action/DeleteLifecycleAction.java index 623c9797ffde1..2d59d07a55d0d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/action/DeleteLifecycleAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/action/DeleteLifecycleAction.java @@ -18,6 +18,8 @@ import java.io.IOException; import java.util.Objects; +import static org.elasticsearch.core.Strings.format; + public class DeleteLifecycleAction extends ActionType { public static final DeleteLifecycleAction INSTANCE = new DeleteLifecycleAction(); public static final String NAME = "cluster:admin/ilm/delete"; @@ -75,6 +77,11 @@ public boolean equals(Object obj) { return Objects.equals(policyName, other.policyName); } + @Override + public String toString() { + return format("delete lifecycle policy [%s]", policyName); + } + } } diff --git a/x-pack/plugin/ilm/src/main/java/module-info.java b/x-pack/plugin/ilm/src/main/java/module-info.java index dfcb404fb4e56..69348eedcfc60 100644 --- a/x-pack/plugin/ilm/src/main/java/module-info.java +++ b/x-pack/plugin/ilm/src/main/java/module-info.java @@ -16,4 +16,6 @@ exports org.elasticsearch.xpack.ilm to org.elasticsearch.server; exports org.elasticsearch.xpack.slm.action to org.elasticsearch.server; exports org.elasticsearch.xpack.slm to org.elasticsearch.server; + + provides org.elasticsearch.operator.OperatorHandlerProvider with org.elasticsearch.xpack.ilm.operator.ILMOperatorHandlerProvider; } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java index 1387093782e48..5fc29ed66f5fc 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java @@ -29,8 +29,11 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction.Request; +import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; import java.util.List; +import java.util.Optional; +import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; @@ -70,6 +73,11 @@ public DeleteLifecyclePolicyTask(Request request, ActionListener operatorHandlerName() { + return Optional.of(OperatorLifecycleAction.NAME); + } + + @Override + protected Set modifiedKeys(Request request) { + return Set.of(request.getPolicyName()); + } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java index 7baecc4754672..d1d75d7333d85 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java @@ -41,10 +41,14 @@ import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction.Request; import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata; +import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; import java.time.Instant; +import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; @@ -111,7 +115,7 @@ protected void masterOperation(Task task, Request request, ClusterState state, A submitUnbatchedTask( "put-lifecycle-" + request.getPolicy().getName(), - new UpdateLifecyclePolicyTask(request, listener, licenseState, filteredHeaders, xContentRegistry, client) + new UpdateLifecyclePolicyTask(request, listener, licenseState, filteredHeaders, xContentRegistry, client, true) ); } @@ -121,6 +125,7 @@ public static class UpdateLifecyclePolicyTask extends AckedClusterStateUpdateTas private final Map filteredHeaders; private final NamedXContentRegistry xContentRegistry; private final Client client; + private final boolean verboseLogging; public UpdateLifecyclePolicyTask( Request request, @@ -128,7 +133,8 @@ public UpdateLifecyclePolicyTask( XPackLicenseState licenseState, Map filteredHeaders, NamedXContentRegistry xContentRegistry, - Client client + Client client, + boolean verboseLogging ) { super(request, listener); this.request = request; @@ -136,6 +142,24 @@ public UpdateLifecyclePolicyTask( this.filteredHeaders = filteredHeaders; this.xContentRegistry = xContentRegistry; this.client = client; + this.verboseLogging = verboseLogging; + } + + /** + * Constructor used in operator mode. It disables verbose logging and has no filtered headers. + * + * @param request + * @param licenseState + * @param xContentRegistry + * @param client + */ + public UpdateLifecyclePolicyTask( + Request request, + XPackLicenseState licenseState, + NamedXContentRegistry xContentRegistry, + Client client + ) { + this(request, null, licenseState, new HashMap<>(), xContentRegistry, client, false); } @Override @@ -161,10 +185,12 @@ public ClusterState execute(ClusterState currentState) throws Exception { Instant.now().toEpochMilli() ); LifecyclePolicyMetadata oldPolicy = newPolicies.put(lifecyclePolicyMetadata.getName(), lifecyclePolicyMetadata); - if (oldPolicy == null) { - logger.info("adding index lifecycle policy [{}]", request.getPolicy().getName()); - } else { - logger.info("updating index lifecycle policy [{}]", request.getPolicy().getName()); + if (verboseLogging) { + if (oldPolicy == null) { + logger.info("adding index lifecycle policy [{}]", request.getPolicy().getName()); + } else { + logger.info("updating index lifecycle policy [{}]", request.getPolicy().getName()); + } } IndexLifecycleMetadata newMetadata = new IndexLifecycleMetadata(newPolicies, currentMetadata.getOperationMode()); stateBuilder.metadata(Metadata.builder(currentState.getMetadata()).putCustom(IndexLifecycleMetadata.TYPE, newMetadata).build()); @@ -285,4 +311,14 @@ private static void validatePrerequisites(LifecyclePolicy policy, ClusterState s protected ClusterBlockException checkBlock(Request request, ClusterState state) { return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE); } + + @Override + protected Optional operatorHandlerName() { + return Optional.of(OperatorLifecycleAction.NAME); + } + + @Override + protected Set modifiedKeys(Request request) { + return Set.of(request.getPolicy().getName()); + } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java new file mode 100644 index 0000000000000..5577401719cfd --- /dev/null +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.operator; + +import org.elasticsearch.operator.OperatorHandler; +import org.elasticsearch.operator.OperatorHandlerProvider; + +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +/** + * ILM Provider implementation for the OperatorHandlerProvider service interface + */ +public class ILMOperatorHandlerProvider implements OperatorHandlerProvider { + private static final Set> handlers = ConcurrentHashMap.newKeySet(); + + @Override + public Collection> handlers() { + return handlers; + } + + public static void handler(OperatorHandler handler) { + handlers.add(handler); + } +} diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java new file mode 100644 index 0000000000000..0092db643568c --- /dev/null +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java @@ -0,0 +1,113 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.operator.action; + +import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.operator.OperatorHandler; +import org.elasticsearch.operator.TransformState; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; +import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; +import org.elasticsearch.xpack.core.template.LifecyclePolicyConfig; +import org.elasticsearch.xpack.ilm.action.TransportDeleteLifecycleAction; +import org.elasticsearch.xpack.ilm.action.TransportPutLifecycleAction; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.common.util.Maps.asMap; +import static org.elasticsearch.common.xcontent.XContentHelper.mapToXContentParser; + +/** + * This {@link OperatorHandler} is responsible for CRUD operations on ILM policies in + * operator mode, e.g. file based settings. + *

+ * Internally it uses {@link TransportPutLifecycleAction} and + * {@link TransportDeleteLifecycleAction} to add, update and delete ILM policies. + */ +public class OperatorLifecycleAction implements OperatorHandler { + + private final NamedXContentRegistry xContentRegistry; + private final Client client; + private final XPackLicenseState licenseState; + + public static final String NAME = "ilm"; + + public OperatorLifecycleAction(NamedXContentRegistry xContentRegistry, Client client, XPackLicenseState licenseState) { + this.xContentRegistry = xContentRegistry; + this.client = client; + this.licenseState = licenseState; + } + + @Override + public String name() { + return NAME; + } + + @SuppressWarnings("unchecked") + public Collection prepare(Object input) throws IOException { + List result = new ArrayList<>(); + + Map source = asMap(input); + + for (String name : source.keySet()) { + Map content = (Map) source.get(name); + var config = XContentParserConfiguration.EMPTY.withRegistry(LifecyclePolicyConfig.DEFAULT_X_CONTENT_REGISTRY); + try (XContentParser parser = mapToXContentParser(config, content)) { + LifecyclePolicy policy = LifecyclePolicy.parse(parser, name); + PutLifecycleAction.Request request = new PutLifecycleAction.Request(policy); + validate(request); + result.add(request); + } + } + + return result; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + var requests = prepare(source); + + ClusterState state = prevState.state(); + + for (var request : requests) { + TransportPutLifecycleAction.UpdateLifecyclePolicyTask task = new TransportPutLifecycleAction.UpdateLifecyclePolicyTask( + request, + licenseState, + xContentRegistry, + client + ); + + state = task.execute(state); + } + + Set entities = requests.stream().map(r -> r.getPolicy().getName()).collect(Collectors.toSet()); + + Set toDelete = new HashSet<>(prevState.keys()); + toDelete.removeAll(entities); + + for (var policyToDelete : toDelete) { + TransportDeleteLifecycleAction.DeleteLifecyclePolicyTask task = new TransportDeleteLifecycleAction.DeleteLifecyclePolicyTask( + policyToDelete + ); + state = task.execute(state); + } + + return new TransformState(state, entities); + } +} diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java new file mode 100644 index 0000000000000..51df10fb7a190 --- /dev/null +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java @@ -0,0 +1,36 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.action; + +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; +import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.mockito.Mockito.mock; + +public class TransportDeleteLifecycleActionTests extends ESTestCase { + public void testOperatorHandler() { + TransportDeleteLifecycleAction putAction = new TransportDeleteLifecycleAction( + mock(TransportService.class), + mock(ClusterService.class), + mock(ThreadPool.class), + mock(ActionFilters.class), + mock(IndexNameExpressionResolver.class) + ); + assertEquals(OperatorLifecycleAction.NAME, putAction.operatorHandlerName().get()); + + DeleteLifecycleAction.Request request = new DeleteLifecycleAction.Request("my_timeseries_lifecycle2"); + assertThat(putAction.modifiedKeys(request), containsInAnyOrder("my_timeseries_lifecycle2")); + } +} diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java index b2202f0e5126f..6731957cb43ab 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java @@ -7,13 +7,29 @@ package org.elasticsearch.xpack.ilm.action; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; +import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.ilm.LifecyclePolicyTestsUtils; +import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; import java.util.Map; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.mockito.Mockito.mock; + public class TransportPutLifecycleActionTests extends ESTestCase { public void testIsNoop() { LifecyclePolicy policy1 = LifecyclePolicyTestsUtils.randomTimeseriesLifecyclePolicy("policy"); @@ -29,4 +45,37 @@ public void testIsNoop() { assertFalse(TransportPutLifecycleAction.isNoopUpdate(existing, policy1, headers2)); assertFalse(TransportPutLifecycleAction.isNoopUpdate(null, policy1, headers1)); } + + public void testOperatorHandler() throws Exception { + TransportPutLifecycleAction putAction = new TransportPutLifecycleAction( + mock(TransportService.class), + mock(ClusterService.class), + mock(ThreadPool.class), + mock(ActionFilters.class), + mock(IndexNameExpressionResolver.class), + mock(NamedXContentRegistry.class), + mock(XPackLicenseState.class), + mock(Client.class) + ); + assertEquals(OperatorLifecycleAction.NAME, putAction.operatorHandlerName().get()); + + String json = """ + { + "policy": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + } + } + } + }"""; + + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { + PutLifecycleAction.Request request = PutLifecycleAction.Request.parseRequest("my_timeseries_lifecycle2", parser); + + assertThat(putAction.modifiedKeys(request), containsInAnyOrder("my_timeseries_lifecycle2")); + } + } } From 407fd72a89137145acc474fbbfd8b4521dc6a9ff Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 27 Jun 2022 16:59:26 -0400 Subject: [PATCH 02/14] Add few ILM operator handler tests --- .../operator/OperatorILMControllerTests.java | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java new file mode 100644 index 0000000000000..ed279c213693d --- /dev/null +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java @@ -0,0 +1,213 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.action.operator; + +import org.elasticsearch.client.internal.Client; +import org.elasticsearch.cluster.ClusterModule; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.operator.TransformState; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.NamedXContentRegistry; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.XContentParseException; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; +import org.elasticsearch.xpack.core.ilm.AllocateAction; +import org.elasticsearch.xpack.core.ilm.DeleteAction; +import org.elasticsearch.xpack.core.ilm.ForceMergeAction; +import org.elasticsearch.xpack.core.ilm.FreezeAction; +import org.elasticsearch.xpack.core.ilm.IndexLifecycleMetadata; +import org.elasticsearch.xpack.core.ilm.LifecycleAction; +import org.elasticsearch.xpack.core.ilm.LifecycleType; +import org.elasticsearch.xpack.core.ilm.MigrateAction; +import org.elasticsearch.xpack.core.ilm.ReadOnlyAction; +import org.elasticsearch.xpack.core.ilm.RolloverAction; +import org.elasticsearch.xpack.core.ilm.RollupILMAction; +import org.elasticsearch.xpack.core.ilm.SearchableSnapshotAction; +import org.elasticsearch.xpack.core.ilm.SetPriorityAction; +import org.elasticsearch.xpack.core.ilm.ShrinkAction; +import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType; +import org.elasticsearch.xpack.core.ilm.UnfollowAction; +import org.elasticsearch.xpack.core.ilm.WaitForSnapshotAction; +import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class OperatorILMControllerTests extends ESTestCase { + + protected NamedXContentRegistry xContentRegistry() { + List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); + entries.addAll( + Arrays.asList( + new NamedXContentRegistry.Entry( + LifecycleType.class, + new ParseField(TimeseriesLifecycleType.TYPE), + (p) -> TimeseriesLifecycleType.INSTANCE + ), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(AllocateAction.NAME), AllocateAction::parse), + new NamedXContentRegistry.Entry( + LifecycleAction.class, + new ParseField(WaitForSnapshotAction.NAME), + WaitForSnapshotAction::parse + ), + new NamedXContentRegistry.Entry( + LifecycleAction.class, + new ParseField(SearchableSnapshotAction.NAME), + SearchableSnapshotAction::parse + ), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(DeleteAction.NAME), DeleteAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(ForceMergeAction.NAME), ForceMergeAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(ReadOnlyAction.NAME), ReadOnlyAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(RolloverAction.NAME), RolloverAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(ShrinkAction.NAME), ShrinkAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(FreezeAction.NAME), FreezeAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(SetPriorityAction.NAME), SetPriorityAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(MigrateAction.NAME), MigrateAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(UnfollowAction.NAME), UnfollowAction::parse), + new NamedXContentRegistry.Entry(LifecycleAction.class, new ParseField(RollupILMAction.NAME), RollupILMAction::parse) + ) + ); + return new NamedXContentRegistry(entries); + } + + private TransformState processJSON(OperatorLifecycleAction action, TransformState prevState, String json) throws Exception { + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { + return action.transform(parser.map(), prevState); + } + } + + public void testValidationFails() { + Client client = mock(Client.class); + when(client.settings()).thenReturn(Settings.EMPTY); + final ClusterName clusterName = new ClusterName("elasticsearch"); + + ClusterState state = ClusterState.builder(clusterName).build(); + OperatorLifecycleAction action = new OperatorLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); + TransformState prevState = new TransformState(state, Collections.emptySet()); + + String badPolicyJSON = """ + { + "my_timeseries_lifecycle": { + "phase": { + "warm": { + "min_age": "10s", + "actions": { + } + } + } + } + }"""; + + assertEquals( + "[1:2] [lifecycle_policy] unknown field [phase] did you mean [phases]?", + expectThrows(XContentParseException.class, () -> processJSON(action, prevState, badPolicyJSON)).getMessage() + ); + } + + public void testActionAddRemove() throws Exception { + Client client = mock(Client.class); + when(client.settings()).thenReturn(Settings.EMPTY); + final ClusterName clusterName = new ClusterName("elasticsearch"); + + ClusterState state = ClusterState.builder(clusterName).build(); + + OperatorLifecycleAction action = new OperatorLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); + + String emptyJSON = ""; + + TransformState prevState = new TransformState(state, Collections.emptySet()); + + TransformState updatedState = processJSON(action, prevState, emptyJSON); + assertEquals(0, updatedState.keys().size()); + assertEquals(prevState.state(), updatedState.state()); + + String twoPoliciesJSON = """ + { + "my_timeseries_lifecycle": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + } + } + }, + "my_timeseries_lifecycle1": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + }, + "delete": { + "min_age": "30s", + "actions": { + } + } + } + } + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, twoPoliciesJSON); + assertThat(updatedState.keys(), containsInAnyOrder("my_timeseries_lifecycle", "my_timeseries_lifecycle1")); + IndexLifecycleMetadata ilmMetadata = updatedState.state() + .metadata() + .custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); + assertThat(ilmMetadata.getPolicyMetadatas().keySet(), containsInAnyOrder("my_timeseries_lifecycle", "my_timeseries_lifecycle1")); + + String onePolicyRemovedJSON = """ + { + "my_timeseries_lifecycle": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + } + } + } + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, onePolicyRemovedJSON); + assertThat(updatedState.keys(), containsInAnyOrder("my_timeseries_lifecycle")); + ilmMetadata = updatedState.state().metadata().custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); + assertThat(ilmMetadata.getPolicyMetadatas().keySet(), containsInAnyOrder("my_timeseries_lifecycle")); + + String onePolicyRenamedJSON = """ + { + "my_timeseries_lifecycle2": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + } + } + } + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, onePolicyRenamedJSON); + assertThat(updatedState.keys(), containsInAnyOrder("my_timeseries_lifecycle2")); + ilmMetadata = updatedState.state().metadata().custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); + assertThat(ilmMetadata.getPolicyMetadatas().keySet(), containsInAnyOrder("my_timeseries_lifecycle2")); + } +} From 2836ff77f412983cf2b380b30925bcb519f30a5a Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Wed, 29 Jun 2022 19:12:50 -0400 Subject: [PATCH 03/14] Rename operator to immutablestate --- server/src/main/java/module-info.java | 2 +- .../TransportClusterUpdateSettingsAction.java | 10 +- .../master/TransportMasterNodeAction.java | 12 +-- .../ImmutableClusterSettingsAction.java | 90 ++++++++++++++++ .../OperatorClusterUpdateSettingsAction.java | 0 .../ImmutableClusterSettingsActionTests.java | 100 ++++++++++++++++++ ...ratorClusterUpdateSettingsActionTests.java | 0 .../plugin/ilm/src/main/java/module-info.java | 4 +- .../TransportDeleteLifecycleAction.java | 12 ++- .../action/TransportPutLifecycleAction.java | 11 +- .../ILMImmutableStateHandlerProvider.java | 31 ++++++ .../action/ImmutableLifecycleAction.java} | 14 +-- .../operator/ILMOperatorHandlerProvider.java | 31 ------ .../TransportDeleteLifecycleActionTests.java | 6 +- .../TransportPutLifecycleActionTests.java | 6 +- .../ImmutableILMStateControllerTests.java} | 14 +-- 16 files changed, 272 insertions(+), 71 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java delete mode 100644 server/src/main/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsAction.java create mode 100644 server/src/test/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsActionTests.java delete mode 100644 server/src/test/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsActionTests.java create mode 100644 x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java rename x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/{operator/action/OperatorLifecycleAction.java => immutablestate/action/ImmutableLifecycleAction.java} (86%) delete mode 100644 x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java rename x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/{operator/OperatorILMControllerTests.java => immutablestate/ImmutableILMStateControllerTests.java} (93%) diff --git a/server/src/main/java/module-info.java b/server/src/main/java/module-info.java index 8cc384df8b5b7..c6dcf3306729c 100644 --- a/server/src/main/java/module-info.java +++ b/server/src/main/java/module-info.java @@ -360,5 +360,5 @@ org.elasticsearch.index.shard.ShardToolCliProvider; provides org.apache.logging.log4j.util.PropertySource with org.elasticsearch.common.logging.ESSystemPropertiesPropertySource; - uses org.elasticsearch.operator.OperatorHandlerProvider; + uses org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; } diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java index c182862edfd3c..90b70368bbb7d 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java @@ -29,7 +29,7 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.SuppressForbidden; -import org.elasticsearch.operator.action.OperatorClusterUpdateSettingsAction; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.tasks.Task; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; @@ -131,8 +131,8 @@ private static boolean checkClearedBlockAndArchivedSettings( } @Override - protected Optional operatorHandlerName() { - return Optional.of(OperatorClusterUpdateSettingsAction.NAME); + protected Optional immutableStateHandlerName() { + return Optional.of(ImmutableClusterSettingsAction.NAME); } @Override @@ -256,7 +256,9 @@ public ClusterUpdateSettingsTask( this.request = request; } - // Used by the operator handler + /** + * Used by the immutable state handler {@link ImmutableClusterSettingsAction} + */ public ClusterUpdateSettingsTask(final ClusterSettings clusterSettings, ClusterUpdateSettingsRequest request) { this(clusterSettings, Priority.IMMEDIATE, request, null); } diff --git a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java index a13a8d19601dc..777b0c2356a72 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java @@ -146,15 +146,15 @@ private ClusterBlockException checkBlockIfStateRecovered(Request request, Cluste } /** - * Override this method if the master node action also has an {@link org.elasticsearch.operator.OperatorHandler} + * Override this method if the master node action also has an {@link org.elasticsearch.immutablestate.ImmutableClusterStateHandler} * interaction. *

* We need to check if certain settings or entities are allowed to be modified by the master node - * action, depending on if they are set already in operator mode. + * action, depending on if they are set as immutable in 'operator' mode (file based settings, modules, plugins). * - * @return an Optional of the operator handler name + * @return an Optional of the {@link org.elasticsearch.immutablestate.ImmutableClusterStateHandler} name */ - protected Optional operatorHandlerName() { + protected Optional immutableStateHandlerName() { return Optional.empty(); } @@ -162,8 +162,8 @@ protected Optional operatorHandlerName() { * Override this method to return the keys of the cluster state or cluster entities that are modified by * the Request object. *

- * This method is used by the operator handler logic (see {@link org.elasticsearch.operator.OperatorHandler}) - * to verify if the keys don't conflict with an existing key set in operator mode. + * This method is used by the immutable state handler logic (see {@link org.elasticsearch.immutablestate.ImmutableClusterStateHandler}) + * to verify if the keys don't conflict with an existing key set as immutable. * * @param request the TransportMasterNode request * @return set of String keys intended to be modified/set/deleted by this request diff --git a/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java b/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java new file mode 100644 index 0000000000000..cd82f17ecfa8f --- /dev/null +++ b/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.action; + +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.elasticsearch.action.admin.cluster.settings.TransportClusterUpdateSettingsAction; +import org.elasticsearch.client.internal.Requests; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.TransformState; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; + +import static org.elasticsearch.common.util.Maps.asMap; + +/** + * This Action is the immutable state save version of RestClusterUpdateSettingsAction + *

+ * It is used by the ImmutableClusterStateController to update the persistent cluster settings. + * Since transient cluster settings are deprecated, this action doesn't support updating transient cluster settings. + */ +public class ImmutableClusterSettingsAction implements ImmutableClusterStateHandler { + + public static final String NAME = "cluster_settings"; + + private final ClusterSettings clusterSettings; + + public ImmutableClusterSettingsAction(ClusterSettings clusterSettings) { + this.clusterSettings = clusterSettings; + } + + @Override + public String name() { + return NAME; + } + + @SuppressWarnings("unchecked") + private ClusterUpdateSettingsRequest prepare(Object input, Set previouslySet) { + final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = Requests.clusterUpdateSettingsRequest(); + + Map source = asMap(input); + Map persistentSettings = new HashMap<>(); + Set toDelete = new HashSet<>(previouslySet); + + source.forEach((k, v) -> { + persistentSettings.put(k, v); + toDelete.remove(k); + }); + + toDelete.forEach(k -> persistentSettings.put(k, null)); + + clusterUpdateSettingsRequest.persistentSettings(persistentSettings); + return clusterUpdateSettingsRequest; + } + + @Override + public TransformState transform(Object input, TransformState prevState) { + ClusterUpdateSettingsRequest request = prepare(input, prevState.keys()); + + // allow empty requests, this is how we clean up settings + if (request.persistentSettings().isEmpty() == false) { + validate(request); + } + + ClusterState state = prevState.state(); + + TransportClusterUpdateSettingsAction.ClusterUpdateSettingsTask updateSettingsTask = + new TransportClusterUpdateSettingsAction.ClusterUpdateSettingsTask(clusterSettings, request); + + state = updateSettingsTask.execute(state); + Set currentKeys = request.persistentSettings() + .keySet() + .stream() + .filter(k -> request.persistentSettings().hasValue(k)) + .collect(Collectors.toSet()); + + return new TransformState(state, currentKeys); + } +} diff --git a/server/src/main/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsAction.java b/server/src/main/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsAction.java deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/server/src/test/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsActionTests.java b/server/src/test/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsActionTests.java new file mode 100644 index 0000000000000..3ff01b2627021 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsActionTests.java @@ -0,0 +1,100 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.action; + +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.immutablestate.TransformState; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; + +import java.util.Collections; + +import static org.hamcrest.Matchers.containsInAnyOrder; + +public class ImmutableClusterSettingsActionTests extends ESTestCase { + + private TransformState processJSON(ImmutableClusterSettingsAction action, TransformState prevState, String json) throws Exception { + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { + return action.transform(parser.map(), prevState); + } + } + + public void testValidation() throws Exception { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + TransformState prevState = new TransformState(state, Collections.emptySet()); + ImmutableClusterSettingsAction action = new ImmutableClusterSettingsAction(clusterSettings); + + String badPolicyJSON = """ + { + "indices.recovery.min_bytes_per_sec": "50mb" + }"""; + + assertEquals( + "persistent setting [indices.recovery.min_bytes_per_sec], not recognized", + expectThrows(IllegalArgumentException.class, () -> processJSON(action, prevState, badPolicyJSON)).getMessage() + ); + } + + public void testSetUnsetSettings() throws Exception { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + ClusterState state = ClusterState.builder(new ClusterName("elasticsearch")).build(); + TransformState prevState = new TransformState(state, Collections.emptySet()); + ImmutableClusterSettingsAction action = new ImmutableClusterSettingsAction(clusterSettings); + + String emptyJSON = ""; + + TransformState updatedState = processJSON(action, prevState, emptyJSON); + assertEquals(0, updatedState.keys().size()); + assertEquals(prevState.state(), updatedState.state()); + + String settingsJSON = """ + { + "indices.recovery.max_bytes_per_sec": "50mb", + "cluster": { + "remote": { + "cluster_one": { + "seeds": [ + "127.0.0.1:9300" + ] + } + } + } + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, settingsJSON); + assertThat(updatedState.keys(), containsInAnyOrder("indices.recovery.max_bytes_per_sec", "cluster.remote.cluster_one.seeds")); + assertEquals("50mb", updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + assertEquals("[127.0.0.1:9300]", updatedState.state().metadata().persistentSettings().get("cluster.remote.cluster_one.seeds")); + + String oneSettingJSON = """ + { + "indices.recovery.max_bytes_per_sec": "25mb" + }"""; + + prevState = updatedState; + updatedState = processJSON(action, prevState, oneSettingJSON); + assertThat(updatedState.keys(), containsInAnyOrder("indices.recovery.max_bytes_per_sec")); + assertEquals("25mb", updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + assertNull(updatedState.state().metadata().persistentSettings().get("cluster.remote.cluster_one.seeds")); + + prevState = updatedState; + updatedState = processJSON(action, prevState, emptyJSON); + assertEquals(0, updatedState.keys().size()); + assertNull(updatedState.state().metadata().persistentSettings().get("indices.recovery.max_bytes_per_sec")); + } +} diff --git a/server/src/test/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsActionTests.java b/server/src/test/java/org/elasticsearch/immutablestate/action/OperatorClusterUpdateSettingsActionTests.java deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/x-pack/plugin/ilm/src/main/java/module-info.java b/x-pack/plugin/ilm/src/main/java/module-info.java index 69348eedcfc60..10e68f0a13ff2 100644 --- a/x-pack/plugin/ilm/src/main/java/module-info.java +++ b/x-pack/plugin/ilm/src/main/java/module-info.java @@ -1,3 +1,5 @@ +import org.elasticsearch.xpack.ilm.immutablestate.ILMImmutableStateHandlerProvider; + /* * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one * or more contributor license agreements. Licensed under the Elastic License @@ -17,5 +19,5 @@ exports org.elasticsearch.xpack.slm.action to org.elasticsearch.server; exports org.elasticsearch.xpack.slm to org.elasticsearch.server; - provides org.elasticsearch.operator.OperatorHandlerProvider with org.elasticsearch.xpack.ilm.operator.ILMOperatorHandlerProvider; + provides org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider with ILMImmutableStateHandlerProvider; } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java index 5fc29ed66f5fc..4d3cf3d613a11 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java @@ -29,7 +29,7 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction.Request; -import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; +import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.List; import java.util.Optional; @@ -73,7 +73,11 @@ public DeleteLifecyclePolicyTask(Request request, ActionListener operatorHandlerName() { - return Optional.of(OperatorLifecycleAction.NAME); + protected Optional immutableStateHandlerName() { + return Optional.of(ImmutableLifecycleAction.NAME); } @Override diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java index d1d75d7333d85..08b9147f55376 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleAction.java @@ -41,7 +41,7 @@ import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction.Request; import org.elasticsearch.xpack.core.slm.SnapshotLifecycleMetadata; -import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; +import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.time.Instant; import java.util.HashMap; @@ -146,7 +146,10 @@ public UpdateLifecyclePolicyTask( } /** - * Constructor used in operator mode. It disables verbose logging and has no filtered headers. + * Used by the {@link org.elasticsearch.immutablestate.ImmutableClusterStateHandler} for ILM + * {@link ImmutableLifecycleAction} + *

+ * It disables verbose logging and has no filtered headers. * * @param request * @param licenseState @@ -313,8 +316,8 @@ protected ClusterBlockException checkBlock(Request request, ClusterState state) } @Override - protected Optional operatorHandlerName() { - return Optional.of(OperatorLifecycleAction.NAME); + protected Optional immutableStateHandlerName() { + return Optional.of(ImmutableLifecycleAction.NAME); } @Override diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java new file mode 100644 index 0000000000000..764a087dd9264 --- /dev/null +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java @@ -0,0 +1,31 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.ilm.immutablestate; + +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; + +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +/** + * ILM Provider implementation for the {@link ImmutableClusterStateHandlerProvider} service interface + */ +public class ILMImmutableStateHandlerProvider implements ImmutableClusterStateHandlerProvider { + private static final Set> handlers = ConcurrentHashMap.newKeySet(); + + @Override + public Collection> handlers() { + return handlers; + } + + public static void handler(ImmutableClusterStateHandler handler) { + handlers.add(handler); + } +} diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java similarity index 86% rename from x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java rename to x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java index 0092db643568c..9efaa3434915b 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/action/OperatorLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java @@ -5,13 +5,13 @@ * 2.0. */ -package org.elasticsearch.xpack.ilm.operator.action; +package org.elasticsearch.xpack.ilm.immutablestate.action; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.TransformState; import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.operator.OperatorHandler; -import org.elasticsearch.operator.TransformState; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.XContentParser; import org.elasticsearch.xcontent.XContentParserConfiguration; @@ -34,13 +34,13 @@ import static org.elasticsearch.common.xcontent.XContentHelper.mapToXContentParser; /** - * This {@link OperatorHandler} is responsible for CRUD operations on ILM policies in - * operator mode, e.g. file based settings. + * This {@link org.elasticsearch.immutablestate.ImmutableClusterStateHandler} is responsible for immutable state + * CRUD operations on ILM policies in, e.g. file based settings. *

* Internally it uses {@link TransportPutLifecycleAction} and * {@link TransportDeleteLifecycleAction} to add, update and delete ILM policies. */ -public class OperatorLifecycleAction implements OperatorHandler { +public class ImmutableLifecycleAction implements ImmutableClusterStateHandler { private final NamedXContentRegistry xContentRegistry; private final Client client; @@ -48,7 +48,7 @@ public class OperatorLifecycleAction implements OperatorHandler public static final String NAME = "ilm"; - public OperatorLifecycleAction(NamedXContentRegistry xContentRegistry, Client client, XPackLicenseState licenseState) { + public ImmutableLifecycleAction(NamedXContentRegistry xContentRegistry, Client client, XPackLicenseState licenseState) { this.xContentRegistry = xContentRegistry; this.client = client; this.licenseState = licenseState; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java deleted file mode 100644 index 5577401719cfd..0000000000000 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/operator/ILMOperatorHandlerProvider.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License - * 2.0; you may not use this file except in compliance with the Elastic License - * 2.0. - */ - -package org.elasticsearch.xpack.ilm.operator; - -import org.elasticsearch.operator.OperatorHandler; -import org.elasticsearch.operator.OperatorHandlerProvider; - -import java.util.Collection; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; - -/** - * ILM Provider implementation for the OperatorHandlerProvider service interface - */ -public class ILMOperatorHandlerProvider implements OperatorHandlerProvider { - private static final Set> handlers = ConcurrentHashMap.newKeySet(); - - @Override - public Collection> handlers() { - return handlers; - } - - public static void handler(OperatorHandler handler) { - handlers.add(handler); - } -} diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java index 51df10fb7a190..db1edda71b39f 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java @@ -14,13 +14,13 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; -import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; +import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.mockito.Mockito.mock; public class TransportDeleteLifecycleActionTests extends ESTestCase { - public void testOperatorHandler() { + public void testImmutableHandler() { TransportDeleteLifecycleAction putAction = new TransportDeleteLifecycleAction( mock(TransportService.class), mock(ClusterService.class), @@ -28,7 +28,7 @@ public void testOperatorHandler() { mock(ActionFilters.class), mock(IndexNameExpressionResolver.class) ); - assertEquals(OperatorLifecycleAction.NAME, putAction.operatorHandlerName().get()); + assertEquals(ImmutableLifecycleAction.NAME, putAction.immutableStateHandlerName().get()); DeleteLifecycleAction.Request request = new DeleteLifecycleAction.Request("my_timeseries_lifecycle2"); assertThat(putAction.modifiedKeys(request), containsInAnyOrder("my_timeseries_lifecycle2")); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java index 6731957cb43ab..030bd55703e94 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java @@ -23,7 +23,7 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.ilm.LifecyclePolicyTestsUtils; -import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; +import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.Map; @@ -46,7 +46,7 @@ public void testIsNoop() { assertFalse(TransportPutLifecycleAction.isNoopUpdate(null, policy1, headers1)); } - public void testOperatorHandler() throws Exception { + public void testImmutableStateHandler() throws Exception { TransportPutLifecycleAction putAction = new TransportPutLifecycleAction( mock(TransportService.class), mock(ClusterService.class), @@ -57,7 +57,7 @@ public void testOperatorHandler() throws Exception { mock(XPackLicenseState.class), mock(Client.class) ); - assertEquals(OperatorLifecycleAction.NAME, putAction.operatorHandlerName().get()); + assertEquals(ImmutableLifecycleAction.NAME, putAction.immutableStateHandlerName().get()); String json = """ { diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java similarity index 93% rename from x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java rename to x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java index ed279c213693d..4f8c672a24ab5 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/operator/OperatorILMControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java @@ -5,15 +5,15 @@ * 2.0. */ -package org.elasticsearch.xpack.ilm.action.operator; +package org.elasticsearch.xpack.ilm.action.immutablestate; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.immutablestate.TransformState; import org.elasticsearch.license.XPackLicenseState; -import org.elasticsearch.operator.TransformState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; import org.elasticsearch.xcontent.ParseField; @@ -38,7 +38,7 @@ import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.ilm.UnfollowAction; import org.elasticsearch.xpack.core.ilm.WaitForSnapshotAction; -import org.elasticsearch.xpack.ilm.operator.action.OperatorLifecycleAction; +import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.ArrayList; import java.util.Arrays; @@ -49,7 +49,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -public class OperatorILMControllerTests extends ESTestCase { +public class ImmutableILMStateControllerTests extends ESTestCase { protected NamedXContentRegistry xContentRegistry() { List entries = new ArrayList<>(ClusterModule.getNamedXWriteables()); @@ -86,7 +86,7 @@ protected NamedXContentRegistry xContentRegistry() { return new NamedXContentRegistry(entries); } - private TransformState processJSON(OperatorLifecycleAction action, TransformState prevState, String json) throws Exception { + private TransformState processJSON(ImmutableLifecycleAction action, TransformState prevState, String json) throws Exception { try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { return action.transform(parser.map(), prevState); } @@ -98,7 +98,7 @@ public void testValidationFails() { final ClusterName clusterName = new ClusterName("elasticsearch"); ClusterState state = ClusterState.builder(clusterName).build(); - OperatorLifecycleAction action = new OperatorLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); + ImmutableLifecycleAction action = new ImmutableLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); TransformState prevState = new TransformState(state, Collections.emptySet()); String badPolicyJSON = """ @@ -127,7 +127,7 @@ public void testActionAddRemove() throws Exception { ClusterState state = ClusterState.builder(clusterName).build(); - OperatorLifecycleAction action = new OperatorLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); + ImmutableLifecycleAction action = new ImmutableLifecycleAction(xContentRegistry(), client, mock(XPackLicenseState.class)); String emptyJSON = ""; From 2c11e746ac4faecb3f775db07aa48b791aa8dc26 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Thu, 30 Jun 2022 15:49:02 -0400 Subject: [PATCH 04/14] Address PR review feedback --- .../plugin/ilm/src/main/java/module-info.java | 2 +- .../ILMImmutableStateHandlerProvider.java | 7 +++-- .../xpack/ilm/IndexLifecycle.java | 5 ++++ .../action/ImmutableLifecycleAction.java | 4 +-- .../TransportDeleteLifecycleAction.java | 4 +-- .../action/TransportPutLifecycleAction.java | 30 ++++++++----------- .../ImmutableILMStateControllerTests.java | 3 +- .../TransportDeleteLifecycleActionTests.java | 1 - .../TransportPutLifecycleActionTests.java | 1 - 9 files changed, 25 insertions(+), 32 deletions(-) rename x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/{immutablestate => }/ILMImmutableStateHandlerProvider.java (81%) rename x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/{immutablestate => }/action/ImmutableLifecycleAction.java (95%) rename x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/{immutablestate => }/ImmutableILMStateControllerTests.java (98%) diff --git a/x-pack/plugin/ilm/src/main/java/module-info.java b/x-pack/plugin/ilm/src/main/java/module-info.java index 10e68f0a13ff2..b15ff4baecb30 100644 --- a/x-pack/plugin/ilm/src/main/java/module-info.java +++ b/x-pack/plugin/ilm/src/main/java/module-info.java @@ -1,4 +1,4 @@ -import org.elasticsearch.xpack.ilm.immutablestate.ILMImmutableStateHandlerProvider; +import org.elasticsearch.xpack.ilm.ILMImmutableStateHandlerProvider; /* * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ILMImmutableStateHandlerProvider.java similarity index 81% rename from x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java rename to x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ILMImmutableStateHandlerProvider.java index 764a087dd9264..afd2397b8fb6f 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/ILMImmutableStateHandlerProvider.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ILMImmutableStateHandlerProvider.java @@ -5,11 +5,12 @@ * 2.0. */ -package org.elasticsearch.xpack.ilm.immutablestate; +package org.elasticsearch.xpack.ilm; import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; import org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; +import java.util.Arrays; import java.util.Collection; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -25,7 +26,7 @@ public Collection> handlers() { return handlers; } - public static void handler(ImmutableClusterStateHandler handler) { - handlers.add(handler); + public static void registerHandlers(ImmutableClusterStateHandler... stateHandlers) { + handlers.addAll(Arrays.asList(stateHandlers)); } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java index c18b0d632a8bc..b7f824b4c2845 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycle.java @@ -83,6 +83,7 @@ import org.elasticsearch.xpack.core.slm.action.PutSnapshotLifecycleAction; import org.elasticsearch.xpack.core.slm.action.StartSLMAction; import org.elasticsearch.xpack.core.slm.action.StopSLMAction; +import org.elasticsearch.xpack.ilm.action.ImmutableLifecycleAction; import org.elasticsearch.xpack.ilm.action.RestDeleteLifecycleAction; import org.elasticsearch.xpack.ilm.action.RestExplainLifecycleAction; import org.elasticsearch.xpack.ilm.action.RestGetLifecycleAction; @@ -267,6 +268,10 @@ public Collection createComponents( components.addAll(Arrays.asList(snapshotLifecycleService.get(), snapshotHistoryStore.get(), snapshotRetentionService.get())); ilmHealthIndicatorService.set(new IlmHealthIndicatorService(clusterService)); slmHealthIndicatorService.set(new SlmHealthIndicatorService(clusterService)); + + ILMImmutableStateHandlerProvider.registerHandlers( + new ImmutableLifecycleAction(xContentRegistry, client, XPackPlugin.getSharedLicenseState()) + ); return components; } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java similarity index 95% rename from x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java rename to x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java index 9efaa3434915b..68b6d994ae63f 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/immutablestate/action/ImmutableLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java @@ -5,7 +5,7 @@ * 2.0. */ -package org.elasticsearch.xpack.ilm.immutablestate.action; +package org.elasticsearch.xpack.ilm.action; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterState; @@ -18,8 +18,6 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.core.template.LifecyclePolicyConfig; -import org.elasticsearch.xpack.ilm.action.TransportDeleteLifecycleAction; -import org.elasticsearch.xpack.ilm.action.TransportPutLifecycleAction; import java.io.IOException; import java.util.ArrayList; diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java index 4d3cf3d613a11..29282740072f4 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleAction.java @@ -29,7 +29,6 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction.Request; -import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.List; import java.util.Optional; @@ -76,9 +75,8 @@ public DeleteLifecyclePolicyTask(Request request, ActionListener filteredHeaders, NamedXContentRegistry xContentRegistry, - Client client, - boolean verboseLogging + Client client ) { super(request, listener); this.request = request; @@ -142,7 +140,7 @@ public UpdateLifecyclePolicyTask( this.filteredHeaders = filteredHeaders; this.xContentRegistry = xContentRegistry; this.client = client; - this.verboseLogging = verboseLogging; + this.verboseLogging = true; } /** @@ -150,19 +148,15 @@ public UpdateLifecyclePolicyTask( * {@link ImmutableLifecycleAction} *

* It disables verbose logging and has no filtered headers. - * - * @param request - * @param licenseState - * @param xContentRegistry - * @param client */ - public UpdateLifecyclePolicyTask( - Request request, - XPackLicenseState licenseState, - NamedXContentRegistry xContentRegistry, - Client client - ) { - this(request, null, licenseState, new HashMap<>(), xContentRegistry, client, false); + UpdateLifecyclePolicyTask(Request request, XPackLicenseState licenseState, NamedXContentRegistry xContentRegistry, Client client) { + super(request, null); + this.request = request; + this.licenseState = licenseState; + this.filteredHeaders = Collections.emptyMap(); + this.xContentRegistry = xContentRegistry; + this.client = client; + this.verboseLogging = false; } @Override diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java similarity index 98% rename from x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java rename to x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index 4f8c672a24ab5..a9c776f100623 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/immutablestate/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -5,7 +5,7 @@ * 2.0. */ -package org.elasticsearch.xpack.ilm.action.immutablestate; +package org.elasticsearch.xpack.ilm.action; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterModule; @@ -38,7 +38,6 @@ import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.ilm.UnfollowAction; import org.elasticsearch.xpack.core.ilm.WaitForSnapshotAction; -import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.ArrayList; import java.util.Arrays; diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java index db1edda71b39f..588eee06777d8 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportDeleteLifecycleActionTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.ilm.action.DeleteLifecycleAction; -import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.mockito.Mockito.mock; diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java index 030bd55703e94..4a54b4cf696a7 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/TransportPutLifecycleActionTests.java @@ -23,7 +23,6 @@ import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.action.PutLifecycleAction; import org.elasticsearch.xpack.ilm.LifecyclePolicyTestsUtils; -import org.elasticsearch.xpack.ilm.immutablestate.action.ImmutableLifecycleAction; import java.util.Map; From c636026713a50c6be237803b8706c53ebb236cd7 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Thu, 30 Jun 2022 17:54:26 -0400 Subject: [PATCH 05/14] Add ImmutableClusterStateController --- .../elasticsearch/action/ActionModule.java | 33 +- .../master/TransportMasterNodeAction.java | 33 ++ .../ImmutableClusterStateController.java | 306 +++++++++++ .../ImmutableStateUpdateErrorTask.java | 90 ++++ .../ImmutableStateUpdateStateTask.java | 174 +++++++ .../service/StateVersionMetadata.java | 57 +++ .../java/org/elasticsearch/node/Node.java | 5 +- .../action/ActionModuleTests.java | 4 + .../ClusterUpdateSettingsRequestTests.java | 49 ++ .../TransportMasterNodeActionTests.java | 112 +++- .../ImmutableClusterStateControllerTests.java | 483 ++++++++++++++++++ .../ImmutableILMStateControllerTests.java | 76 +++ .../xpack/security/SecurityTests.java | 1 + 13 files changed, 1420 insertions(+), 3 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java create mode 100644 server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java create mode 100644 server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java create mode 100644 server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java create mode 100644 server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index 1858d8ab28784..b24f8c60b979c 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -251,6 +251,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.NamedRegistry; import org.elasticsearch.common.inject.AbstractModule; import org.elasticsearch.common.inject.TypeLiteral; @@ -262,6 +263,10 @@ import org.elasticsearch.gateway.TransportNodesListGatewayStartedShards; import org.elasticsearch.health.GetHealthAction; import org.elasticsearch.health.RestGetHealthAction; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; +import org.elasticsearch.immutablestate.service.ImmutableClusterStateController; import org.elasticsearch.index.seqno.GlobalCheckpointSyncAction; import org.elasticsearch.index.seqno.RetentionLeaseActions; import org.elasticsearch.indices.SystemIndices; @@ -273,6 +278,7 @@ import org.elasticsearch.persistent.UpdatePersistentTaskStatusAction; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; +import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.plugins.interceptor.RestInterceptorActionPlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; @@ -448,6 +454,7 @@ public class ActionModule extends AbstractModule { private final RequestValidators mappingRequestValidators; private final RequestValidators indicesAliasesRequestRequestValidators; private final ThreadPool threadPool; + private final ImmutableClusterStateController immutableStateController; public ActionModule( Settings settings, @@ -460,7 +467,8 @@ public ActionModule( NodeClient nodeClient, CircuitBreakerService circuitBreakerService, UsageService usageService, - SystemIndices systemIndices + SystemIndices systemIndices, + ClusterService clusterService ) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; @@ -511,6 +519,7 @@ public ActionModule( ); restController = new RestController(headers, restInterceptor, nodeClient, circuitBreakerService, usageService); + immutableStateController = new ImmutableClusterStateController(clusterService); } public Map> getActions() { @@ -888,6 +897,24 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestCatAction(catActions)); } + /** + * Initializes the immutable cluster state handlers for Elasticsearch and it's modules/plugins + * + * @param pluginsService needed to load all modules/plugins immutable state handlers through SPI + */ + public void initImmutableClusterStateHandlers(PluginsService pluginsService) { + List> handlers = new ArrayList<>(); + + List pluginHandlers = pluginsService.loadServiceProviders( + ImmutableClusterStateHandlerProvider.class + ); + + handlers.add(new ImmutableClusterSettingsAction(clusterSettings)); + pluginHandlers.forEach(h -> handlers.addAll(h.handlers())); + + immutableStateController.initHandlers(handlers); + } + @Override protected void configure() { bind(ActionFilters.class).toInstance(actionFilters); @@ -920,4 +947,8 @@ public ActionFilters getActionFilters() { public RestController getRestController() { return restController; } + + public ImmutableClusterStateController getImmutableClusterStateController() { + return immutableStateController; + } } diff --git a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java index 777b0c2356a72..26f9de8dd380b 100644 --- a/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java +++ b/server/src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeAction.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.block.ClusterBlockLevel; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; @@ -42,7 +43,9 @@ import org.elasticsearch.transport.TransportException; import org.elasticsearch.transport.TransportService; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.Optional; import java.util.Set; import java.util.function.Predicate; @@ -172,9 +175,39 @@ protected Set modifiedKeys(Request request) { return Collections.emptySet(); } + // package private for testing + void validateForImmutableState(Request request, ClusterState state) { + Optional handlerName = immutableStateHandlerName(); + assert handlerName.isPresent(); + + Set modified = modifiedKeys(request); + List errors = new ArrayList<>(); + + for (ImmutableStateMetadata metadata : state.metadata().immutableStateMetadata().values()) { + Set conflicts = metadata.conflicts(handlerName.get(), modified); + if (conflicts.isEmpty() == false) { + errors.add(format("[%s] set as read-only by [%s]", String.join(",", conflicts), metadata.namespace())); + } + } + + if (errors.isEmpty() == false) { + throw new IllegalArgumentException( + format("Failed to process request [%s] with errors: %s", request, String.join(System.lineSeparator(), errors)) + ); + } + } + + // package private for testing + boolean supportsImmutableState() { + return immutableStateHandlerName().isPresent(); + } + @Override protected void doExecute(Task task, final Request request, ActionListener listener) { ClusterState state = clusterService.state(); + if (supportsImmutableState()) { + validateForImmutableState(request, state); + } logger.trace("starting processing request [{}] with cluster state version [{}]", request, state.version()); if (task != null) { request.setParentTask(clusterService.localNode().getId(), task.getId()); diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java new file mode 100644 index 0000000000000..f0ca10473fec2 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java @@ -0,0 +1,306 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.service; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateTaskConfig; +import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Priority; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.xcontent.ConstructingObjectParser; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.XContentParser; + +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Collectors; + +import static org.elasticsearch.core.Strings.format; + +/** + * Controller class for applying file based settings to ClusterState. + * This class contains the logic about validation, ordering and applying of + * the cluster state specified in a file. + */ +public class ImmutableClusterStateController { + private static final Logger logger = LogManager.getLogger(ImmutableClusterStateController.class); + + public static final String SETTINGS = "settings"; + public static final String METADATA = "metadata"; + + Map> handlers = null; + final ClusterService clusterService; + + public ImmutableClusterStateController(ClusterService clusterService) { + this.clusterService = clusterService; + } + + /** + * Initializes the controller with the currently implemented state handlers + * + * @param handlerList the list of supported immutable cluster state handlers + */ + public void initHandlers(List> handlerList) { + handlers = handlerList.stream().collect(Collectors.toMap(ImmutableClusterStateHandler::name, Function.identity())); + } + + /** + * A package class containing the composite immutable cluster state + *

+ * Apart from the cluster state we want to store as immutable, the package requires that + * you supply the version metadata. This version metadata (see {@link StateVersionMetadata}) is checked to ensure + * that the update is safe, and it's not unnecessarily repeated. + */ + public static class Package { + public static final ParseField STATE_FIELD = new ParseField("state"); + public static final ParseField METADATA_FIELD = new ParseField("metadata"); + @SuppressWarnings("unchecked") + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "immutable_cluster_state", + a -> new Package((Map) a[0], (StateVersionMetadata) a[1]) + ); + static { + PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), STATE_FIELD); + PARSER.declareObject(ConstructingObjectParser.constructorArg(), StateVersionMetadata::parse, METADATA_FIELD); + } + + Map state; + StateVersionMetadata metadata; + + /** + * A package class containing the composite immutable cluster state + * @param state a {@link Map} of immutable state handler name and data + * @param metadata a version metadata + */ + public Package(Map state, StateVersionMetadata metadata) { + this.state = state; + this.metadata = metadata; + } + } + + /** + * Saves an immutable cluster state for a given 'namespace' from {@link XContentParser} + * + * @param namespace the namespace under which we'll store the immutable keys in the cluster state metadata + * @param parser the XContentParser to process + * @param errorListener a consumer called with IllegalStateException if the content has errors and the + * cluster state cannot be correctly applied, IncompatibleVersionException if the content is stale or + * incompatible with this node {@link Version}, null if successful. + */ + public void process(String namespace, XContentParser parser, Consumer errorListener) { + Package immutableStatePackage; + + try { + immutableStatePackage = Package.PARSER.apply(parser, null); + } catch (Exception e) { + List errors = List.of(e.getMessage()); + recordErrorState(new ImmutableUpdateErrorState(namespace, -1L, errors, ImmutableStateErrorMetadata.ErrorKind.PARSING)); + logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); + + errorListener.accept(new IllegalStateException("Error processing state change request for " + namespace, e)); + return; + } + + process(namespace, immutableStatePackage, errorListener); + } + + /** + * Saves an immutable cluster state for a given 'namespace' from {@link Package} + * + * @param namespace the namespace under which we'll store the immutable keys in the cluster state metadata + * @param immutableStateFilePackage a {@link Package} composite state object to process + * @param errorListener a consumer called with IllegalStateException if the content has errors and the + * cluster state cannot be correctly applied, IncompatibleVersionException if the content is stale or + * incompatible with this node {@link Version}, null if successful. + */ + public void process(String namespace, Package immutableStateFilePackage, Consumer errorListener) { + Map immutableState = immutableStateFilePackage.state; + StateVersionMetadata stateVersionMetadata = immutableStateFilePackage.metadata; + + LinkedHashSet orderedHandlers; + try { + orderedHandlers = orderedStateHandlers(immutableState.keySet()); + } catch (Exception e) { + List errors = List.of(e.getMessage()); + recordErrorState( + new ImmutableUpdateErrorState( + namespace, + stateVersionMetadata.version(), + errors, + ImmutableStateErrorMetadata.ErrorKind.PARSING + ) + ); + logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); + + errorListener.accept(new IllegalStateException("Error processing state change request for " + namespace, e)); + return; + } + + ClusterState state = clusterService.state(); + ImmutableStateMetadata existingMetadata = state.metadata().immutableStateMetadata().get(namespace); + if (checkMetadataVersion(existingMetadata, stateVersionMetadata, errorListener) == false) { + return; + } + + // Do we need to retry this, or it retries automatically? + clusterService.submitStateUpdateTask( + "immutable cluster state [" + namespace + "]", + new ImmutableStateUpdateStateTask( + namespace, + immutableStateFilePackage, + handlers, + orderedHandlers, + (errorState) -> recordErrorState(errorState), + new ActionListener<>() { + @Override + public void onResponse(ActionResponse.Empty empty) { + logger.info("Successfully applied new cluster state for namespace [{}]", namespace); + errorListener.accept(null); + } + + @Override + public void onFailure(Exception e) { + logger.error("Failed to apply immutable cluster state", e); + errorListener.accept(e); + } + } + ), + ClusterStateTaskConfig.build(Priority.URGENT), + new ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor(namespace, clusterService.getRerouteService()) + ); + } + + // package private for testing + static boolean checkMetadataVersion( + ImmutableStateMetadata existingMetadata, + StateVersionMetadata stateVersionMetadata, + Consumer errorListener + ) { + if (Version.CURRENT.before(stateVersionMetadata.minCompatibleVersion())) { + errorListener.accept( + new IncompatibleVersionException( + format( + "Cluster state version [%s] is not compatible with this Elasticsearch node", + stateVersionMetadata.minCompatibleVersion() + ) + ) + ); + return false; + } + + if (existingMetadata != null && existingMetadata.version() >= stateVersionMetadata.version()) { + errorListener.accept( + new IncompatibleVersionException( + format( + "Not updating cluster state because version [%s] is less or equal to the current metadata version [%s]", + stateVersionMetadata.version(), + existingMetadata.version() + ) + ) + ); + return false; + } + + return true; + } + + record ImmutableUpdateErrorState( + String namespace, + Long version, + List errors, + ImmutableStateErrorMetadata.ErrorKind errorKind + ) {} + + private void recordErrorState(ImmutableUpdateErrorState state) { + clusterService.submitStateUpdateTask( + "immutable cluster state update error for [ " + state.namespace + "]", + new ImmutableStateUpdateErrorTask(state, new ActionListener<>() { + @Override + public void onResponse(ActionResponse.Empty empty) { + logger.info("Successfully applied new immutable error state for namespace [{}]", state.namespace); + } + + @Override + public void onFailure(Exception e) { + logger.error("Failed to apply immutable error cluster state", e); + } + }), + ClusterStateTaskConfig.build(Priority.URGENT), + new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor() + ); + } + + // package private for testing + LinkedHashSet orderedStateHandlers(Set keys) { + LinkedHashSet orderedHandlers = new LinkedHashSet<>(); + LinkedHashSet dependencyStack = new LinkedHashSet<>(); + + for (String key : keys) { + addStateHandler(key, keys, orderedHandlers, dependencyStack); + } + + return orderedHandlers; + } + + private void addStateHandler(String key, Set keys, LinkedHashSet ordered, LinkedHashSet visited) { + if (visited.contains(key)) { + StringBuilder msg = new StringBuilder("Cycle found in settings dependencies: "); + visited.forEach(s -> { + msg.append(s); + msg.append(" -> "); + }); + msg.append(key); + throw new IllegalStateException(msg.toString()); + } + + if (ordered.contains(key)) { + // already added by another dependent handler + return; + } + + visited.add(key); + ImmutableClusterStateHandler handler = handlers.get(key); + + if (handler == null) { + throw new IllegalStateException("Unknown settings definition type: " + key); + } + + for (String dependency : handler.dependencies()) { + if (keys.contains(dependency) == false) { + throw new IllegalStateException("Missing settings dependency definition: " + key + " -> " + dependency); + } + addStateHandler(dependency, keys, ordered, visited); + } + + visited.remove(key); + ordered.add(key); + } + + /** + * {@link IncompatibleVersionException} is thrown when we try to update the cluster state + * without changing the update version id, or if we try to update cluster state on + * an incompatible Elasticsearch version in mixed cluster mode. + */ + public static class IncompatibleVersionException extends RuntimeException { + public IncompatibleVersionException(String message) { + super(message); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java new file mode 100644 index 0000000000000..dd4b336aeded1 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java @@ -0,0 +1,90 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.service; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; +import org.elasticsearch.cluster.ClusterStateTaskListener; +import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; +import org.elasticsearch.cluster.metadata.Metadata; + +import java.util.List; + +/** + * Cluster state update task that sets the error state of the immutable cluster state metadata. + *

+ * This is used when an immutable cluster state update encounters error(s) while processing + * the {@link org.elasticsearch.immutablestate.service.ImmutableClusterStateController.Package}. + */ +public class ImmutableStateUpdateErrorTask implements ClusterStateTaskListener { + + private final ImmutableClusterStateController.ImmutableUpdateErrorState errorState; + private final ActionListener listener; + + public ImmutableStateUpdateErrorTask( + ImmutableClusterStateController.ImmutableUpdateErrorState errorState, + ActionListener listener + ) { + this.errorState = errorState; + this.listener = listener; + } + + private static final Logger logger = LogManager.getLogger(ImmutableStateUpdateErrorTask.class); + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + + ActionListener listener() { + return listener; + } + + ClusterState execute(ClusterState currentState) { + ClusterState.Builder stateBuilder = new ClusterState.Builder(currentState); + Metadata.Builder metadataBuilder = Metadata.builder(currentState.metadata()); + ImmutableStateMetadata immutableMetadata = currentState.metadata().immutableStateMetadata().get(errorState.namespace()); + ImmutableStateMetadata.Builder immMetadataBuilder = ImmutableStateMetadata.builder(errorState.namespace(), immutableMetadata); + immMetadataBuilder.errorMetadata( + new ImmutableStateErrorMetadata(errorState.version(), errorState.errorKind(), errorState.errors()) + ); + metadataBuilder.put(immMetadataBuilder.build()); + ClusterState newState = stateBuilder.metadata(metadataBuilder).build(); + + return newState; + } + + /** + * Immutable cluster error state task executor + */ + public record ImmutableUpdateErrorTaskExecutor() implements ClusterStateTaskExecutor { + + @Override + public ClusterState execute(ClusterState currentState, List> taskContexts) + throws Exception { + for (final var taskContext : taskContexts) { + currentState = taskContext.getTask().execute(currentState); + taskContext.success( + () -> taskContext.getTask().listener().delegateFailure((l, s) -> l.onResponse(ActionResponse.Empty.INSTANCE)) + ); + } + return currentState; + } + + @Override + public void clusterStatePublished(ClusterState newClusterState) { + logger.info("Wrote new error state in immutable metadata"); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java new file mode 100644 index 0000000000000..f50edd24ecafd --- /dev/null +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java @@ -0,0 +1,174 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.service; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; +import org.elasticsearch.cluster.ClusterStateTaskListener; +import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateHandlerMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.routing.RerouteService; +import org.elasticsearch.common.Priority; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.TransformState; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Consumer; + +import static org.elasticsearch.core.Strings.format; + +/** + * Generic immutable cluster state update task + */ +public class ImmutableStateUpdateStateTask implements ClusterStateTaskListener { + private static final Logger logger = LogManager.getLogger(ImmutableStateUpdateStateTask.class); + + private final String namespace; + private final ImmutableClusterStateController.Package immutableStatePackage; + private final Map> handlers; + private final Collection orderedHandlers; + private final Consumer recordErrorState; + private final ActionListener listener; + + public ImmutableStateUpdateStateTask( + String namespace, + ImmutableClusterStateController.Package immutableStatePackage, + Map> handlers, + Collection orderedHandlers, + Consumer recordErrorState, + ActionListener listener + ) { + this.namespace = namespace; + this.immutableStatePackage = immutableStatePackage; + this.handlers = handlers; + this.orderedHandlers = orderedHandlers; + this.recordErrorState = recordErrorState; + this.listener = listener; + } + + @Override + public void onFailure(Exception e) { + listener.onFailure(e); + } + + ActionListener listener() { + return listener; + } + + protected ClusterState execute(ClusterState state) { + ImmutableStateMetadata existingMetadata = state.metadata().immutableStateMetadata().get(namespace); + Map immutableState = immutableStatePackage.state; + StateVersionMetadata stateVersionMetadata = immutableStatePackage.metadata; + + var immutableMetadataBuilder = new ImmutableStateMetadata.Builder(namespace).version(stateVersionMetadata.version()); + List errors = new ArrayList<>(); + + for (var handlerName : orderedHandlers) { + ImmutableClusterStateHandler handler = handlers.get(handlerName); + try { + Set existingKeys = keysForHandler(existingMetadata, handlerName); + TransformState transformState = handler.transform(immutableState.get(handlerName), new TransformState(state, existingKeys)); + state = transformState.state(); + immutableMetadataBuilder.putHandler(new ImmutableStateHandlerMetadata(handlerName, transformState.keys())); + } catch (Exception e) { + errors.add(format("Error processing %s state change: %s", handler.name(), e.getMessage())); + } + } + + if (errors.isEmpty() == false) { + // Check if we had previous error metadata with version information, don't spam with cluster state updates, if the + // version hasn't been updated. + if (existingMetadata != null + && existingMetadata.errorMetadata() != null + && existingMetadata.errorMetadata().version() >= stateVersionMetadata.version()) { + logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); + + throw new ImmutableClusterStateController.IncompatibleVersionException( + format( + "Not updating error state because version [%s] is less or equal to the last state error version [%s]", + stateVersionMetadata.version(), + existingMetadata.errorMetadata().version() + ) + ); + } + + recordErrorState.accept( + new ImmutableClusterStateController.ImmutableUpdateErrorState( + namespace, + stateVersionMetadata.version(), + errors, + ImmutableStateErrorMetadata.ErrorKind.VALIDATION + ) + ); + logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); + + throw new IllegalStateException("Error processing state change request for " + namespace); + } + + // remove the last error if we had previously encountered any + immutableMetadataBuilder.errorMetadata(null); + + ClusterState.Builder stateBuilder = new ClusterState.Builder(state); + Metadata.Builder metadataBuilder = Metadata.builder(state.metadata()).put(immutableMetadataBuilder.build()); + + return stateBuilder.metadata(metadataBuilder).build(); + } + + private Set keysForHandler(ImmutableStateMetadata immutableStateMetadata, String handlerName) { + if (immutableStateMetadata == null || immutableStateMetadata.handlers().get(handlerName) == null) { + return Collections.emptySet(); + } + + return immutableStateMetadata.handlers().get(handlerName).keys(); + } + + /** + * Immutable cluster state update task executor + * + * @param namespace of the state we are updating + * @param rerouteService instance of {@link RerouteService}, so that we can execute reroute after cluster state is published + */ + public record ImmutableUpdateStateTaskExecutor(String namespace, RerouteService rerouteService) + implements + ClusterStateTaskExecutor { + + @Override + public ClusterState execute(ClusterState currentState, List> taskContexts) + throws Exception { + for (final var taskContext : taskContexts) { + currentState = taskContext.getTask().execute(currentState); + taskContext.success(() -> taskContext.getTask().listener().onResponse(ActionResponse.Empty.INSTANCE)); + } + return currentState; + } + + @Override + public void clusterStatePublished(ClusterState newClusterState) { + rerouteService.reroute( + "reroute after applying immutable cluster state for namespace [" + namespace + "]", + Priority.NORMAL, + ActionListener.wrap( + r -> logger.trace("reroute after applying immutable cluster state for [{}] succeeded", namespace), + e -> logger.debug("reroute after applying immutable cluster state failed", e) + ) + ); + } + } +} diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java b/server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java new file mode 100644 index 0000000000000..807197c638d3b --- /dev/null +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.service; + +import org.elasticsearch.Version; +import org.elasticsearch.xcontent.ConstructingObjectParser; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.XContentParser; + +/** + * File settings metadata class that holds information about + * versioning and Elasticsearch version compatibility + */ +public class StateVersionMetadata { + public static final ParseField VERSION = new ParseField("version"); + public static final ParseField COMPATIBILITY = new ParseField("compatibility"); + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "immutable_cluster_state_version_metadata", + a -> { + Long updateId = Long.parseLong((String) a[0]); + Version minCompatVersion = Version.fromString((String) a[1]); + + return new StateVersionMetadata(updateId, minCompatVersion); + } + ); + static { + PARSER.declareString(ConstructingObjectParser.constructorArg(), VERSION); + PARSER.declareString(ConstructingObjectParser.constructorArg(), COMPATIBILITY); + } + + private Long version; + private Version compatibleWith; + + public StateVersionMetadata(Long version, Version compatibleWith) { + this.version = version; + this.compatibleWith = compatibleWith; + } + + public static StateVersionMetadata parse(XContentParser parser, Void v) { + return PARSER.apply(parser, v); + } + + public Long version() { + return version; + } + + public Version minCompatibleVersion() { + return compatibleWith; + } +} diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 519ac92ceb494..ab951deb643d3 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -698,7 +698,8 @@ protected Node( client, circuitBreakerService, usageService, - systemIndices + systemIndices, + clusterService ); modules.add(actionModule); @@ -1037,6 +1038,8 @@ protected Node( logger.debug("initializing HTTP handlers ..."); actionModule.initRestHandlers(() -> clusterService.state().nodesIfRecovered()); + logger.debug("initializing operator handlers ..."); + actionModule.initImmutableClusterStateHandlers(pluginsService); logger.info("initialized"); success = true; diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index a1993bd942f91..ebc869a3aba2c 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -116,6 +116,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() { null, null, usageService, + null, null ); actionModule.initRestHandlers(null); @@ -172,6 +173,7 @@ public String getName() { null, null, usageService, + null, null ); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); @@ -221,6 +223,7 @@ public List getRestHandlers( null, null, usageService, + null, null ); actionModule.initRestHandlers(null); @@ -265,6 +268,7 @@ public void test3rdPartyHandlerIsNotInstalled() { null, null, usageService, + null, null ) ); diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java index 042f7f150788a..d31c53adfae63 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/settings/ClusterUpdateSettingsRequestTests.java @@ -8,9 +8,17 @@ package org.elasticsearch.action.admin.cluster.settings; +import org.elasticsearch.action.support.ActionFilters; +import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.bytes.BytesReference; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.XContentTestUtils; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.transport.TransportService; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentParseException; import org.elasticsearch.xcontent.XContentParser; @@ -21,6 +29,8 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.mockito.Mockito.mock; public class ClusterUpdateSettingsRequestTests extends ESTestCase { @@ -71,4 +81,43 @@ private static ClusterUpdateSettingsRequest createTestItem() { request.transientSettings(ClusterUpdateSettingsResponseTests.randomClusterSettings(0, 2)); return request; } + + public void testOperatorHandler() throws IOException { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + + TransportClusterUpdateSettingsAction action = new TransportClusterUpdateSettingsAction( + mock(TransportService.class), + mock(ClusterService.class), + mock(ThreadPool.class), + mock(ActionFilters.class), + mock(IndexNameExpressionResolver.class), + clusterSettings + ); + + assertEquals(ImmutableClusterSettingsAction.NAME, action.immutableStateHandlerName().get()); + + String oneSettingJSON = """ + { + "persistent": { + "indices.recovery.max_bytes_per_sec": "25mb", + "cluster": { + "remote": { + "cluster_one": { + "seeds": [ + "127.0.0.1:9300" + ] + } + } + } + } + }"""; + + try (XContentParser parser = createParser(XContentType.JSON.xContent(), oneSettingJSON)) { + ClusterUpdateSettingsRequest parsedRequest = ClusterUpdateSettingsRequest.fromXContent(parser); + assertThat( + action.modifiedKeys(parsedRequest), + containsInAnyOrder("indices.recovery.max_bytes_per_sec", "cluster.remote.cluster_one.seeds") + ); + } + } } diff --git a/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java b/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java index 9770b1c42dc0f..5bef002fc3511 100644 --- a/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/support/master/TransportMasterNodeActionTests.java @@ -14,12 +14,14 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.IndicesRequest; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.action.support.ActionFilters; import org.elasticsearch.action.support.ActionTestUtils; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.ThreadedActionListener; import org.elasticsearch.action.support.replication.ClusterStateCreationUtils; +import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.NotMasterException; import org.elasticsearch.cluster.block.ClusterBlock; @@ -27,6 +29,8 @@ import org.elasticsearch.cluster.block.ClusterBlockLevel; import org.elasticsearch.cluster.block.ClusterBlocks; import org.elasticsearch.cluster.coordination.FailedToCommitClusterStateException; +import org.elasticsearch.cluster.metadata.ImmutableStateHandlerMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.Metadata; @@ -41,6 +45,7 @@ import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor; import org.elasticsearch.core.TimeValue; import org.elasticsearch.discovery.MasterNotDiscoveredException; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.indices.TestIndexNameExpressionResolver; import org.elasticsearch.node.NodeClosedException; import org.elasticsearch.rest.RestStatus; @@ -65,6 +70,7 @@ import java.util.HashSet; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; @@ -254,6 +260,63 @@ protected ClusterBlockException checkBlock(Request request, ClusterState state) } } + class ImmutableStateAction extends Action { + ImmutableStateAction(String actionName, TransportService transportService, ClusterService clusterService, ThreadPool threadPool) { + super(actionName, transportService, clusterService, threadPool, ThreadPool.Names.SAME); + } + + @Override + protected Optional immutableStateHandlerName() { + return Optional.of("test_immutable_state_action"); + } + } + + class FakeClusterStateUpdateAction extends TransportMasterNodeAction { + FakeClusterStateUpdateAction( + String actionName, + TransportService transportService, + ClusterService clusterService, + ThreadPool threadPool, + String executor + ) { + super( + actionName, + transportService, + clusterService, + threadPool, + new ActionFilters(new HashSet<>()), + ClusterUpdateSettingsRequest::new, + TestIndexNameExpressionResolver.newInstance(), + Response::new, + executor + ); + } + + @Override + protected void masterOperation( + Task task, + ClusterUpdateSettingsRequest request, + ClusterState state, + ActionListener listener + ) {} + + @Override + protected ClusterBlockException checkBlock(ClusterUpdateSettingsRequest request, ClusterState state) { + return null; + } + + @Override + protected Optional immutableStateHandlerName() { + return Optional.of(ImmutableClusterSettingsAction.NAME); + } + + @Override + protected Set modifiedKeys(ClusterUpdateSettingsRequest request) { + Settings allSettings = Settings.builder().put(request.persistentSettings()).put(request.transientSettings()).build(); + return allSettings.keySet(); + } + } + public void testLocalOperationWithoutBlocks() throws ExecutionException, InterruptedException { final boolean masterOperationFailure = randomBoolean(); @@ -686,7 +749,6 @@ protected ClusterBlockException checkBlock(Request request, ClusterState state) indexNameExpressionResolver.concreteIndexNamesWithSystemIndexAccess(state, request) ); } - }; PlainActionFuture listener = new PlainActionFuture<>(); @@ -697,6 +759,54 @@ protected ClusterBlockException checkBlock(Request request, ClusterState state) assertThat(ex.getCause().getCause(), instanceOf(ClusterBlockException.class)); } + public void testRejectImmutableConflictClusterStateUpdate() { + ImmutableStateHandlerMetadata hmOne = new ImmutableStateHandlerMetadata(ImmutableClusterSettingsAction.NAME, Set.of("a", "b")); + ImmutableStateHandlerMetadata hmThree = new ImmutableStateHandlerMetadata(ImmutableClusterSettingsAction.NAME, Set.of("e", "f")); + ImmutableStateMetadata omOne = ImmutableStateMetadata.builder("namespace_one").putHandler(hmOne).build(); + ImmutableStateMetadata omTwo = ImmutableStateMetadata.builder("namespace_two").putHandler(hmThree).build(); + + Metadata metadata = Metadata.builder().put(omOne).put(omTwo).build(); + + ClusterState clusterState = ClusterState.builder(new ClusterName("test")).metadata(metadata).build(); + + Action noHandler = new Action("internal:testAction", transportService, clusterService, threadPool, ThreadPool.Names.SAME); + + assertFalse(noHandler.supportsImmutableState()); + + noHandler = new ImmutableStateAction("internal:testOpAction", transportService, clusterService, threadPool); + + assertTrue(noHandler.supportsImmutableState()); + + // nothing should happen here, since the request doesn't touch any of the immutable state keys + noHandler.validateForImmutableState(new Request(), clusterState); + + ClusterUpdateSettingsRequest request = new ClusterUpdateSettingsRequest().persistentSettings( + Settings.builder().put("a", "a value").build() + ).transientSettings(Settings.builder().put("e", "e value").build()); + + FakeClusterStateUpdateAction action = new FakeClusterStateUpdateAction( + "internal:testClusterSettings", + transportService, + clusterService, + threadPool, + ThreadPool.Names.SAME + ); + + assertTrue(action.supportsImmutableState()); + + assertTrue( + expectThrows(IllegalArgumentException.class, () -> action.validateForImmutableState(request, clusterState)).getMessage() + .contains("with errors: [a] set as read-only by [namespace_one]\n" + "[e] set as read-only by [namespace_two]") + ); + + ClusterUpdateSettingsRequest okRequest = new ClusterUpdateSettingsRequest().persistentSettings( + Settings.builder().put("m", "m value").build() + ).transientSettings(Settings.builder().put("n", "n value").build()); + + // this should just work, no conflicts + action.validateForImmutableState(okRequest, clusterState); + } + private Runnable blockAllThreads(String executorName) throws Exception { final int numberOfThreads = threadPool.info(executorName).getMax(); final EsThreadPoolExecutor executor = (EsThreadPoolExecutor) threadPool.executor(executorName); diff --git a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java new file mode 100644 index 0000000000000..a7a3b2eb43dca --- /dev/null +++ b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java @@ -0,0 +1,483 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.immutablestate.service; + +import org.elasticsearch.Version; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionResponse; +import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateAckListener; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; +import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateHandlerMetadata; +import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; +import org.elasticsearch.cluster.metadata.Metadata; +import org.elasticsearch.cluster.routing.RerouteService; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.ClusterSettings; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.TransformState; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.XContentParserConfiguration; +import org.elasticsearch.xcontent.XContentType; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.anyOf; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.is; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class ImmutableClusterStateControllerTests extends ESTestCase { + + public void testOperatorController() throws IOException { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterService clusterService = mock(ClusterService.class); + final ClusterName clusterName = new ClusterName("elasticsearch"); + + ClusterState state = ClusterState.builder(clusterName).build(); + when(clusterService.state()).thenReturn(state); + + ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); + controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); + + String testJSON = """ + { + "metadata": { + "version": "1234", + "compatibility": "8.4.0" + }, + "state": { + "cluster_settings": { + "indices.recovery.max_bytes_per_sec": "50mb" + + } + } + """; + + AtomicReference x = new AtomicReference<>(); + + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { + controller.process("operator", parser, (e) -> x.set(e)); + + assertTrue(x.get() instanceof IllegalStateException); + assertEquals("Error processing state change request for operator", x.get().getMessage()); + } + + testJSON = """ + { + "metadata": { + "version": "1234", + "compatibility": "8.4.0" + }, + "state": { + "cluster_settings": { + "indices.recovery.max_bytes_per_sec": "50mb", + "cluster": { + "remote": { + "cluster_one": { + "seeds": [ + "127.0.0.1:9300" + ] + } + } + } + } + } + } + """; + + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { + controller.process("operator", parser, (e) -> { + if (e != null) { + fail("Should not fail"); + } + }); + } + } + + public void testUpdateStateTasks() throws Exception { + ClusterService clusterService = mock(ClusterService.class); + RerouteService rerouteService = mock(RerouteService.class); + + when(clusterService.getRerouteService()).thenReturn(rerouteService); + ClusterState state = ClusterState.builder(new ClusterName("test")).build(); + + ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor taskExecutor = + new ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor("test", clusterService.getRerouteService()); + + AtomicBoolean successCalled = new AtomicBoolean(false); + + ImmutableStateUpdateStateTask task = spy( + new ImmutableStateUpdateStateTask( + "test", + null, + Collections.emptyMap(), + Collections.emptySet(), + (errorState) -> {}, + new ActionListener<>() { + @Override + public void onResponse(ActionResponse.Empty empty) {} + + @Override + public void onFailure(Exception e) {} + } + ) + ); + + doReturn(state).when(task).execute(any()); + + ClusterStateTaskExecutor.TaskContext taskContext = new ClusterStateTaskExecutor.TaskContext<>() { + @Override + public ImmutableStateUpdateStateTask getTask() { + return task; + } + + @Override + public void success(Runnable onPublicationSuccess) { + onPublicationSuccess.run(); + successCalled.set(true); + } + + @Override + public void success(Consumer publishedStateConsumer) {} + + @Override + public void success(Runnable onPublicationSuccess, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void success(Consumer publishedStateConsumer, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void onFailure(Exception failure) {} + }; + + ClusterState newState = taskExecutor.execute(state, List.of(taskContext)); + assertEquals(state, newState); + assertTrue(successCalled.get()); + verify(task, times(1)).execute(any()); + + taskExecutor.clusterStatePublished(state); + verify(rerouteService, times(1)).reroute(anyString(), any(), any()); + } + + public void testErrorStateTask() throws Exception { + ClusterState state = ClusterState.builder(new ClusterName("test")).build(); + + ImmutableStateUpdateErrorTask task = spy( + new ImmutableStateUpdateErrorTask( + new ImmutableClusterStateController.ImmutableUpdateErrorState( + "test", + 1L, + List.of("some parse error", "some io error"), + ImmutableStateErrorMetadata.ErrorKind.PARSING + ), + new ActionListener<>() { + @Override + public void onResponse(ActionResponse.Empty empty) {} + + @Override + public void onFailure(Exception e) {} + } + ) + ); + + ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor.TaskContext taskContext = + new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor.TaskContext<>() { + @Override + public ImmutableStateUpdateErrorTask getTask() { + return task; + } + + @Override + public void success(Runnable onPublicationSuccess) { + onPublicationSuccess.run(); + } + + @Override + public void success(Consumer publishedStateConsumer) {} + + @Override + public void success(Runnable onPublicationSuccess, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void success(Consumer publishedStateConsumer, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void onFailure(Exception failure) {} + }; + + ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor executor = + new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor(); + + ClusterState newState = executor.execute(state, List.of(taskContext)); + + verify(task, times(1)).execute(any()); + + ImmutableStateMetadata operatorMetadata = newState.metadata().immutableStateMetadata().get("test"); + assertNotNull(operatorMetadata); + assertNotNull(operatorMetadata.errorMetadata()); + assertEquals(1L, (long) operatorMetadata.errorMetadata().version()); + assertEquals(ImmutableStateErrorMetadata.ErrorKind.PARSING, operatorMetadata.errorMetadata().errorKind()); + assertThat(operatorMetadata.errorMetadata().errors(), contains("some parse error", "some io error")); + } + + public void testUpdateTaskDuplicateError() { + ImmutableClusterStateHandler dummy = new ImmutableClusterStateHandler<>() { + @Override + public String name() { + return "one"; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + throw new Exception("anything"); + } + }; + + ImmutableStateUpdateStateTask task = spy( + new ImmutableStateUpdateStateTask( + "namespace_one", + new ImmutableClusterStateController.Package(Map.of("one", "two"), new StateVersionMetadata(1L, Version.CURRENT)), + Map.of("one", dummy), + List.of(dummy.name()), + (errorState) -> {}, + new ActionListener<>() { + @Override + public void onResponse(ActionResponse.Empty empty) {} + + @Override + public void onFailure(Exception e) {} + } + ) + ); + + ImmutableStateHandlerMetadata hmOne = new ImmutableStateHandlerMetadata("one", Set.of("a", "b")); + ImmutableStateErrorMetadata emOne = new ImmutableStateErrorMetadata( + 1L, + ImmutableStateErrorMetadata.ErrorKind.VALIDATION, + List.of("Test error 1", "Test error 2") + ); + + ImmutableStateMetadata operatorMetadata = ImmutableStateMetadata.builder("namespace_one") + .errorMetadata(emOne) + .version(1L) + .putHandler(hmOne) + .build(); + + Metadata metadata = Metadata.builder().put(operatorMetadata).build(); + ClusterState state = ClusterState.builder(new ClusterName("test")).metadata(metadata).build(); + + // We exit on duplicate errors before we update the cluster state error metadata + assertEquals( + "Not updating error state because version [1] is less or equal to the last state error version [1]", + expectThrows(ImmutableClusterStateController.IncompatibleVersionException.class, () -> task.execute(state)).getMessage() + ); + + emOne = new ImmutableStateErrorMetadata( + 0L, + ImmutableStateErrorMetadata.ErrorKind.VALIDATION, + List.of("Test error 1", "Test error 2") + ); + + // If we are writing with older error metadata, we should get proper IllegalStateException + operatorMetadata = ImmutableStateMetadata.builder("namespace_one").errorMetadata(emOne).version(0L).putHandler(hmOne).build(); + + metadata = Metadata.builder().put(operatorMetadata).build(); + ClusterState newState = ClusterState.builder(new ClusterName("test")).metadata(metadata).build(); + + // We exit on duplicate errors before we update the cluster state error metadata + assertEquals( + "Error processing state change request for namespace_one", + expectThrows(IllegalStateException.class, () -> task.execute(newState)).getMessage() + ); + } + + public void testCheckMetadataVersion() { + ImmutableStateMetadata operatorMetadata = ImmutableStateMetadata.builder("test").version(123L).build(); + + assertTrue( + ImmutableClusterStateController.checkMetadataVersion( + operatorMetadata, + new StateVersionMetadata(124L, Version.CURRENT), + (e) -> {} + ) + ); + + AtomicReference x = new AtomicReference<>(); + + assertFalse( + ImmutableClusterStateController.checkMetadataVersion( + operatorMetadata, + new StateVersionMetadata(123L, Version.CURRENT), + (e) -> x.set(e) + ) + ); + + assertTrue(x.get() instanceof ImmutableClusterStateController.IncompatibleVersionException); + assertTrue(x.get().getMessage().contains("is less or equal to the current metadata version")); + + assertFalse( + ImmutableClusterStateController.checkMetadataVersion( + operatorMetadata, + new StateVersionMetadata(124L, Version.fromId(Version.CURRENT.id + 1)), + (e) -> x.set(e) + ) + ); + + assertEquals(ImmutableClusterStateController.IncompatibleVersionException.class, x.get().getClass()); + assertTrue(x.get().getMessage().contains("is not compatible with this Elasticsearch node")); + } + + public void testHandlerOrdering() { + ImmutableClusterStateHandler oh1 = new ImmutableClusterStateHandler<>() { + @Override + public String name() { + return "one"; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + return null; + } + + @Override + public Collection dependencies() { + return List.of("two", "three"); + } + }; + + ImmutableClusterStateHandler oh2 = new ImmutableClusterStateHandler<>() { + @Override + public String name() { + return "two"; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + return null; + } + }; + + ImmutableClusterStateHandler oh3 = new ImmutableClusterStateHandler<>() { + @Override + public String name() { + return "three"; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + return null; + } + + @Override + public Collection dependencies() { + return List.of("two"); + } + }; + + ClusterService clusterService = mock(ClusterService.class); + ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); + + controller.initHandlers(List.of(oh1, oh2, oh3)); + Collection ordered = controller.orderedStateHandlers(Set.of("one", "two", "three")); + assertThat(ordered, contains("two", "three", "one")); + + // assure that we bail on unknown handler + assertEquals( + "Unknown settings definition type: four", + expectThrows(IllegalStateException.class, () -> controller.orderedStateHandlers(Set.of("one", "two", "three", "four"))) + .getMessage() + ); + + // assure that we bail on missing dependency link + assertEquals( + "Missing settings dependency definition: one -> three", + expectThrows(IllegalStateException.class, () -> controller.orderedStateHandlers(Set.of("one", "two"))).getMessage() + ); + + // Change the second handler so that we create cycle + oh2 = new ImmutableClusterStateHandler<>() { + @Override + public String name() { + return "two"; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + return null; + } + + @Override + public Collection dependencies() { + return List.of("one"); + } + }; + + controller.initHandlers(List.of(oh1, oh2)); + assertThat( + expectThrows(IllegalStateException.class, () -> controller.orderedStateHandlers(Set.of("one", "two"))).getMessage(), + anyOf( + is("Cycle found in settings dependencies: one -> two -> one"), + is("Cycle found in settings dependencies: two -> one -> two") + ) + ); + } + + public void testDuplicateHandlerNames() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterService clusterService = mock(ClusterService.class); + final ClusterName clusterName = new ClusterName("elasticsearch"); + + ClusterState state = ClusterState.builder(clusterName).build(); + when(clusterService.state()).thenReturn(state); + + ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); + + assertTrue( + expectThrows( + IllegalStateException.class, + () -> controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings), new TestHandler())) + ).getMessage().startsWith("Duplicate key cluster_settings") + ); + } + + class TestHandler implements ImmutableClusterStateHandler { + + @Override + public String name() { + return ImmutableClusterSettingsAction.NAME; + } + + @Override + public TransformState transform(Object source, TransformState prevState) throws Exception { + return prevState; + } + } +} diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index a9c776f100623..5f0a77f0f6ce2 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -11,8 +11,12 @@ import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.immutablestate.TransformState; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; +import org.elasticsearch.immutablestate.service.ImmutableClusterStateController; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -39,10 +43,12 @@ import org.elasticsearch.xpack.core.ilm.UnfollowAction; import org.elasticsearch.xpack.core.ilm.WaitForSnapshotAction; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.mockito.Mockito.mock; @@ -209,4 +215,74 @@ public void testActionAddRemove() throws Exception { ilmMetadata = updatedState.state().metadata().custom(IndexLifecycleMetadata.TYPE, IndexLifecycleMetadata.EMPTY); assertThat(ilmMetadata.getPolicyMetadatas().keySet(), containsInAnyOrder("my_timeseries_lifecycle2")); } + + public void testOperatorController() throws IOException { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterService clusterService = mock(ClusterService.class); + final ClusterName clusterName = new ClusterName("elasticsearch"); + + ClusterState state = ClusterState.builder(clusterName).build(); + when(clusterService.state()).thenReturn(state); + + ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); + controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); + + String testJSON = """ + { + "metadata": { + "version": "1234", + "compatibility": "8.4.0" + }, + "state": { + "cluster_settings": { + "indices.recovery.max_bytes_per_sec": "50mb" + }, + "ilm": { + "my_timeseries_lifecycle": { + "phases": { + "warm": { + "min_age": "10s", + "actions": { + } + }, + "delete": { + "min_age": "30s", + "actions": { + } + } + } + } + } + } + }"""; + + AtomicReference x = new AtomicReference<>(); + + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { + controller.process("operator", parser, (e) -> x.set(e)); + + assertTrue(x.get() instanceof IllegalStateException); + assertEquals("Error processing state change request for operator", x.get().getMessage()); + } + + Client client = mock(Client.class); + when(client.settings()).thenReturn(Settings.EMPTY); + + XPackLicenseState licenseState = mock(XPackLicenseState.class); + + controller.initHandlers( + List.of( + new ImmutableClusterSettingsAction(clusterSettings), + new ImmutableLifecycleAction(xContentRegistry(), client, licenseState) + ) + ); + + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { + controller.process("operator", parser, (e) -> { + if (e != null) { + fail("Should not fail"); + } + }); + } + } } diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 65d4267f541ec..604a521da9a8e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -778,6 +778,7 @@ public void testSecurityRestHandlerInterceptorCanBeInstalled() throws IllegalAcc null, null, usageService, + null, null ); actionModule.initRestHandlers(null); From 4f543935d95abae4bdedaf0cf652be6a35536012 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 5 Jul 2022 10:34:19 -0400 Subject: [PATCH 06/14] Fix test to run the transforms --- .../ImmutableILMStateControllerTests.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index 5f0a77f0f6ce2..8ef6a4b3d4717 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -11,12 +11,15 @@ import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ClusterStateAckListener; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.immutablestate.TransformState; import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.immutablestate.service.ImmutableClusterStateController; +import org.elasticsearch.immutablestate.service.ImmutableStateUpdateStateTask; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -42,6 +45,7 @@ import org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType; import org.elasticsearch.xpack.core.ilm.UnfollowAction; import org.elasticsearch.xpack.core.ilm.WaitForSnapshotAction; +import org.mockito.stubbing.Answer; import java.io.IOException; import java.util.ArrayList; @@ -49,8 +53,12 @@ import java.util.Collections; import java.util.List; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -277,6 +285,45 @@ public void testOperatorController() throws IOException { ) ); + doAnswer((Answer) invocation -> { + Object[] args = invocation.getArguments(); + + if ((args[3] instanceof ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) == false) { + fail("Should have gotten a state update task to execute, instead got: " + args[3].getClass().getName()); + } + + ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor task = + (ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) args[3]; + + ClusterStateTaskExecutor.TaskContext context = new ClusterStateTaskExecutor.TaskContext<>() { + @Override + public ImmutableStateUpdateStateTask getTask() { + return (ImmutableStateUpdateStateTask) args[1]; + } + + @Override + public void success(Runnable onPublicationSuccess) {} + + @Override + public void success(Consumer publishedStateConsumer) {} + + @Override + public void success(Runnable onPublicationSuccess, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void success(Consumer publishedStateConsumer, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void onFailure(Exception failure) { + fail("Shouldn't fail here"); + } + }; + + task.execute(state, List.of(context)); + + return null; + }).when(clusterService).submitStateUpdateTask(anyString(), any(), any(), any()); + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { controller.process("operator", parser, (e) -> { if (e != null) { From 16c3272a424c984f2cbb315a342926f0f9bff206 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 5 Jul 2022 20:54:21 -0400 Subject: [PATCH 07/14] Refactor to split out parsing in handlers --- .../org/elasticsearch/common/util/Maps.java | 14 -- .../ImmutableClusterStateHandler.java | 20 ++- .../ImmutableClusterSettingsAction.java | 16 +- .../ImmutableClusterStateController.java | 86 +++++---- .../ImmutableStateUpdateStateTask.java | 12 +- ...rsionMetadata.java => PackageVersion.java} | 21 +-- .../ImmutableClusterStateHandlerTests.java | 59 +------ .../ImmutableClusterStateControllerTests.java | 53 ++++-- .../ilm/action/ImmutableLifecycleAction.java | 37 ++-- .../ImmutableILMStateControllerTests.java | 167 ++++++++++++++---- 10 files changed, 278 insertions(+), 207 deletions(-) rename server/src/main/java/org/elasticsearch/immutablestate/service/{StateVersionMetadata.java => PackageVersion.java} (70%) diff --git a/server/src/main/java/org/elasticsearch/common/util/Maps.java b/server/src/main/java/org/elasticsearch/common/util/Maps.java index 5b02f9af64a06..f0a7f464ed8f3 100644 --- a/server/src/main/java/org/elasticsearch/common/util/Maps.java +++ b/server/src/main/java/org/elasticsearch/common/util/Maps.java @@ -284,18 +284,4 @@ static int capacity(int expectedSize) { return expectedSize < 2 ? expectedSize + 1 : (int) (expectedSize / 0.75 + 1.0); } - /** - * Convenience method to convert the passed in input object as a map with String keys. - * - * @param input the input passed into the operator handler after parsing the content - * @return - */ - @SuppressWarnings("unchecked") - public static Map asMap(Object input) { - if (input instanceof Map source) { - return (Map) source; - } - throw new IllegalStateException("Unsupported input format"); - } - } diff --git a/server/src/main/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandler.java b/server/src/main/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandler.java index f8428a9423c51..5ab1023d9c0f1 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandler.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandler.java @@ -10,7 +10,9 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.master.MasterNodeRequest; +import org.elasticsearch.xcontent.XContentParser; +import java.io.IOException; import java.util.Collection; import java.util.Collections; @@ -35,7 +37,6 @@ public interface ImmutableClusterStateHandler { * cluster state update and the cluster state is typically supplied as a combined content, * unlike the REST handlers. This name must match a desired content key name in the combined * cluster state update, e.g. "ilm" or "cluster_settings" (for persistent cluster settings update). - *

* * @return a String with the handler name, e.g "ilm". */ @@ -51,7 +52,6 @@ public interface ImmutableClusterStateHandler { * state in one go. For that reason, we supply a wrapper class to the cluster state called * {@link TransformState}, which contains the current cluster state as well as any previous keys * set by this handler on prior invocation. - *

* * @param source The parsed information specific to this handler from the combined cluster state content * @param prevState The previous cluster state and keys set by this handler (if any) @@ -70,7 +70,6 @@ public interface ImmutableClusterStateHandler { * to any immutable handler to declare other immutable state handlers it depends on. Given dependencies exist, * the ImmutableClusterStateController will order those handlers such that the handlers that are dependent * on are processed first. - *

* * @return a collection of immutable state handler names */ @@ -85,7 +84,6 @@ default Collection dependencies() { * All implementations of {@link ImmutableClusterStateHandler} should call the request validate method, by calling this default * implementation. To aid in any special validation logic that may need to be implemented by the immutable cluster state handler * we provide this convenience method. - *

* * @param request the master node request that we base this immutable state handler on */ @@ -95,4 +93,18 @@ default void validate(MasterNodeRequest request) { throw new IllegalStateException("Validation error", exception); } } + + /** + * The parse content method which is called during parsing of file based content. + * + *

+ * The immutable state can be provided as XContent, which means that each handler needs + * to implement a method to convert an XContent to an object it can consume later in + * transform + * + * @param parser the XContent parser we are parsing from + * @return + * @throws IOException + */ + T fromXContent(XContentParser parser) throws IOException; } diff --git a/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java b/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java index cd82f17ecfa8f..249914276d83d 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/action/ImmutableClusterSettingsAction.java @@ -15,22 +15,22 @@ import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; import org.elasticsearch.immutablestate.TransformState; +import org.elasticsearch.xcontent.XContentParser; +import java.io.IOException; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.stream.Collectors; -import static org.elasticsearch.common.util.Maps.asMap; - /** * This Action is the immutable state save version of RestClusterUpdateSettingsAction *

* It is used by the ImmutableClusterStateController to update the persistent cluster settings. * Since transient cluster settings are deprecated, this action doesn't support updating transient cluster settings. */ -public class ImmutableClusterSettingsAction implements ImmutableClusterStateHandler { +public class ImmutableClusterSettingsAction implements ImmutableClusterStateHandler> { public static final String NAME = "cluster_settings"; @@ -49,11 +49,12 @@ public String name() { private ClusterUpdateSettingsRequest prepare(Object input, Set previouslySet) { final ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = Requests.clusterUpdateSettingsRequest(); - Map source = asMap(input); Map persistentSettings = new HashMap<>(); Set toDelete = new HashSet<>(previouslySet); - source.forEach((k, v) -> { + Map settings = (Map) input; + + settings.forEach((k, v) -> { persistentSettings.put(k, v); toDelete.remove(k); }); @@ -87,4 +88,9 @@ public TransformState transform(Object input, TransformState prevState) { return new TransformState(state, currentKeys); } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } } diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java index f0ca10473fec2..8edc93261597a 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java @@ -19,11 +19,13 @@ import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; +import org.elasticsearch.core.Tuple; import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; import org.elasticsearch.xcontent.ConstructingObjectParser; import org.elasticsearch.xcontent.ParseField; import org.elasticsearch.xcontent.XContentParser; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -35,21 +37,45 @@ import static org.elasticsearch.core.Strings.format; /** - * Controller class for applying file based settings to ClusterState. + * Controller class for applying immutable state to ClusterState. + *

* This class contains the logic about validation, ordering and applying of - * the cluster state specified in a file. + * the cluster state specified in a file or through plugins/modules. */ public class ImmutableClusterStateController { private static final Logger logger = LogManager.getLogger(ImmutableClusterStateController.class); - public static final String SETTINGS = "settings"; - public static final String METADATA = "metadata"; + public static final ParseField STATE_FIELD = new ParseField("state"); + public static final ParseField METADATA_FIELD = new ParseField("metadata"); Map> handlers = null; final ClusterService clusterService; + @SuppressWarnings("unchecked") + private final ConstructingObjectParser packageParser = new ConstructingObjectParser<>("immutable_cluster_package", a -> { + List> tuples = (List>) a[0]; + Map stateMap = new HashMap<>(); + for (var tuple : tuples) { + stateMap.put(tuple.v1(), tuple.v2()); + } + + return new Package(stateMap, (PackageVersion) a[1]); + }); + + /** + * Controller class for saving immutable ClusterState. + * @param clusterService for fetching and saving the modified state + */ public ImmutableClusterStateController(ClusterService clusterService) { this.clusterService = clusterService; + packageParser.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, name) -> { + if (handlers.containsKey(name) == false) { + throw new IllegalStateException("Missing handler definition for content key [" + name + "]"); + } + p.nextToken(); + return new Tuple<>(name, handlers.get(name).fromXContent(p)); + }, STATE_FIELD); + packageParser.declareObject(ConstructingObjectParser.constructorArg(), PackageVersion::parse, METADATA_FIELD); } /** @@ -65,35 +91,10 @@ public void initHandlers(List> handlerList) { * A package class containing the composite immutable cluster state *

* Apart from the cluster state we want to store as immutable, the package requires that - * you supply the version metadata. This version metadata (see {@link StateVersionMetadata}) is checked to ensure + * you supply the version metadata. This version metadata (see {@link PackageVersion}) is checked to ensure * that the update is safe, and it's not unnecessarily repeated. */ - public static class Package { - public static final ParseField STATE_FIELD = new ParseField("state"); - public static final ParseField METADATA_FIELD = new ParseField("metadata"); - @SuppressWarnings("unchecked") - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( - "immutable_cluster_state", - a -> new Package((Map) a[0], (StateVersionMetadata) a[1]) - ); - static { - PARSER.declareObject(ConstructingObjectParser.constructorArg(), (p, c) -> p.map(), STATE_FIELD); - PARSER.declareObject(ConstructingObjectParser.constructorArg(), StateVersionMetadata::parse, METADATA_FIELD); - } - - Map state; - StateVersionMetadata metadata; - - /** - * A package class containing the composite immutable cluster state - * @param state a {@link Map} of immutable state handler name and data - * @param metadata a version metadata - */ - public Package(Map state, StateVersionMetadata metadata) { - this.state = state; - this.metadata = metadata; - } - } + public record Package(Map state, PackageVersion metadata) {} /** * Saves an immutable cluster state for a given 'namespace' from {@link XContentParser} @@ -108,7 +109,7 @@ public void process(String namespace, XContentParser parser, Consumer Package immutableStatePackage; try { - immutableStatePackage = Package.PARSER.apply(parser, null); + immutableStatePackage = packageParser.apply(parser, null); } catch (Exception e) { List errors = List.of(e.getMessage()); recordErrorState(new ImmutableUpdateErrorState(namespace, -1L, errors, ImmutableStateErrorMetadata.ErrorKind.PARSING)); @@ -132,7 +133,7 @@ public void process(String namespace, XContentParser parser, Consumer */ public void process(String namespace, Package immutableStateFilePackage, Consumer errorListener) { Map immutableState = immutableStateFilePackage.state; - StateVersionMetadata stateVersionMetadata = immutableStateFilePackage.metadata; + PackageVersion packageVersion = immutableStateFilePackage.metadata; LinkedHashSet orderedHandlers; try { @@ -140,12 +141,7 @@ public void process(String namespace, Package immutableStateFilePackage, Consume } catch (Exception e) { List errors = List.of(e.getMessage()); recordErrorState( - new ImmutableUpdateErrorState( - namespace, - stateVersionMetadata.version(), - errors, - ImmutableStateErrorMetadata.ErrorKind.PARSING - ) + new ImmutableUpdateErrorState(namespace, packageVersion.version(), errors, ImmutableStateErrorMetadata.ErrorKind.PARSING) ); logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); @@ -155,7 +151,7 @@ public void process(String namespace, Package immutableStateFilePackage, Consume ClusterState state = clusterService.state(); ImmutableStateMetadata existingMetadata = state.metadata().immutableStateMetadata().get(namespace); - if (checkMetadataVersion(existingMetadata, stateVersionMetadata, errorListener) == false) { + if (checkMetadataVersion(existingMetadata, packageVersion, errorListener) == false) { return; } @@ -190,27 +186,27 @@ public void onFailure(Exception e) { // package private for testing static boolean checkMetadataVersion( ImmutableStateMetadata existingMetadata, - StateVersionMetadata stateVersionMetadata, + PackageVersion packageVersion, Consumer errorListener ) { - if (Version.CURRENT.before(stateVersionMetadata.minCompatibleVersion())) { + if (Version.CURRENT.before(packageVersion.minCompatibleVersion())) { errorListener.accept( new IncompatibleVersionException( format( "Cluster state version [%s] is not compatible with this Elasticsearch node", - stateVersionMetadata.minCompatibleVersion() + packageVersion.minCompatibleVersion() ) ) ); return false; } - if (existingMetadata != null && existingMetadata.version() >= stateVersionMetadata.version()) { + if (existingMetadata != null && existingMetadata.version() >= packageVersion.version()) { errorListener.accept( new IncompatibleVersionException( format( "Not updating cluster state because version [%s] is less or equal to the current metadata version [%s]", - stateVersionMetadata.version(), + packageVersion.version(), existingMetadata.version() ) ) diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java index f50edd24ecafd..11e3e1362476d 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java @@ -74,10 +74,10 @@ ActionListener listener() { protected ClusterState execute(ClusterState state) { ImmutableStateMetadata existingMetadata = state.metadata().immutableStateMetadata().get(namespace); - Map immutableState = immutableStatePackage.state; - StateVersionMetadata stateVersionMetadata = immutableStatePackage.metadata; + Map immutableState = immutableStatePackage.state(); + PackageVersion packageVersion = immutableStatePackage.metadata(); - var immutableMetadataBuilder = new ImmutableStateMetadata.Builder(namespace).version(stateVersionMetadata.version()); + var immutableMetadataBuilder = new ImmutableStateMetadata.Builder(namespace).version(packageVersion.version()); List errors = new ArrayList<>(); for (var handlerName : orderedHandlers) { @@ -97,13 +97,13 @@ protected ClusterState execute(ClusterState state) { // version hasn't been updated. if (existingMetadata != null && existingMetadata.errorMetadata() != null - && existingMetadata.errorMetadata().version() >= stateVersionMetadata.version()) { + && existingMetadata.errorMetadata().version() >= packageVersion.version()) { logger.error("Error processing state change request for [{}] with the following errors [{}]", namespace, errors); throw new ImmutableClusterStateController.IncompatibleVersionException( format( "Not updating error state because version [%s] is less or equal to the last state error version [%s]", - stateVersionMetadata.version(), + packageVersion.version(), existingMetadata.errorMetadata().version() ) ); @@ -112,7 +112,7 @@ protected ClusterState execute(ClusterState state) { recordErrorState.accept( new ImmutableClusterStateController.ImmutableUpdateErrorState( namespace, - stateVersionMetadata.version(), + packageVersion.version(), errors, ImmutableStateErrorMetadata.ErrorKind.VALIDATION ) diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java b/server/src/main/java/org/elasticsearch/immutablestate/service/PackageVersion.java similarity index 70% rename from server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java rename to server/src/main/java/org/elasticsearch/immutablestate/service/PackageVersion.java index 807197c638d3b..95a48112515da 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/StateVersionMetadata.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/PackageVersion.java @@ -17,40 +17,29 @@ * File settings metadata class that holds information about * versioning and Elasticsearch version compatibility */ -public class StateVersionMetadata { +public record PackageVersion(Long version, Version compatibleWith) { public static final ParseField VERSION = new ParseField("version"); public static final ParseField COMPATIBILITY = new ParseField("compatibility"); - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "immutable_cluster_state_version_metadata", a -> { Long updateId = Long.parseLong((String) a[0]); Version minCompatVersion = Version.fromString((String) a[1]); - return new StateVersionMetadata(updateId, minCompatVersion); + return new PackageVersion(updateId, minCompatVersion); } ); + static { PARSER.declareString(ConstructingObjectParser.constructorArg(), VERSION); PARSER.declareString(ConstructingObjectParser.constructorArg(), COMPATIBILITY); } - private Long version; - private Version compatibleWith; - - public StateVersionMetadata(Long version, Version compatibleWith) { - this.version = version; - this.compatibleWith = compatibleWith; - } - - public static StateVersionMetadata parse(XContentParser parser, Void v) { + public static PackageVersion parse(XContentParser parser, Void v) { return PARSER.apply(parser, v); } - public Long version() { - return version; - } - public Version minCompatibleVersion() { return compatibleWith; } diff --git a/server/src/test/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandlerTests.java b/server/src/test/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandlerTests.java index 86eddbaadad17..c46ac036eb823 100644 --- a/server/src/test/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandlerTests.java +++ b/server/src/test/java/org/elasticsearch/immutablestate/ImmutableClusterStateHandlerTests.java @@ -10,18 +10,11 @@ import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.master.MasterNodeRequest; -import org.elasticsearch.common.util.Maps; -import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.indices.settings.InternalOrPrivateSettingsPlugin; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.XContentParser; -import org.elasticsearch.xcontent.XContentParserConfiguration; -import org.elasticsearch.xcontent.XContentType; import java.io.IOException; -import java.util.Map; - -import static org.hamcrest.Matchers.containsInAnyOrder; public class ImmutableClusterStateHandlerTests extends ESTestCase { public void testValidation() { @@ -35,6 +28,11 @@ public String name() { public TransformState transform(Object source, TransformState prevState) throws Exception { return prevState; } + + @Override + public ValidRequest fromXContent(XContentParser parser) throws IOException { + return new ValidRequest(); + } }; handler.validate(new ValidRequest()); @@ -44,53 +42,6 @@ public TransformState transform(Object source, TransformState prevState) throws ); } - public void testAsMapAndFromMap() throws IOException { - String someJSON = """ - { - "persistent": { - "indices.recovery.max_bytes_per_sec": "25mb", - "cluster": { - "remote": { - "cluster_one": { - "seeds": [ - "127.0.0.1:9300" - ] - } - } - } - } - }"""; - - ImmutableClusterStateHandler persistentHandler = new ImmutableClusterStateHandler<>() { - @Override - public String name() { - return "persistent"; - } - - @Override - public TransformState transform(Object source, TransformState prevState) throws Exception { - return prevState; - } - }; - - try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, someJSON)) { - Map originalMap = parser.map(); - - Map internalHandlerMap = Maps.asMap(originalMap.get(persistentHandler.name())); - assertThat(internalHandlerMap.keySet(), containsInAnyOrder("indices.recovery.max_bytes_per_sec", "cluster")); - assertEquals( - "Unsupported input format", - expectThrows(IllegalStateException.class, () -> Maps.asMap(Integer.valueOf(123))).getMessage() - ); - - try (XContentParser newParser = XContentHelper.mapToXContentParser(XContentParserConfiguration.EMPTY, originalMap)) { - Map newMap = newParser.map(); - - assertThat(newMap.keySet(), containsInAnyOrder("persistent")); - } - } - } - static class ValidRequest extends MasterNodeRequest { @Override public ActionRequestValidationException validate() { diff --git a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java index a7a3b2eb43dca..60f841895b72b 100644 --- a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java +++ b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java @@ -11,7 +11,6 @@ import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionResponse; -import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateAckListener; @@ -249,7 +248,7 @@ public void onFailure(Exception failure) {} } public void testUpdateTaskDuplicateError() { - ImmutableClusterStateHandler dummy = new ImmutableClusterStateHandler<>() { + ImmutableClusterStateHandler> dummy = new ImmutableClusterStateHandler<>() { @Override public String name() { return "one"; @@ -259,12 +258,17 @@ public String name() { public TransformState transform(Object source, TransformState prevState) throws Exception { throw new Exception("anything"); } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } }; ImmutableStateUpdateStateTask task = spy( new ImmutableStateUpdateStateTask( "namespace_one", - new ImmutableClusterStateController.Package(Map.of("one", "two"), new StateVersionMetadata(1L, Version.CURRENT)), + new ImmutableClusterStateController.Package(Map.of("one", "two"), new PackageVersion(1L, Version.CURRENT)), Map.of("one", dummy), List.of(dummy.name()), (errorState) -> {}, @@ -323,11 +327,7 @@ public void testCheckMetadataVersion() { ImmutableStateMetadata operatorMetadata = ImmutableStateMetadata.builder("test").version(123L).build(); assertTrue( - ImmutableClusterStateController.checkMetadataVersion( - operatorMetadata, - new StateVersionMetadata(124L, Version.CURRENT), - (e) -> {} - ) + ImmutableClusterStateController.checkMetadataVersion(operatorMetadata, new PackageVersion(124L, Version.CURRENT), (e) -> {}) ); AtomicReference x = new AtomicReference<>(); @@ -335,7 +335,7 @@ public void testCheckMetadataVersion() { assertFalse( ImmutableClusterStateController.checkMetadataVersion( operatorMetadata, - new StateVersionMetadata(123L, Version.CURRENT), + new PackageVersion(123L, Version.CURRENT), (e) -> x.set(e) ) ); @@ -346,7 +346,7 @@ public void testCheckMetadataVersion() { assertFalse( ImmutableClusterStateController.checkMetadataVersion( operatorMetadata, - new StateVersionMetadata(124L, Version.fromId(Version.CURRENT.id + 1)), + new PackageVersion(124L, Version.fromId(Version.CURRENT.id + 1)), (e) -> x.set(e) ) ); @@ -356,7 +356,7 @@ public void testCheckMetadataVersion() { } public void testHandlerOrdering() { - ImmutableClusterStateHandler oh1 = new ImmutableClusterStateHandler<>() { + ImmutableClusterStateHandler> oh1 = new ImmutableClusterStateHandler<>() { @Override public String name() { return "one"; @@ -371,9 +371,14 @@ public TransformState transform(Object source, TransformState prevState) throws public Collection dependencies() { return List.of("two", "three"); } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } }; - ImmutableClusterStateHandler oh2 = new ImmutableClusterStateHandler<>() { + ImmutableClusterStateHandler> oh2 = new ImmutableClusterStateHandler<>() { @Override public String name() { return "two"; @@ -383,9 +388,14 @@ public String name() { public TransformState transform(Object source, TransformState prevState) throws Exception { return null; } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } }; - ImmutableClusterStateHandler oh3 = new ImmutableClusterStateHandler<>() { + ImmutableClusterStateHandler> oh3 = new ImmutableClusterStateHandler<>() { @Override public String name() { return "three"; @@ -400,6 +410,11 @@ public TransformState transform(Object source, TransformState prevState) throws public Collection dependencies() { return List.of("two"); } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } }; ClusterService clusterService = mock(ClusterService.class); @@ -438,6 +453,11 @@ public TransformState transform(Object source, TransformState prevState) throws public Collection dependencies() { return List.of("one"); } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } }; controller.initHandlers(List.of(oh1, oh2)); @@ -468,7 +488,7 @@ public void testDuplicateHandlerNames() { ); } - class TestHandler implements ImmutableClusterStateHandler { + class TestHandler implements ImmutableClusterStateHandler> { @Override public String name() { @@ -479,5 +499,10 @@ public String name() { public TransformState transform(Object source, TransformState prevState) throws Exception { return prevState; } + + @Override + public Map fromXContent(XContentParser parser) throws IOException { + return parser.map(); + } } } diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java index 68b6d994ae63f..0cb6b115d158e 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/action/ImmutableLifecycleAction.java @@ -28,7 +28,6 @@ import java.util.Set; import java.util.stream.Collectors; -import static org.elasticsearch.common.util.Maps.asMap; import static org.elasticsearch.common.xcontent.XContentHelper.mapToXContentParser; /** @@ -38,7 +37,7 @@ * Internally it uses {@link TransportPutLifecycleAction} and * {@link TransportDeleteLifecycleAction} to add, update and delete ILM policies. */ -public class ImmutableLifecycleAction implements ImmutableClusterStateHandler { +public class ImmutableLifecycleAction implements ImmutableClusterStateHandler> { private final NamedXContentRegistry xContentRegistry; private final Client client; @@ -60,18 +59,12 @@ public String name() { @SuppressWarnings("unchecked") public Collection prepare(Object input) throws IOException { List result = new ArrayList<>(); + List policies = (List) input; - Map source = asMap(input); - - for (String name : source.keySet()) { - Map content = (Map) source.get(name); - var config = XContentParserConfiguration.EMPTY.withRegistry(LifecyclePolicyConfig.DEFAULT_X_CONTENT_REGISTRY); - try (XContentParser parser = mapToXContentParser(config, content)) { - LifecyclePolicy policy = LifecyclePolicy.parse(parser, name); - PutLifecycleAction.Request request = new PutLifecycleAction.Request(policy); - validate(request); - result.add(request); - } + for (var policy : policies) { + PutLifecycleAction.Request request = new PutLifecycleAction.Request(policy); + validate(request); + result.add(request); } return result; @@ -108,4 +101,22 @@ public TransformState transform(Object source, TransformState prevState) throws return new TransformState(state, entities); } + + @Override + public List fromXContent(XContentParser parser) throws IOException { + List result = new ArrayList<>(); + + Map source = parser.map(); + var config = XContentParserConfiguration.EMPTY.withRegistry(LifecyclePolicyConfig.DEFAULT_X_CONTENT_REGISTRY); + + for (String name : source.keySet()) { + @SuppressWarnings("unchecked") + Map content = (Map) source.get(name); + try (XContentParser policyParser = mapToXContentParser(config, content)) { + result.add(LifecyclePolicy.parse(policyParser, name)); + } + } + + return result; + } } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index 8ef6a4b3d4717..b3c2b843370cb 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.ilm.action; +import org.elasticsearch.Version; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.ClusterModule; import org.elasticsearch.cluster.ClusterName; @@ -16,10 +17,12 @@ import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.immutablestate.TransformState; import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.immutablestate.service.ImmutableClusterStateController; import org.elasticsearch.immutablestate.service.ImmutableStateUpdateStateTask; +import org.elasticsearch.immutablestate.service.PackageVersion; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; @@ -34,8 +37,10 @@ import org.elasticsearch.xpack.core.ilm.FreezeAction; import org.elasticsearch.xpack.core.ilm.IndexLifecycleMetadata; import org.elasticsearch.xpack.core.ilm.LifecycleAction; +import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; import org.elasticsearch.xpack.core.ilm.LifecycleType; import org.elasticsearch.xpack.core.ilm.MigrateAction; +import org.elasticsearch.xpack.core.ilm.Phase; import org.elasticsearch.xpack.core.ilm.ReadOnlyAction; import org.elasticsearch.xpack.core.ilm.RolloverAction; import org.elasticsearch.xpack.core.ilm.RollupILMAction; @@ -52,6 +57,8 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; @@ -224,7 +231,48 @@ public void testActionAddRemove() throws Exception { assertThat(ilmMetadata.getPolicyMetadatas().keySet(), containsInAnyOrder("my_timeseries_lifecycle2")); } - public void testOperatorController() throws IOException { + private void setupTaskMock(ClusterService clusterService, ClusterState state) { + doAnswer((Answer) invocation -> { + Object[] args = invocation.getArguments(); + + if ((args[3] instanceof ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) == false) { + fail("Should have gotten a state update task to execute, instead got: " + args[3].getClass().getName()); + } + + ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor task = + (ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) args[3]; + + ClusterStateTaskExecutor.TaskContext context = new ClusterStateTaskExecutor.TaskContext<>() { + @Override + public ImmutableStateUpdateStateTask getTask() { + return (ImmutableStateUpdateStateTask) args[1]; + } + + @Override + public void success(Runnable onPublicationSuccess) {} + + @Override + public void success(Consumer publishedStateConsumer) {} + + @Override + public void success(Runnable onPublicationSuccess, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void success(Consumer publishedStateConsumer, ClusterStateAckListener clusterStateAckListener) {} + + @Override + public void onFailure(Exception failure) { + fail("Shouldn't fail here"); + } + }; + + task.execute(state, List.of(context)); + + return null; + }).when(clusterService).submitStateUpdateTask(anyString(), any(), any(), any()); + } + + public void testOperatorControllerFromJSONContent() throws IOException { ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); ClusterService clusterService = mock(ClusterService.class); final ClusterName clusterName = new ClusterName("elasticsearch"); @@ -247,10 +295,34 @@ public void testOperatorController() throws IOException { }, "ilm": { "my_timeseries_lifecycle": { + "phases": { + "hot": { + "min_age": "10s", + "actions": { + "rollover": { + "max_primary_shard_size": "50gb", + "max_age": "30d" + } + } + }, + "delete": { + "min_age": "30s", + "actions": { + } + } + } + }, + "my_timeseries_lifecycle1": { "phases": { "warm": { "min_age": "10s", "actions": { + "shrink": { + "number_of_shards": 1 + }, + "forcemerge": { + "max_num_segments": 1 + } } }, "delete": { @@ -285,51 +357,74 @@ public void testOperatorController() throws IOException { ) ); - doAnswer((Answer) invocation -> { - Object[] args = invocation.getArguments(); + setupTaskMock(clusterService, state); - if ((args[3] instanceof ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) == false) { - fail("Should have gotten a state update task to execute, instead got: " + args[3].getClass().getName()); - } + try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { + controller.process("operator", parser, (e) -> { + if (e != null) { + fail("Should not fail"); + } + }); + } + } - ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor task = - (ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) args[3]; + public void testOperatorControllerWithPluginPackage() { + ClusterSettings clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterService clusterService = mock(ClusterService.class); + final ClusterName clusterName = new ClusterName("elasticsearch"); - ClusterStateTaskExecutor.TaskContext context = new ClusterStateTaskExecutor.TaskContext<>() { - @Override - public ImmutableStateUpdateStateTask getTask() { - return (ImmutableStateUpdateStateTask) args[1]; - } + ClusterState state = ClusterState.builder(clusterName).build(); + when(clusterService.state()).thenReturn(state); - @Override - public void success(Runnable onPublicationSuccess) {} + ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); + controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); - @Override - public void success(Consumer publishedStateConsumer) {} + AtomicReference x = new AtomicReference<>(); - @Override - public void success(Runnable onPublicationSuccess, ClusterStateAckListener clusterStateAckListener) {} + ImmutableClusterStateController.Package pack = new ImmutableClusterStateController.Package( + Map.of( + ImmutableClusterSettingsAction.NAME, + Map.of("indices.recovery.max_bytes_per_sec", "50mb"), + ImmutableLifecycleAction.NAME, + List.of( + new LifecyclePolicy( + "my_timeseries_lifecycle", + Map.of( + "warm", + new Phase("warm", new TimeValue(10, TimeUnit.SECONDS), Collections.emptyMap()), + "delete", + new Phase("delete", new TimeValue(30, TimeUnit.SECONDS), Collections.emptyMap()) + ) + ) + ) + ), + new PackageVersion(123L, Version.CURRENT) + ); - @Override - public void success(Consumer publishedStateConsumer, ClusterStateAckListener clusterStateAckListener) {} + controller.process("operator", pack, (e) -> x.set(e)); - @Override - public void onFailure(Exception failure) { - fail("Shouldn't fail here"); - } - }; + assertTrue(x.get() instanceof IllegalStateException); + assertEquals("Error processing state change request for operator", x.get().getMessage()); - task.execute(state, List.of(context)); + Client client = mock(Client.class); + when(client.settings()).thenReturn(Settings.EMPTY); - return null; - }).when(clusterService).submitStateUpdateTask(anyString(), any(), any(), any()); + XPackLicenseState licenseState = mock(XPackLicenseState.class); - try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, testJSON)) { - controller.process("operator", parser, (e) -> { - if (e != null) { - fail("Should not fail"); - } - }); - } + controller.initHandlers( + List.of( + new ImmutableClusterSettingsAction(clusterSettings), + new ImmutableLifecycleAction(xContentRegistry(), client, licenseState) + ) + ); + + setupTaskMock(clusterService, state); + + controller.process("operator", pack, (e) -> { + if (e != null) { + fail("Should not fail"); + } + }); } + } From d9273f4f21438ef61a4550bf535ddd307722a03b Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 5 Jul 2022 21:20:59 -0400 Subject: [PATCH 08/14] Fix test JSON parsing --- .../xpack/ilm/action/ImmutableILMStateControllerTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index b3c2b843370cb..96e013ada95e0 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -108,7 +108,7 @@ protected NamedXContentRegistry xContentRegistry() { private TransformState processJSON(ImmutableLifecycleAction action, TransformState prevState, String json) throws Exception { try (XContentParser parser = XContentType.JSON.xContent().createParser(XContentParserConfiguration.EMPTY, json)) { - return action.transform(parser.map(), prevState); + return action.transform(action.fromXContent(parser), prevState); } } From 925d5e3c9588931ee146e2a9a4db1e8d13c51799 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski <6207777+grcevski@users.noreply.github.com> Date: Sun, 10 Jul 2022 19:57:59 -0400 Subject: [PATCH 09/14] Update docs/changelog/88224.yaml --- docs/changelog/88224.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/88224.yaml diff --git a/docs/changelog/88224.yaml b/docs/changelog/88224.yaml new file mode 100644 index 0000000000000..0e6265eef9354 --- /dev/null +++ b/docs/changelog/88224.yaml @@ -0,0 +1,5 @@ +pr: 88224 +summary: Immutable cluster state controller +area: Infra/Core +type: enhancement +issues: [] From 28349791d08b0d146f30726131dbf16e4182b2aa Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 11 Jul 2022 11:08:11 -0400 Subject: [PATCH 10/14] Reuse the executors --- .../ImmutableClusterStateController.java | 67 ++++++++++++++++++- .../ImmutableStateUpdateErrorTask.java | 26 ------- .../ImmutableStateUpdateStateTask.java | 36 ---------- .../ImmutableClusterStateControllerTests.java | 12 ++-- .../ImmutableILMStateControllerTests.java | 6 +- 5 files changed, 73 insertions(+), 74 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java index 8edc93261597a..f9488e63b1d11 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java @@ -15,8 +15,10 @@ import org.elasticsearch.action.ActionResponse; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.ClusterStateTaskConfig; +import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; +import org.elasticsearch.cluster.routing.RerouteService; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.Priority; import org.elasticsearch.core.Tuple; @@ -50,6 +52,8 @@ public class ImmutableClusterStateController { Map> handlers = null; final ClusterService clusterService; + private final ImmutableUpdateStateTaskExecutor updateStateTaskExecutor; + private final ImmutableUpdateErrorTaskExecutor errorStateTaskExecutor; @SuppressWarnings("unchecked") private final ConstructingObjectParser packageParser = new ConstructingObjectParser<>("immutable_cluster_package", a -> { @@ -68,6 +72,8 @@ public class ImmutableClusterStateController { */ public ImmutableClusterStateController(ClusterService clusterService) { this.clusterService = clusterService; + this.updateStateTaskExecutor = new ImmutableUpdateStateTaskExecutor(clusterService.getRerouteService()); + this.errorStateTaskExecutor = new ImmutableUpdateErrorTaskExecutor(); packageParser.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, name) -> { if (handlers.containsKey(name) == false) { throw new IllegalStateException("Missing handler definition for content key [" + name + "]"); @@ -155,7 +161,6 @@ public void process(String namespace, Package immutableStateFilePackage, Consume return; } - // Do we need to retry this, or it retries automatically? clusterService.submitStateUpdateTask( "immutable cluster state [" + namespace + "]", new ImmutableStateUpdateStateTask( @@ -179,7 +184,7 @@ public void onFailure(Exception e) { } ), ClusterStateTaskConfig.build(Priority.URGENT), - new ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor(namespace, clusterService.getRerouteService()) + updateStateTaskExecutor ); } @@ -239,7 +244,7 @@ public void onFailure(Exception e) { } }), ClusterStateTaskConfig.build(Priority.URGENT), - new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor() + errorStateTaskExecutor ); } @@ -299,4 +304,60 @@ public IncompatibleVersionException(String message) { super(message); } } + + /** + * Immutable cluster state update task executor + * + * @param rerouteService instance of {@link RerouteService}, so that we can execute reroute after cluster state is published + */ + public record ImmutableUpdateStateTaskExecutor(RerouteService rerouteService) + implements + ClusterStateTaskExecutor { + + @Override + public ClusterState execute(ClusterState currentState, List> taskContexts) + throws Exception { + for (final var taskContext : taskContexts) { + currentState = taskContext.getTask().execute(currentState); + taskContext.success(() -> taskContext.getTask().listener().onResponse(ActionResponse.Empty.INSTANCE)); + } + return currentState; + } + + @Override + public void clusterStatePublished(ClusterState newClusterState) { + rerouteService.reroute( + "reroute after applying immutable cluster state", + Priority.NORMAL, + ActionListener.wrap( + r -> logger.trace("reroute after applying immutable cluster state succeeded"), + e -> logger.debug("reroute after applying immutable cluster state failed", e) + ) + ); + } + } + + /** + * Immutable cluster error state task executor + *

+ * We use this task executor to record any errors while updating immutable cluster state + */ + public record ImmutableUpdateErrorTaskExecutor() implements ClusterStateTaskExecutor { + @Override + public ClusterState execute(ClusterState currentState, List> taskContexts) + throws Exception { + for (final var taskContext : taskContexts) { + currentState = taskContext.getTask().execute(currentState); + taskContext.success( + () -> taskContext.getTask().listener().delegateFailure((l, s) -> l.onResponse(ActionResponse.Empty.INSTANCE)) + ); + } + return currentState; + } + + @Override + public void clusterStatePublished(ClusterState newClusterState) { + logger.info("Wrote new error state in immutable metadata"); + } + } } diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java index dd4b336aeded1..2e2c276a3ac1e 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateErrorTask.java @@ -13,14 +13,11 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.ClusterStateTaskListener; import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; import org.elasticsearch.cluster.metadata.Metadata; -import java.util.List; - /** * Cluster state update task that sets the error state of the immutable cluster state metadata. *

@@ -64,27 +61,4 @@ ClusterState execute(ClusterState currentState) { return newState; } - - /** - * Immutable cluster error state task executor - */ - public record ImmutableUpdateErrorTaskExecutor() implements ClusterStateTaskExecutor { - - @Override - public ClusterState execute(ClusterState currentState, List> taskContexts) - throws Exception { - for (final var taskContext : taskContexts) { - currentState = taskContext.getTask().execute(currentState); - taskContext.success( - () -> taskContext.getTask().listener().delegateFailure((l, s) -> l.onResponse(ActionResponse.Empty.INSTANCE)) - ); - } - return currentState; - } - - @Override - public void clusterStatePublished(ClusterState newClusterState) { - logger.info("Wrote new error state in immutable metadata"); - } - } } diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java index 11e3e1362476d..97e005b4f23a2 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableStateUpdateStateTask.java @@ -13,14 +13,11 @@ import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ClusterStateTaskExecutor; import org.elasticsearch.cluster.ClusterStateTaskListener; import org.elasticsearch.cluster.metadata.ImmutableStateErrorMetadata; import org.elasticsearch.cluster.metadata.ImmutableStateHandlerMetadata; import org.elasticsearch.cluster.metadata.ImmutableStateMetadata; import org.elasticsearch.cluster.metadata.Metadata; -import org.elasticsearch.cluster.routing.RerouteService; -import org.elasticsearch.common.Priority; import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; import org.elasticsearch.immutablestate.TransformState; @@ -138,37 +135,4 @@ private Set keysForHandler(ImmutableStateMetadata immutableStateMetadata return immutableStateMetadata.handlers().get(handlerName).keys(); } - - /** - * Immutable cluster state update task executor - * - * @param namespace of the state we are updating - * @param rerouteService instance of {@link RerouteService}, so that we can execute reroute after cluster state is published - */ - public record ImmutableUpdateStateTaskExecutor(String namespace, RerouteService rerouteService) - implements - ClusterStateTaskExecutor { - - @Override - public ClusterState execute(ClusterState currentState, List> taskContexts) - throws Exception { - for (final var taskContext : taskContexts) { - currentState = taskContext.getTask().execute(currentState); - taskContext.success(() -> taskContext.getTask().listener().onResponse(ActionResponse.Empty.INSTANCE)); - } - return currentState; - } - - @Override - public void clusterStatePublished(ClusterState newClusterState) { - rerouteService.reroute( - "reroute after applying immutable cluster state for namespace [" + namespace + "]", - Priority.NORMAL, - ActionListener.wrap( - r -> logger.trace("reroute after applying immutable cluster state for [{}] succeeded", namespace), - e -> logger.debug("reroute after applying immutable cluster state failed", e) - ) - ); - } - } } diff --git a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java index 60f841895b72b..58f86f54393dc 100644 --- a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java +++ b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java @@ -128,8 +128,8 @@ public void testUpdateStateTasks() throws Exception { when(clusterService.getRerouteService()).thenReturn(rerouteService); ClusterState state = ClusterState.builder(new ClusterName("test")).build(); - ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor taskExecutor = - new ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor("test", clusterService.getRerouteService()); + ImmutableClusterStateController.ImmutableUpdateStateTaskExecutor taskExecutor = + new ImmutableClusterStateController.ImmutableUpdateStateTaskExecutor(clusterService.getRerouteService()); AtomicBoolean successCalled = new AtomicBoolean(false); @@ -207,8 +207,8 @@ public void onFailure(Exception e) {} ) ); - ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor.TaskContext taskContext = - new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor.TaskContext<>() { + ImmutableClusterStateController.ImmutableUpdateErrorTaskExecutor.TaskContext taskContext = + new ImmutableClusterStateController.ImmutableUpdateErrorTaskExecutor.TaskContext<>() { @Override public ImmutableStateUpdateErrorTask getTask() { return task; @@ -232,8 +232,8 @@ public void success(Consumer publishedStateConsumer, ClusterStateA public void onFailure(Exception failure) {} }; - ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor executor = - new ImmutableStateUpdateErrorTask.ImmutableUpdateErrorTaskExecutor(); + ImmutableClusterStateController.ImmutableUpdateErrorTaskExecutor executor = + new ImmutableClusterStateController.ImmutableUpdateErrorTaskExecutor(); ClusterState newState = executor.execute(state, List.of(taskContext)); diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index 96e013ada95e0..a890d4a3673c5 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -235,12 +235,12 @@ private void setupTaskMock(ClusterService clusterService, ClusterState state) { doAnswer((Answer) invocation -> { Object[] args = invocation.getArguments(); - if ((args[3] instanceof ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) == false) { + if ((args[3] instanceof ImmutableClusterStateController.ImmutableUpdateStateTaskExecutor) == false) { fail("Should have gotten a state update task to execute, instead got: " + args[3].getClass().getName()); } - ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor task = - (ImmutableStateUpdateStateTask.ImmutableUpdateStateTaskExecutor) args[3]; + ImmutableClusterStateController.ImmutableUpdateStateTaskExecutor task = + (ImmutableClusterStateController.ImmutableUpdateStateTaskExecutor) args[3]; ClusterStateTaskExecutor.TaskContext context = new ClusterStateTaskExecutor.TaskContext<>() { @Override From ebe0a749a4dcc7bb07c67b9b103a7de010a22c6b Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Mon, 11 Jul 2022 11:45:15 -0400 Subject: [PATCH 11/14] Fix tests --- .../org/elasticsearch/action/ActionModuleTests.java | 10 ++++++---- .../elasticsearch/xpack/security/SecurityTests.java | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index ebc869a3aba2c..fcb2d0ea4ffbb 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.IndexScopedSettings; import org.elasticsearch.common.settings.Settings; @@ -49,6 +50,7 @@ import static org.elasticsearch.rest.RestRequest.Method.GET; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.startsWith; +import static org.mockito.Mockito.mock; public class ActionModuleTests extends ESTestCase { public void testSetupActionsContainsKnownBuiltin() { @@ -117,7 +119,7 @@ public void testSetupRestHandlerContainsKnownBuiltin() { null, usageService, null, - null + mock(ClusterService.class) ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -174,7 +176,7 @@ public String getName() { null, usageService, null, - null + mock(ClusterService.class) ); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET")); @@ -224,7 +226,7 @@ public List getRestHandlers( null, usageService, null, - null + mock(ClusterService.class) ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -269,7 +271,7 @@ public void test3rdPartyHandlerIsNotInstalled() { null, usageService, null, - null + mock(ClusterService.class) ) ); assertThat( diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 604a521da9a8e..589addb95408c 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -779,7 +779,7 @@ public void testSecurityRestHandlerInterceptorCanBeInstalled() throws IllegalAcc null, usageService, null, - null + mock(ClusterService.class) ); actionModule.initRestHandlers(null); From c8cb7ddfd332549974c807aed46bd7c62affeab2 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 12 Jul 2022 10:29:26 -0400 Subject: [PATCH 12/14] Apply review feedback on action module --- .../elasticsearch/action/ActionModule.java | 26 +++---------------- .../ImmutableClusterStateController.java | 14 +++------- .../java/org/elasticsearch/node/Node.java | 19 +++++++++++--- .../action/ActionModuleTests.java | 13 +++++++--- .../ImmutableClusterStateControllerTests.java | 22 +++++++++------- .../ImmutableILMStateControllerTests.java | 18 ++++++++----- .../xpack/security/SecurityTests.java | 3 ++- 7 files changed, 57 insertions(+), 58 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index b24f8c60b979c..0cef2657f2090 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -264,8 +264,6 @@ import org.elasticsearch.health.GetHealthAction; import org.elasticsearch.health.RestGetHealthAction; import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; -import org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; -import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.immutablestate.service.ImmutableClusterStateController; import org.elasticsearch.index.seqno.GlobalCheckpointSyncAction; import org.elasticsearch.index.seqno.RetentionLeaseActions; @@ -278,7 +276,6 @@ import org.elasticsearch.persistent.UpdatePersistentTaskStatusAction; import org.elasticsearch.plugins.ActionPlugin; import org.elasticsearch.plugins.ActionPlugin.ActionHandler; -import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.plugins.interceptor.RestInterceptorActionPlugin; import org.elasticsearch.rest.RestController; import org.elasticsearch.rest.RestHandler; @@ -468,7 +465,8 @@ public ActionModule( CircuitBreakerService circuitBreakerService, UsageService usageService, SystemIndices systemIndices, - ClusterService clusterService + ClusterService clusterService, + List> reservedStateHandlers ) { this.settings = settings; this.indexNameExpressionResolver = indexNameExpressionResolver; @@ -519,7 +517,7 @@ public ActionModule( ); restController = new RestController(headers, restInterceptor, nodeClient, circuitBreakerService, usageService); - immutableStateController = new ImmutableClusterStateController(clusterService); + immutableStateController = new ImmutableClusterStateController(clusterService, reservedStateHandlers); } public Map> getActions() { @@ -897,24 +895,6 @@ public void initRestHandlers(Supplier nodesInCluster) { registerHandler.accept(new RestCatAction(catActions)); } - /** - * Initializes the immutable cluster state handlers for Elasticsearch and it's modules/plugins - * - * @param pluginsService needed to load all modules/plugins immutable state handlers through SPI - */ - public void initImmutableClusterStateHandlers(PluginsService pluginsService) { - List> handlers = new ArrayList<>(); - - List pluginHandlers = pluginsService.loadServiceProviders( - ImmutableClusterStateHandlerProvider.class - ); - - handlers.add(new ImmutableClusterSettingsAction(clusterSettings)); - pluginHandlers.forEach(h -> handlers.addAll(h.handlers())); - - immutableStateController.initHandlers(handlers); - } - @Override protected void configure() { bind(ActionFilters.class).toInstance(actionFilters); diff --git a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java index f9488e63b1d11..5e763625792f3 100644 --- a/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java +++ b/server/src/main/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateController.java @@ -50,7 +50,7 @@ public class ImmutableClusterStateController { public static final ParseField STATE_FIELD = new ParseField("state"); public static final ParseField METADATA_FIELD = new ParseField("metadata"); - Map> handlers = null; + final Map> handlers; final ClusterService clusterService; private final ImmutableUpdateStateTaskExecutor updateStateTaskExecutor; private final ImmutableUpdateErrorTaskExecutor errorStateTaskExecutor; @@ -70,10 +70,11 @@ public class ImmutableClusterStateController { * Controller class for saving immutable ClusterState. * @param clusterService for fetching and saving the modified state */ - public ImmutableClusterStateController(ClusterService clusterService) { + public ImmutableClusterStateController(ClusterService clusterService, List> handlerList) { this.clusterService = clusterService; this.updateStateTaskExecutor = new ImmutableUpdateStateTaskExecutor(clusterService.getRerouteService()); this.errorStateTaskExecutor = new ImmutableUpdateErrorTaskExecutor(); + this.handlers = handlerList.stream().collect(Collectors.toMap(ImmutableClusterStateHandler::name, Function.identity())); packageParser.declareNamedObjects(ConstructingObjectParser.constructorArg(), (p, c, name) -> { if (handlers.containsKey(name) == false) { throw new IllegalStateException("Missing handler definition for content key [" + name + "]"); @@ -84,15 +85,6 @@ public ImmutableClusterStateController(ClusterService clusterService) { packageParser.declareObject(ConstructingObjectParser.constructorArg(), PackageVersion::parse, METADATA_FIELD); } - /** - * Initializes the controller with the currently implemented state handlers - * - * @param handlerList the list of supported immutable cluster state handlers - */ - public void initHandlers(List> handlerList) { - handlers = handlerList.stream().collect(Collectors.toMap(ImmutableClusterStateHandler::name, Function.identity())); - } - /** * A package class containing the composite immutable cluster state *

diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index 8ebaf4a59a2fc..9f32316694d3a 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -105,6 +105,9 @@ import org.elasticsearch.health.node.selection.HealthNode; import org.elasticsearch.health.node.selection.HealthNodeTaskExecutor; import org.elasticsearch.http.HttpServerTransport; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandler; +import org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider; +import org.elasticsearch.immutablestate.action.ImmutableClusterSettingsAction; import org.elasticsearch.index.IndexSettingProvider; import org.elasticsearch.index.IndexSettingProviders; import org.elasticsearch.index.IndexSettings; @@ -705,6 +708,17 @@ protected Node( ) ).toList(); + List> reservedStateHandlers = new ArrayList<>(); + + // add all reserved state handlers from server + reservedStateHandlers.add(new ImmutableClusterSettingsAction(settingsModule.getClusterSettings())); + + // add all reserved state handlers from plugins + List pluginHandlers = pluginsService.loadServiceProviders( + ImmutableClusterStateHandlerProvider.class + ); + pluginHandlers.forEach(h -> reservedStateHandlers.addAll(h.handlers())); + ActionModule actionModule = new ActionModule( settings, clusterModule.getIndexNameExpressionResolver(), @@ -717,7 +731,8 @@ protected Node( circuitBreakerService, usageService, systemIndices, - clusterService + clusterService, + reservedStateHandlers ); modules.add(actionModule); @@ -1056,8 +1071,6 @@ protected Node( logger.debug("initializing HTTP handlers ..."); actionModule.initRestHandlers(() -> clusterService.state().nodesIfRecovered()); - logger.debug("initializing operator handlers ..."); - actionModule.initImmutableClusterStateHandlers(pluginsService); logger.info("initialized"); success = true; diff --git a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java index fcb2d0ea4ffbb..bdac66ba98acf 100644 --- a/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java +++ b/server/src/test/java/org/elasticsearch/action/ActionModuleTests.java @@ -41,6 +41,7 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.function.Supplier; import java.util.function.UnaryOperator; @@ -119,7 +120,8 @@ public void testSetupRestHandlerContainsKnownBuiltin() { null, usageService, null, - mock(ClusterService.class) + mock(ClusterService.class), + Collections.emptyList() ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -176,7 +178,8 @@ public String getName() { null, usageService, null, - mock(ClusterService.class) + mock(ClusterService.class), + Collections.emptyList() ); Exception e = expectThrows(IllegalArgumentException.class, () -> actionModule.initRestHandlers(null)); assertThat(e.getMessage(), startsWith("Cannot replace existing handler for [/] for method: GET")); @@ -226,7 +229,8 @@ public List getRestHandlers( null, usageService, null, - mock(ClusterService.class) + mock(ClusterService.class), + Collections.emptyList() ); actionModule.initRestHandlers(null); // At this point the easiest way to confirm that a handler is loaded is to try to register another one on top of it and to fail @@ -271,7 +275,8 @@ public void test3rdPartyHandlerIsNotInstalled() { null, usageService, null, - mock(ClusterService.class) + mock(ClusterService.class), + Collections.emptyList() ) ); assertThat( diff --git a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java index 58f86f54393dc..baf282805eda0 100644 --- a/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java +++ b/server/src/test/java/org/elasticsearch/immutablestate/service/ImmutableClusterStateControllerTests.java @@ -63,8 +63,10 @@ public void testOperatorController() throws IOException { ClusterState state = ClusterState.builder(clusterName).build(); when(clusterService.state()).thenReturn(state); - ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); - controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); + ImmutableClusterStateController controller = new ImmutableClusterStateController( + clusterService, + List.of(new ImmutableClusterSettingsAction(clusterSettings)) + ); String testJSON = """ { @@ -418,9 +420,7 @@ public Map fromXContent(XContentParser parser) throws IOExceptio }; ClusterService clusterService = mock(ClusterService.class); - ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); - - controller.initHandlers(List.of(oh1, oh2, oh3)); + final var controller = new ImmutableClusterStateController(clusterService, List.of(oh1, oh2, oh3)); Collection ordered = controller.orderedStateHandlers(Set.of("one", "two", "three")); assertThat(ordered, contains("two", "three", "one")); @@ -460,9 +460,10 @@ public Map fromXContent(XContentParser parser) throws IOExceptio } }; - controller.initHandlers(List.of(oh1, oh2)); + final var controller1 = new ImmutableClusterStateController(clusterService, List.of(oh1, oh2)); + assertThat( - expectThrows(IllegalStateException.class, () -> controller.orderedStateHandlers(Set.of("one", "two"))).getMessage(), + expectThrows(IllegalStateException.class, () -> controller1.orderedStateHandlers(Set.of("one", "two"))).getMessage(), anyOf( is("Cycle found in settings dependencies: one -> two -> one"), is("Cycle found in settings dependencies: two -> one -> two") @@ -478,12 +479,13 @@ public void testDuplicateHandlerNames() { ClusterState state = ClusterState.builder(clusterName).build(); when(clusterService.state()).thenReturn(state); - ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); - assertTrue( expectThrows( IllegalStateException.class, - () -> controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings), new TestHandler())) + () -> new ImmutableClusterStateController( + clusterService, + List.of(new ImmutableClusterSettingsAction(clusterSettings), new TestHandler()) + ) ).getMessage().startsWith("Duplicate key cluster_settings") ); } diff --git a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java index a890d4a3673c5..3c72e6e846f54 100644 --- a/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java +++ b/x-pack/plugin/ilm/src/test/java/org/elasticsearch/xpack/ilm/action/ImmutableILMStateControllerTests.java @@ -280,8 +280,10 @@ public void testOperatorControllerFromJSONContent() throws IOException { ClusterState state = ClusterState.builder(clusterName).build(); when(clusterService.state()).thenReturn(state); - ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); - controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); + ImmutableClusterStateController controller = new ImmutableClusterStateController( + clusterService, + List.of(new ImmutableClusterSettingsAction(clusterSettings)) + ); String testJSON = """ { @@ -350,7 +352,8 @@ public void testOperatorControllerFromJSONContent() throws IOException { XPackLicenseState licenseState = mock(XPackLicenseState.class); - controller.initHandlers( + controller = new ImmutableClusterStateController( + clusterService, List.of( new ImmutableClusterSettingsAction(clusterSettings), new ImmutableLifecycleAction(xContentRegistry(), client, licenseState) @@ -376,8 +379,10 @@ public void testOperatorControllerWithPluginPackage() { ClusterState state = ClusterState.builder(clusterName).build(); when(clusterService.state()).thenReturn(state); - ImmutableClusterStateController controller = new ImmutableClusterStateController(clusterService); - controller.initHandlers(List.of(new ImmutableClusterSettingsAction(clusterSettings))); + ImmutableClusterStateController controller = new ImmutableClusterStateController( + clusterService, + List.of(new ImmutableClusterSettingsAction(clusterSettings)) + ); AtomicReference x = new AtomicReference<>(); @@ -411,7 +416,8 @@ public void testOperatorControllerWithPluginPackage() { XPackLicenseState licenseState = mock(XPackLicenseState.class); - controller.initHandlers( + controller = new ImmutableClusterStateController( + clusterService, List.of( new ImmutableClusterSettingsAction(clusterSettings), new ImmutableLifecycleAction(xContentRegistry(), client, licenseState) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 589addb95408c..4e739c1111042 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -779,7 +779,8 @@ public void testSecurityRestHandlerInterceptorCanBeInstalled() throws IllegalAcc null, usageService, null, - mock(ClusterService.class) + mock(ClusterService.class), + Collections.emptyList() ); actionModule.initRestHandlers(null); From 84681f72adb72b82661502d82d88ecdf7bee81b4 Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 12 Jul 2022 14:19:45 -0400 Subject: [PATCH 13/14] Add meta-inf for createExtensions --- ...ch.immutablestate.ImmutableClusterStateHandlerProvider | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider diff --git a/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider b/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider new file mode 100644 index 0000000000000..5a9c1fe140016 --- /dev/null +++ b/x-pack/plugin/ilm/src/main/resources/META-INF/services/org.elasticsearch.immutablestate.ImmutableClusterStateHandlerProvider @@ -0,0 +1,8 @@ +# +# Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +# or more contributor license agreements. Licensed under the Elastic License +# 2.0; you may not use this file except in compliance with the Elastic License +# 2.0. +# + +org.elasticsearch.xpack.ilm.ILMImmutableStateHandlerProvider From e55c6c7caa51397ffb8d7d841f0c8d26c2f59c0a Mon Sep 17 00:00:00 2001 From: Nikola Grcevski Date: Tue, 12 Jul 2022 15:32:38 -0400 Subject: [PATCH 14/14] Add classloader check --- .../main/java/org/elasticsearch/plugins/PluginsService.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 7e2e13d5343f5..9e251fae881c5 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -309,7 +309,10 @@ public List loadServiceProviders(Class service) { List result = new ArrayList<>(); for (LoadedPlugin pluginTuple : plugins()) { - result.addAll(createExtensions(service, pluginTuple.instance)); + // Only load SPI providers if they are loaded by different class loader + if (pluginTuple.loader().equals(this.getClass().getClassLoader()) == false) { + result.addAll(createExtensions(service, pluginTuple.instance())); + } } return Collections.unmodifiableList(result);