OCPCLOUD-3647: Consolidate install-time object processing in toBoxcutterRevision - #633
Conversation
We were previously pre-converting all API revisions into installer revisions. The purpose of this was to assemble all related objects before reconciliation. However, we weren't actually writing them so this served no purpose. We take the opportunity to remove the unnecessary complexity.
This allows us to add code to component generation which can fail.
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mdbooth: This pull request explicitly references no jira issue. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughRevision rendering now exposes all manifests as pointer-based unstructured objects. Installer conversion builds native Boxcutter phases with CRD separation and adopt-existing options, while reconciliation processes API revisions directly and resolves collected objects afterward. ChangesRevision rendering and validation
Boxcutter conversion
API reconciliation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant revisionReconciler
participant reconcileRevision
participant toBoxcutterRevision
participant ObjectResolver
revisionReconciler->>reconcileRevision: reconcile API revision
reconcileRevision->>toBoxcutterRevision: construct Boxcutter revision and phases
revisionReconciler->>ObjectResolver: resolve collected objects after reconciliation
ObjectResolver-->>revisionReconciler: return resolution errors
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 14 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/pipeline auto |
|
Pipeline controller notification The |
|
Scheduling tests matching the |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
76a6184 to
f9e7299
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/revisiongenerator/revision_test.go`:
- Line 313: Strengthen the object-contract tests: in
pkg/revisiongenerator/revision_test.go at lines 313-313, 571-571, and 706-706,
assert the expected CRD and non-CRD kind/name identities returned by Objects()
and the matched profile, not just counts; in
pkg/revisiongenerator/validate_test.go at lines 108-123, retain or adapt the
CRD-shaped invalid-annotation case to verify CRDs receive the same annotation
validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c19846cf-ecef-4284-9c61-5400acf6839d
📒 Files selected for processing (7)
pkg/controllers/installer/boxcutter.gopkg/controllers/installer/boxcutter_test.gopkg/controllers/installer/revision_reconciler.gopkg/revisiongenerator/revision.gopkg/revisiongenerator/revision_test.gopkg/revisiongenerator/validate.gopkg/revisiongenerator/validate_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/revisiongenerator/validate.go
- pkg/revisiongenerator/revision.go
- pkg/controllers/installer/revision_reconciler.go
- pkg/controllers/installer/boxcutter_test.go
- pkg/controllers/installer/boxcutter.go
This is now handled in toBoxcutterRevision
f9e7299 to
e612ad6
Compare
|
Scheduling tests matching the |
|
Infra flakes. /retest-required |
mdbooth
changed the title
|
@mdbooth: This pull request references OCPCLOUD-3647 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Filed OCPBUGS-100155 for the flake. /test e2e-aws-capi-techpreview |
|
Infra flake /test regression-clusterinfra-aws-ipi-techpreview-capi |
|
@mdbooth: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@mdbooth: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
2 participants
This refactor forms the basis of a simplification to enable several other
features:
one place
Summary by CodeRabbit