Check both node and cluster settings in NodeDeprecationChecks #82487
Conversation
elasticmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample |
dakrone
left a comment
dakrone
left a comment
There was a problem hiding this comment.
LGTM, thanks for working on this so quickly. I left two really minor comments.
In the future, maybe we should consider creating a NodeDeprecationContext that encapsulates all of these things (node settings, cluster settings, license, etc) so that if we need to pass additional information into the deprecation methods we don't have to manually change a bunch of method arities. Not that we should make any of those changes here, but maybe in future work on the deprecation framework.
| final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList()); | ||
| final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0); | ||
| Metadata metadata = Metadata.builder().persistentSettings(clusterSettings).build(); | ||
| ; |
There was a problem hiding this comment.
I'd apparently typed two semicolons at the end of the previous line, and spotlessApply fixed that for me. :)
| Settings nodettings = Settings.builder().build(); | ||
| final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList()); | ||
| final XPackLicenseState licenseState = new XPackLicenseState(Settings.EMPTY, () -> 0); | ||
| Metadata metadata = Metadata.builder().persistentSettings(clusterSettings).build(); |
There was a problem hiding this comment.
Perhaps we should randomize here between using persistentSettings and transientSettings to make sure we catch both?
|
@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample |
1 similar comment
|
@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample |
masseyke
deleted the
fix/check-dynamic-settings-in-node-deprecation-checks
branch
albertzaharovits
changed the title
This is a forward-port of elastic#82487, elastic#83544, elastic#83601, elastic#84145, and elastic#84246, but given that the branches had diverged so much they were not a straightforward cherry-picks. It required modifying the interface of the NodeDeprecationChecks to include ClusterState as we do in 7.x.
This is a forward-port of elastic#82487, elastic#83544, elastic#83601, elastic#84145, and elastic#84246, but given that the branches had diverged so much they were not a straightforward cherry-picks. It required modifying the interface of the NodeDeprecationChecks to include ClusterState as we do in 7.x.
This is a forward-port of elastic#82487, elastic#83544, elastic#83601, elastic#84145, and elastic#84246, but given that the branches had diverged so much they were not a straightforward cherry-picks. It required modifying the interface of the NodeDeprecationChecks to include ClusterState as we do in 7.x.
3 participants
This commit changes NodeDeprecationChecks to look in both node settings and cluster settings when looking
for deprecated or removed settings. This allows it to find settings that were set dynamically.
Closes #82484