Use original settings on full-cluster restart (#30780)

When doing a node restart using the test framework, the restarted node does not only use the
settings provided to the original node, but also additional settings provided by plugin extensions,
which does not correspond to the settings that a node would have on a true restart.

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

@@ -230,6 +230,7 @@ public class Node implements Closeable {
     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 @@ public class Node implements Closeable {
             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 @@ public class Node implements Closeable {
     }
 
     /**
-     * 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 @@ public final class InternalTestCluster extends TestCluster {
 
         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 class InternalTestClusterTests extends ESTestCase {
         };
         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 class InternalTestClusterTests extends ESTestCase {
             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 class MachineLearning extends Plugin implements ActionPlugin, AnalysisPlu
     }
 
     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 class MachineLearning extends Plugin implements ActionPlugin, AnalysisPlu
             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),