feat: decouple console plugin from default ArgoCD instance - #1231
feat: decouple console plugin from default ArgoCD instance #1231Rizwana777 wants to merge 2 commits into
Conversation
|
[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 |
|
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:
📝 WalkthroughWalkthroughThe operator derives a plugin namespace from the service-account namespace, configures reconciliation with it, creates plugin resources there, removes legacy resources from the previous namespace, and adds controller and E2E coverage for the resulting placement. ChangesConsole plugin namespace migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant ReconcileGitopsService
participant KubernetesAPI
participant reconcilePlugin
participant cleanupOldPluginResources
Operator->>ReconcileGitopsService: provide PluginNamespace
ReconcileGitopsService->>KubernetesAPI: create namespace when needed
ReconcileGitopsService->>reconcilePlugin: reconcile resources in PluginNamespace
ReconcileGitopsService->>cleanupOldPluginResources: remove legacy resources
cleanupOldPluginResources->>KubernetesAPI: delete old plugin resources
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsRepository analysis: Couldn't refresh 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@controllers/consoleplugin.go`:
- Line 455: Update the migration flow around consolePlugin so the fetched
existingPlugin receives the desired fields from newConsolePlugin before the
update is submitted. Use existingPlugin for the Kubernetes update, preserving
its resourceVersion, rather than updating the newly constructed object; ensure
the CR is updated before deleting the old Service.
In `@controllers/gitopsservice_controller.go`:
- Line 321: Move the cleanupOldPluginResources call out of the pre-version-gate
path and invoke it only after replacement resource reconciliation completes
successfully. Ensure unsupported clusters and target-resource creation failures
leave legacy resources intact, and propagate any cleanupOldPluginResources error
so reconciliation retries.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 7f53c0bc-d230-4f9b-b873-8d1cd1a58751
📒 Files selected for processing (8)
cmd/main.gocontrollers/consoleplugin.gocontrollers/consoleplugin_test.gocontrollers/gitopsservice_controller.gocontrollers/gitopsservice_controller_test.gotest/e2e/suite_test.gotest/nondefaulte2e/suite_test.gotest/openshift/e2e/ginkgo/sequential/1-124_validate_console_plugin_in_operator_namespace_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
e8d9721 to
91e8e6d
Compare
|
/test v4.14-ci-index-gitops-operator-bundle |
07b5c1e to
7cc0707
Compare
|
/test v4.14-kuttl-sequential |
|
/test v4.14-e2e |
|
/test v4.19-kuttl-sequential |
anandrkskd
left a comment
anandrkskd
left a comment
There was a problem hiding this comment.
some nit comments, changes LGTM
…-by: Claude Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
7cc0707 to
42765d3
Compare
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)
controllers/consoleplugin.go (1)
138-158: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPut the runtime-namespace label on the pod template.
kubeAppLabelRuntimeNamespaceis currently added toDeployment.ObjectMeta.Labelsat Line 158, whileSpec.Template.ObjectMeta.Labelsstill contains onlykubeAppLabelApp. Pods therefore do not receive the namespace label, contrary to the deployment reconciliation contract covered bycontrollers/consoleplugin_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 `@controllers/consoleplugin.go` around lines 138 - 158, Add kubeAppLabelRuntimeNamespace with the namespace value to the pod template labels in pluginDeployment, alongside kubeAppLabelApp. Keep the existing Deployment.ObjectMeta label unchanged so both the Deployment and its Pods receive the runtime-namespace label required by reconciliation.
🤖 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 `@controllers/consoleplugin.go`:
- Line 415: Update both reconciliation paths in controllers/consoleplugin.go: at
lines 415-415, submit the fetched existingServiceRef after applying mutations
instead of the fresh pluginServiceRef; at lines 492-492, submit the fetched
existingPluginConfigMap instead of a newly constructed desired object. Preserve
the existing namespace-aware update behavior and resource metadata from the
fetched resources.
---
Outside diff comments:
In `@controllers/consoleplugin.go`:
- Around line 138-158: Add kubeAppLabelRuntimeNamespace with the namespace value
to the pod template labels in pluginDeployment, alongside kubeAppLabelApp. Keep
the existing Deployment.ObjectMeta label unchanged so both the Deployment and
its Pods receive the runtime-namespace label required by reconciliation.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: cf4f56e1-ca8a-4364-8325-feced152e39a
📒 Files selected for processing (12)
cmd/main.gocontrollers/consoleplugin.gocontrollers/consoleplugin_test.gocontrollers/gitopsservice_controller.gocontrollers/gitopsservice_controller_test.gotest/e2e/suite_test.gotest/nondefaulte2e/suite_test.gotest/openshift/e2e/ginkgo/sequential/1-085_validate_dynamic_plugin_installation_test.gotest/openshift/e2e/ginkgo/sequential/1-115_validate_imagepullpolicy_console_plugin_test.gotest/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.gotest/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.gotest/openshift/e2e/ginkgo/sequential/1-124_validate_console_plugin_in_operator_namespace_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
🚧 Files skipped from review as they are similar to previous changes (11)
- test/openshift/e2e/ginkgo/sequential/1-124_validate_console_plugin_in_operator_namespace_test.go
- test/openshift/e2e/ginkgo/sequential/1-115_validate_imagepullpolicy_console_plugin_test.go
- controllers/consoleplugin_test.go
- controllers/gitopsservice_controller_test.go
- test/e2e/suite_test.go
- cmd/main.go
- test/openshift/e2e/ginkgo/sequential/1-085_validate_dynamic_plugin_installation_test.go
- test/openshift/e2e/ginkgo/sequential/1-121-valiate_resource_constraints_gitopsservice_test.go
- test/openshift/e2e/ginkgo/sequential/1-123_validate_list_order_comparison_test.go
- controllers/gitopsservice_controller.go
- test/nondefaulte2e/suite_test.go
a5e2b9d to
0359154
Compare
Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com>
0359154 to
0337564
Compare
|
@Rizwana777: The following test failed, say
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. |
3 participants
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
Moves console plugin resources (Deployment, Service, ConfigMap) from 'openshift-gitops' namespace to the operator's own namespace (openshift-gitops-operator) so the plugin operates independently of the default ArgoCD instance. This is a prerequisite for allowing the default ArgoCD instance to be disabled without breaking the console plugin.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
make install
make run
kubectl get deployment gitops-plugin -n openshift-gitops-operator
NAME READY UP-TO-DATE AVAILABLE AGE
gitops-plugin 1/1 1 1 19m
kubectl get service gitops-plugin -n openshift-gitops-operator
NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE
gitops-plugin ClusterIP 172.xyz.83 9001/TCP 19m
kubectl get configmap httpd-cfg -n openshift-gitops-operator
NAME DATA AGE
httpd-cfg 1 20m
kubectl get consoleplugin gitops-plugin -o jsonpath='{.spec.backend.service.namespace}'
openshift-gitops-operator