From 12110b9e1087b2698a2774ddcfe128e88bf6e65f Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Fri, 18 May 2018 17:19:24 +0200 Subject: [PATCH 1/8] Only activate x-pack if all nodes ready for it --- .../elasticsearch/license/LicenseService.java | 9 +- .../license/StartBasicClusterTask.java | 2 + .../license/StartTrialClusterTask.java | 2 + .../StartupSelfGeneratedLicenseTask.java | 2 + .../elasticsearch/xpack/core/XPackPlugin.java | 86 +++++++++++++++++++ .../AbstractLicenseServiceTestCase.java | 8 +- .../xpack/core/XPackPluginTests.java | 57 ++++++++++++ .../xpack/ml/MlInitializationService.java | 7 ++ .../action/TransportDeleteDatafeedAction.java | 2 + .../TransportFinalizeJobExecutionAction.java | 2 + .../ml/action/TransportPutDatafeedAction.java | 2 + .../xpack/ml/job/JobManager.java | 5 ++ .../ml/MlInitializationServiceTests.java | 37 ++++++-- .../xpack/security/Security.java | 9 +- .../xpack/security/authc/TokenService.java | 45 ++++++++++ .../TransportWatcherServiceAction.java | 3 + 16 files changed, 263 insertions(+), 15 deletions(-) create mode 100644 x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java index fa0c239aab17d..99e6a10ad92de 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/LicenseService.java @@ -223,6 +223,7 @@ protected PutLicenseResponse newResponse(boolean acknowledged) { @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); MetaData currentMetadata = currentState.metaData(); LicensesMetaData licensesMetaData = currentMetadata.custom(LicensesMetaData.TYPE); Version trialVersion = null; @@ -341,7 +342,7 @@ protected void doStart() throws ElasticsearchException { if (clusterService.lifecycleState() == Lifecycle.State.STARTED) { final ClusterState clusterState = clusterService.state(); if (clusterState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK) == false && - clusterState.nodes().getMasterNode() != null) { + clusterState.nodes().getMasterNode() != null && XPackPlugin.isReadyForXPackCustomMetadata(clusterState)) { final LicensesMetaData currentMetaData = clusterState.metaData().custom(LicensesMetaData.TYPE); boolean noLicense = currentMetaData == null || currentMetaData.getLicense() == null; if (clusterState.getNodes().isLocalNodeElectedMaster() && @@ -374,6 +375,12 @@ public void clusterChanged(ClusterChangedEvent event) { final ClusterState previousClusterState = event.previousState(); final ClusterState currentClusterState = event.state(); if (!currentClusterState.blocks().hasGlobalBlock(GatewayService.STATE_NOT_RECOVERED_BLOCK)) { + if (XPackPlugin.isReadyForXPackCustomMetadata(currentClusterState) == false) { + logger.debug("cannot add license to cluster as the following nodes might not understand the license metadata: {}", + () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(currentClusterState)); + return; + } + final LicensesMetaData prevLicensesMetaData = previousClusterState.getMetaData().custom(LicensesMetaData.TYPE); final LicensesMetaData currentLicensesMetaData = currentClusterState.getMetaData().custom(LicensesMetaData.TYPE); if (logger.isDebugEnabled()) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java index 355482872d629..0cf949a69906f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartBasicClusterTask.java @@ -13,6 +13,7 @@ import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.Nullable; +import org.elasticsearch.xpack.core.XPackPlugin; import java.time.Clock; import java.util.Collections; @@ -59,6 +60,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); LicensesMetaData licensesMetaData = currentState.metaData().custom(LicensesMetaData.TYPE); License currentLicense = LicensesMetaData.extractLicense(licensesMetaData); if (currentLicense == null || currentLicense.type().equals("basic") == false) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java index 355672dedf717..5c5c03151ba26 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartTrialClusterTask.java @@ -13,6 +13,7 @@ import org.elasticsearch.cluster.ClusterStateUpdateTask; import org.elasticsearch.cluster.metadata.MetaData; import org.elasticsearch.common.Nullable; +import org.elasticsearch.xpack.core.XPackPlugin; import java.time.Clock; import java.util.Collections; @@ -64,6 +65,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); LicensesMetaData currentLicensesMetaData = currentState.metaData().custom(LicensesMetaData.TYPE); if (request.isAcknowledged() == false) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java index 77695f64538bc..823283ac5a852 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/license/StartupSelfGeneratedLicenseTask.java @@ -16,6 +16,7 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.xpack.core.XPackPlugin; import java.time.Clock; import java.util.UUID; @@ -49,6 +50,7 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); final MetaData metaData = currentState.metaData(); final LicensesMetaData currentLicensesMetaData = metaData.custom(LicensesMetaData.TYPE); // do not generate a license if any license is present diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 5ee46f3b3c97a..84ad5885f738f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -9,15 +9,20 @@ import org.apache.lucene.util.SetOnce; import org.bouncycastle.operator.OperatorCreationException; import org.elasticsearch.SpecialPermission; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.GenericAction; import org.elasticsearch.action.support.ActionFilter; import org.elasticsearch.client.Client; import org.elasticsearch.client.transport.TransportClient; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.service.ClusterService; +import org.elasticsearch.common.Booleans; import org.elasticsearch.common.inject.Binder; import org.elasticsearch.common.inject.Module; import org.elasticsearch.common.inject.multibindings.Multibinder; @@ -33,6 +38,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.env.NodeEnvironment; import org.elasticsearch.license.LicenseService; +import org.elasticsearch.license.LicensesMetaData; import org.elasticsearch.license.Licensing; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.ExtensiblePlugin; @@ -46,10 +52,13 @@ import org.elasticsearch.xpack.core.action.TransportXPackUsageAction; import org.elasticsearch.xpack.core.action.XPackInfoAction; import org.elasticsearch.xpack.core.action.XPackUsageAction; +import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.rest.action.RestXPackInfoAction; import org.elasticsearch.xpack.core.rest.action.RestXPackUsageAction; +import org.elasticsearch.xpack.core.security.authc.TokenMetaData; import org.elasticsearch.xpack.core.ssl.SSLConfigurationReloader; import org.elasticsearch.xpack.core.ssl.SSLService; +import org.elasticsearch.xpack.core.watcher.WatcherMetaData; import javax.security.auth.DestroyFailedException; @@ -62,14 +71,19 @@ import java.time.Clock; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.function.Supplier; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; public class XPackPlugin extends XPackClientPlugin implements ScriptPlugin, ExtensiblePlugin { private static Logger logger = ESLoggerFactory.getLogger(XPackPlugin.class); private static DeprecationLogger deprecationLogger = new DeprecationLogger(logger); + public static final String XPACK_INSTALLED_NODE_ATTR = "xpack.installed"; + // TODO: clean up this library to not ask for write access to all system properties! static { // invoke this clinit in unbound with permissions to access all system properties @@ -138,6 +152,78 @@ protected Clock getClock() { public static LicenseService getSharedLicenseService() { return licenseService.get(); } public static XPackLicenseState getSharedLicenseState() { return licenseState.get(); } + /** + * Checks if the cluster state allows this node to add x-pack metadata to the cluster state, + * and throws an exception otherwise. + * This check should be called before installing any x-pack metadata to the cluster state, + * to ensure that the other nodes that are part of the cluster will be able to deserialize + * that metadata. + * Having this check properly in place everywhere allows to install x-pack into a cluster + * using a rolling restart. + */ + public static void checkReadyForXPackCustomMetadata(ClusterState clusterState) { + List notReadyNodes = nodesNotReadyForXPackCustomMetadata(clusterState); + if (notReadyNodes.isEmpty() == false) { + throw new IllegalStateException("The following nodes are not ready yet for enabling x-pack custom metadata: " + notReadyNodes); + } + } + + /** + * Checks if the cluster state allows this node to add x-pack metadata to the cluster state. + * See {@link #checkReadyForXPackCustomMetadata} for more details. + */ + public static boolean isReadyForXPackCustomMetadata(ClusterState clusterState) { + return nodesNotReadyForXPackCustomMetadata(clusterState).isEmpty(); + } + + /** + * Returns the list of nodes that won't allow this node from adding x-pack metadata to the cluster state. + * See {@link #checkReadyForXPackCustomMetadata} for more details. + */ + public static List nodesNotReadyForXPackCustomMetadata(ClusterState clusterState) { + // check if there's already x-pack metadata in the cluster state; if so, any further metadata won't hurt + final MetaData metaData = clusterState.metaData(); + if (metaData.custom(LicensesMetaData.TYPE) != null || + metaData.custom(MLMetadataField.TYPE) != null || + metaData.custom(WatcherMetaData.TYPE) != null || + clusterState.custom(TokenMetaData.TYPE) != null) { + return Collections.emptyList(); + } + + // if there's no x-pack metadata yet in the cluster state, check that all nodes would be capable + // of deserializing newly added x-pack metadata + final List notReadyNodes = StreamSupport.stream(clusterState.nodes().spliterator(), false).filter(node -> { + final String xpackInstalledAttr = node.getAttributes().getOrDefault(XPACK_INSTALLED_NODE_ATTR, "false"); + + // The node attribute XPACK_INSTALLED_NODE_ATTR was only introduced in 6.3.0, so when + // we have an older node in this mixed-version cluster without any x-pack metadata, + // we want to prevent x-pack from adding custom metadata + return node.getVersion().before(Version.V_6_3_0) || Booleans.parseBoolean(xpackInstalledAttr) == false; + }).collect(Collectors.toList()); + + return notReadyNodes; + } + + @Override + public Settings additionalSettings() { + final String xpackInstalledNodeAttrSetting = "node.attr." + XPACK_INSTALLED_NODE_ATTR; + + if (transportClientMode) { + if (settings.get(xpackInstalledNodeAttrSetting) != null) { + throw new IllegalArgumentException("Directly setting [" + xpackInstalledNodeAttrSetting + "] is not permitted"); + } + + return super.additionalSettings(); + } else { + if (settings.get(xpackInstalledNodeAttrSetting) != null && + settings.get(xpackInstalledNodeAttrSetting).equals("true") == false) { + throw new IllegalArgumentException("Conflicting setting [" + xpackInstalledNodeAttrSetting + "]"); + } + + return Settings.builder().put(super.additionalSettings()).put(xpackInstalledNodeAttrSetting, "true").build(); + } + } + @Override public Collection createGuiceModules() { ArrayList modules = new ArrayList<>(); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/AbstractLicenseServiceTestCase.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/AbstractLicenseServiceTestCase.java index 2f110f4f8a9e8..5bc33ae330a18 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/license/AbstractLicenseServiceTestCase.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/license/AbstractLicenseServiceTestCase.java @@ -18,14 +18,16 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.watcher.ResourceWatcherService; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.watcher.watch.ClockMock; import org.junit.After; import org.junit.Before; import java.nio.file.Path; +import java.util.Arrays; -import static java.util.Collections.emptyMap; import static java.util.Collections.emptySet; +import static java.util.Collections.singletonMap; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -66,6 +68,7 @@ protected void setInitialState(License license, XPackLicenseState licenseState, when(state.metaData()).thenReturn(metaData); final DiscoveryNode mockNode = getLocalNode(); when(discoveryNodes.getMasterNode()).thenReturn(mockNode); + when(discoveryNodes.spliterator()).thenReturn(Arrays.asList(mockNode).spliterator()); when(discoveryNodes.isLocalNodeElectedMaster()).thenReturn(false); when(state.nodes()).thenReturn(discoveryNodes); when(state.getNodes()).thenReturn(discoveryNodes); // it is really ridiculous we have nodes() and getNodes()... @@ -76,7 +79,8 @@ protected void setInitialState(License license, XPackLicenseState licenseState, } protected DiscoveryNode getLocalNode() { - return new DiscoveryNode("b", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT); + return new DiscoveryNode("b", buildNewFakeTransportAddress(), singletonMap(XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "true"), + emptySet(), Version.CURRENT); } @After diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java new file mode 100644 index 0000000000000..ae193850347a9 --- /dev/null +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.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; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.core; + +import org.elasticsearch.client.Client; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.license.XPackLicenseState; +import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.xpack.core.ssl.SSLService; + +import static org.hamcrest.Matchers.containsString; + +public class XPackPluginTests extends ESTestCase { + + public void testXPackInstalledAttrClashOnTransport() throws Exception { + Settings.Builder builder = Settings.builder(); + builder.put("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "true"); + builder.put(Client.CLIENT_TYPE_SETTING_S.getKey(), "transport"); + XPackPlugin xpackPlugin = createXPackPlugin(builder.put("path.home", createTempDir()).build()); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, xpackPlugin::additionalSettings); + assertThat(e.getMessage(), + containsString("Directly setting [node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR + "] is not permitted")); + } + + public void testXPackInstalledAttrClashOnNode() throws Exception { + Settings.Builder builder = Settings.builder(); + builder.put("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "false"); + XPackPlugin xpackPlugin = createXPackPlugin(builder.put("path.home", createTempDir()).build()); + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, xpackPlugin::additionalSettings); + assertThat(e.getMessage(), + containsString("Conflicting setting [node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR + "]")); + } + + public void testXPackInstalledAttrExists() throws Exception { + XPackPlugin xpackPlugin = createXPackPlugin(Settings.builder().put("path.home", createTempDir()).build()); + assertEquals("true", xpackPlugin.additionalSettings().get("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR)); + } + + private XPackPlugin createXPackPlugin(Settings settings) throws Exception { + return new XPackPlugin(settings, null){ + + @Override + protected void setSslService(SSLService sslService) { + // disable + } + + @Override + protected void setLicenseState(XPackLicenseState licenseState) { + // disable + } + }; + } + +} diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java index 5dba8ce943c44..9fed7924a2c52 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java @@ -17,6 +17,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.gateway.GatewayService; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetadata; @@ -48,6 +49,11 @@ public void clusterChanged(ClusterChangedEvent event) { } if (event.localNodeMaster()) { + if (XPackPlugin.isReadyForXPackCustomMetadata(event.state()) == false) { + logger.debug("cannot add ML metadata to cluster as the following nodes might not understand the ML metadata: {}", + () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(event.state())); + return; + } MetaData metaData = event.state().metaData(); installMlMetadata(metaData); installDailyMaintenanceService(); @@ -63,6 +69,7 @@ private void installMlMetadata(MetaData metaData) { clusterService.submitStateUpdateTask("install-ml-metadata", new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); // If the metadata has been added already don't try to update if (currentState.metaData().custom(MLMetadataField.TYPE) != null) { return currentState; diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java index 79995af9e62aa..a015d612c7778 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteDatafeedAction.java @@ -21,6 +21,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetadata; import org.elasticsearch.xpack.core.ml.action.DeleteDatafeedAction; @@ -120,6 +121,7 @@ protected DeleteDatafeedAction.Response newResponse(boolean acknowledged) { @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); MlMetadata currentMetadata = currentState.getMetaData().custom(MLMetadataField.TYPE); PersistentTasksCustomMetaData persistentTasks = currentState.getMetaData().custom(PersistentTasksCustomMetaData.TYPE); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportFinalizeJobExecutionAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportFinalizeJobExecutionAction.java index 3f8ca39a0f124..9749d0f33c946 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportFinalizeJobExecutionAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportFinalizeJobExecutionAction.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.action.FinalizeJobExecutionAction; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetadata; @@ -57,6 +58,7 @@ protected void masterOperation(FinalizeJobExecutionAction.Request request, Clust clusterService.submitStateUpdateTask(source, new ClusterStateUpdateTask() { @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); MlMetadata mlMetadata = currentState.metaData().custom(MLMetadataField.TYPE); MlMetadata.Builder mlMetadataBuilder = new MlMetadata.Builder(mlMetadata); Date finishedTime = new Date(); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java index 0c492a0817c98..d9c8c2580cff0 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutDatafeedAction.java @@ -28,6 +28,7 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.XPackField; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetadata; @@ -141,6 +142,7 @@ public ClusterState execute(ClusterState currentState) { } private ClusterState putDatafeed(PutDatafeedAction.Request request, ClusterState clusterState) { + XPackPlugin.checkReadyForXPackCustomMetadata(clusterState); MlMetadata currentMetadata = clusterState.getMetaData().custom(MLMetadataField.TYPE); MlMetadata newMetadata = new MlMetadata.Builder(currentMetadata) .putDatafeed(request.getDatafeed(), threadPool.getThreadContext()).build(); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java index 0f6a0f44cbf02..1a02379026f43 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/JobManager.java @@ -30,6 +30,7 @@ import org.elasticsearch.env.Environment; import org.elasticsearch.index.analysis.AnalysisRegistry; import org.elasticsearch.persistent.PersistentTasksCustomMetaData; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MachineLearningField; import org.elasticsearch.xpack.core.ml.MlMetadata; @@ -188,6 +189,9 @@ public void putJob(PutJobAction.Request request, AnalysisRegistry analysisRegist DEPRECATION_LOGGER.deprecated("Creating jobs with delimited data format is deprecated. Please use xcontent instead."); } + // pre-flight check, not necessarily required, but avoids figuring this out while on the CS update thread + XPackPlugin.checkReadyForXPackCustomMetadata(state); + MlMetadata currentMlMetadata = state.metaData().custom(MLMetadataField.TYPE); if (currentMlMetadata != null && currentMlMetadata.getJobs().containsKey(job.getId())) { actionListener.onFailure(ExceptionsHelper.jobAlreadyExists(job.getId())); @@ -565,6 +569,7 @@ private static MlMetadata.Builder createMlMetadataBuilder(ClusterState currentSt } private static ClusterState buildNewClusterState(ClusterState currentState, MlMetadata.Builder builder) { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); ClusterState.Builder newState = ClusterState.builder(currentState); newState.metaData(MetaData.builder(currentState.getMetaData()).putCustom(MLMetadataField.TYPE, builder.build()).build()); return newState.build(); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java index ce46139a18bdb..607787c3b9c2c 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlInitializationServiceTests.java @@ -19,12 +19,14 @@ import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ml.MLMetadataField; import org.elasticsearch.xpack.core.ml.MlMetadata; import org.junit.Before; import org.mockito.Mockito; import java.net.InetAddress; +import java.util.Collections; import java.util.concurrent.ExecutorService; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.atomic.AtomicBoolean; @@ -68,12 +70,18 @@ public void setUpMocks() { when(clusterService.getClusterName()).thenReturn(CLUSTER_NAME); } + private static DiscoveryNode newNode(String name) { + return new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), + Collections.singletonMap(XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "true"), Collections.emptySet(), + Version.CURRENT); + } + public void testInitialize() throws Exception { MlInitializationService initializationService = new MlInitializationService(Settings.EMPTY, threadPool, clusterService, client); ClusterState cs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT)) + .add(newNode("_node_id")) .localNodeId("_node_id") .masterNodeId("_node_id")) .metaData(MetaData.builder()) @@ -89,7 +97,7 @@ public void testInitialize_noMasterNode() throws Exception { ClusterState cs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT))) + .add(newNode("_node_id"))) .metaData(MetaData.builder()) .build(); initializationService.clusterChanged(new ClusterChangedEvent("_source", cs, cs)); @@ -98,12 +106,29 @@ public void testInitialize_noMasterNode() throws Exception { assertThat(initializationService.getDailyMaintenanceService(), is(nullValue())); } + public void testInitialize_nonXpackNodePresent() throws Exception { + MlInitializationService initializationService = new MlInitializationService(Settings.EMPTY, threadPool, clusterService, client); + + ClusterState cs = ClusterState.builder(new ClusterName("_name")) + .nodes(DiscoveryNodes.builder() + .add(newNode("_node_id")) + .add(new DiscoveryNode("_node_id2", new TransportAddress(InetAddress.getLoopbackAddress(), 9201), Version.CURRENT)) + .localNodeId("_node_id") + .masterNodeId("_node_id")) + .metaData(MetaData.builder()) + .build(); + initializationService.clusterChanged(new ClusterChangedEvent("_source", cs, cs)); + + verify(clusterService, times(0)).submitStateUpdateTask(eq("install-ml-metadata"), any()); + assertThat(initializationService.getDailyMaintenanceService(), is(nullValue())); + } + public void testInitialize_alreadyInitialized() throws Exception { MlInitializationService initializationService = new MlInitializationService(Settings.EMPTY, threadPool, clusterService, client); ClusterState cs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT)) + .add(newNode("_node_id")) .localNodeId("_node_id") .masterNodeId("_node_id")) .metaData(MetaData.builder() @@ -122,7 +147,7 @@ public void testInitialize_onlyOnce() throws Exception { ClusterState cs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT)) + .add(newNode("_node_id")) .localNodeId("_node_id") .masterNodeId("_node_id")) .metaData(MetaData.builder()) @@ -147,7 +172,7 @@ public void testInitialize_reintialiseAfterFailure() throws Exception { ClusterState cs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT)) + .add(newNode("_node_id")) .localNodeId("_node_id") .masterNodeId("_node_id")) .metaData(MetaData.builder()) @@ -180,7 +205,7 @@ public void testNodeGoesFromMasterToNonMasterAndBack() throws Exception { ClusterState masterCs = ClusterState.builder(new ClusterName("_name")) .nodes(DiscoveryNodes.builder() - .add(new DiscoveryNode("_node_id", new TransportAddress(InetAddress.getLoopbackAddress(), 9200), Version.CURRENT)) + .add(newNode("_node_id")) .localNodeId("_node_id") .masterNodeId("_node_id")) .metaData(MetaData.builder()) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 44f5afd4bdb07..0f183395df3f4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -317,7 +317,7 @@ public Collection createGuiceModules() { } modules.add(b -> XPackPlugin.bindFeatureSet(b, SecurityFeatureSet.class)); - + if (enabled == false) { modules.add(b -> { b.bind(Realms.class).toProvider(Providers.of(null)); // for SecurityFeatureSet @@ -905,11 +905,8 @@ public UnaryOperator> getIndexTemplateMetaDat @Override public Map> getInitialClusterStateCustomSupplier() { - if (enabled) { - return Collections.singletonMap(TokenMetaData.TYPE, () -> tokenService.get().getTokenMetaData()); - } else { - return Collections.emptyMap(); - } + // TODO: Remove this whole concept of InitialClusterStateCustomSupplier + return Collections.emptyMap(); } @Override diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index d23415f87dfcc..05ca6d9c0b90b 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -8,6 +8,9 @@ import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRefBuilder; +import org.elasticsearch.cluster.ClusterStateUpdateTask; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.common.Priority; import org.elasticsearch.core.internal.io.IOUtils; import org.apache.lucene.util.UnicodeUtil; import org.elasticsearch.ElasticsearchSecurityException; @@ -63,6 +66,7 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchHit; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.security.ScrollHelper; @@ -107,6 +111,7 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -1327,6 +1332,8 @@ public TimeValue masterNodeTimeout() { @Override public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); + if (tokenMetaData.equals(currentState.custom(TokenMetaData.TYPE))) { return currentState; } @@ -1347,6 +1354,15 @@ private void initialize(ClusterService clusterService) { return; } + if (state.nodes().isLocalNodeElectedMaster()) { + if (XPackPlugin.isReadyForXPackCustomMetadata(state)) { + installTokenMetadata(state.metaData()); + } else { + logger.debug("cannot add token metadata to cluster as the following nodes might not understand the metadata: {}", + () -> XPackPlugin.nodesNotReadyForXPackCustomMetadata(state)); + } + } + TokenMetaData custom = event.state().custom(TokenMetaData.TYPE); if (custom != null && custom.equals(getTokenMetaData()) == false) { logger.info("refresh keys"); @@ -1360,6 +1376,35 @@ private void initialize(ClusterService clusterService) { }); } + private final AtomicBoolean installTokenMetadataCheck = new AtomicBoolean(false); + + private void installTokenMetadata(MetaData metaData) { + if (metaData.custom(TokenMetaData.TYPE) == null) { + if (installTokenMetadataCheck.compareAndSet(false, true)) { + clusterService.submitStateUpdateTask("install-token-metadata", new ClusterStateUpdateTask(Priority.URGENT) { + @Override + public ClusterState execute(ClusterState currentState) throws Exception { + XPackPlugin.checkReadyForXPackCustomMetadata(currentState); + + if (currentState.custom(TokenMetaData.TYPE) == null) { + return ClusterState.builder(currentState).putCustom(TokenMetaData.TYPE, getTokenMetaData()).build(); + } else { + return currentState; + } + } + + @Override + public void onFailure(String source, Exception e) { + installTokenMetadataCheck.set(false); + logger.error("unable to install token metadata", e); + } + }); + } + } else { + installTokenMetadataCheck.set(false); + } + } + /** * For testing */ diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/service/TransportWatcherServiceAction.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/service/TransportWatcherServiceAction.java index fa78208494f94..6b2bb26ef45f0 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/service/TransportWatcherServiceAction.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/transport/actions/service/TransportWatcherServiceAction.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.watcher.WatcherMetaData; import org.elasticsearch.xpack.core.watcher.transport.actions.service.WatcherServiceAction; import org.elasticsearch.xpack.core.watcher.transport.actions.service.WatcherServiceRequest; @@ -86,6 +87,8 @@ protected WatcherServiceResponse newResponse(boolean acknowledged) { @Override public ClusterState execute(ClusterState clusterState) { + XPackPlugin.checkReadyForXPackCustomMetadata(clusterState); + WatcherMetaData newWatcherMetaData = new WatcherMetaData(manuallyStopped); WatcherMetaData currentMetaData = clusterState.metaData().custom(WatcherMetaData.TYPE); From ba94bdd14aff367e9c5d1b6549571757c1019857 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 May 2018 13:36:41 +0200 Subject: [PATCH 2/8] add comment about node setting --- .../main/java/org/elasticsearch/xpack/core/XPackPlugin.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 84ad5885f738f..8e80b8100b562 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -215,6 +215,10 @@ public Settings additionalSettings() { return super.additionalSettings(); } else { + // Unfortunately we cannot simply disallow any value for xpackInstalledNodeAttrSetting, because the + // internal cluster integration test framework will restart nodes with settings copied from the node + // immediately before it was stopped. The best we can do is reject inconsistencies. + // TODO: fix the test framework not to copy derived node settings upon restart. if (settings.get(xpackInstalledNodeAttrSetting) != null && settings.get(xpackInstalledNodeAttrSetting).equals("true") == false) { throw new IllegalArgumentException("Conflicting setting [" + xpackInstalledNodeAttrSetting + "]"); From 5f25ea3c14afc8f2b40d3542e8b0f26a1ddd88d2 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 May 2018 13:49:40 +0200 Subject: [PATCH 3/8] refactor checkReadyForXPackCustomMetadata into two methods and add javadoc --- .../elasticsearch/xpack/core/XPackPlugin.java | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 8e80b8100b562..131a4597444e2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -157,11 +157,15 @@ protected Clock getClock() { * and throws an exception otherwise. * This check should be called before installing any x-pack metadata to the cluster state, * to ensure that the other nodes that are part of the cluster will be able to deserialize - * that metadata. + * that metadata. Note that if the cluster state already contains x-pack metadata, this + * check assumes that the nodes are already ready to receive additional x-pack metadata. * Having this check properly in place everywhere allows to install x-pack into a cluster * using a rolling restart. */ public static void checkReadyForXPackCustomMetadata(ClusterState clusterState) { + if (alreadyContainsXPackCustomMetadata(clusterState)) { + return; + } List notReadyNodes = nodesNotReadyForXPackCustomMetadata(clusterState); if (notReadyNodes.isEmpty() == false) { throw new IllegalStateException("The following nodes are not ready yet for enabling x-pack custom metadata: " + notReadyNodes); @@ -173,7 +177,7 @@ public static void checkReadyForXPackCustomMetadata(ClusterState clusterState) { * See {@link #checkReadyForXPackCustomMetadata} for more details. */ public static boolean isReadyForXPackCustomMetadata(ClusterState clusterState) { - return nodesNotReadyForXPackCustomMetadata(clusterState).isEmpty(); + return alreadyContainsXPackCustomMetadata(clusterState) || nodesNotReadyForXPackCustomMetadata(clusterState).isEmpty(); } /** @@ -181,17 +185,7 @@ public static boolean isReadyForXPackCustomMetadata(ClusterState clusterState) { * See {@link #checkReadyForXPackCustomMetadata} for more details. */ public static List nodesNotReadyForXPackCustomMetadata(ClusterState clusterState) { - // check if there's already x-pack metadata in the cluster state; if so, any further metadata won't hurt - final MetaData metaData = clusterState.metaData(); - if (metaData.custom(LicensesMetaData.TYPE) != null || - metaData.custom(MLMetadataField.TYPE) != null || - metaData.custom(WatcherMetaData.TYPE) != null || - clusterState.custom(TokenMetaData.TYPE) != null) { - return Collections.emptyList(); - } - - // if there's no x-pack metadata yet in the cluster state, check that all nodes would be capable - // of deserializing newly added x-pack metadata + // check that all nodes would be capable of deserializing newly added x-pack metadata final List notReadyNodes = StreamSupport.stream(clusterState.nodes().spliterator(), false).filter(node -> { final String xpackInstalledAttr = node.getAttributes().getOrDefault(XPACK_INSTALLED_NODE_ATTR, "false"); @@ -204,6 +198,14 @@ public static List nodesNotReadyForXPackCustomMetadata(ClusterSta return notReadyNodes; } + private static boolean alreadyContainsXPackCustomMetadata(ClusterState clusterState) { + final MetaData metaData = clusterState.metaData(); + return metaData.custom(LicensesMetaData.TYPE) != null || + metaData.custom(MLMetadataField.TYPE) != null || + metaData.custom(WatcherMetaData.TYPE) != null || + clusterState.custom(TokenMetaData.TYPE) != null; + } + @Override public Settings additionalSettings() { final String xpackInstalledNodeAttrSetting = "node.attr." + XPACK_INSTALLED_NODE_ATTR; From e1437e8d4dded66c359b61d003ad94caf2407ab0 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 May 2018 14:19:36 +0200 Subject: [PATCH 4/8] move note about removal of InitialClusterStateCustomSupplier to top-level class --- .../main/java/org/elasticsearch/plugins/ClusterPlugin.java | 2 ++ .../java/org/elasticsearch/xpack/security/Security.java | 6 ------ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/ClusterPlugin.java b/server/src/main/java/org/elasticsearch/plugins/ClusterPlugin.java index 5e58aa5a3a926..61145c7a1d7cf 100644 --- a/server/src/main/java/org/elasticsearch/plugins/ClusterPlugin.java +++ b/server/src/main/java/org/elasticsearch/plugins/ClusterPlugin.java @@ -70,6 +70,8 @@ default void onNodeStarted() { * Returns a map of {@link ClusterState.Custom} supplier that should be invoked to initialize the initial clusterstate. * This allows custom clusterstate extensions to be always present and prevents invariants where clusterstates are published * but customs are not initialized. + * + * TODO: Remove this whole concept of InitialClusterStateCustomSupplier, it's not used anymore */ default Map> getInitialClusterStateCustomSupplier() { return Collections.emptyMap(); } } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 0f183395df3f4..133093df33a13 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -903,12 +903,6 @@ public UnaryOperator> getIndexTemplateMetaDat }; } - @Override - public Map> getInitialClusterStateCustomSupplier() { - // TODO: Remove this whole concept of InitialClusterStateCustomSupplier - return Collections.emptyMap(); - } - @Override public Function> getFieldFilter() { if (enabled) { From 87ad80f214bbac9c6657ff2adde3143186a25b08 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 May 2018 14:27:15 +0200 Subject: [PATCH 5/8] simplify installTokenMetadata --- .../xpack/security/authc/TokenService.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index 05ca6d9c0b90b..2934fb8062de4 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -1376,14 +1376,15 @@ private void initialize(ClusterService clusterService) { }); } - private final AtomicBoolean installTokenMetadataCheck = new AtomicBoolean(false); + // to prevent too many cluster state update tasks to be queued for doing the same update + private final AtomicBoolean installTokenMetadataInProgress = new AtomicBoolean(false); private void installTokenMetadata(MetaData metaData) { if (metaData.custom(TokenMetaData.TYPE) == null) { - if (installTokenMetadataCheck.compareAndSet(false, true)) { + if (installTokenMetadataInProgress.compareAndSet(false, true)) { clusterService.submitStateUpdateTask("install-token-metadata", new ClusterStateUpdateTask(Priority.URGENT) { @Override - public ClusterState execute(ClusterState currentState) throws Exception { + public ClusterState execute(ClusterState currentState) { XPackPlugin.checkReadyForXPackCustomMetadata(currentState); if (currentState.custom(TokenMetaData.TYPE) == null) { @@ -1395,13 +1396,16 @@ public ClusterState execute(ClusterState currentState) throws Exception { @Override public void onFailure(String source, Exception e) { - installTokenMetadataCheck.set(false); + installTokenMetadataInProgress.set(false); logger.error("unable to install token metadata", e); } + + @Override + public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) { + installTokenMetadataInProgress.set(false); + } }); } - } else { - installTokenMetadataCheck.set(false); } } From cec970456e9595cdfb31d265e0fd535c3ae92a9e Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Tue, 22 May 2018 17:23:04 +0200 Subject: [PATCH 6/8] add unit test for isReadyForXPackCustomMetadata --- .../xpack/core/XPackPluginTests.java | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java index ae193850347a9..46468bcc83ef5 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java @@ -5,12 +5,22 @@ */ package org.elasticsearch.xpack.core; +import org.elasticsearch.Version; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.xpack.core.security.authc.TokenMetaData; import org.elasticsearch.xpack.core.ssl.SSLService; +import java.util.Collections; +import java.util.Map; + import static org.hamcrest.Matchers.containsString; public class XPackPluginTests extends ESTestCase { @@ -39,6 +49,45 @@ public void testXPackInstalledAttrExists() throws Exception { assertEquals("true", xpackPlugin.additionalSettings().get("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR)); } + public void testNodesNotReadyForXPackCustomMetadata() { + boolean compatible; + boolean nodesCompatible = true; + DiscoveryNodes.Builder discoveryNodes = DiscoveryNodes.builder(); + + for (int i = 0; i < randomInt(3); i++) { + final Version version = VersionUtils.randomVersion(random()); + final Map attributes; + if (randomBoolean() && version.onOrAfter(Version.V_6_3_0)) { + attributes = Collections.singletonMap(XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "true"); + } else { + nodesCompatible = false; + attributes = Collections.emptyMap(); + } + + discoveryNodes.add(new DiscoveryNode("node_" + i, buildNewFakeTransportAddress(), attributes, Collections.emptySet(), + Version.CURRENT)); + } + ClusterState.Builder clusterStateBuilder = ClusterState.builder(ClusterName.DEFAULT); + + if (randomBoolean()) { + clusterStateBuilder.putCustom(TokenMetaData.TYPE, new TokenMetaData(Collections.emptyList(), new byte[0])); + compatible = true; + } else { + compatible = nodesCompatible; + } + + ClusterState clusterState = clusterStateBuilder.nodes(discoveryNodes.build()).build(); + + assertEquals(XPackPlugin.nodesNotReadyForXPackCustomMetadata(clusterState).isEmpty(), nodesCompatible); + assertEquals(XPackPlugin.isReadyForXPackCustomMetadata(clusterState), compatible); + + if (compatible == false) { + IllegalStateException e = expectThrows(IllegalStateException.class, + () -> XPackPlugin.checkReadyForXPackCustomMetadata(clusterState)); + assertThat(e.getMessage(), containsString("The following nodes are not ready yet for enabling x-pack custom metadata:")); + } + } + private XPackPlugin createXPackPlugin(Settings settings) throws Exception { return new XPackPlugin(settings, null){ From 1b6ae1d9a5976a8178948afbdc71a4aeb872d301 Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Wed, 23 May 2018 09:16:50 +0200 Subject: [PATCH 7/8] Simplify node attributes check --- .../elasticsearch/xpack/core/XPackPlugin.java | 17 ++++------------- .../xpack/core/XPackPluginTests.java | 17 +++++------------ 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 131a4597444e2..77d521e2d4322 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -210,22 +210,13 @@ private static boolean alreadyContainsXPackCustomMetadata(ClusterState clusterSt public Settings additionalSettings() { final String xpackInstalledNodeAttrSetting = "node.attr." + XPACK_INSTALLED_NODE_ATTR; - if (transportClientMode) { - if (settings.get(xpackInstalledNodeAttrSetting) != null) { - throw new IllegalArgumentException("Directly setting [" + xpackInstalledNodeAttrSetting + "] is not permitted"); - } + if (settings.get(xpackInstalledNodeAttrSetting) != null) { + throw new IllegalArgumentException("Directly setting [" + xpackInstalledNodeAttrSetting + "] is not permitted"); + } + if (transportClientMode) { return super.additionalSettings(); } else { - // Unfortunately we cannot simply disallow any value for xpackInstalledNodeAttrSetting, because the - // internal cluster integration test framework will restart nodes with settings copied from the node - // immediately before it was stopped. The best we can do is reject inconsistencies. - // TODO: fix the test framework not to copy derived node settings upon restart. - if (settings.get(xpackInstalledNodeAttrSetting) != null && - settings.get(xpackInstalledNodeAttrSetting).equals("true") == false) { - throw new IllegalArgumentException("Conflicting setting [" + xpackInstalledNodeAttrSetting + "]"); - } - return Settings.builder().put(super.additionalSettings()).put(xpackInstalledNodeAttrSetting, "true").build(); } } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java index 46468bcc83ef5..59731cab71db8 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/XPackPluginTests.java @@ -25,25 +25,18 @@ public class XPackPluginTests extends ESTestCase { - public void testXPackInstalledAttrClashOnTransport() throws Exception { + public void testXPackInstalledAttrClash() throws Exception { Settings.Builder builder = Settings.builder(); - builder.put("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "true"); - builder.put(Client.CLIENT_TYPE_SETTING_S.getKey(), "transport"); + builder.put("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR, randomBoolean()); + if (randomBoolean()) { + builder.put(Client.CLIENT_TYPE_SETTING_S.getKey(), "transport"); + } XPackPlugin xpackPlugin = createXPackPlugin(builder.put("path.home", createTempDir()).build()); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, xpackPlugin::additionalSettings); assertThat(e.getMessage(), containsString("Directly setting [node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR + "] is not permitted")); } - public void testXPackInstalledAttrClashOnNode() throws Exception { - Settings.Builder builder = Settings.builder(); - builder.put("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR, "false"); - XPackPlugin xpackPlugin = createXPackPlugin(builder.put("path.home", createTempDir()).build()); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, xpackPlugin::additionalSettings); - assertThat(e.getMessage(), - containsString("Conflicting setting [node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR + "]")); - } - public void testXPackInstalledAttrExists() throws Exception { XPackPlugin xpackPlugin = createXPackPlugin(Settings.builder().put("path.home", createTempDir()).build()); assertEquals("true", xpackPlugin.additionalSettings().get("node.attr." + XPackPlugin.XPACK_INSTALLED_NODE_ATTR)); From a70ea645a78504f15b51cec37715de5430b09bfd Mon Sep 17 00:00:00 2001 From: Yannick Welsch Date: Wed, 23 May 2018 11:37:47 +0200 Subject: [PATCH 8/8] Fix ML tests --- .../xpack/ml/MachineLearningTests.java | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java index 9b141380c65eb..2ced5cb6f8bd2 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MachineLearningTests.java @@ -37,13 +37,11 @@ public void testNoAttributes_givenNoClash() { public void testNoAttributes_givenSameAndMlEnabled() { Settings.Builder builder = Settings.builder(); if (randomBoolean()) { - builder.put("xpack.ml.enabled", true); - builder.put("node.attr.ml.enabled", true); + builder.put("xpack.ml.enabled", randomBoolean()); } if (randomBoolean()) { int maxOpenJobs = randomIntBetween(5, 15); builder.put("xpack.ml.max_open_jobs", maxOpenJobs); - builder.put("node.attr.ml.max_open_jobs", maxOpenJobs); } MachineLearning machineLearning = createMachineLearning(builder.put("path.home", createTempDir()).build()); assertNotNull(machineLearning.additionalSettings()); @@ -51,16 +49,8 @@ public void testNoAttributes_givenSameAndMlEnabled() { public void testNoAttributes_givenClash() { Settings.Builder builder = Settings.builder(); - boolean enabled = true; - if (randomBoolean()) { - enabled = randomBoolean(); - builder.put("xpack.ml.enabled", enabled); - } - if (randomBoolean()) { - builder.put("xpack.ml.max_open_jobs", randomIntBetween(9, 12)); - } if (randomBoolean()) { - builder.put("node.attr.ml.enabled", !enabled); + builder.put("node.attr.ml.enabled", randomBoolean()); } else { builder.put("node.attr.ml.max_open_jobs", randomIntBetween(13, 15)); }