Introduce minimumCompatibilityVersion to BuildVersion (ES-9378) #119101
Conversation
…removing all occurrences of the Java annotation @UpdateForV9 (ES-9378)
alexey-ivanov-es
added
:Core/Infra/Core
| return previousNodeVersion; | ||
| } | ||
| return "7.17.0"; | ||
| return BuildVersion.fromVersionId(Version.CURRENT.minimumCompatibilityVersion().id); |
There was a problem hiding this comment.
Is it ok to use Version here?
In previous PR: #117992 (comment)
There was a problem hiding this comment.
We should add this to BuildVersion to avoid inferring build versions from version ids.
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
| return previousNodeVersion; | ||
| } | ||
| return "7.17.0"; | ||
| return BuildVersion.fromVersionId(Version.CURRENT.minimumCompatibilityVersion().id); |
There was a problem hiding this comment.
We should add this to BuildVersion to avoid inferring build versions from version ids.
| assertThat(nodeMetadata.nodeId(), equalTo("y6VUVMSaStO4Tz-B5BxcOw")); | ||
| assertThat(nodeMetadata.nodeVersion(), equalTo(BuildVersion.fromVersionId(0))); | ||
|
|
||
| ElasticsearchException ex = assertThrows( |
There was a problem hiding this comment.
assertThrows comes from junit and is a bit wonky in my experience. We use expectThrows in other parts of the codebase (which is our own, and existed before junit added a method for this). Let's prefer using that.
| public void testUpgradeMarksPreviousVersion() { | ||
| final String nodeId = randomAlphaOfLength(10); | ||
| final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_8_0_0); | ||
| final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_9_0_0); |
There was a problem hiding this comment.
Can this test be rewritten to be built on BuildVersion and IndexVersion? We seem to be still using Version, but NodeMetadata no longer uses that class.
There was a problem hiding this comment.
I don't see how to do it without big refactoring - BuildVersion currently is essentially a wrapper around Version, so it doesn't seem possible to create it without having a version id within a valid range
alexey-ivanov-es
changed the title
elasticsearchmachine
added
the
serverless-linked
| * <p>If a user is trying to install 8.0 but has incompatible indices, the user should | ||
| * downgrade to 7.17.x. We return 7.17.0, unless the user is trying to upgrade from | ||
| * a 7.17.x release, in which case we return the last installed version. | ||
| * <p>If a user is trying to install 9.0 (current major) but has incompatible indices, the user should |
There was a problem hiding this comment.
Could you rewrite this in abstract terms so we don't need to update it with each major release? You could use N.0 and (N-1).last
|
@alexey-ivanov-es Please also adjust the PR title. It should be clearer by the title what is changing. eg this seems to now be about minimum compatibility version. |
alexey-ivanov-es
changed the title
3 participants
No description provided.