Skip to content

show image details even if flashing fails#170

Merged
bennyz merged 2 commits into
centos-automotive-suite:mainfrom
bennyz:show-build-flash-fail
Mar 15, 2026
Merged

show image details even if flashing fails#170
bennyz merged 2 commits into
centos-automotive-suite:mainfrom
bennyz:show-build-flash-fail

Conversation

@bennyz

@bennyz bennyz commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

So users can flash themselves

Summary by CodeRabbit

  • Bug Fixes

    • Build results now display for flash-related failures (or when flash-after-build is enabled), improving visibility during error conditions.
    • Failure messages now surface more detailed flash-task diagnostics when available, making root causes clearer.
  • Style

    • Console output simplified: removed the prior manual-flash header and reduced header color usage for cleaner logs.

So users can flash themselves

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
@coderabbitai

coderabbitai Bot commented Mar 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 943872f7-bdd6-4324-97eb-d3ce629fdae6

📥 Commits

Reviewing files that changed from the base of the PR and between e275de6 and 155bf58.

📒 Files selected for processing (4)
  • cmd/caib/buildcmd/build.go
  • cmd/caib/buildcmd/flash_feedback.go
  • cmd/caib/buildcmd/logs.go
  • internal/controller/imagebuild/controller.go
💤 Files with no reviewable changes (1)
  • cmd/caib/buildcmd/build.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmd/caib/buildcmd/logs.go

📝 Walkthrough

Walkthrough

Reworks flash-related failure handling across build logs, flash feedback, and the image build reconciler: displays build results before invoking flash error handling, simplifies flash-detection logic, removes some formatting constants, and adds a reconciler helper that inspects PipelineRun child TaskRuns to produce more specific flash-task failure details.

Changes

Cohort / File(s) Summary
Build log failure handling
cmd/caib/buildcmd/logs.go
Simplified flash detection (message contains "flash" or lastPhase == phaseFlashing). On phaseFailed, use raw message as error and call h.displayBuildResults(ctx, api, name) before h.handleFlashError(...) when flash is detected or FlashAfterBuild is set; otherwise call h.handleError(...).
Build command constants
cmd/caib/buildcmd/build.go
Removed two unused error-prefix constants (errPrefixBuild, errPrefixPush), leaving only errPrefixFlash.
Flash feedback output
cmd/caib/buildcmd/flash_feedback.go
Removed header color/prefix variables and suppressed the "Manual Flash Required" header line; retains command/info color and prints only the info line on failure/success paths.
ImageBuild controller failure detail
internal/controller/imagebuild/controller.go
Added pipelineRunFailureDetail(ctx, pipelineRun) used when marking ImageBuild as Failed — inspects PipelineRun child refs to locate a flash-image TaskRun and, if completed and unsuccessful, returns the TaskRun failure message; otherwise falls back to existing pipeline-level message.

Sequence Diagram(s)

sequenceDiagram
  rect rgba(200,200,255,0.5)
    participant Reconciler
    participant K8sAPI
    participant TektonTaskRun
  end

  Reconciler->>K8sAPI: Get PipelineRun
  K8sAPI-->>Reconciler: PipelineRun (with child refs)
  Reconciler->>K8sAPI: Get referenced TaskRun (flash-image)
  K8sAPI-->>Reconciler: TaskRun (status)
  alt TaskRun completed & unsuccessful
    Reconciler->>Reconciler: extract TaskRun failure message
    Reconciler->>Reconciler: set ImageBuild.Status.Failure = TaskRun message
  else fallback
    Reconciler->>Reconciler: fallback to pipelineRunFailureMessage(pipelineRun)
    Reconciler->>Reconciler: set ImageBuild.Status.Failure = pipeline message
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • caib: improve build flow #111 — Related controller changes that introduced/centralized pipelineRun/taskRun failure-message extraction used as precedent for pipelineRunFailureDetail.

Suggested reviewers

  • bkhizgiy

Poem

🐰 A tiny hop, a careful peek,
I chase the flash through logs that speak,
I show the build before the cry,
A carrot line to catch the why,
Hooray — small hops make errors meek! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main objective: surfacing image details when flashing fails, which is the core purpose of changes across multiple files simplifying error handling and improving failure messaging.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@cmd/caib/buildcmd/logs.go`:
- Around line 164-166: When isFlashFailure is true, displayBuildResults may call
h.handleError() and exit before h.handleFlashError() runs; ensure flash
instructions are always shown by invoking h.handleFlashError(handleErr, st)
before calling h.displayBuildResults(ctx, api, name), or alternatively modify
displayBuildResults to catch OCI-download errors and log them instead of calling
h.handleError; update the invocation order in the block handling isFlashFailure
(or make displayBuildResults non-fatal) so handleFlashError executes regardless
of displayBuildResults failures.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4af20b5e-dbca-469f-9a1c-7e197c3572c6

📥 Commits

Reviewing files that changed from the base of the PR and between abecc4c and e275de6.

📒 Files selected for processing (1)
  • cmd/caib/buildcmd/logs.go

Comment thread cmd/caib/buildcmd/logs.go Outdated
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
Assisted-by: claude-sonnet-4.6
@bennyz bennyz requested a review from bkhizgiy March 15, 2026 10:53
Comment thread internal/controller/imagebuild/controller.go
@bennyz bennyz merged commit 5f19471 into centos-automotive-suite:main Mar 15, 2026
4 checks passed
@bennyz bennyz deleted the show-build-flash-fail branch March 15, 2026 11:49
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.

2 participants