Conversation
Signed-off-by: Anatolii Bazko <abazko@redhat.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: tolusha 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the WorkspacesConfigReconciler to use a non-cached client for removing and recreating objects that already exist without the expected cache labels, and it centralizes generic sync logic into a new pkg/common/utils/sync.go while removing the old pkg/deploy/sync/sync.go.
- Introduces
nonCachedClientinWorkspacesConfigReconcilerand its instantiation inmain.go - Updates
doCreateObjectto delete existing non-cached objects and recreate them - Adds a shared
pkg/common/utils/sync.goand removes the legacy syncer
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/deploy/sync/sync.go | Removed (replaced by common utils) |
| pkg/common/utils/sync.go | Added new generic sync utility |
| main.go | Passes nonCachingClient to the reconciler |
| controllers/usernamespace/workspaces_config_controller.go | Enhanced doCreateObject logic to delete/recreate on conflict |
Multiple *_test.go files |
Updated calls to NewWorkspacesConfigReconciler to include the new client parameter and bumped copyrights |
Comments suppressed due to low confidence (1)
pkg/common/utils/sync.go:47
- [nitpick] The parameter name 'context' shadows the imported package 'context'. Rename this parameter (and similar occurrences) to 'ctx' for clarity and to avoid shadowing.
func (s ObjSyncer) Get(context context.Context, key client.ObjectKey, objectMeta client.Object) (bool, error) {
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue with recreating non-cached objects in a user namespace by revising the sync functionality and updating the WorkspacesConfigReconciler.
- Migrates sync logic from pkg/deploy/sync/sync.go into pkg/common/sync and updates corresponding tests.
- Updates WorkspacesConfigReconciler and related controller and test files to accept a non-cached client, facilitating deletion and recreation of objects when necessary.
- Updates main.go and various usernamespace controller tests to reflect the new API for handling cached versus non-cached client operations.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/deploy/sync/sync.go | Removed legacy sync implementation. |
| pkg/common/sync/sync.go and pkg/common/sync/sync_test.go | Introduced new generic sync interfaces and helper functions/tests. |
| main.go | Updated instantiation of WorkspacesConfigReconciler with a non-cached client. |
| controllers/usernamespace/workspaces_config_controller.go | Modified controller logic to delete/recreate objects using non-cached client if needed. |
| controllers/usernamespace/*_test.go | Updated tests to pass non-cached client parameter and adapt to new API. |
Comments suppressed due to low confidence (1)
main.go:293
- [nitpick] Consider unifying the parameter naming between 'nonCachingClient' used here and the function parameter 'nonCachedClient' in NewWorkspacesConfigReconciler for consistency across the codebase.
workspacesConfigReconciler := usernamespace.NewWorkspacesConfigReconciler(mgr.GetClient(), nonCachingClient, mgr.GetScheme(), namespacechace)
|
/retest |
|
Build 3.22 :: operator_3.x/463: Console, Changes, Git Data |
|
Build 3.22 :: sync-to-downstream_3.x/9351: Console, Changes, Git Data |
|
Build 3.22 :: push-latest-container-to-quay_3.x/5191: Console, Changes, Git Data |
|
Build 3.22 :: get-sources-rhpkg-container-build_3.x/9498: devspaces-operator : 3.x :: Build 67809959 : quay.io/devspaces/devspaces-rhel9-operator:3.22-5 |
|
Build 3.22 :: operator_3.x/463: Upstream sync done; /DS_CI/sync-to-downstream_3.x/9351 triggered |
|
Build 3.22 :: update-digests_3.x/9330: Console, Changes, Git Data |
|
Build 3.22 :: operator-bundle_3.x/4919: Console, Changes, Git Data |
|
Build 3.22 :: sync-to-downstream_3.x/9352: Console, Changes, Git Data |
|
Build 3.22 :: push-latest-container-to-quay_3.x/5192: Console, Changes, Git Data |
|
Build 3.22 :: copyIIBsToQuay/3007: Console, Changes, Git Data |
|
Build 3.22 :: sync-to-downstream_3.x/9352: Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/9499 triggered; /job/DS_CI/job/dsc_3.x triggered; |
|
Build 3.22 :: operator-bundle_3.x/4919: Upstream sync done; /DS_CI/sync-to-downstream_3.x/9352 triggered |
|
Build 3.22 :: update-digests_3.x/9330: Detected new images: rebuild operator-bundle |
|
Build 3.22 :: dsc_3.x/2065: Console, Changes, Git Data |
|
Build 3.22 :: dsc_3.x/2065: 3.22.0-CI |
|
Build 3.22 :: operator-bundle_3.x/4920: Console, Changes, Git Data |
|
Build 3.22 :: sync-to-downstream_3.x/9354: Console, Changes, Git Data |
|
Build 3.22 :: push-latest-container-to-quay_3.x/5194: Console, Changes, Git Data |
|
Build 3.22 :: copyIIBsToQuay/3008: Console, Changes, Git Data |
|
Build 3.22 :: sync-to-downstream_3.x/9354: Build container: devspaces-operator-bundle synced; /DS_CI/get-sources-rhpkg-container-build_3.x/9501 triggered; /job/DS_CI/job/dsc_3.x triggered; |
|
Build 3.22 :: operator-bundle_3.x/4920: Upstream sync done; /DS_CI/sync-to-downstream_3.x/9354 triggered |
|
Build 3.22 :: dsc_3.x/2066: Console, Changes, Git Data |
|
Build 3.22 :: dsc_3.x/2066: 3.22.0-CI |
What does this PR do?
Recreate non cached object in a user namespace
Screenshot/screencast of this PR
N/A
What issues does this PR fix or reference?
https://issues.redhat.com/browse/CRW-8900
How to test this PR?
OpenShift
on Minikube
PR Checklist
As the author of this Pull Request I made sure that:
What issues does this PR fix or referenceandHow to test this PRcompletedReviewers
Reviewers, please comment how you tested the PR when approving it.