Skip to content

feat(skills): add rhdh-must-gather-helm-bump skill and script - #86

Open
nickboldt wants to merge 12 commits into
mainfrom
feat/RHIDP-16046-must-gather-helm-bump
Open

feat(skills): add rhdh-must-gather-helm-bump skill and script#86
nickboldt wants to merge 12 commits into
mainfrom
feat/RHIDP-16046-must-gather-helm-bump

Conversation

@nickboldt

@nickboldt nickboldt commented Aug 12, 2026

Copy link
Copy Markdown
Member

Summary

Adds the rhdh-must-gather-helm-bump skill (skills/ci/) and bundled scripts/bump-must-gather-helm.py to propagate a Helm CLI version bump across:

Repo What the skill changes
redhat-developer/rhdh-must-gather Makefile HELM_VERSION, CGW artifacts.lock.yaml or vendor/helm/, Stage 2a/2b on Containerfile and .rhdh/docker/Containerfile
gitlab.cee.redhat.com/rhidp/rhdh Mirror helm-related files into distgit/containers/rhdh-must-gather/; regenerate the hermetic distgit Containerfile; patch must-gather Tekton prefetch; write sync/upstream_SHA_rhdh-must-gather

This is the Helm CLI in the must-gather image, not the RHDH Helm chart (oci://quay.io/rhdh/chart). The script does not commit, push, or open a PR/MR.

Catalog: registered under skills/ci/ in catalog.json, marketplace, and /ask-rhdh routes.

https://redhat.atlassian.net/browse/RHIDP-16046

What the script does

Preview (ungated): --check probes CGW and prints helm_version= / mode=cgw|vendor. --dry-run prints the planned writes without touching trees. Approval to preview is not approval to apply.

Apply (after /mutation-gate; upstream and distgit/Tekton as separate operations):

  1. Choose install path by probing upstream hack/check-helm-binary-available.sh (same probe for --check and apply, including --skip-upstream):
    • CGW (preferred): hack/update-helm-lockfile.sh → Konflux generic prefetch on distgit root → Stage 2a active
    • Vendor fallback: hack/update-vendor.sh helmgomod prefetch on vendor/helm → Stage 2b active
  2. Flip Stage 2a/2b bidirectionally (doc-only comments stay commented) on upstream Containerfile, .rhdh/docker/Containerfile, and the regenerated distgit Containerfile.
  3. Sync into distgit: Makefile, artifacts.lock.yaml, entire hack/, .rhdh/docker/Containerfile, entire vendor/ (on CGW, omit/delete vendor/helm; keep websocat etc.). Does not copy upstream root Containerfile onto hermetic distgit.
  4. Regenerate distgit Containerfile from .rhdh/docker/Containerfile, preserving RHDH_MUST_GATHER_VERSION and the Brew/Konflux ENV SUMMARY= / LABEL footer. Increments release="N"N+1 and the konflux.additional-tags version-release suffix with a numeric boundary (2.0-12.0-2, 2.0-102.0-11 not 2.0-20).
  5. Patch Tekton prefetch for must-gather only: .tekton/rhdh-must-gather-2-{pull,push}.yaml and must-gather.prefetch_input in .tekton-templates/components.yaml (generic vs gomod). Other components.yaml entries are left untouched.
  6. Write sync/upstream_SHA_rhdh-must-gather from a committed upstream HEAD. Refuses to pin a dirty HEAD (--allow-dirty still does not write a SHA from uncommitted work).

--skip-upstream syncs distgit + Tekton + SHA only. --skip-downstream bumps upstream only.

Discovery and guards

  • --parent-dir finds must-gather (1-must-gather, …) and midstream (4-rhdh, rhdh-downstream, rhidp-rhdh) only when the candidate has distgit/containers/rhdh-must-gather (skips a GitHub rhdh clone).
  • Requires hack/check-helm-binary-available.sh on upstream; missing bash / rsync is a hard error.
  • git status failures (fatal: not a git repository, etc.) die instead of looking clean.
  • Requires Python 3.9+, bash, curl, git, rsync.

Agent workflow (in SKILL.md)

  1. Confirm --to against Helm releases.
  2. Resolve checkouts.
  3. --check + --dry-run (preview).
  4. Apply only after /mutation-gate.
  5. Review git diff + references/verification.md (unit tests, Konflux, E2E triage including Helm 4 OCI stdout).
  6. Commit / PR or MR only when asked, again through /mutation-gate (Jira via /rhdh-jira-link).

References: install-paths (CGW vs vendor, tarball layout), tekton-prefetch, verification, helm4-notes.

Related upstream work: rhdh-must-gather#284 (CGW binary), rhdh-must-gather#282 (vendored fallback).

Tests

tests/unit/test_must_gather_helm_bump.py (invoked via sys.executable):

  • Real --to bump off the 4.2.3 fixture (lockfile/vendor stubs update HELM_VERSION)
  • CGW sync omits vendor/helm, keeps websocat, scopes prefetch to must-gather, updates pull and push PLRs
  • Vendor mode flips 2a→2b; a second case flips 2b back to 2a
  • --dry-run writes nothing
  • release="10" / 2.0-1011 / 2.0-11
  • Failed git status, dirty-HEAD SHA refusal, rhdh-downstream discovery, missing CGW probe script
  • --skip-upstream --check still reports mode=cgw when upstream CGW is available (stale distgit vendor/helm does not win)

Test plan

  • uv run pytest tests/unit/test_must_gather_helm_bump.py
  • uv run pytest tests/unit/test_skill_catalog.py
  • --check / --dry-run smoke test against local must-gather + midstream checkouts

Generated-by: cursor

nickboldt and others added 2 commits August 12, 2026 10:02
Automate Helm CLI version bumps across rhdh-must-gather upstream and
rhidp/rhdh distgit, including Konflux .tekton prefetch (CGW generic vs
vendored gomod). Covers the multi-repo workflow introduced for RHIDP-16046.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Restructure SKILL.md with verification gates, success criteria, and
E2E triage pointers from rhdh-must-gather#284. Add references for Helm 4
OCI stdout, Konflux/E2E verification, and CGW flat tarball layout.

Script now updates sync/upstream_SHA_rhdh-must-gather and removes stale
distgit vendor/helm on the CGW path.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
nickboldt and others added 8 commits August 13, 2026 18:01
Reflect merged upstream checksum verify (verify-helm-tarball.sh),
TARGETPLATFORM arch field-2 notes, and midstream !697 lesson: sync
hack/ into distgit and stop excluding it in upstream_repos.yml so
Stage 2a COPY does not break after bot sync.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Midstream main already keeps hack/ in sync; the bump skill only needs to
copy verify-helm-tarball.sh and document #284 hardening (checksum verify,
TARGETPLATFORM field-2, E2E pipefail). Remove ensure_hack_not_excluded
and related docs/tests.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Scope components.yaml prefetch to must-gather only, stop copying
upstream root Containerfile onto hermetic distgit, regenerate distgit
Containerfile from .rhdh/docker (preserving VERSION/MIDSTREAM_REPO),
flip Stage 2a/2b bidirectionally, sync whole hack/ and vendor/ (omit
helm on CGW), and tighten script guards plus skill prose.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Resolve README conflict with main's catalog-first layout, move
rhdh-must-gather-helm-bump under skills/ci/, register it in
catalog.json / marketplace / ask-rhdh routes, and satisfy catalog
Completion + name-only composition rules.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
CI lint job failed on `ruff format --check` for the new unit test file.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Regenerating from .rhdh/docker/Containerfile was dropping the Brew/Konflux
ENV SUMMARY/LABEL block that sync-midstream appends. Keep that footer, and
stop flipping upstream Containerfiles on --skip-upstream.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
When regenerating distgit Containerfile from .rhdh/docker, increment
LABEL release and the matching konflux.additional-tags version-release
suffix (e.g. 2.0-1 → 2.0-2), mirroring sync-midstream.sh.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Match the rhdh-skill convention of Python bundled scripts so the Helm
bump workflow is easier to test and maintain than the previous bash
implementation, while preserving CGW/vendor sync and Tekton prefetch
behavior.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>

@Fortune-Ndlovu Fortune-Ndlovu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@schultzp2020 schultzp2020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CGW vs vendor split, and editing only must-gather's prefetch, are the right idea. I'd tighten the apply path before this runs against rhidp/rhdh. --check and --skip-upstream can pick different modes, 2.0-10 becomes 2.0-20, and a failed git status looks clean. Tests don't hit those cases. On Windows the new file fails on shebang exec (WinError 193) before rsync is involved.

Comments below are the hunks I'd change. While you're there: don't write upstream_SHA from a dirty HEAD; require hack/check-helm-binary-available.sh and turn a missing bash/rsync into die(); run tests via sys.executable and actually bump --to off the 4.2.3 fixture (the lockfile stubs are no-ops); cover --dry-run, push.yaml, and 2b back to 2a. Same skills/ci/... path is in the script docstring. PR body still names bump-must-gather-helm.sh.

## Usage

```bash
SKILL=skills/ci/rhdh-must-gather-helm-bump # under rhdh-skill checkout

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The category folder is gone after install, so an agent that copies this Usage block misses the script and starts editing Tekton by hand. rhdh-yarn-bump uses a placeholder for that reason.

Suggested change
SKILL=skills/ci/rhdh-must-gather-helm-bump # under rhdh-skill checkout
SKILL=<this skill's directory>

Comment on lines +62 to +66
4. Run `--dry-run`, then the script without `--dry-run`.
5. Review `git diff` in **both** repos.
6. Run every verification gate — read [references/verification.md](references/verification.md).
7. Commit / PR·MR only when the user asks.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--check and --dry-run can stay ungated. The next line writes two other working trees. /mutation-gate calls that a write. rhdh-base-images gates the apply; rhdh-prow-trigger says a dry-run is not approval to execute. Completion below currently treats the real run as done before anyone signed off.

Suggested change
4. Run `--dry-run`, then the script without `--dry-run`.
5. Review `git diff` in **both** repos.
6. Run every verification gate — read [references/verification.md](references/verification.md).
7. Commit / PR·MR only when the user asks.
3. Run `--check` and `--dry-run`. That pair is the preview.
4. Apply only after `/mutation-gate` approval, with upstream and distgit/Tekton as separate operations. Approval to dry-run is not approval to write.
5. Review `git diff` in both repos, then the gates in [references/verification.md](references/verification.md).
6. Commit / PR or MR only when the user asks, again through `/mutation-gate`.

check_only: bool,
) -> str:
mode = "cgw"
if upstream is not None and (not skip_upstream or check_only):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If upstream is the sync source, probe it on apply the same way you do for --check. --skip-upstream currently looks at distgit: leftover vendor/helm (the test fixture always has this) plus an old lock for a different version becomes mode=vendor, while --check still prints mode=cgw. Then you copy a CGW lockfile and set gomod prefetch.

Suggested change
if upstream is not None and (not skip_upstream or check_only):
if upstream is not None:

Comment on lines +92 to +98
result = subprocess.run(
["git", "-C", str(path), "status", "--porcelain"],
capture_output=True,
text=True,
check=False,
)
if result.stdout.strip():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check=False plus "empty stdout means clean" treats fatal: not a git repository as success. That path is rc 128 with empty stdout, then distgit still gets written. Tests always pass --allow-dirty, so this never runs.

Suggested change
result = subprocess.run(
["git", "-C", str(path), "status", "--porcelain"],
capture_output=True,
text=True,
check=False,
)
if result.stdout.strip():
result = subprocess.run(
["git", "-C", str(path), "status", "--porcelain"],
capture_output=True,
text=True,
check=False,
)
if result.returncode != 0:
die(
f"{label} is not a git repository or git status failed ({path}): "
f"{result.stderr.strip() or 'unknown error'}"
)
if result.stdout.strip():

Comment on lines +297 to +304
if version:
footer = footer.replace(f"{version}-{current}", f"{version}-{next_n}")
else:
footer = re.sub(
rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}',
rf"\g<1>{next_n}",
footer,
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace("2.0-1", "2.0-2") also rewrites 2.0-10. Same hole in the no-version regex. Add a test with release="10" / 2.0-10; the fixture only covers 1 -> 2.

Suggested change
if version:
footer = footer.replace(f"{version}-{current}", f"{version}-{next_n}")
else:
footer = re.sub(
rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}',
rf"\g<1>{next_n}",
footer,
)
footer = footer.replace(f'release="{current}"', f'release="{next_n}"')
if version:
footer = re.sub(
rf"{re.escape(version)}-{re.escape(current)}(?![0-9])",
f"{version}-{next_n}",
footer,
)
else:
footer = re.sub(
rf'(konflux\.additional-tags="[^\"]*-){re.escape(current)}(?![0-9])',
rf"\g<1>{next_n}",
footer,
)

die(f"Could not find must-gather under {parent}")
upstream = found
if downstream is None:
found = discover_repo(parent, "4-rhdh", "rhdh", "rhidp-rhdh")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--parent-dir never looks for rhdh-downstream, which is the name /rhdh-context uses. rhdh is the GitHub clone. Validate fails (no silent write), but a sibling GitLab checkout is never tried. Stronger still: only accept a candidate that has distgit/containers/rhdh-must-gather.

Suggested change
found = discover_repo(parent, "4-rhdh", "rhdh", "rhidp-rhdh")
found = discover_repo(parent, "4-rhdh", "rhdh-downstream", "rhidp-rhdh")

Address review on the apply path: probe upstream for CGW vs vendor on
apply, fail git status errors, numeric-boundary release tags, discover
rhdh-downstream, gate writes behind /mutation-gate, and run tests via
sys.executable with a real --to bump.

https://redhat.atlassian.net/browse/RHIDP-16046

Generated-by: cursor
Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants