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 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),