Skip to content

caib: improve build flow#111

Merged
bennyz merged 6 commits into
centos-automotive-suite:mainfrom
bennyz:improve-build-flow
Feb 14, 2026
Merged

caib: improve build flow#111
bennyz merged 6 commits into
centos-automotive-suite:mainfrom
bennyz:improve-build-flow

Conversation

@bennyz

@bennyz bennyz commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added a logs subcommand to follow and stream build logs.
    • Builds now apply target-specific defaults (architecture and extra args) when available.
  • Improvements

    • Improved log streaming with reconnects, incremental fetches, lease hints, and follow guidance after builds.
    • More informative failure and status messages across build/flash flows.
    • Configurable upload timeout for uploads.
  • Tests

    • Added unit tests covering target defaulting and failure-message logic.

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

coderabbitai Bot commented Feb 13, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Added a CLI logs <build-name> command and enhanced log streaming; introduced target-default fetching/applying (architecture and extra args) for Jumpstarter mappings across CLI and API; changed log-streaming API to return success bool; added failure-message helpers in the imagebuild controller; updated CRD, types, and tests.

Changes

Cohort / File(s) Summary
CLI — logs command & target-default wiring
cmd/caib/main.go
Added logsCmd and runLogs to follow build logs; added fetchTargetDefaults and applyTargetDefaults; integrated default application into runBuild, runDisk, and runBuildDev; updated handlers to accept *cobra.Command for richer flag checks and follow hints.
Build API — streaming & target types
internal/buildapi/server.go, internal/buildapi/types.go, internal/buildapi/server_test.go
Changed streamContainerLogs to return bool indicating stream success; updated callers to respect return; introduced TargetDefaults type and changed OperatorConfigResponse.JumpstarterTargets to map[string]TargetDefaults; tests updated accordingly.
Controller — failure messaging & upload timeout
internal/controller/imagebuild/controller.go, internal/controller/imagebuild/controller_test.go
Added conditionSucceeded constant and helpers pipelineRunFailureMessage and taskRunFailureMessage; used helpers for failure messages; added upload timeout handling for uploading state and unit tests for failure-message logic.
API types, deepcopy & CRD
api/v1alpha1/operatorconfig_types.go, api/v1alpha1/zz_generated.deepcopy.go, config/crd/bases/..._operatorconfigs.yaml
Added Architecture string and ExtraArgs []string to Jumpstarter mappings; deep-copy logic updated for ExtraArgs; CRD schema extended with architecture, extraArgs, and uploadTimeoutMinutes under OS builds.
CLI tests — target defaulting
cmd/caib/main_test.go
Added tests covering target default application: architecture selection, precedence of explicit --arch, and concatenation/prepending of extra args.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI (caib)
  participant API as Build API Server
  participant Controller as ImageBuild Controller
  participant K8s as Kubernetes (pods)

  CLI->>API: Request build status & start log follow (caib logs)
  API->>Controller: Query build status / operator config
  Controller-->>API: Current build state & config (including TargetDefaults)
  API->>K8s: Open/stream pod/container logs (sinceTime)
  K8s-->>API: Stream log lines / EOF / errors
  API-->>CLI: Relay log lines (streaming)
  Note right of API: on reconnect, advance startTime\nand only fetch new logs
  CLI->>API: Poll/wait until build finished
  API->>Controller: Finalize status request
  Controller-->>API: Final build result (success/failure + lease id)
  API-->>CLI: Final result, hints (e.g., lease info)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • bkhizgiy

Poem

🐰
I hopped into logs with eager paws,
Followed streams across restart flaws,
Defaults tucked into arch and args,
Reconnects hum beneath moon and stars,
Hop on — your build applause.

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'caib: improve build flow' is generic and vague, failing to specify the key improvements being made. Consider revising to a more specific title that highlights the main feature, such as 'caib: add logs subcommand and target defaults mechanism' or 'caib: enhance build flow with logs command and target configuration defaults'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
cmd/caib/main.go (1)

2193-2198: Set a TLS minimum version even in insecure mode.
InsecureSkipVerify disables cert checks but still allows protocol downgrade; pinning TLS 1.2+ hardens the client.

🔒 Proposed tweak
-        if insecureSkipTLS {
-            logTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}
-        }
+        if insecureSkipTLS {
+            logTransport.TLSClientConfig = &tls.Config{
+                InsecureSkipVerify: true,
+                MinVersion:         tls.VersionTLS13,
+            }
+        }

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.

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

@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

🤖 Fix all issues with AI agents
In `@cmd/caib/main.go`:
- Around line 2170-2194: The runLogs handler currently returns immediately if
st.Phase == phaseCompleted || st.Phase == phaseFailed which prevents emitting
any logs for finished builds; modify runLogs to fetch and print the latest logs
once before returning by invoking the existing log-streaming/log-fetching API
(e.g., call api.StreamBuildLogs(ctx, name) or api.GetBuildLogs(ctx, name) —
whichever exists in the client) after printing the build status, waiting for
that call to finish, then return; keep the early-return behavior only after the
one-time log fetch/print so active builds still stream as before.

Comment thread cmd/caib/main.go
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
configurable

Signed-off-by: Benny Zlotnik <bzlotnik@redhat.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.

1 participant