initial cleanup of deprecation checks for 6.x #35326
Conversation
The deprecation checks were primarily used for helping identity breaking changes before upgrading latest minor releases to next majors. This meant that these checks were not necessarily maintained across future minors. This cleanup is a first step in preparing for catching up on reporting all the existing deprecations in the 6.x branch. changes: - added cluster deprecation tests for existing updated checks - added node deprecation checks for azure and gcs plugins - removed stale index checks - added index check for indices created before 6.0
|
Should the cleanup target master and be backported so there is a clean slate? |
|
@tylersmalley probably. I'll open a new PR that just deletes everything |
|
the goal of this PR is to have at at least one Node, Cluster, and Index check each, so they can be more easily appended to. A PR against |
|
Pinging @elastic/es-core-infra |
| "breaking_60_indices_changes.html#_shadow_replicas_have_been_removed", null); | ||
| "Index created before 6.0", | ||
| "https://www.elastic.co/guide/en/elasticsearch/reference/master/" + | ||
| "breaking-changes-7.0.html#breaking_70_packaging_changes", |
There was a problem hiding this comment.
| "breaking-changes-7.0.html#breaking_70_packaging_changes", | |
| "breaking-changes-7.0.html", |
I think this link should just link to https://www.elastic.co/guide/en/elasticsearch/reference/master/breaking-changes-7.0.html - the warning about indices created in old versions is at the top of the page and the text at this anchor doesn't really relate to this issue.
| import java.util.stream.Collectors; | ||
|
|
||
| /** | ||
| * Index-specific deprecation checks |
There was a problem hiding this comment.
| * Index-specific deprecation checks | |
| * Node-specific deprecation checks |
| nodeInfo.getPlugins().getPluginInfos().stream() | ||
| .anyMatch(pluginInfo -> "repository-azure".equals(pluginInfo.getName())) | ||
| ).map(nodeInfo -> nodeInfo.getNode().getName()).collect(Collectors.toList()); | ||
| if (nodesFound.size() > 0) { |
There was a problem hiding this comment.
Should these check to see if the deprecated settings are in use, rather than just that the plugin is present? I'm not very familiar with these plugins but it looks like you can use the new settings in 6.x so it would make sense to be more specific with the warnings.
That might be too much effort for too little gain, though.
There was a problem hiding this comment.
I thought it would be too much effort for too little gain. maybe this can be discussed further in future discussions?
There was a problem hiding this comment.
do you recommend I just remove these checks from here for now?
There was a problem hiding this comment.
No, this is fine, we can revise if necessary.
| int maxShardsInCluster = shardsPerNode * nodeCount; | ||
| int currentOpenShards = maxShardsInCluster + randomIntBetween(0, 100); | ||
|
|
||
|
|
|
|
||
| public class ClusterDeprecationChecksTests extends ESTestCase { | ||
|
|
||
| public void testCheckShardLimit() { |
There was a problem hiding this comment.
++, I should have had this in the PR to add the check in the first place. Thanks for adding it!
| "[[type: testIncludeInAll, field: my_field]]"); | ||
| "Index created before 6.0", | ||
| "https://www.elastic.co/guide/en/elasticsearch/reference/master/" + | ||
| "breaking-changes-7.0.html#breaking_70_packaging_changes", |
There was a problem hiding this comment.
| "breaking-changes-7.0.html#breaking_70_packaging_changes", | |
| "breaking-changes-7.0.html", |
Same reasoning as above.
gwbrown
added
:Core/Features/Features
and removed
:Core/Infra/Core
|
Pinging @elastic/es-core-features |
|
Thanks Tal, LGTM! |
The deprecation checks were primarily used for helping identity
breaking changes before upgrading latest minor releases to next
majors. This meant that these checks were not necessarily maintained
across future minors. This cleanup is a first step in preparing for
catching up on reporting all the existing deprecations in the 6.x
branch.
changes: