Make resolve conflict in transport version its own task (#134842) (#135487)

This commit moves the action for resolving merge conflicts in transport version files a separate task instead of a command line flag to generate.

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/ResolveTransportVersionConflictFuncTest.groovy

@@ -0,0 +1,90 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+package org.elasticsearch.gradle.internal.transport
+
+import org.gradle.testkit.runner.BuildResult
+import org.gradle.testkit.runner.GradleRunner
+import org.gradle.testkit.runner.TaskOutcome
+
+class ResolveTransportVersionConflictFuncTest extends AbstractTransportVersionFuncTest {
+
+    GradleRunner runResolveAndValidateTask() {
+        List<String> args = List.of(":myserver:validateTransportVersionResources", ":myserver:resolveTransportVersionConflict")
+        return gradleRunner(args.toArray())
+    }
+
+    void assertResolveAndValidateSuccess(BuildResult result) {
+        assert result.task(":myserver:resolveTransportVersionConflict").outcome == TaskOutcome.SUCCESS
+        assert result.task(":myserver:validateTransportVersionResources").outcome == TaskOutcome.SUCCESS
+    }
+
+    def "update flag works with current"() {
+        given:
+        referableAndReferencedTransportVersion("new_tv", "8123000")
+        file("myserver/src/main/resources/transport/latest/9.2.csv").text =
+            """
+            <<<<<<< HEAD
+            existing_92,8123000
+            =======
+            new_tv,8123000
+            >>>>>> name
+            """.strip()
+
+        when:
+        def result = runResolveAndValidateTask().build()
+
+        then:
+        assertResolveAndValidateSuccess(result)
+        assertReferableDefinition("existing_92", "8123000,8012001")
+        assertReferableDefinition("new_tv", "8124000")
+        assertUpperBound("9.2", "new_tv,8124000")
+    }
+
+    def "update flag works with multiple branches"() {
+        given:
+        referableAndReferencedTransportVersion("new_tv", "8123000,8012001,7123001")
+        file("myserver/src/main/resources/transport/latest/9.2.csv").text =
+            """
+            <<<<<<< HEAD
+            existing_92,8123000
+            =======
+            new_tv,8123000
+            >>>>>> name
+            """.strip()
+        file("myserver/src/main/resources/transport/latest/9.1.csv").text =
+            """
+            <<<<<<< HEAD
+            existing_92,8012001
+            =======
+            new_tv,8012001
+            >>>>>> name
+            """.strip()
+        file("myserver/src/main/resources/transport/latest/8.19.csv").text =
+            """
+            <<<<<<< HEAD
+            initial_8.19.7,7123001
+            =======
+            new_tv,7123001
+            >>>>>> name
+            """.strip()
+
+        when:
+        def result = runResolveAndValidateTask().build()
+
+        then:
+        assertResolveAndValidateSuccess(result)
+        assertReferableDefinition("existing_92", "8123000,8012001")
+        assertUnreferableDefinition("initial_8.19.7", "7123001")
+        assertReferableDefinition("new_tv", "8124000,8012002,7123002")
+        assertUpperBound("9.2", "new_tv,8124000")
+        assertUpperBound("9.1", "new_tv,8012002")
+        assertUpperBound("8.19", "new_tv,7123002")
+    }
+}

build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/transport/TransportVersionGenerationFuncTest.groovy

@@ -19,14 +19,14 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
         List<String> args = new ArrayList<>()
         args.add(":myserver:validateTransportVersionResources")
         args.add(":myserver:generateTransportVersion")
-        args.addAll(additionalArgs);
+        args.addAll(additionalArgs)
         return gradleRunner(args.toArray())
     }
 
     def runGenerateTask(String... additionalArgs) {
         List<String> args = new ArrayList<>()
         args.add(":myserver:generateTransportVersion")
-        args.addAll(additionalArgs);
+        args.addAll(additionalArgs)
         return gradleRunner(args.toArray())
     }
 
@@ -283,77 +283,6 @@ class TransportVersionGenerationFuncTest extends AbstractTransportVersionFuncTes
         assertUpperBound("9.2", "second_tv,8124000")
     }
 
-    def "update flag works with current"() {
-        given:
-        referableAndReferencedTransportVersion("new_tv", "8123000")
-        file("myserver/src/main/resources/transport/latest/9.2.csv").text =
-            """
-            <<<<<<< HEAD
-            existing_92,8123000
-            =======
-            new_tv,8123000
-            >>>>>> name
-            """.strip()
-
-        when:
-        def result = runGenerateAndValidateTask("--resolve-conflict").build()
-
-        then:
-        assertGenerateAndValidateSuccess(result)
-        assertReferableDefinition("existing_92", "8123000,8012001")
-        assertReferableDefinition("new_tv", "8124000")
-        assertUpperBound("9.2", "new_tv,8124000")
-    }
-
-    def "update flag works with multiple branches"() {
-        given:
-        referableAndReferencedTransportVersion("new_tv", "8123000,8012001,7123001")
-        file("myserver/src/main/resources/transport/latest/9.2.csv").text =
-            """
-            <<<<<<< HEAD
-            existing_92,8123000
-            =======
-            new_tv,8123000
-            >>>>>> name
-            """.strip()
-        file("myserver/src/main/resources/transport/latest/9.1.csv").text =
-            """
-            <<<<<<< HEAD
-            existing_92,8012001
-            =======
-            new_tv,8012001
-            >>>>>> name
-            """.strip()
-        file("myserver/src/main/resources/transport/latest/8.19.csv").text =
-            """
-            <<<<<<< HEAD
-            initial_8.19.7,7123001
-            =======
-            new_tv,7123001
-            >>>>>> name
-            """.strip()
-
-        when:
-        def result = runGenerateAndValidateTask("--resolve-conflict").build()
-
-        then:
-        assertGenerateAndValidateSuccess(result)
-        assertReferableDefinition("existing_92", "8123000,8012001")
-        assertUnreferableDefinition("initial_8.19.7", "7123001")
-        assertReferableDefinition("new_tv", "8124000,8012002,7123002")
-        assertUpperBound("9.2", "new_tv,8124000")
-        assertUpperBound("9.1", "new_tv,8012002")
-        assertUpperBound("8.19", "new_tv,7123002")
-    }
-
-    def "update flag cannot be used with backport branches"() {
-        when:
-        def result = runGenerateTask("--resolve-conflict", "--backport-branches=9.1").buildAndFail()
-
-        then:
-        assertGenerateFailure(result, "Cannot use --resolve-conflict with --backport-branches")
-    }
-
     def "branches param order does not matter"() {
         given:
         referencedTransportVersion("test_tv")

build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/transport/GenerateTransportVersionDefinitionTask.java

@@ -74,11 +74,7 @@ public abstract class GenerateTransportVersionDefinitionTask extends DefaultTask
 
     @Input
     @Optional
-    @Option(
-        option = "resolve-conflict",
-        description = "Regenerate the transport version currently being added to upstream to resolve a merge conflict"
-    )
-    public abstract Property<Boolean> getResolveConflict();
+    abstract Property<Boolean> getResolveConflict();
 
     /**
      * The name of the upper bounds file which will be used at runtime on the current branch. Normally

docs/internal/Versioning.md

@@ -22,68 +22,126 @@ it could be any arbitrary string.
 
 ## Transport protocol
 
-The transport protocol is used to send binary data between Elasticsearch nodes;
-`TransportVersion` is the version number used for this protocol.
-This version number is negotiated between each pair of nodes in the cluster
-on first connection, and is set as the lower of the highest transport version
-understood by each node.
+The transport protocol is used to send binary data between Elasticsearch nodes; a
+`TransportVersion` encapsulates versioning of this protocol.
+This version is negotiated between each pair of nodes in the cluster
+on first connection, selecting the highest shared version.
 This version is then accessible through the `getTransportVersion` method
 on `StreamInput` and `StreamOutput`, so serialization code can read/write
 objects in a form that will be understood by the other node.
 
-Every change to the transport protocol is represented by a new transport version,
-higher than all previous transport versions, which then becomes the highest version
-recognized by that build of Elasticsearch. The version ids are stored
-as constants in the `TransportVersions` class.
-Each id has a standard pattern `M_NNN_S_PP`, where:
-* `M` is the major version
-* `NNN` is an incrementing id
-* `S` is used in subsidiary repos amending the default transport protocol
-* `PP` is used for patches and backports
-
-When you make a change to the serialization form of any object,
-you need to create a new sequential constant in `TransportVersions`,
-introduced in the same PR that adds the change, that increments
-the `NNN` component from the previous highest version,
-with other components  set to zero.
-For example, if the previous version number is `8_413_0_01`,
-the next version number should be `8_414_0_00`.
-
-Once you have defined your constant, you then need to use it
-in serialization code. If the transport version is at or above the new id,
-the modified protocol should be used:
-
-    str = in.readString();
-    bool = in.readBoolean();
-    if (in.getTransportVersion().onOrAfter(TransportVersions.NEW_CONSTANT)) {
-        num = in.readVInt();
-    }
+At a high level a `TransportVersion` contains one id per release branch it will
+be committed to. Each `TransportVersion` has a name selected when it is generated.
+In order to ensure consistency and robustness, all new `TransportVersion`s
+must first be created in the `main` branch and then backported to the relevant
+release branches.
+
+### Internal state files
+
+The Elasticsearch server jar contains resource files representing each
+transport version. These files are loaded at runtime to construct
+`TransportVersion` instances. Since each transport version has its own file
+they can be backported without conflict.
 
-If a transport version change needs to be reverted, a **new** version constant
-should be added representing the revert, and the version id checks
-adjusted appropriately to only use the modified protocol between the version id
-the change was added, and the new version id used for the revert (exclusive).
-The `between` method can be used for this.
+Additional resource files represent the latest transport version known on
+each release branch. If two transport versions are added at the same time,
+there will be a conflict in these internal state files, forcing one to be
+regenerated to resolve the conflict before merging to `main`.
 
-Once a transport change with a new version has been merged into main or a release branch,
-it **must not** be modified - this is so the meaning of that specific
-transport version does not change.
+All of these internal state files are managed by gradle tasks; they should
+not be edited directly.
 
 _Elastic developers_ - please see corresponding documentation for Serverless
 on creating transport versions for Serverless changes.
 
-### Collapsing transport versions
+### Creating transport versions locally
+
+To create a transport version, declare a reference anywhere in java code. For example:
+
+    private static final TransportVersion MY_NEW_TV = TransportVersion.fromName("my_new_tv");
+
+`fromName` takes an arbitrary String name. The String must be a String literal;
+it cannot be a reference to a String. It must match the regex `[_0-9a-zA-Z]+`.
+You can reference the same transport version name from multiple classes, but
+you must not use an existing transport version name after it as already been
+committed to `main`.
+
+Once you have declared your `TransportVersion` you can use it in serialization code.
+For example, in a constructor that takes `StreamInput in`:
+
+    if (in.getTransportVersion().supports(MY_NEW_TV)) {
+        // new serialization code
+    }
+
+Finally, in order to run Elasticsearch or run tests, the transport version ids
+must be generated. Run the following gradle task:
+
+    ./gradlew generateTransportVersion
+
+This will generate the internal state to support the new transport version. If
+you also intend to backport your code, include branches you will backport to:
+
+    ./gradlew generateTransportVersion --backport-branches=9.1,8.19
+
+### Updating transport versions
+
+You can modify a transport version before it is merged to `main`. This includes
+renaming the transport version, updating the branches it will be backported to,
+or even removing the transport version itself.
+
+The generation task is idempotent. It can be re-run at any time and will result
+in a valid internal state. For example, if you want to add an additional
+backport branch, re-run the generation task with all the target backport
+branches:
+
+    ./gradlew generateTransportVersion --backport-branches=9.1,9.0,8.19,8.18
+
+You can also let CI handle updating transport versions for you. As version
+labels are updated on your PR, the generation task is automatically run with
+the appropriate backport branches and any changes to the internal state files
+are committed to your branch.
+
+Transport versions can also have additional branches added after merging to
+`main`. When doing so, you must include all branches the transport version was
+added to in addition to new branch. For example, if you originally committed
+your transport version `my_tv` to `main` and `9.1`, and then realized you also
+needed to backport to `8.19` you would run (in `main`):
+
+    ./gradlew generateTransportVersion --name=my_tv --backport-branches=9.1,8.19
+
+In the above case CI will not know what transport version name to update, so you
+must run the generate task again as described. After merging the updated transport
+version it will need to be backported to all the applicable branches.
+
+### Resolving merge conflicts
 
-As each change adds a new constant, the list of constants in `TransportVersions`
-will keep growing. However, once there has been an official release of Elasticsearch,
-that includes that change, that specific transport version is no longer needed,
-apart from constants that happen to be used for release builds.
-As part of managing transport versions, consecutive transport versions can be
-periodically collapsed together into those that are only used for release builds.
-This task is normally performed by Core/Infra on a semi-regular basis,
-usually after each new minor release, to collapse the transport versions
-for the previous minor release. An example of such an operation can be found
-[here](https://github.com/elastic/elasticsearch/pull/104937).
+Transport versions are created sequentially. If two developers create a transport
+version at the same time, based on the same `main` commit, they will generate
+the same internal ids. The first of these two merged into `main` will "win", and
+the latter will have a merge conflict with `main`.
+
+In the event of a conflict, merge `main` into your branch. You will have
+conflict(s) with transport version internal state files. Run the following
+task to resolve the conflict(s):
+
+    ./gradlew resolveTransportVersionConflict
+
+This command will regenerate your transport version and stage the updated
+state files in git. You can then proceed with your merge as usual.
+
+### Reverting changes
+
+Transport versions cannot be removed, they can only be added. If the logic
+using a transport version needs to be reverted, it must be done with a
+new transport version.
+
+For example, if you have previously added a transport version named
+`original_tv` you could add `revert_tv` reversing the logic:
+
+    TransportVersion tv = in.getTransportVersion();
+    if (tv.supports(ORIGINAL_TV) && tv.supports(REVERT_TV) == false) {
+        // serialization code being reverted
+    }
 
 ### Minimum compatibility versions
 
@@ -114,70 +172,22 @@ is updated automatically as part of performing a release.
 In releases that do not have a release version number, that method becomes
 a no-op.
 
-### Managing patches and backports
-
-Backporting transport version changes to previous releases
-should only be done if absolutely necessary, as it is very easy to get wrong
-and break the release in a way that is very hard to recover from.
-
-If we consider the version number as an incrementing line, what we are doing is
-grafting a change that takes effect at a certain point in the line,
-to additionally take effect in a fixed window earlier in the line.
-
-To take an example, using indicative version numbers, when the latest
-transport version is 52, we decide we need to backport a change done in
-transport version 50 to transport version 45. We use the `P` version id component
-to create version 45.1 with the backported change.
-This change will apply for version ids 45.1 to 45.9 (should they exist in the future).
-
-The serialization code in the backport needs to use the backported protocol
-for all version numbers 45.1 to 45.9. The `TransportVersion.isPatchFrom` method
-can be used to easily determine if this is the case: `streamVersion.isPatchFrom(45.1)`.
-However, the `onOrAfter` also does what is needed on patch branches.
-
-The serialization code in version 53 then needs to additionally check
-version numbers 45.1-45.9 to use the backported protocol, also using the `isPatchFrom` method.
-
-As an example, [this transport change](https://github.com/elastic/elasticsearch/pull/107862)
-was backported from 8.15 to [8.14.0](https://github.com/elastic/elasticsearch/pull/108251)
-and [8.13.4](https://github.com/elastic/elasticsearch/pull/108250) at the same time
-(8.14 was a build candidate at the time).
-
-The 8.13 PR has:
-
-    if (transportVersion.onOrAfter(8.13_backport_id))
-
-The 8.14 PR has:
-
-    if (transportVersion.isPatchFrom(8.13_backport_id)
-        || transportVersion.onOrAfter(8.14_backport_id))
-
-The 8.15 PR has:
-
-    if (transportVersion.isPatchFrom(8.13_backport_id)
-        || transportVersion.isPatchFrom(8.14_backport_id)
-        || transportVersion.onOrAfter(8.15_transport_id))
-
-In particular, if you are backporting a change to a patch release,
-you also need to make sure that any subsequent released version on any branch
-also has that change, and knows about the patch backport ids and what they mean.
-
 ## Index version
 
 Index version is a single incrementing version number for the index data format,
-metadata, and associated mappings. It is declared with the pattern `M_NNN_S_PP`,
-for the major version, version id, subsidiary version id, and patch number
-respectively.
+metadata, and associated mappings. It is declared the same way as the
+transport version - with the pattern `M_NNN_S_PP`, for the major version, version id,
+subsidiary version id, and patch number respectively.
 
 Index version is stored in index metadata when an index is created,
 and it is used to determine the storage format and what functionality that index supports.
 The index version does not change once an index is created.
 
-When a change is needed to the index data format or metadata, or new mapping
-types are added, create a new version constant below the last one, incrementing
-the `NNN` version component.
+In the same way as transport versions, when a change is needed to the index
+data format or metadata, or new mapping types are added, create a new version constant
+below the last one, incrementing the `NNN` version component.
 
-Index version constants cannot be collapsed together,
+Unlike transport version, version constants cannot be collapsed together,
 as an index keeps its creation version id once it is created.
 Fortunately, new index versions are only created once a month or so,
 so we don’t have a large list of index versions that need managing.