Add DRA version bump automation - #3986
Conversation
Replace the block step placeholder with an automated version bump script that handles both patch and minor workflows. Fix Slack notification channel and catalog-info.yaml trailing newline. Split DRA artifact polling into separate staging (conditional) and snapshot steps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b10ae5a to
9766b58
Compare
Replace fragile sed-based DRA branch list update with a Python helper that uses the ci:packaging condition as an anchor point, is idempotent, and fails clearly if the expected pipeline.yml structure changes. Add DRY_RUN=true support to preview all operations without committing or pushing. Add push_with_retry to handle concurrent commits landing on main during the minor workflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace direct pushes with gh pr create + gh pr merge --auto --squash so version bump changes go through PRs, matching the expected pattern across other teams in the unified release process. Add ci:version-bump label fast path to pipeline.yml so version bump PRs skip the full CI suite (notice, lint, tests, packaging) and only run a lightweight gate step. Increase version-bump pipeline timeout to 90min to accommodate waiting for PR CI and merge. Requires pull_requests: write in the GithubPermissionSet (elastic/terrazzo#1190). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add idempotency checks so re-runs skip already-completed steps. Clean up review feedback: build link in PR body, proper file handling in Python, remove dead checkouts, gate token check on DRY_RUN. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Buildkite expression language doesn't support `not` as a prefix operator. Use `!(... includes ...)` instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Overall - looking pretty good. Since this script is manually launched from Buildkite, I'm also wondering if it should also bump the DRA version numbers for minor branches here in the main Buildkite file? |
|
@markjhoy the |
Ah - I missed that! Thanks for pointing it out. |
Defaults to true (preserves production behavior). When false, the script creates the PR but skips enabling auto-merge and the merge-wait poll, leaving the PR open for inspection. Useful for testing the real PR creation path in Buildkite without landing changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi, this is Claude (driving Apostolos's terminal) -- we added an |
|
@markjhoy if you're okay with the above, you can give me a ✅ and I'll move onto testing. |
| DRA_PYTHON_VERSION: "3.11" | ||
|
|
||
| steps: | ||
| # Version bump PRs only touch VERSION files -- skip the full CI suite. |
There was a problem hiding this comment.
If we're checking the labels for each step below - we don't really need this step to just print out that we're skipping CI... I'd remove this block
There was a problem hiding this comment.
The more I look at this - since the catalog-info.yaml changes already call ".buildkite/version-bump-pipeline.yml" instead of using the main pipeline, I don't think it's necessary to change this main pipeline file at all... @seanstory - you probably have a bit more context here with the CI, thoughts?
There was a problem hiding this comment.
Not sure I've got more context. But I do question if we actually want to skip CI on version bump. I think we probably don't. Our CI is not that long, and if there's something that breaks, I'd rather it break on the branch than on main.
There was a problem hiding this comment.
The only concern and why I did this is that we don't control the timeout set on the Buildkite trigger step that calls us to do this bump. I agree it's better to break on the PR - but adding ~X min per PR could eventually push us past whatever timeout release-eng set on their side, and we'd have to figure out what's happening to adjust.
This matters more for minor bumps, where the script opens two PRs sequentially (one to add the new branch to pipeline.yml's DRA list on main, then a second to bump main to the next minor after cutting the release branch).
I can change this if you guys are sure this is preferable.
There was a problem hiding this comment.
Did they provide explicit guidance that CI should be skipped and that they have a timeout we need to worry about? If they did, I'll bow to your judgement. But if they haven't explicitly asked us to disable CI, I'd rather not.
There was a problem hiding this comment.
Actually dug out the following guidance - in PSI Version Bump Automation
*Team pipelines should aim to achieve within a maximum of 2–3 hours (including DRA builds), when full automation is in place, to ensure an efficient full version bump process
*Teams should ideally configure their version bump PRs to skip ITs and E2E tests to reduce the duration of the process; version bump PRs only increment the version number (displayed with --version flag), presenting low risk of introducing critical bugs, and if bugs are introduced, DRA artifacts can be published and fixed in a follow-up DRA build to unblock dependent teams
Running the tests X times as we do means we might end up pushing close to the suggested 2-3 hours. I know guidance says "ITs and E2E" but I think that's shorthand for "non-instant tests".
@markjhoy I don't think skipping tests gives up any real safety though - DRA publishing is gated on tests passing, it just means we will have to revert from a protected branch which is a hassle.
Updated thoughts? 😁
There was a problem hiding this comment.
Our builds on main reliably take about 30 min, that that includes a packaging phase that won't run on PRs. We should not be anywhere near the 2-3 hour mark.
I expect that guidance is more applicable for monorepos like elasticsearch/cloud/kibana that have obscene numbers of tests.
There was a problem hiding this comment.
Should this be a fully separate pipeline instead that calls non-conditionally all tests/ftests?
There was a problem hiding this comment.
Two separate things.
One pipeline creates the version bump as a PR.
This generated PR runs our normal pipeline. And I maintain should run it normally.
There was a problem hiding this comment.
Alright- undid this.
I got Claude to look at 50 past PR and build runs on buildkite, run a simulation and give me p values for the total run time to run tests 2 or 3 times depending on the flow.
| Scenario | Mean | P50 | P90 | P95 | P99 |
|---|---|---|---|---|---|
| Patch (PR + triggered build) | 48.8 | 47.3 | 65.6 | 73.8 | 89.0 |
| Minor (PR1 + PR2 + triggered build) | 71.6 | 69.8 | 94.9 | 104.5 | 120.9 |
So for now we're mostly safely under 2 hours.
If our times grow we can look at skipping tests at some point in this process while keeping them in the PR for revert-aversion 😁
| with open(pipeline_yml) as f: | ||
| pipeline = f.read() | ||
|
|
||
| dra_marker = "ci:packaging" |
There was a problem hiding this comment.
It might be safer to search for the comment # Add new maintenance branches here as well as a comment in the main .yml file telling people to keep the comment as-is...) as who knows what might happen in the future with this script (we might add a step in that meets these conditions 🤷 I tend to err on the side of caution)
There was a problem hiding this comment.
Had claude add a unit test for this then realized that it doesn't actually work because it needs to be discovered. Followed what you suggested. 👍
| python3 -c ' | ||
| import sys | ||
|
|
||
| pipeline_yml = sys.argv[1] |
There was a problem hiding this comment.
(sorry about the last comment I made here --- just realized this was within a python code block 🤦 ) - ignore the (now deleted) last comment I had
| local version="$1" | ||
| local current | ||
| current=$(cat "${VERSION_FILES[0]}" 2>/dev/null || echo "") | ||
| [[ "${current}" == "${version}" ]] |
There was a problem hiding this comment.
Will it be enough to check to equality here? Or should this check be to ensure that the current version is >= to the version? It's a bit more work to separate / check the version parts as numeric, but might be safer.
There was a problem hiding this comment.
Good call, Claude added a version_is_downgrade check here.
|
|
||
| insertion = f"build.branch == {escaped}{new_branch}{escaped} || " | ||
| lines[target_idx] = line.replace(pr_label_marker, insertion + pr_label_marker) | ||
|
|
There was a problem hiding this comment.
Would we want to consider removing older branches on this line at some point? IIRC - we only need to keep the last two minors of a major
There was a problem hiding this comment.
I don't know if this is a stable thing historically, and if we want to encode it in the code.
I trust you know better, so if you tell me it makes sense I can do it, but I would myself try to contain the changes.
I think it's good... I'd like to get a second set of eyes on it (maybe @artem-shelkovnikov ) though, especially around the questions I have about needing to edit the primary |
Require both the "ci:packaging" token and the "# Add new maintenance branches here" trailing comment when locating the DRA condition line. Prevents accidental rewrites if a future `if:` condition elsewhere in pipeline.yml happens to reference "ci:packaging". Also add a note above the target line reminding editors that the trailing comment is load-bearing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
eaaee4b to
a600739
Compare
| echo "Adding ${new_branch} to DRA branch list in pipeline.yml" | ||
|
|
||
| python3 -c ' | ||
| import sys |
There was a problem hiding this comment.
We can just make it a separate script file?
Also, we can theoretically make this whole script python if needed.
There was a problem hiding this comment.
++ to this - might make it easier to maintain (at least the inline Python script here - I'm OK with the primary pipeline being in Bash)
There was a problem hiding this comment.
Done. Tests not added because I'm unsure when they would run and how. If you have ideas tell me.
Per review feedback (Sean, Mark), don't skip CI on version bump PRs. Without the bypass, PR CI runs the full test suite and catches breakage before the change lands on a protected release branch, instead of relying on post-merge CI that doesn't even auto-trigger on release-branch pushes (confirmed empirically: release branches are only built via daily schedule or manual UI trigger, never via webhook on push). Remove: - `ci:version-bump` gate step in pipeline.yml - `!(build.pull_request.labels includes "ci:version-bump")` conditions gating notice/lint/unit_tests/smoke/ftests/test_packages - `--label ci:version-bump` on gh pr create in version-bump.sh Bump timeouts to accommodate full PR CI (~30 min each): - Version-bump step timeout: 90 -> 180 minutes - PR_MERGE_TIMEOUT: 3600 -> 5400 seconds (60 -> 90 minutes) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@markjhoy @seanstory @artem-shelkovnikov addressed most things I think - what we're not covering is this part of the RM duty on minor versions:
Worth trying to take care of these as well? |
|
Let's get this in for now, and we can tackle those later if necessary. I don't personally think that the RM duties are too heavy right now, the motivation for this is more about the release team not being blocked by our convenience, and none of those that you list matter to the release team, so they can still wait on our convenience. |
|
@markjhoy @artem-shelkovnikov let me know if anything important remains unadddressed |
markjhoy
left a comment
There was a problem hiding this comment.
👍 I think this is good for now. As @seanstory mentioned, we can address the other items later after we get this initial piece in.
`gh pr view <branch>` matches any state, so on re-runs with a recycled branch name it returns a stale merged PR and the script then fails trying to auto-merge it. Made-with: Cursor
Missing `${BRANCH}` / `${NEW_VERSION}` interpolate to empty strings at
upload time and surface only as a 4-hour json-watcher timeout. Guard up
front so the failure is immediate and obvious.
Made-with: Cursor
|
Merging and moving to dry run tests - will coordinate for end to end later. |
💔 Failed to create backport PR(s)The backport operation could not be completed due to the following error: The backport PRs will be merged automatically after passing CI. To backport manually run: |
## Summary Adds support for pre-9.3 release branches (8.19, 9.0, 9.1, 9.2) in the version-bump script introduced in #3986. The original PR's description noted that pre-monorepo branches (single `connectors/VERSION` file at a different path) couldn't be supported as-is and would need a separate backport. That turns out not to be necessary: the script always runs from `main`'s checkout (the `connectors-version-bump` Buildkite pipeline uploads `version-bump-pipeline.yml` from `main`, regardless of the target release branch), so a single `case` on `${BRANCH}` in the on-`main` script is sufficient. No backport, no per-branch script copies. I had originally assumed the script ran from each release branch's checkout, which is why #3986 left this as a follow-up. After verifying how CI actually invokes the script, this turned out to be a one-file change. ## Changes In `.buildkite/publish/version-bump.sh`, replace the hardcoded `VERSION_FILES` array with a `case` on `${BRANCH}`: - `8.*`, `9.0`, `9.1`, `9.2` → `connectors/VERSION` (pre-monorepo single-file layout) - everything else → the two post-monorepo paths (current behavior, unchanged for 9.3+/main) Made with [Cursor](https://cursor.com) --------- Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
## Summary Adds `release-eng: BUILD_AND_READ` to the `connectors` pipeline's teams block so release-eng team members can trigger builds. ## Why The `connectors-version-bump` pipeline (introduced in #3986) executes a `trigger:` step that starts a DRA build on the target release branch on the main `connectors` pipeline. Buildkite propagates the original triggering user's identity through trigger chains, so the downstream build on `connectors` is created as that user -- and the user needs Build access on `connectors` for the trigger to succeed. We already granted release-eng `BUILD_AND_READ` on `connectors-version-bump` (so they can trigger our pipeline at all), but missed adding it on the main `connectors` pipeline. This caused failures in [build #3](https://buildkite.com/elastic/connectors-version-bump/builds/3) (9.3.5) and [build #4](https://buildkite.com/elastic/connectors-version-bump/builds/4) (8.19.16) when triggered by Nina Lee, while [build #2](https://buildkite.com/elastic/connectors-version-bump/builds/2) (9.4.1) triggered by Julien Mailleret succeeded -- presumably via individual grants. This grants team-level access so any release-eng member can complete the version bump end-to-end. ## Test plan - [ ] After merge and Backstage sync (~few minutes), re-trigger build #3 or #4 from the centralized pipeline and confirm the `Trigger DRA build on ${BRANCH}` step succeeds --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Includes and builds on #3951. Replaces the block step with automated version bump logic.
The centralized release-eng pipeline triggers
connectors-version-bumpwithNEW_VERSION,BRANCH, andWORKFLOW. Forpatch, the script opens an auto-merging PR bumping both VERSION files on the release branch. Forminor, it updatespipeline.yml's DRA branch list, creates the new release branch, and bumps main to the next minor -- each step idempotent. The merged commits trigger existing CI which publishes DRA artifacts, and the json-watcher steps confirm completion.Requires elastic/terrazzo#1190 for
pull_requests: writepermission. SetDRY_RUN=trueto preview without side effects.Test plan