-
Notifications
You must be signed in to change notification settings - Fork 68
⚠️ Remove Migrated reason from ClusterExtensionRevision Available condition #2374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Per Goncalves da Silva <[email protected]>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the transient "Migrated" reason from the ClusterExtensionRevision Available condition during storage migration from Helm to ClusterExtensionRevision. The change simplifies the migration flow by allowing the revision controller to manage all condition states uniformly, rather than having the migrator set a short-lived condition.
- Removed the
ClusterExtensionRevisionReasonMigratedconstant from the API types - Eliminated status condition update code in the migration flow that set
Available=Unknownwith reason "Migrated" - Simplified the
BoxcutterStorageMigrator.Migrate()method by removing the post-creation status update logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| api/v1/clusterextensionrevision_types.go | Removed the unused ClusterExtensionRevisionReasonMigrated constant from condition reasons |
| internal/operator-controller/applier/boxcutter.go | Removed code that sets Available=Unknown status condition after creating a migrated revision, along with the subsequent status update and re-fetch operations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2374 +/- ##
==========================================
+ Coverage 72.26% 74.81% +2.55%
==========================================
Files 95 95
Lines 7336 7327 -9
==========================================
+ Hits 5301 5482 +181
+ Misses 1631 1410 -221
- Partials 404 435 +31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Removes the Migrated reason from the ClusterExtensionRevision Available condition. It would be short-lived and it makes more sense to let the revision controller set the conditions. If there is need to call out migration, we could set an annotation on the revision resource.
Reviewer Checklist