Use original settings on full-cluster restart

server/src/main/java/org/elasticsearch/node/Node.java

@@ -230,6 +230,7 @@ public static final Settings addNodeNameIfNeeded(Settings settings, final String
     private final Lifecycle lifecycle = new Lifecycle();
     private final Injector injector;
     private final Settings settings;
+    private final Settings originalSettings;
     private final Environment environment;
     private final NodeEnvironment nodeEnvironment;
     private final PluginsService pluginsService;
@@ -260,6 +261,7 @@ protected Node(final Environment environment, Collection<Class<? extends Plugin>
             logger.info("initializing ...");
         }
         try {
+            originalSettings = environment.settings();
             Settings tmpSettings = Settings.builder().put(environment.settings())
                 .put(Client.CLIENT_TYPE_SETTING_S.getKey(), CLIENT_TYPE).build();
 
@@ -563,7 +565,14 @@ protected void processRecoverySettings(ClusterSettings clusterSettings, Recovery
     }
 
     /**
-     * The settings that were used to create the node.
+     * The original settings that were used to create the node
+     */
+    public Settings originalSettings() {
+        return originalSettings;
+    }
+
+    /**
+     * The settings that are used by this node. Contains original settings as well as additional settings provided by plugins.
      */
     public Settings settings() {
         return this.settings;

test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java

@@ -906,7 +906,7 @@ private void clearDataIfNeeded(RestartCallback callback) throws IOException {
 
         private void createNewNode(final Settings newSettings) {
             final long newIdSeed = NodeEnvironment.NODE_ID_SEED_SETTING.get(node.settings()) + 1; // use a new seed to make sure we have new node id
-            Settings finalSettings = Settings.builder().put(node.settings()).put(newSettings).put(NodeEnvironment.NODE_ID_SEED_SETTING.getKey(), newIdSeed).build();
+            Settings finalSettings = Settings.builder().put(node.originalSettings()).put(newSettings).put(NodeEnvironment.NODE_ID_SEED_SETTING.getKey(), newIdSeed).build();
             if (DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.exists(finalSettings) == false) {
                 throw new IllegalStateException(DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey() +
                     " is not configured after restart of [" + name + "]");

test/framework/src/test/java/org/elasticsearch/test/test/InternalTestClusterTests.java

@@ -469,9 +469,11 @@ public Settings transportClientSettings() {
         };
         String nodePrefix = "test";
         Path baseDir = createTempDir();
+        List<Class<? extends Plugin>> plugins = new ArrayList<>(mockPlugins());
+        plugins.add(NodeAttrCheckPlugin.class);
         InternalTestCluster cluster = new InternalTestCluster(randomLong(), baseDir, false, true, 2, 2,
             "test", nodeConfigurationSource, 0, nodePrefix,
-            mockPlugins(), Function.identity());
+            plugins, Function.identity());
         try {
             cluster.beforeTest(random(), 0.0);
             assertMMNinNodeSetting(cluster, 2);
@@ -502,4 +504,26 @@ public Settings onNodeStopped(String nodeName) throws Exception {
             cluster.close();
         }
     }
+
+    /**
+     * Plugin that adds a simple node attribute as setting and checks if that node attribute is not already defined.
+     * Allows to check that the full-cluster restart logic does not copy over plugin-derived settings.
+     */
+    public static class NodeAttrCheckPlugin extends Plugin {
+
+        private final Settings settings;
+
+        public NodeAttrCheckPlugin(Settings settings) {
+            this.settings = settings;
+        }
+
+        @Override
+        public Settings additionalSettings() {
+            if (settings.get("node.attr.dummy") != null) {
+                fail("dummy setting already exists");
+            }
+            return Settings.builder().put("node.attr.dummy", true).build();
+        }
+
+    }
 }

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java

@@ -316,12 +316,8 @@ public Settings additionalSettings() {
     }
 
     private void addMlNodeAttribute(Settings.Builder additionalSettings, String attrName, String value) {
-        // Unfortunately we cannot simply disallow any value, 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, and report this in a way that
-        // makes clear that setting the node attribute directly is not allowed.
         String oldValue = settings.get(attrName);
-        if (oldValue == null || oldValue.equals(value)) {
+        if (oldValue == null) {
             additionalSettings.put(attrName, value);
         } else {
             reportClashingNodeAttribute(attrName);
@@ -487,7 +483,7 @@ public List<RestHandler> getRestHandlers(Settings settings, RestController restC
             new RestStartDatafeedAction(settings, restController),
             new RestStopDatafeedAction(settings, restController),
             new RestDeleteModelSnapshotAction(settings, restController),
-            new RestDeleteExpiredDataAction(settings, restController), 
+            new RestDeleteExpiredDataAction(settings, restController),
             new RestForecastJobAction(settings, restController),
             new RestGetCalendarsAction(settings, restController),
             new RestPutCalendarAction(settings, restController),