Teach the build about betas and rcs #26066

nik9000 · 2017-08-04T19:07:44Z

The build was ignoring suffixes like "beta1" and "rc1" on the version numbers which was causing the backwards compatibility packaging tests to fail because they expected to be upgrading from 6.0.0 even though they were actually upgrading from 6.0.0-beta1. This adds the suffixes to the information that the build scrapes from Version.java. It then uses those suffixes when it resolves artifacts build from the bwc branch and for testing.

Closes #26017

This should fix the upgrade tests when the upgrade from version is 6.0.0-beta1. Version.java and Version.groovy don't line up 100% but they are closer now, which is useful.

nik9000 · 2017-08-04T19:09:07Z

I'm still running through the test suite for this but it fixes the packaging tests and seems fine on the regular test suite. I'd also like to run the gradle bwcTest build before merging this, but I figure it'd be good to get some feedback on the approach.

nik9000 · 2017-08-04T19:44:46Z

This is failing in the mixed cluster tests with:

  2> REPRODUCE WITH: gradle :qa:mixed-cluster:v6.1.0-SNAPSHOT#mixedClusterTestRunner -Dtests.seed=6B1C9F49AFC53C45 -Dtests.class=org.elasticsearch.backwards.IndexingIT -Dtests.meth
od="testIndexVersionPropagation" -Dtests.security.manager=true -Dtests.jvms=12 -Dtests.locale=es -Dtests.timezone=America/Danmarkshavn
ERROR   1.47s | IndexingIT.testIndexVersionPropagation <<< FAILURES!
   > Throwable #1: org.elasticsearch.client.ResponseException: DELETE http://[::1]:41849*: HTTP/1.1 400 Bad Request
   > {"error":{"root_cause":[{"type":"remote_transport_exception","reason":"[node-3][127.0.0.1:43495][indices:admin/delete]"}],"type":"illegal_argument_exception","reason":"Cannot 
delete indices that are being snapshotted: [[index1/UsojX7mgT_K23SvdPdlavA], [index2/jRZeN7MnTHuFxnKiGAM7mw]]. Try again after snapshot finishes or cancel the currently running sna
pshot."},"status":400}

I'm not sure what is up with that but I don't see CI failing so it must be this change, somehow.

nik9000 · 2017-08-04T21:12:00Z

After merging from master everything looks like it works now.

rjernst · 2017-08-04T21:29:11Z

test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java

@@ -195,7 +199,12 @@ public void testGradleVersionsMatchVersionUtils() {
                /* Gradle skips the current version because being backwards compatible
                 * with yourself is implied. Java lists the version because it is useful. */
                .filter(v -> v != Version.CURRENT)
-                .map(v -> v.major + "." + v.minor + "." + v.revision)
+                /* Note that gradle does *not* skip alphas, betas, or rcs here even
+                 * though we don't have backwards compatibility for alphas, betas,


I thought we agreed to only have the last beta/rc in the wire/index compat versions in gradle in the special case of a new major release like we have now. So we should not have betas/rcs here in most cases. Also, gradle still does not ever have alphas, that would not be matched by the regex for Version.java.

I believe only the last beta/rc in a branch is unreleased so I think we're good there. I mean, it is technically possible for the release branch and the stable branch to have a beta but I don't think we'll do that so I'm not sure it is worth filtering it out.

I'm not sure why we don't have alphas in the Version.groovy. I mean, I know the regex doesn't match them, I just don't know why we treat them differently then betas. Either way, I can remove the word alpha here and make the filter just allow betas and rcs if you like.

I haven't changed the way gradle scrapes the versions. Did you want me to do that as part of this PR? We talked about wanting that "only the last beta/rc" behavior but I was under the impression we'd already implemented it. When I went to make this change it looks like we haven't and the reason our list lines up now is because we only pick up alphas.

I'm not sure why we don't have alphas in the Version.groovy. I mean, I know the regex doesn't match them, I just don't know why we treat them differently then betas

Because alphas are only cut from master, so there is never a case where we need to worry about bwc with an alpha. Only once we branch for a major release will we have betas/rcs.

We talked about wanting that "only the last beta/rc" behavior but I was under the impression we'd already implemented it

You are right, I think I already did this. I forgot.

nik9000

@rjernst I pushed an update. I believe this matches up with the logic we've talked about. I made it so gradle wouldn't keep alphas and updated some comments and tests around it keeping betas.

The tests found that gradle was keeping 6.0.0-beta1 instead of 6.0.0-beta2 and I believe I've fixed that was well.

nik9000 · 2017-08-09T18:42:05Z

build.gradle

@@ -64,10 +64,10 @@ configure(subprojects.findAll { it.projectDir.toPath().startsWith(rootPath) }) {

 /* Introspect all versions of ES that may be tested agains for backwards
 * compatibility. It is *super* important that this logic is the same as the
- * logic in VersionUtils.java, modulo alphas, betas, and rcs which are ignored


This was out of date.

nik9000 · 2017-08-09T18:42:22Z

build.gradle

    if (currentVersion != foundVersion
-        && (major == prevMajor || major == currentVersion.major)
-        && (versions.isEmpty() || versions.last() != foundVersion)) {
-      versions.add(foundVersion)


This was keeping beta-1 when it should have been keeping beta-2.

rjernst

Looks good, thanks @nik9000

rjernst · 2017-08-09T21:40:42Z

build.gradle

+        versions.add(foundVersion)
+      } else {
+        // Replace the earlier betas with later ones
+        versions.set(versions.size() - 1, foundVersion)


Can you assert/error here if foundVersion and/or the previous version.last() is not a beta/rc?

nik9000 · 2017-08-10T19:44:27Z

I've merged this to master but I don't believe I can merge it to 6.x or 6.0 because that'd cause all kinds of "fun" problems.

nik9000 · 2017-08-10T19:52:48Z

This change works in master because master only needs to test against 6.0.0-beta2 and 6.1.0, both of which are unreleased. On the other hand, cleanly backporting this to 6.x would have it testing against all the released 5.x versions and three unreleased versions: 5.5.2, 5.6.0, and 6.0.0-beta2. The build doesn't support testing against three unreleased versions, only 2.

* master: (30 commits) Rewrite range queries with open bounds to exists query (elastic#26160) Fix eclipse compilation problem (elastic#26170) Epoch millis and second formats parse float implicitly (Closes elastic#14641) (elastic#26119) fix SplitProcessor targetField test (elastic#26178) Fixed typo in README.textile (elastic#26168) Fix incorrect class name in deleteByQuery docs (elastic#26151) Move more token filters to analysis-common module reindex: automatically choose the number of slices (elastic#26030) Fix serialization of the `_all` field. (elastic#26143) percolator: Hint what clauses are important in a conjunction query based on fields Remove unused Netty-related settings (elastic#26161) Remove SimpleQueryStringIT#testPhraseQueryOnFieldWithNoPositions. Tests: reenable ShardReduceIT#testIpRange. Allow `ClusterState.Custom` to be created on initial cluster states (elastic#26144) Teach the build about betas and rcs (elastic#26066) Fix wrong header level inner hits: Unfiltered nested source should keep its full path Document how to import Lucene Snapshot libs when elasticsearch clients (elastic#26113) Use `global_ordinals_hash` execution mode when sorting by sub aggregations. (elastic#26014) Make the README use a single type in examples. (elastic#26098) ...

nik9000 added 2 commits August 4, 2017 14:28

Teach build about version suffixes like beta and rc

dff04f4

This should fix the upgrade tests when the upgrade from version is 6.0.0-beta1. Version.java and Version.groovy don't line up 100% but they are closer now, which is useful.

Update version test

8dd3e82

nik9000 added :Delivery/Build Build or test infrastructure review v6.1.0 v7.0.0 labels Aug 4, 2017

nik9000 requested a review from rjernst August 4, 2017 19:07

Merge branch 'master' into build_knows_alpha_beta

bbeb7b7

rjernst reviewed Aug 4, 2017

View reviewed changes

talevy mentioned this pull request Aug 9, 2017

[CI] full-cluster-restart BWC-Test fails because 6.0 is not released yet #26120

Closed

nik9000 added 2 commits August 9, 2017 13:53

Merge branch 'master' into build_knows_alpha_beta

d9055b3

Cleanup alphas and betas

5df0430

nik9000 commented Aug 9, 2017

View reviewed changes

imotov added v6.0.0 and removed v6.0.0-beta2 labels Aug 9, 2017

rjernst approved these changes Aug 9, 2017

View reviewed changes

nik9000 added 2 commits August 10, 2017 13:02

Merge branch 'master' into build_knows_alpha_beta

0254466

Assert that replaced version is beta or rc or something like that

e109d86

nik9000 merged commit 99ac7be into elastic:master Aug 10, 2017

nik9000 added v6.0.0-beta1 v6.0.0 and removed v6.0.0 v6.0.0-beta1 v6.1.0 labels Aug 10, 2017

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Teach the build about betas and rcs #26066

Teach the build about betas and rcs #26066

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

rjernst Aug 4, 2017 •

edited

Loading

nik9000 Aug 5, 2017

rjernst Aug 9, 2017

rjernst Aug 9, 2017

nik9000 left a comment

nik9000 Aug 9, 2017

nik9000 Aug 9, 2017

rjernst left a comment

rjernst Aug 9, 2017

nik9000 Aug 10, 2017

nik9000 commented Aug 10, 2017

nik9000 commented Aug 10, 2017

Teach the build about betas and rcs #26066

Teach the build about betas and rcs #26066

Conversation

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

nik9000 commented Aug 4, 2017

rjernst Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

nik9000 Aug 5, 2017

Choose a reason for hiding this comment

rjernst Aug 9, 2017

Choose a reason for hiding this comment

rjernst Aug 9, 2017

Choose a reason for hiding this comment

nik9000 left a comment

Choose a reason for hiding this comment

nik9000 Aug 9, 2017

Choose a reason for hiding this comment

nik9000 Aug 9, 2017

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

rjernst Aug 9, 2017

Choose a reason for hiding this comment

nik9000 Aug 10, 2017

Choose a reason for hiding this comment

nik9000 commented Aug 10, 2017

nik9000 commented Aug 10, 2017

rjernst Aug 4, 2017 •

edited

Loading