add --flash-cmd to override OperatorConfig#149
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds a FlashCmd override that travels from CLI flags into BuildRequest/ImageBuild spec and is respected by the controller, which prefers the user-supplied FlashCmd over OperatorConfig defaults when preparing flash-related tasks. Changes
Sequence Diagram(s)sequenceDiagram
participant User as CLI User
participant CLI as caib CLI
participant Server as Build API Server
participant Controller as ImageBuild Controller
participant Jumpstarter as Jumpstarter Flash
User->>CLI: run command with --flash-cmd
CLI->>Server: POST BuildRequest (FlashCmd)
Server->>Server: create ImageBuild (FlashSpec.FlashCmd set)
Server->>Controller: notify/create reconcile
Controller->>Controller: read ImageBuild.Spec.GetFlashCmd()
alt user FlashCmd present
Controller->>Jumpstarter: invoke flash with user FlashCmd
else no user FlashCmd
Controller->>Jumpstarter: invoke flash with OperatorConfig FlashCmd
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/v1alpha1/imagebuild_types.go (1)
60-72:⚠️ Potential issue | 🔴 CriticalRun
make generate manifeststo regenerate all artifacts after API type changes.The CRD YAML includes
flashCmdcorrectly, but the generated deepcopy file (api/v1alpha1/zz_generated.deepcopy.go) has not been updated to reflect the newFlashCmdfield inFlashSpec. TheDeepCopyInto()method shows only a shallow copy without explicit field handling for the new field. Per the coding guidelines, all generated artifacts must be refreshed whenever types inapi/v1alpha1/are modified.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/v1alpha1/imagebuild_types.go` around lines 60 - 72, The FlashSpec type was updated with a new FlashCmd field but generated artifacts weren't regenerated; run the project code generators (e.g., execute "make generate manifests") to regenerate zz_generated.deepcopy.go and related CRD manifests so DeepCopyInto/DeepCopy methods include proper handling of FlashCmd for the FlashSpec struct; verify the regenerated zz_generated.deepcopy.go contains explicit copying of FlashCmd in DeepCopyInto for FlashSpec and commit the updated generated files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/buildapi/server.go`:
- Around line 1590-1594: getBuild currently ignores a persisted manual flash
override and reconstructs JumpstarterInfo.FlashCmd from the OperatorConfig
mapping; change the response construction in getBuild to prefer the persisted
value by using build.Spec.GetFlashCmd() (or equivalent accessor) before falling
back to the target/OperatorConfig mapping when setting JumpstarterInfo.FlashCmd,
ensuring builds created with --flash-cmd show the stored manual flash command in
API responses.
---
Outside diff comments:
In `@api/v1alpha1/imagebuild_types.go`:
- Around line 60-72: The FlashSpec type was updated with a new FlashCmd field
but generated artifacts weren't regenerated; run the project code generators
(e.g., execute "make generate manifests") to regenerate zz_generated.deepcopy.go
and related CRD manifests so DeepCopyInto/DeepCopy methods include proper
handling of FlashCmd for the FlashSpec struct; verify the regenerated
zz_generated.deepcopy.go contains explicit copying of FlashCmd in DeepCopyInto
for FlashSpec and commit the updated generated files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6306c39d-d342-4069-9847-89b6e723838b
⛔ Files ignored due to path filters (1)
config/crd/bases/automotive.sdv.cloud.redhat.com_imagebuilds.yamlis excluded by!config/crd/bases/**
📒 Files selected for processing (9)
api/v1alpha1/imagebuild_types.gocmd/caib/buildcmd/build.gocmd/caib/flashcmd/flash.gocmd/caib/image/image.gocmd/caib/main.gocmd/caib/runtime_wiring.gointernal/buildapi/server.gointernal/buildapi/types.gointernal/controller/imagebuild/controller.go
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> Assisted-by: claude-sonnet-4.6
783b405 to
bf1c422
Compare
Add --flash-cmd to allow users to override the flashCmd set in the OperatorConfig
Summary by CodeRabbit