From 2d49d7d74b3ada79139c9dba8f8d4e2a4809b6b1 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Thu, 13 Nov 2025 16:11:17 +0000 Subject: [PATCH] Fix for GET /_migration/deprecations doesn't report node deprecations if low watermark exceeded and GET /_migration/deprecations doesn't report node-level failures properly (#137964) Fix for GET /_migration/deprecations doesn't report node deprecations if low watermark exceeded and GET /_migration/deprecations doesn't report node-level failures properly Closes #137010 Closes #137004 --- docs/changelog/137964.yaml | 9 ++ .../deprecation/NodeDeprecationChecker.java | 2 +- .../TransportNodeDeprecationCheckAction.java | 21 ++-- ...nsportNodeDeprecationCheckActionTests.java | 106 +++++++++++++++++- 4 files changed, 126 insertions(+), 12 deletions(-) create mode 100644 docs/changelog/137964.yaml diff --git a/docs/changelog/137964.yaml b/docs/changelog/137964.yaml new file mode 100644 index 0000000000000..aac2fbcd171b9 --- /dev/null +++ b/docs/changelog/137964.yaml @@ -0,0 +1,9 @@ +pr: 137964 +summary: Fix for GET /_migration/deprecations doesn't report node deprecations if + low watermark exceeded and GET /_migration/deprecations doesn't report node-level + failures properly +area: Infra/Core +type: bug +issues: + - 137010 + - 137004 diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecker.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecker.java index a2e9ed12a2298..4342e16444489 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecker.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecker.java @@ -52,7 +52,7 @@ public void check(Client client, ActionListener> listener .collect(Collectors.toList()); logger.warn("nodes failed to run deprecation checks: {}", failedNodeIds); for (FailedNodeException failure : response.failures()) { - logger.debug("node {} failed to run deprecation checks: {}", failure.nodeId(), failure); + logger.atWarn().withThrowable(failure).log("node {} failed to run deprecation checks", failure.nodeId()); } } l.onResponse(reduceToDeprecationIssues(response)); diff --git a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java index befe0bd6b41a4..0f885b7d27856 100644 --- a/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java +++ b/x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckAction.java @@ -33,10 +33,10 @@ import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; -import java.util.Objects; import static org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING; @@ -107,11 +107,10 @@ protected NodesDeprecationCheckAction.NodeResponse newNodeResponse(StreamInput i @Override protected NodesDeprecationCheckAction.NodeResponse nodeOperation(NodesDeprecationCheckAction.NodeRequest request, Task task) { - return nodeOperation(request, NodeDeprecationChecks.SINGLE_NODE_CHECKS); + return nodeOperation(NodeDeprecationChecks.SINGLE_NODE_CHECKS); } NodesDeprecationCheckAction.NodeResponse nodeOperation( - NodesDeprecationCheckAction.NodeRequest request, List< NodeDeprecationChecks.NodeDeprecationCheck< Settings, @@ -131,10 +130,18 @@ NodesDeprecationCheckAction.NodeResponse nodeOperation( .metadata(Metadata.builder(metadata).transientSettings(transientSettings).persistentSettings(persistentSettings).build()) .build(); - List issues = nodeSettingsChecks.stream() - .map(c -> c.apply(filteredNodeSettings, pluginsService.info(), filteredClusterState, licenseState)) - .filter(Objects::nonNull) - .toList(); + List issues = new ArrayList<>(); + for (NodeDeprecationChecks.NodeDeprecationCheck< + Settings, + PluginsAndModules, + ClusterState, + XPackLicenseState, + DeprecationIssue> c : nodeSettingsChecks) { + DeprecationIssue deprecationIssue = c.apply(filteredNodeSettings, pluginsService.info(), filteredClusterState, licenseState); + if (deprecationIssue != null) { + issues.add(deprecationIssue); + } + } DeprecationIssue watermarkIssue = checkDiskLowWatermark( filteredNodeSettings, filteredClusterState.metadata().settings(), diff --git a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java index a0a37f2bb52d1..fb884d988beb1 100644 --- a/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java +++ b/x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/TransportNodeDeprecationCheckActionTests.java @@ -16,9 +16,11 @@ import org.elasticsearch.cluster.DiskUsage; import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.routing.allocation.DiskThresholdSettings; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.ClusterSettings; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.CollectionUtils; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.plugins.PluginsService; import org.elasticsearch.test.ESTestCase; @@ -26,17 +28,25 @@ import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.transport.TransportService; import org.elasticsearch.xpack.core.deprecation.DeprecationIssue; +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.hamcrest.Matcher; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.mockito.Mockito; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; +import static org.elasticsearch.common.settings.ClusterSettings.BUILT_IN_CLUSTER_SETTINGS; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; import static org.mockito.Mockito.when; public class TransportNodeDeprecationCheckActionTests extends ESTestCase { @@ -98,7 +108,6 @@ public void testNodeOperation() { actionFilters, clusterInfoService ); - NodesDeprecationCheckAction.NodeRequest nodeRequest = null; AtomicReference visibleNodeSettings = new AtomicReference<>(); AtomicReference visibleClusterStateMetadataSettings = new AtomicReference<>(); NodeDeprecationChecks.NodeDeprecationCheck< @@ -118,7 +127,7 @@ public void testNodeOperation() { ClusterState, XPackLicenseState, DeprecationIssue>> nodeSettingsChecks = List.of(nodeSettingCheck); - transportNodeDeprecationCheckAction.nodeOperation(nodeRequest, nodeSettingsChecks); + transportNodeDeprecationCheckAction.nodeOperation(nodeSettingsChecks); settingsBuilder = Settings.builder(); settingsBuilder.put("some.undeprecated.property", "someValue3"); settingsBuilder.putList("some.undeprecated.list.property", List.of("someValue4", "someValue5")); @@ -137,7 +146,7 @@ public void testNodeOperation() { .putList(TransportDeprecationInfoAction.SKIP_DEPRECATIONS_SETTING.getKey(), List.of("some.undeprecated.property")) .build(); clusterSettings.applySettings(newSettings); - transportNodeDeprecationCheckAction.nodeOperation(nodeRequest, nodeSettingsChecks); + transportNodeDeprecationCheckAction.nodeOperation(nodeSettingsChecks); settingsBuilder = Settings.builder(); settingsBuilder.put("some.deprecated.property", "someValue1"); settingsBuilder.put("some.other.bad.deprecated.property", "someValue2"); @@ -163,7 +172,7 @@ public void testCheckDiskLowWatermark() { settingsBuilder.put("cluster.routing.allocation.disk.watermark.low", "10%"); Settings settingsWithLowWatermark = settingsBuilder.build(); Settings dynamicSettings = settingsWithLowWatermark; - ClusterSettings clusterSettings = new ClusterSettings(nodeSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + ClusterSettings clusterSettings = new ClusterSettings(nodeSettings, BUILT_IN_CLUSTER_SETTINGS); String nodeId = "123"; long totalBytesOnMachine = 100; long totalBytesFree = 70; @@ -208,4 +217,93 @@ public void testCheckDiskLowWatermark() { assertNotNull(issue); assertEquals("Disk usage exceeds low watermark", issue.getMessage()); } + + public void testDiskLowWatermarkIsIncludedInDeprecationWarnings() { + Settings.Builder settingsBuilder = Settings.builder(); + String deprecatedSettingKey = "some.deprecated.property"; + settingsBuilder.put(deprecatedSettingKey, "someValue1"); + settingsBuilder.put(DiskThresholdSettings.CLUSTER_ROUTING_ALLOCATION_LOW_DISK_WATERMARK_SETTING.getKey(), "10%"); + Settings nodeSettings = settingsBuilder.build(); + final XPackLicenseState licenseState = null; + ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metadata(Metadata.builder().build()).build(); + ClusterService clusterService = Mockito.mock(ClusterService.class); + when(clusterService.state()).thenReturn(clusterState); + ClusterSettings clusterSettings = new ClusterSettings( + nodeSettings, + Set.copyOf(CollectionUtils.appendToCopy(BUILT_IN_CLUSTER_SETTINGS, TransportDeprecationInfoAction.SKIP_DEPRECATIONS_SETTING)) + ); + when((clusterService.getClusterSettings())).thenReturn(clusterSettings); + DiscoveryNode node = Mockito.mock(DiscoveryNode.class); + String nodeId = "123"; + when(node.getId()).thenReturn(nodeId); + TransportService transportService = Mockito.mock(TransportService.class); + when(transportService.getThreadPool()).thenReturn(threadPool); + when(transportService.getLocalNode()).thenReturn(node); + PluginsService pluginsService = Mockito.mock(PluginsService.class); + ActionFilters actionFilters = Mockito.mock(ActionFilters.class); + ClusterInfoService clusterInfoService = Mockito.mock(ClusterInfoService.class); + long totalBytesOnMachine = 100; + long totalBytesFree = 70; + ClusterInfo clusterInfo = ClusterInfo.builder() + .mostAvailableSpaceUsage(Map.of(nodeId, new DiskUsage(nodeId, "", "", totalBytesOnMachine, totalBytesFree))) + .build(); + when(clusterInfoService.getClusterInfo()).thenReturn(clusterInfo); + TransportNodeDeprecationCheckAction transportNodeDeprecationCheckAction = new TransportNodeDeprecationCheckAction( + nodeSettings, + threadPool, + licenseState, + clusterService, + transportService, + pluginsService, + actionFilters, + clusterInfoService + ); + + NodeDeprecationChecks.NodeDeprecationCheck< + Settings, + PluginsAndModules, + ClusterState, + XPackLicenseState, + DeprecationIssue> deprecationCheck = (first, second, third, fourth) -> { + if (first.keySet().contains(deprecatedSettingKey)) { + return new DeprecationIssue(DeprecationIssue.Level.WARNING, "Deprecated setting", null, null, false, null); + } + return null; + }; + NodesDeprecationCheckAction.NodeResponse nodeResponse = transportNodeDeprecationCheckAction.nodeOperation( + Collections.singletonList(deprecationCheck) + ); + List deprecationIssues = nodeResponse.getDeprecationIssues(); + assertThat(deprecationIssues, hasSize(2)); + assertThat(deprecationIssues, hasItem(new DeprecationIssueMatcher(DeprecationIssue.Level.WARNING, equalTo("Deprecated setting")))); + assertThat( + deprecationIssues, + hasItem(new DeprecationIssueMatcher(DeprecationIssue.Level.CRITICAL, equalTo("Disk usage exceeds low watermark"))) + ); + } + + private static class DeprecationIssueMatcher extends BaseMatcher { + private final DeprecationIssue.Level level; + private final Matcher messageMatcher; + + private DeprecationIssueMatcher(DeprecationIssue.Level level, Matcher messageMatcher) { + this.level = level; + this.messageMatcher = messageMatcher; + } + + @Override + public boolean matches(Object actual) { + if (actual instanceof DeprecationIssue == false) { + return false; + } + DeprecationIssue actualDeprecationIssue = (DeprecationIssue) actual; + return level.equals(actualDeprecationIssue.getLevel()) && messageMatcher.matches(actualDeprecationIssue.getMessage()); + } + + @Override + public void describeTo(Description description) { + description.appendText("deprecation issue with level: ").appendValue(level).appendText(" and message: "); + messageMatcher.describeTo(description); + } + } }