Skip to content

ART-14708: Auto-sync golang builders to layered operator branches #2814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rayfordj wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

ART-14708: Auto-sync golang builders to layered operator branches #2814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3c19b94
ART-14708: Auto-sync golang builders to layered operator branches
rayfordj Apr 23, 2026
596fa00
uv run ruff check --fix
rayfordj Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
144 changes: 139 additions & 5 deletions pyartcd/pyartcd/pipelines/update_golang.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from artcommonlib.util import new_roundtrip_yaml_handler
from elliottlib import util as elliottutil
from elliottlib.constants import GOLANG_BUILDER_CVE_COMPONENT
from github import GithubException

from pyartcd import constants, jenkins
from pyartcd.cli import cli, click_coroutine, pass_runtime
Expand Down Expand Up @@ -342,7 +343,8 @@ async def run(self):
else: # konflux
builder_nvrs = konflux_nvrs

await self.update_golang_streams(go_version, builder_nvrs)
replacements = await self.update_golang_streams(go_version, builder_nvrs)
await self.update_layered_branches(go_version, replacements)

await move_golang_bugs(
ocp_version=self.ocp_version,
Expand Down Expand Up @@ -495,13 +497,15 @@ def _get_builder_pullspec(self, parsed_nvr, build_system: str):
# TODO: This is temporary. In the future we need a location to share with multiple teams.
return f'{KONFLUX_DEFAULT_IMAGE_REPO}:golang-builder-{parsed_nvr["version"]}-{parsed_nvr["release"]}'

async def update_golang_streams(self, go_version, builder_nvrs):
async def update_golang_streams(self, go_version, builder_nvrs) -> dict[str, str]:
"""
Update golang builders for current release in ocp-build-data
1. First check go verion from group.yml var to decide if it's a major version bump or minor version bump
2. Get the golang image value, find and replace for each item in streams.yml
3. If it'a major version bump, also need to update key in streams.yml and vars in group.yml
4. Create pr to update changes

Returns a dict mapping old pullspecs to new pullspecs for all replacements made.
"""
branch = f"openshift-{self.ocp_version}"
upstream_repo = get_github_client_for_org("openshift-eng").get_repo("openshift-eng/ocp-build-data")
Expand Down Expand Up @@ -529,6 +533,7 @@ def previous_go_stream_name(el_v):
return f'rhel-{el_v}-golang-{go_previous_var_template}'

update_streams = update_group = False
replacements: dict[str, str] = {}

# register aliases
stream_alias_map = {}
Expand Down Expand Up @@ -557,6 +562,7 @@ def get_stream(stream_name):
latest_go = get_stream(latest_go_stream_name(el_v))['image']

new_latest_go = self._get_builder_pullspec(parsed_nvr, self.build_system)
replacements[latest_go] = new_latest_go
for _, info in streams_content.items():
if info['image'] == latest_go:
info['image'] = new_latest_go
Expand All @@ -570,6 +576,7 @@ def get_stream(stream_name):
previous_go = get_stream(previous_go_stream_name(el_v))['image']

new_previous_go = self._get_builder_pullspec(parsed_nvr, self.build_system)
replacements[previous_go] = new_previous_go
for _, info in streams_content.items():
if info['image'] == previous_go:
info['image'] = new_previous_go
Expand All @@ -586,10 +593,13 @@ def get_stream(stream_name):
previous_go = get_stream(previous_go_stream_name(el_v))['image'] if go_previous else None

new_latest_go = self._get_builder_pullspec(parsed_nvr, self.build_system)
replacements[latest_go] = new_latest_go
if previous_go:
replacements[previous_go] = latest_go
Comment on lines +596 to +598
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python - <<'PY'
cases = [("1.10.1", "1.9"), ("2.0.0", "1.25")]
for go_version, go_latest in cases:
    current_expr = (
        go_version.split(".")[0] >= go_latest.split(".")[0]
        and go_version.split(".")[1] > go_latest.split(".")[1]
    )
    print(f"{go_version} vs {go_latest}: current_expr={current_expr}")
PY

Repository: openshift-eng/art-tools

Length of output: 134

🏁 Script executed:

sed -n '580,620p' pyartcd/pyartcd/pipelines/update_golang.py | cat -n

Repository: openshift-eng/art-tools

Length of output: 2728

🏁 Script executed:

sed -n '590,610p' pyartcd/pyartcd/pipelines/update_golang.py | cat -n

Repository: openshift-eng/art-tools

Length of output: 1370

🏁 Script executed:

python - <<'PY'
cases = [("1.10.1", "1.9"), ("2.0.0", "1.25"), ("1.9", "1.10"), ("1.20", "1.20")]
for go_version, go_latest in cases:
    # Original logic
    original = (
        go_version.split(".")[0] >= go_latest.split(".")[0]
        and go_version.split(".")[1] > go_latest.split(".")[1]
    )
    # Suggested fix
    suggested = tuple(map(int, go_version.split('.')[:2])) > tuple(map(int, go_latest.split('.')[:2]))
    print(f"{go_version} vs {go_latest}: original={original}, suggested={suggested}")
PY

Repository: openshift-eng/art-tools

Length of output: 248

🏁 Script executed:

python - <<'PY'
# Test edge cases for the suggested fix
cases = [
    ("1", "0"),           # Single digit
    ("1.20.5", "1.20.4"), # Patch version (should compare major.minor only)
    ("1.20.5", "1.19"),   # Patch vs no patch
]

for go_version, go_latest in cases:
    try:
        parts_v = list(map(int, go_version.split('.')[:2]))
        parts_l = list(map(int, go_latest.split('.')[:2]))
        suggested = tuple(parts_v) > tuple(parts_l)
        print(f"{go_version} vs {go_latest}: suggested={suggested}")
    except (ValueError, IndexError) as e:
        print(f"{go_version} vs {go_latest}: ERROR - {e}")
PY

Repository: openshift-eng/art-tools

Length of output: 154

🏁 Script executed:

# Verify the fix handles version strings without proper minor versions
python - <<'PY'
# Test what happens with version strings that don't have 2 components
test_cases = [
    ("1", "1.0"),        # What if go_version has no minor?
    ("1.20", "2"),       # What if go_latest has no minor?
]

for go_version, go_latest in test_cases:
    try:
        parts_v = list(map(int, go_version.split('.')[:2]))
        parts_l = list(map(int, go_latest.split('.')[:2]))
        # Pad with 0 if needed
        while len(parts_v) < 2:
            parts_v.append(0)
        while len(parts_l) < 2:
            parts_l.append(0)
        suggested = tuple(parts_v) > tuple(parts_l)
        print(f"{go_version} vs {go_latest}: parts_v={parts_v}, parts_l={parts_l}, suggested={suggested}")
    except (ValueError, IndexError) as e:
        print(f"{go_version} vs {go_latest}: ERROR - {e}")
PY

Repository: openshift-eng/art-tools

Length of output: 184

Fix version comparison logic to use numeric tuple comparison.

The current string-split comparison at line 585 fails to detect legitimate major-version Go upgrades. For example, 1.10.1 vs 1.9 and 2.0.0 vs 1.25 both incorrectly return False. When the condition is False, the replacements map at lines 596-598 is never populated, causing the GO_PREVIOUS/GO_LATEST remap to be skipped for both the main branch and layered branches.

Use numeric tuple comparison to fix this:

Suggested fix 
-        elif go_version.split('.')[0] >= go_latest.split('.')[0] and go_version.split('.')[1] > go_latest.split('.')[1]:
+        elif tuple(map(int, go_version.split('.')[:2])) > tuple(map(int, go_latest.split('.')[:2])):
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pyartcd/pyartcd/pipelines/update_golang.py` around lines 596 - 598, The
version comparison currently using string-split is incorrect; change the logic
that checks whether latest_go > previous_go to parse both latest_go and
previous_go into numeric tuples (e.g., split on '.' and map to ints, padding
shorter tuples with zeros) and compare those tuples instead, then ensure that
when the numeric comparison is True you populate the replacements dict with
replacements[latest_go] = new_latest_go and replacements[previous_go] =
latest_go so the GO_PREVIOUS/GO_LATEST remap is applied for the main and layered
branches; update the code paths that reference latest_go, previous_go,
replacements, and any helper currently performing the string comparison to use
the numeric-tuple comparison.

for _, info in streams_content.items():
if info['image'] == latest_go:
info['image'] = new_latest_go
if info['image'] == previous_go:
if previous_go and info['image'] == previous_go:
info['image'] = latest_go
group_content['vars'][go_latest_var] = go_version
group_content['vars']['GO_EXTRA'] = go_version
Expand All @@ -600,7 +610,7 @@ def get_stream(stream_name):
if update_streams:
if self.dry_run:
_LOGGER.info(f"[DRY RUN] Would have created PR to update {go_version} golang builders")
return
return replacements
if self.skip_pr:
# Log NVRs and pullspecs for golang builder images
builder_info = []
Expand All @@ -618,7 +628,7 @@ def get_stream(stream_name):
f"Skipping PR creation (--skip-pr flag set) for {go_version} golang builders update.\n"
f"Golang builder images:\n{builder_details}"
)
return
return replacements
fork_repo = get_github_client_for_org("openshift-bot").get_repo("openshift-bot/ocp-build-data")
branch_name = f"update-golang-{self.ocp_version}-{go_version}"
title = f"{self.art_jira} - Bump {self.ocp_version} golang builders to {go_version}"
Expand Down Expand Up @@ -658,6 +668,130 @@ def get_stream(stream_name):
)
else:
_LOGGER.info(f"No update needed in {branch}")
return replacements

async def update_layered_branches(self, go_version: str, replacements: dict[str, str]):
"""
For each layered operator branch listed in golang_sync_branches, update
streams.yml with the same builder pullspec replacements applied to the
main OCP branch.
"""
branch = f"openshift-{self.ocp_version}"
upstream_repo = get_github_client_for_org("openshift-eng").get_repo("openshift-eng/ocp-build-data")
group_content = yaml.load(upstream_repo.get_contents("group.yml", ref=branch).decoded_content)
sync_branches = group_content.get('vars', {}).get('golang_sync_branches', [])

if not sync_branches:
_LOGGER.info("No golang_sync_branches configured for %s, skipping layered branch sync", branch)
return

if not replacements:
_LOGGER.info("No replacements to apply, skipping layered branch sync")
return

if self.dry_run:
_LOGGER.info(
"[DRY RUN] Would update layered branches %s with replacements: %s", sync_branches, replacements
)
return

if self.skip_pr:
_LOGGER.info("Skipping layered branch PRs (--skip-pr flag set) for branches: %s", sync_branches)
return

results = await asyncio.gather(
*[self._update_layered_branch(b, go_version, replacements) for b in sync_branches],
return_exceptions=True,
)

summary_lines = []
for branch_name, result in zip(sync_branches, results):
if isinstance(result, Exception):
summary_lines.append(f" - {branch_name}: :x: error: {result}")
_LOGGER.error("Failed to update layered branch %s: %s", branch_name, result)
elif result is None:
summary_lines.append(f" - {branch_name}: skipped (no matching builders)")
else:
summary_lines.append(f" - {branch_name}: {result}")

summary = f"Layered branch PRs for golang {go_version}:\n" + "\n".join(summary_lines)
_LOGGER.info(summary)
await self._slack_client.say_in_thread(summary)
Comment on lines +702 to +719
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 23, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't mark the pipeline successful when layered sync failed.

Exceptions are summarized and then discarded here. That lets run() continue and later post the final success message even when one or more layered-branch PRs were not created.

💡 Suggested fix 
-        summary_lines = []
+        summary_lines = []
+        errors = []
         for branch_name, result in zip(sync_branches, results):
             if isinstance(result, Exception):
+                errors.append(f"{branch_name}: {result}")
                 summary_lines.append(f"  - {branch_name}: :x: error: {result}")
                 _LOGGER.error("Failed to update layered branch %s: %s", branch_name, result)
             elif result is None:
                 summary_lines.append(f"  - {branch_name}: skipped (no matching builders)")
             else:
                 summary_lines.append(f"  - {branch_name}: {result}")
 
         summary = f"Layered branch PRs for golang {go_version}:\n" + "\n".join(summary_lines)
         _LOGGER.info(summary)
         await self._slack_client.say_in_thread(summary)
+        if errors:
+            raise RuntimeError("Failed to update layered branches:\n" + "\n".join(errors))
🧰 Tools
🪛 Ruff (0.15.10)

[warning] 708-708: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pyartcd/pyartcd/pipelines/update_golang.py` around lines 702 - 719, The code
currently collects exceptions from asyncio.gather for _update_layered_branch but
only logs and summarizes them, allowing run() to continue as a success; change
this by detecting any Exception in results after the loop and raising an error
(e.g., raise RuntimeError or the first exception) with the built summary so the
pipeline fails properly. Concretely: after building summary (the summary
variable and _LOGGER.info call), if any item in results is an Exception, log the
summary, post it via self._slack_client.say_in_thread, then raise a
RuntimeError(f"Layered branch updates failed: {summary}") (or re-raise the first
Exception) so run() observes failure instead of proceeding. Ensure you reference
_update_layered_branch, results, summary and self._slack_client.say_in_thread in
the change.

Copy link
Copy Markdown
Contributor Author

@rayfordj rayfordj Apr 23, 2026

Choose a reason for hiding this comment

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

@coderabbitai autofix


async def _update_layered_branch(
self, branch_name: str, go_version: str, replacements: dict[str, str]
) -> str | None:
"""
Apply pullspec replacements to streams.yml on a single layered branch.
Returns the PR URL if a PR was created, or None if no changes were needed.
"""
upstream_repo = get_github_client_for_org("openshift-eng").get_repo("openshift-eng/ocp-build-data")
try:
upstream_repo.get_branch(branch_name)
except GithubException as e:
if e.status == 404:
raise ValueError(f"Branch '{branch_name}' does not exist in ocp-build-data")
raise
streams_raw = upstream_repo.get_contents("streams.yml", ref=branch_name)
streams_content = yaml.load(streams_raw.decoded_content)

changed = False
for stream_name, info in streams_content.items():
old_image = info.get('image', '')
if old_image in replacements:
_LOGGER.debug(
"Replacing golang builder in %s: %s -> %s", branch_name, old_image, replacements[old_image]
)
info['image'] = replacements[old_image]
changed = True

if not changed:
_LOGGER.info("No matching builders in %s, skipping", branch_name)
return None

fork_repo = get_github_client_for_org("openshift-bot").get_repo("openshift-bot/ocp-build-data")
fork_branch_name = f"update-golang-{branch_name}-{go_version}"
try:
fork_repo.get_git_ref(f"heads/{fork_branch_name}").delete()
except GithubException as e:
if e.status != 404:
raise
fork_repo.create_git_ref(
f"refs/heads/{fork_branch_name}",
upstream_repo.get_branch(branch_name).commit.sha,
)

output = io.BytesIO()
yaml.dump(streams_content, output)
output.seek(0)
fork_file = fork_repo.get_contents("streams.yml", ref=fork_branch_name)
commit_msg = f"Bump {branch_name} golang builders to {go_version}"
fork_repo.update_file("streams.yml", commit_msg, output.read(), fork_file.sha, branch=fork_branch_name)

title = f"{self.art_jira} - Bump {branch_name} golang builders to {go_version}"
build_url = jenkins.get_build_url()

if not branch_name.startswith("openshift-"):
replacements_applied = []
for sname, sinfo in streams_content.items():
for old, new in replacements.items():
if sinfo.get('image') == new:
replacements_applied.append(f"- `{sname}`: {old.split(':')[-1]} \u2192 {new.split(':')[-1]}")
break
body = (
f"Created by job run {build_url or 'manual trigger'}\n\n"
f"This PR was automatically generated to sync golang builder "
f"updates from openshift-{self.ocp_version}.\n\n"
f"**Golang builder replacements:**\n"
+ ("\n".join(replacements_applied) if replacements_applied else "- (no streams updated)")
)
else:
body = f"Created by job run {build_url}" if build_url else ""
pr = upstream_repo.create_pull(
title=title, body=body, base=branch_name, head=f"openshift-bot:{fork_branch_name}"
)
_LOGGER.info("PR created %s for layered branch %s", pr.html_url, branch_name)
return pr.html_url

async def _rebase_brew(self, el_v, go_version):
_LOGGER.info("Rebasing for Brew...")
Expand Down
Loading
Loading