Skip to content

Fix handle_failure() crash when assembly report is missing - #3150

Open
redhat-chai-bot wants to merge 1 commit into
openshift-eng:mainfrom
redhat-chai-bot:fix-build-sync-handle-failure
Open

Fix handle_failure() crash when assembly report is missing#3150
redhat-chai-bot wants to merge 1 commit into
openshift-eng:mainfrom
redhat-chai-bot:fix-build-sync-handle-failure

Conversation

@redhat-chai-bot

@redhat-chai-bot redhat-chai-bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Problem

When doozer crashes early in build-sync-konflux (e.g. OSError: RPM openshift-ansible claims to have a rhel-8 build target, but no build was detected), the assembly report file gen-payload-artifacts/assembly-report.yaml is never generated. The handle_failure() method then crashes with FileNotFoundError when get_unpermissable_assembly_issues() tries to open the missing file. This secondary crash prevents:

  • Slack notifications from being sent to #openshift-{version} or #forum-ocp-release
  • Jenkins title from being tagged with [UNVIABLE] / [FAILURE]
  • The original ChildProcessError from being cleanly re-raised

Fix

1. Guard get_unpermissable_assembly_issues() against missing file

  • Added os.path.exists() check before opening the file
  • Returns empty {'assembly_issues': {}} with a warning when file is missing
  • Added null check for data from yaml.safe_load() (could return None for empty files)
  • Moved return filtered_issues outside the if 'assembly_issues' in data: block to fix implicit None return

2. Safety net in handle_failure()

  • Wrapped the assembly report analysis block in try/except Exception so notification logic always runs even if report analysis fails unexpectedly

Verification

Tested with build-sync-konflux #25999 using ART_TOOLS_COMMIT=redhat-chai-bot@fix-build-sync-handle-failure:

  • ✅ Jenkins title tagged [FAILURE] (was missing before)
  • WARNING Assembly report file not found logged (instead of FileNotFoundError crash)
  • Failure count for 5.0: 10 — counter incremented
  • ✅ Clean re-raise of original ChildProcessError (no secondary exception)

Fixes: ART-21634


@locriandev requested in Slack thread

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling when assembly reports are missing, empty, or invalid.
    • Failure reporting now continues gracefully if assembly issue analysis encounters an error.
    • Added clearer fallback messaging when assembly report details are unavailable.
    • Preserved filtering of permitted issues from assembly reports.

When doozer crashes early (e.g. OSError from assembly_inspector), the
assembly report file gen-payload-artifacts/assembly-report.yaml is
never generated. This caused handle_failure() to crash with
FileNotFoundError via get_unpermissable_assembly_issues(), preventing
Slack notifications and Jenkins title updates from running.

Two fixes:

1. Guard get_unpermissable_assembly_issues() against missing/empty
   file: add os.path.exists() check before opening; add null check
   for data from yaml.safe_load; move return outside the
   'assembly_issues' key check to avoid implicit None return.

2. Wrap the assembly report analysis block in handle_failure() with
   try/except as a safety net, so any unexpected error in that path
   still results in a proper failure report and Jenkins title update.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jupierce for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift-eng/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 18979f63-91b4-4bf4-a4b4-53fc6699baab

📥 Commits

Reviewing files that changed from the base of the PR and between e2fdba2 and 6867992.

📒 Files selected for processing (1)
  • pyartcd/pyartcd/pipelines/build_sync.py

Walkthrough

Assembly report analysis now handles missing or empty YAML safely, preserves permitted-issue filtering, and prevents analysis failures from interrupting Jenkins failure handling.

Changes

Assembly report resilience

Layer / File(s) Summary
Report loading and failure fallback
pyartcd/pyartcd/pipelines/build_sync.py
Missing or empty assembly-report.yaml returns an empty issue structure with warnings; failure analysis exceptions now produce a generic report while preserving Jenkins title selection for successful analysis.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: locriandev, ashwindasr

🚥 Pre-merge checks | ✅ 10 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Ai-Attribution ⚠️ Warning The commit uses Co-Authored-By: Claude Opus 4.6, which the check forbids for AI tools, and no Assisted-by/Generated-by trailer is present. Replace the AI Co-Authored-By trailer with the appropriate Red Hat Assisted-by or Generated-by attribution trailer.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main fix: preventing handle_failure() from crashing when the assembly report is missing.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
No-Weak-Crypto ✅ Passed The diff only adds YAML file-existence/error handling; it introduces no MD5/SHA1/DES/RC4/3DES/Blowfish/ECB, custom crypto, or secret comparisons.
Container-Privileges ✅ Passed PASS: The patch only changes pyartcd/pyartcd/pipelines/build_sync.py logic; no container/K8s manifests or privilege settings were added.
No-Sensitive-Data-In-Logs ✅ Passed New logs only report the report file path and exception text; no passwords, tokens, PII, or customer data are logged.
No-Hardcoded-Secrets ✅ Passed Modified build_sync.py has no API keys, tokens, passwords, private keys, embedded-credential URLs, or long base64 literals.
No-Injection-Vectors ✅ Passed Touched code uses yaml.safe_load/safe_dump on a fixed file path and contains no eval/exec, shell=True, os.system, or similar user-input injection vectors.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@openshift-ci

openshift-ci Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@redhat-chai-bot: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/art-pre-commit-check 6867992 link false /test art-pre-commit-check
ci/prow/security 6867992 link false /test security

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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.

1 participant