Emit settings deprecation logging on empty update

When executing a cluster settings update that leaves the cluster state
unchanged, we skip validation and this avoids deprecation logging for
deprecated settings in the cluster state. This commit addresses this by
running validation even if the settings are unchanged.

Relates #27017

core/src/main/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdater.java

@@ -58,35 +58,40 @@ final class SettingsUpdater {
         persistentSettings.put(currentState.metaData().persistentSettings());
         changed |= clusterSettings.updateDynamicSettings(persistentToApply, persistentSettings, persistentUpdates, "persistent");
 
-        if (!changed) {
-            return currentState;
-        }
-
-        MetaData.Builder metaData = MetaData.builder(currentState.metaData())
-            .persistentSettings(persistentSettings.build())
-            .transientSettings(transientSettings.build());
+        final ClusterState clusterState;
+        if (changed) {
+            MetaData.Builder metaData = MetaData.builder(currentState.metaData())
+                    .persistentSettings(persistentSettings.build())
+                    .transientSettings(transientSettings.build());
 
-        ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
-        boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings())
-            || MetaData.SETTING_READ_ONLY_SETTING.get(metaData.transientSettings());
-        if (updatedReadOnly) {
-            blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
-        } else {
-            blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
-        }
-        boolean updatedReadOnlyAllowDelete = MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.persistentSettings())
-            || MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.transientSettings());
-        if (updatedReadOnlyAllowDelete) {
-            blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
+            ClusterBlocks.Builder blocks = ClusterBlocks.builder().blocks(currentState.blocks());
+            boolean updatedReadOnly = MetaData.SETTING_READ_ONLY_SETTING.get(metaData.persistentSettings())
+                    || MetaData.SETTING_READ_ONLY_SETTING.get(metaData.transientSettings());
+            if (updatedReadOnly) {
+                blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
+            } else {
+                blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_BLOCK);
+            }
+            boolean updatedReadOnlyAllowDelete = MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.persistentSettings())
+                    || MetaData.SETTING_READ_ONLY_ALLOW_DELETE_SETTING.get(metaData.transientSettings());
+            if (updatedReadOnlyAllowDelete) {
+                blocks.addGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
+            } else {
+                blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
+            }
+            clusterState = builder(currentState).metaData(metaData).blocks(blocks).build();
         } else {
-            blocks.removeGlobalBlock(MetaData.CLUSTER_READ_ONLY_ALLOW_DELETE_BLOCK);
+            clusterState = currentState;
         }
-        ClusterState build = builder(currentState).metaData(metaData).blocks(blocks).build();
-        Settings settings = build.metaData().settings();
-        // now we try to apply things and if they are invalid we fail
-        // this dryRun will validate & parse settings but won't actually apply them.
+
+        /*
+         * Now we try to apply things and if they are invalid we fail. This dry run will validate, parse settings, and trigger deprecation
+         * logging, but will not actually apply them.
+         */
+        final Settings settings = clusterState.metaData().settings();
         clusterSettings.validateUpdate(settings);
-        return build;
+
+        return clusterState;
     }
 
 

core/src/test/java/org/elasticsearch/action/admin/cluster/settings/SettingsUpdaterTests.java

@@ -23,10 +23,15 @@ import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.routing.allocation.allocator.BalancedShardsAllocator;
 import org.elasticsearch.common.settings.ClusterSettings;
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.test.ESTestCase;
 
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 public class SettingsUpdaterTests extends ESTestCase {
 
@@ -132,4 +137,30 @@ public class SettingsUpdaterTests extends ESTestCase {
         assertEquals(clusterState.blocks().global().size(), 0);
 
     }
+
+    public void testDeprecationLogging() {
+        Setting<String> deprecatedSetting =
+                Setting.simpleString("deprecated.setting", Property.Dynamic, Property.NodeScope, Property.Deprecated);
+        final Settings settings = Settings.builder().put("deprecated.setting", "foo").build();
+        final Set<Setting<?>> settingsSet =
+                Stream.concat(ClusterSettings.BUILT_IN_CLUSTER_SETTINGS.stream(), Stream.of(deprecatedSetting)).collect(Collectors.toSet());
+        final ClusterSettings clusterSettings = new ClusterSettings(settings, settingsSet);
+        clusterSettings.addSettingsUpdateConsumer(deprecatedSetting, s -> {});
+        final SettingsUpdater settingsUpdater = new SettingsUpdater(clusterSettings);
+        final ClusterState clusterState =
+                ClusterState.builder(new ClusterName("foo")).metaData(MetaData.builder().persistentSettings(settings).build()).build();
+
+        final Settings toApplyDebug = Settings.builder().put("logger.org.elasticsearch", "debug").build();
+        final ClusterState afterDebug = settingsUpdater.updateSettings(clusterState, toApplyDebug, Settings.EMPTY);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] { deprecatedSetting });
+
+        final Settings toApplyUnset = Settings.builder().putNull("logger.org.elasticsearch").build();
+        final ClusterState afterUnset = settingsUpdater.updateSettings(afterDebug, toApplyUnset, Settings.EMPTY);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] { deprecatedSetting });
+
+        // we also check that if no settings are changed, deprecation logging still occurs
+        settingsUpdater.updateSettings(afterUnset, toApplyUnset, Settings.EMPTY);
+        assertSettingDeprecationsAndWarnings(new Setting<?>[] { deprecatedSetting });
+    }
+
 }