OCPCLOUD-3647: Add ManifestTransformers interface - #634
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.
This is now handled in toBoxcutterRevision
|
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (14)
WalkthroughThe change adds a manifest transformer API, unifies rendered component objects as unstructured pointers, applies ordered transformations during Boxcutter phase construction, aggregates transformation errors, and wires configured transformers through revision and installer controllers. ChangesManifest transformation pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InstallerSetup
participant RevisionController
participant InstallerController
participant RevisionReconciler
participant Boxcutter
InstallerSetup->>RevisionController: configure transformers
InstallerSetup->>InstallerController: configure transformers
RevisionController->>RevisionController: validate rendered objects
InstallerController->>RevisionReconciler: reconcile API revisions
RevisionReconciler->>Boxcutter: construct transformed phases
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
pkg/controllers/installer/boxcutter_test.go (2)
255-266: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssert the exact option count instead of
>.Construction is deterministic here — one transformer option on one object — so
HaveLen(baseOptCount + 1)would catch both dropped and duplicated options, which the>comparison cannot.♻️ Proposed refactor
- Expect(len(withTfm.GetPhases()[0].GetReconcileOptions())).To( - BeNumerically(">", baseOptCount), - "transformer options should augment the phase reconcile options", - ) + Expect(withTfm.GetPhases()[0].GetReconcileOptions()).To( + HaveLen(baseOptCount+1), + "transformer options should augment the phase reconcile options", + )🤖 Prompt for 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. In `@pkg/controllers/installer/boxcutter_test.go` around lines 255 - 266, Update the assertion in the toBoxcutterRevision transformer test to require the phase reconcile-options collection to have exactly baseOptCount + 1 entries, replacing the greater-than comparison while preserving the existing context and failure message.
161-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd failure messages to the bare structural assertions.
Expect(phases).To(HaveLen(1))/Expect(phases[0].GetName()).To(Equal(...))here (and theHaveLen(2)at line 161) fail with no context about which phase layout was expected. The neighbouring assertions in this file already carry messages — worth being consistent.As per coding guidelines, "(4) Assertion messages—assertions should include meaningful failure messages, flag missing messages".
🤖 Prompt for 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. In `@pkg/controllers/installer/boxcutter_test.go` around lines 161 - 193, Add meaningful failure messages to the structural assertions in the phase-layout tests: the HaveLen(2) assertion and both HaveLen(1) and phase-name Equal assertions in the no-CRD and only-CRD cases. Describe the expected phase layout in each message, matching the contextual style of the neighboring object-kind assertions.Source: Coding guidelines
pkg/controllers/installer/boxcutter.go (1)
52-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
crdsand avoid appending into the sharedprobeOptsslice.Two things in this closure:
- Line 57 binds the processed objects to
crds, butaddPhaseis used for both the CRD and the non-CRD phase — the name is wrong for half its callers.- Line 58 appends into
probeOpts, which is captured from the enclosing function and reused for every phase. This is only safe whileutil.SliceMapreturns a slice with no spare capacity; a future change there would let one phase's options leak into another's backing array.slices.Concatremoves the hazard.As per coding guidelines, "Use descriptive names for functions, variables, and identifiers".
♻️ Proposed refactor
- objects, xfmrOpts, err := applyTransformers(ctx, transformers, objects) + transformedObjects, xfmrOpts, err := applyTransformers(ctx, transformers, objects) if err != nil { return err } - crds, adoptOpts := processAdoptExistingAnnotations(objects) - allOpts := append(append(probeOpts, adoptOpts...), xfmrOpts...) - bcPhase := boxcutter.NewPhase(name, util.SliceMap(crds, toClientObject)).WithReconcileOptions(allOpts...) + phaseObjects, adoptOpts := processAdoptExistingAnnotations(transformedObjects) + allOpts := slices.Concat(probeOpts, adoptOpts, xfmrOpts) + bcPhase := boxcutter.NewPhase(name, util.SliceMap(phaseObjects, toClientObject)).WithReconcileOptions(allOpts...)Requires adding
"slices"to the imports.🤖 Prompt for 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. In `@pkg/controllers/installer/boxcutter.go` around lines 52 - 59, Rename the processed object variable crds to a descriptive name that applies to both CRD and non-CRD phases, and update its use in util.SliceMap. In addPhase, replace the nested append-based construction of allOpts with slices.Concat(probeOpts, adoptOpts, xfmrOpts) so each phase receives an independent options slice; add the slices import.Source: Coding guidelines
🤖 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/controllers/installer/boxcutter.go`:
- Around line 118-122: Move the WithObjectReconcileOptions registration out of
the transformer loop: collect each object's objOpts while transformers run, then
append the options once after all transformations complete using the final
transformedObj. Ensure the options remain keyed to the final object rather than
an intermediate value.
In `@pkg/controllers/installer/revision_reconciler.go`:
- Around line 345-349: Update the error handling after toBoxcutterRevision in
reconcileRevision so construction failures use the same terminal classification
as the corresponding Boxcutter-build failure path, rather than returning the raw
err. Preserve the existing completion result while ensuring the error is marked
non-retryable.
In `@pkg/controllers/installer/suite_test.go`:
- Line 109: Add a meaningful failure message to the SetupWithManager assertion
identifying InstallerController setup, while preserving the existing success
expectation and arguments.
In `@pkg/controllers/revision/revision_controller_test.go`:
- Line 587: Update the test’s Reconcile invocation to capture and assert the
returned error is nil, preserving the expected non-retryable status path while
ensuring unexpected reconcile failures fail the test.
---
Nitpick comments:
In `@pkg/controllers/installer/boxcutter_test.go`:
- Around line 255-266: Update the assertion in the toBoxcutterRevision
transformer test to require the phase reconcile-options collection to have
exactly baseOptCount + 1 entries, replacing the greater-than comparison while
preserving the existing context and failure message.
- Around line 161-193: Add meaningful failure messages to the structural
assertions in the phase-layout tests: the HaveLen(2) assertion and both
HaveLen(1) and phase-name Equal assertions in the no-CRD and only-CRD cases.
Describe the expected phase layout in each message, matching the contextual
style of the neighboring object-kind assertions.
In `@pkg/controllers/installer/boxcutter.go`:
- Around line 52-59: Rename the processed object variable crds to a descriptive
name that applies to both CRD and non-CRD phases, and update its use in
util.SliceMap. In addPhase, replace the nested append-based construction of
allOpts with slices.Concat(probeOpts, adoptOpts, xfmrOpts) so each phase
receives an independent options slice; add the slices import.
🪄 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: f1196650-e639-402a-9486-ec4aabf74039
📒 Files selected for processing (16)
cmd/capi-installer/main.gopkg/controllers/installer/boxcutter.gopkg/controllers/installer/boxcutter_test.gopkg/controllers/installer/installer_controller.gopkg/controllers/installer/revision_reconciler.gopkg/controllers/installer/suite_test.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/revisiongenerator/revision.gopkg/revisiongenerator/revision_test.gopkg/revisiongenerator/validate.gopkg/revisiongenerator/validate_test.gopkg/runtimetransformer/runtime_transformer.gopkg/runtimetransformer/validate.gopkg/runtimetransformer/validate_test.go
| ) | ||
|
|
||
| Expect(SetupWithManager(mgr, allProviderProfiles, triggerSource)).To(Succeed()) | ||
| Expect(SetupWithManager(mgr, allProviderProfiles, nil, triggerSource)).To(Succeed()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add context to the setup assertion.
Include a failure message identifying InstallerController setup. As per coding guidelines, “assertions should include meaningful failure messages.”
- Expect(SetupWithManager(mgr, allProviderProfiles, nil, triggerSource)).To(Succeed())
+ Expect(SetupWithManager(mgr, allProviderProfiles, nil, triggerSource)).
+ To(Succeed(), "set up InstallerController")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Expect(SetupWithManager(mgr, allProviderProfiles, nil, triggerSource)).To(Succeed()) | |
| Expect(SetupWithManager(mgr, allProviderProfiles, nil, triggerSource)). | |
| To(Succeed(), "set up InstallerController") |
🤖 Prompt for 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.
In `@pkg/controllers/installer/suite_test.go` at line 109, Add a meaningful
failure message to the SetupWithManager assertion identifying
InstallerController setup, while preserving the existing success expectation and
arguments.
Source: Coding guidelines
| Transformers: []runtimetransformer.RuntimeTransformer{&stubTransformer{validateErr: errors.New("invalid manifest")}}, | ||
| } | ||
|
|
||
| _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKey{Name: "cluster"}}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the reconcile error.
Discarding the returned error can let this test pass after an unexpected reconcile failure. The expected non-retryable status path should not produce a Go error.
Proposed fix
- _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKey{Name: "cluster"}})
+ _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKey{Name: "cluster"}})
+ Expect(err).NotTo(HaveOccurred(), "reconcile should return no Go error for transformer validation")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _, _ = r.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKey{Name: "cluster"}}) | |
| _, err := r.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKey{Name: "cluster"}}) | |
| Expect(err).NotTo(HaveOccurred(), "reconcile should return no Go error for transformer validation") |
🤖 Prompt for 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.
In `@pkg/controllers/revision/revision_controller_test.go` at line 587, Update the
test’s Reconcile invocation to capture and assert the returned error is nil,
preserving the expected non-retryable status path while ensuring unexpected
reconcile failures fail the test.
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. |
ea57fb0 to
327b1a9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pkg/controllers/installer/boxcutter_test.go (2)
68-115: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicated
ManifestTransformertest doubles across packages.See consolidated comment for the shared duplication across
pkg/manifesttransformer/validate_test.goandpkg/controllers/revision/helpers_test.go.🤖 Prompt for 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. In `@pkg/controllers/installer/boxcutter_test.go` around lines 68 - 115, Remove the duplicated ManifestTransformer test doubles from boxcutter_test.go, including stubTransformer and fnTransformer, and reuse the shared test helpers established for this interface. Update affected tests to reference the consolidated helpers while preserving their existing transform, validation, and fluent configuration behavior.
410-474: 📐 Maintainability & Code Quality | 🔵 TrivialConsider
DescribeTablefor the error-aggregation cases.These four
Itblocks share the same shape (revision from provider(s) + stub transformer(s) with errors → assert joined error contains expected substrings) and differ only in inputs/expected substrings. As per coding guidelines, "Use Ginkgo/Gomega framework and prefer built-in features: useDescribeTablewithEntryfor table-driven tests."♻️ Illustrative table-driven sketch
DescribeTable("aggregates errors", func(providers []string, transformers []manifesttransformer.ManifestTransformer, wantSubstrings []string) { rev := installerRevisionFromProfiles(providers...) _, err := toBoxcutterRevision(context.Background(), rev, transformers, nil) matchers := make([]any, len(wantSubstrings)) for i, s := range wantSubstrings { matchers[i] = ContainSubstring(s) } Expect(err).To(MatchError(SatisfyAll(matchers...))) }, Entry("multiple objects in the same phase", []string{providerManyClusterScoped}, []manifesttransformer.ManifestTransformer{&stubTransformer{err: errors.New("boom")}}, []string{"/test-cr-1", "/test-cr-2"}), // ... )🤖 Prompt for 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. In `@pkg/controllers/installer/boxcutter_test.go` around lines 410 - 474, Refactor the four error-aggregation It blocks under the “error aggregation” Describe into one DescribeTable with Entries covering their provider inputs, transformer errors, and expected substrings. Build the revision and invoke toBoxcutterRevision in the table body, dynamically combine ContainSubstring matchers with SatisfyAll, and preserve each case’s existing assertions; keep the separate “does not build a Revision” test unchanged.Source: Coding guidelines
pkg/manifesttransformer/validate_test.go (1)
134-180: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicated
ManifestTransformer/RenderedRevisiontest doubles across packages.
stubTransformer,fakeComponent, andfakeRevisionhere closely mirror doubles redefined inpkg/controllers/installer/boxcutter_test.goandpkg/controllers/revision/helpers_test.go. See consolidated comment for details.🤖 Prompt for 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. In `@pkg/manifesttransformer/validate_test.go` around lines 134 - 180, Remove the duplicated test doubles stubTransformer, fakeComponent, and fakeRevision from validate_test.go, then reuse the shared equivalents already defined for ManifestTransformer and revisiongenerator.RenderedRevision tests. Update affected tests to reference the consolidated helpers while preserving their existing validation and fake revision behavior.pkg/controllers/revision/helpers_test.go (1)
201-238: 📐 Maintainability & Code Quality | 🔵 TrivialDuplicated
ManifestTransformer/RenderedRevisiontest doubles across packages.See consolidated comment for the shared duplication across
pkg/manifesttransformer/validate_test.goandpkg/controllers/installer/boxcutter_test.go.🤖 Prompt for 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. In `@pkg/controllers/revision/helpers_test.go` around lines 201 - 238, Replace the duplicated stubTransformer and fakeRevision test doubles in helpers_test.go with the shared test fixtures established for ManifestTransformer and RenderedRevision. Update affected tests to use those shared symbols, preserving validateErr and components behavior, and remove the local type definitions and interface assertions.
🤖 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/controllers/installer/boxcutter.go`:
- Around line 98-129: Update addPhase to detect a nil transformedObj immediately
after applyTransformers returns and skip that source object before processing
objOpts or appending to transformedObjects. Preserve error handling for objErrs,
and ensure skipped objects do not reach processAdoptExistingAnnotations,
WithObjectReconcileOptions, or boxcutter.NewPhase.
---
Nitpick comments:
In `@pkg/controllers/installer/boxcutter_test.go`:
- Around line 68-115: Remove the duplicated ManifestTransformer test doubles
from boxcutter_test.go, including stubTransformer and fnTransformer, and reuse
the shared test helpers established for this interface. Update affected tests to
reference the consolidated helpers while preserving their existing transform,
validation, and fluent configuration behavior.
- Around line 410-474: Refactor the four error-aggregation It blocks under the
“error aggregation” Describe into one DescribeTable with Entries covering their
provider inputs, transformer errors, and expected substrings. Build the revision
and invoke toBoxcutterRevision in the table body, dynamically combine
ContainSubstring matchers with SatisfyAll, and preserve each case’s existing
assertions; keep the separate “does not build a Revision” test unchanged.
In `@pkg/controllers/revision/helpers_test.go`:
- Around line 201-238: Replace the duplicated stubTransformer and fakeRevision
test doubles in helpers_test.go with the shared test fixtures established for
ManifestTransformer and RenderedRevision. Update affected tests to use those
shared symbols, preserving validateErr and components behavior, and remove the
local type definitions and interface assertions.
In `@pkg/manifesttransformer/validate_test.go`:
- Around line 134-180: Remove the duplicated test doubles stubTransformer,
fakeComponent, and fakeRevision from validate_test.go, then reuse the shared
equivalents already defined for ManifestTransformer and
revisiongenerator.RenderedRevision tests. Update affected tests to reference the
consolidated helpers while preserving their existing validation and fake
revision behavior.
🪄 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: 09235b12-e102-4c3b-bc31-5fdee4bc2849
📒 Files selected for processing (12)
cmd/capi-installer/main.gopkg/controllers/installer/boxcutter.gopkg/controllers/installer/boxcutter_test.gopkg/controllers/installer/installer_controller.gopkg/controllers/installer/revision_reconciler.gopkg/controllers/installer/suite_test.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/manifesttransformer/manifest_transformer.gopkg/manifesttransformer/validate.gopkg/manifesttransformer/validate_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/controllers/installer/suite_test.go
- cmd/capi-installer/main.go
- pkg/controllers/revision/revision_controller.go
- pkg/controllers/revision/revision_controller_test.go
- pkg/controllers/installer/revision_reconciler.go
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
327b1a9 to
8d40e45
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/controllers/installer/revision_reconciler_test.go`:
- Line 202: Rename the test case in the transformer validation failure scenario
from its current title to a descriptive title beginning with “should”,
preserving the existing behavior description.
- Around line 43-50: Add meaningful failure messages to the assertions in the
rendered revision setup, including the rendering, install conversion, and API
conversion steps. Update the terminal validation assertions around the
referenced lines to state the expected validation outcome, while preserving the
existing assertion conditions.
🪄 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: c1ffb5d7-44c8-49d3-9bf9-98632f0b9e2a
📒 Files selected for processing (13)
cmd/capi-installer/main.gopkg/controllers/installer/boxcutter.gopkg/controllers/installer/boxcutter_test.gopkg/controllers/installer/installer_controller.gopkg/controllers/installer/revision_reconciler.gopkg/controllers/installer/revision_reconciler_test.gopkg/controllers/installer/suite_test.gopkg/controllers/revision/helpers_test.gopkg/controllers/revision/revision_controller.gopkg/controllers/revision/revision_controller_test.gopkg/manifesttransformer/manifest_transformer.gopkg/manifesttransformer/validate.gopkg/manifesttransformer/validate_test.go
🚧 Files skipped from review as they are similar to previous changes (9)
- pkg/manifesttransformer/validate.go
- pkg/manifesttransformer/manifest_transformer.go
- pkg/controllers/installer/installer_controller.go
- pkg/controllers/installer/boxcutter_test.go
- pkg/controllers/revision/revision_controller.go
- pkg/controllers/revision/revision_controller_test.go
- pkg/manifesttransformer/validate_test.go
- pkg/controllers/installer/revision_reconciler.go
- pkg/controllers/revision/helpers_test.go
mdbooth
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/controllers/installer/installer_controller_test.go`:
- Around line 203-219: Update the test “re-applies an updated transformer output
without a new revision” to capture the current revision name or count after the
initial successful revision and before triggerReconcile(). After reconciliation,
assert that the revision state remains unchanged in addition to verifying the
updated annotation.
In `@pkg/controllers/installer/suite_test.go`:
- Around line 110-113: Add descriptive failure messages to the assertions in
pkg/controllers/installer/suite_test.go lines 110-113, identifying failure to
set up the InstallerController. Also update assertions in
pkg/controllers/installer/installer_controller_test.go lines 207-239 with
messages describing ConfigMap retrieval, annotation validation, update, and
eventual restoration failures.
🪄 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: 4874095c-f99a-4a7e-ac12-1f13a29040b8
📒 Files selected for processing (3)
pkg/controllers/installer/installer_controller_test.gopkg/controllers/installer/suite_test.gopkg/controllers/installer/testvalue_transformer_test.go
ManifestTransformer is a common interface for transformations on resource manifests. It is intended to cover all existing ad-hoc transformations, although they are not implemented in this change.
ee581a5 to
08335ac
Compare
|
@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. |
2 participants
Adds the ManifestTransformers interface and integrates it into the installer and revision controllers. This change does not yet add any ManifestTransformers. It is purely preparatory work.
Builds on #633
TODO:
Summary by CodeRabbit
Summary by CodeRabbit