feat: prevent openshift-gitops namespace creation when DISABLE_DEFAULT_ARGOCD_INSTANCE=true#1206
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 |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe reconcile flow now deletes the default Argo CD namespace when default install is disabled, skips namespace-backed work when the backend namespace is missing or terminating, and keeps plugin reconciliation gated in the enabled path. Tests and docs were updated to match the namespace-removal behavior. ChangesDisable default install deletes namespace
Estimated code review effort: 4 (Complex) | ~40 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/Migration_Guide.md (1)
68-68: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUpdate the
DISABLE_DEFAULT_ARGOCD_INSTANCEdescription at line 68 for consistencyLine 9 was updated to mention namespace deletion, but line 68 still reads "Disables the installation of default instance in openshift-gitops namespace" without mentioning that the namespace itself is also deleted. This is inconsistent with the updated docs in the Usage Guide (line 225) and the non-olm README (line 84).
📝 Proposed fix
-`DISABLE_DEFAULT_ARGOCD_INSTANCE`: Disables the installation of default instance in openshift-gitops namespace. +`DISABLE_DEFAULT_ARGOCD_INSTANCE`: Disables the installation of the default instance and deletes the `openshift-gitops` namespace if it exists.🤖 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 `@docs/Migration_Guide.md` at line 68, Update the DISABLE_DEFAULT_ARGOCD_INSTANCE description in Migration_Guide.md to match the other docs by stating that it disables installation of the default instance in the openshift-gitops namespace and also deletes the namespace itself. Keep the wording consistent with the updated Usage Guide and non-olm README entries, and make the change in the documentation text that references this flag.
🤖 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/gitopsservice_controller.go`:
- Around line 329-343: Skip backend reconciliation when the namespace is already
marked for deletion, because ensureDefaultArgoCDInstanceDoesntExist may leave it
in Terminating and the later Client.Get can still succeed. In the reconcile path
that calls ensureDefaultArgoCDInstanceDoesntExist, inspect the Namespace object
returned by Client.Get and only call reconcileBackend when it exists and has no
DeletionTimestamp; otherwise return without reconciling. Use the existing
symbols ensureDefaultArgoCDInstanceDoesntExist, Client.Get, and reconcileBackend
to keep the fix localized.
- Around line 309-319: Guard the version parsing in gitopsservice_controller.go
before indexing Segments() in the OCP version comparison logic. In the code
around realVersion/startVersion and the
version.NewVersion(dynamicPluginStartOCPVersion) path, check that each slice has
at least 2 elements before reading index 1, and handle shorter versions safely
by returning early or skipping the comparison. This prevents an
index-out-of-range panic when dynamicPluginStartOCPVersion is a single-segment
version like "4".
---
Outside diff comments:
In `@docs/Migration_Guide.md`:
- Line 68: Update the DISABLE_DEFAULT_ARGOCD_INSTANCE description in
Migration_Guide.md to match the other docs by stating that it disables
installation of the default instance in the openshift-gitops namespace and also
deletes the namespace itself. Keep the wording consistent with the updated Usage
Guide and non-olm README entries, and make the change in the documentation text
that references this flag.
🪄 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: 6a8f4a18-d3cf-422c-8289-8219321517c2
📒 Files selected for processing (6)
controllers/gitopsservice_controller.gocontrollers/gitopsservice_controller_test.godocs/Migration_Guide.mddocs/OpenShift GitOps Usage Guide.mdhack/non-olm-install/README.mdtest/openshift/e2e/ginkgo/sequential/1-018_validate_disable_default_instance_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
Rizwana777
force-pushed
the
issue-10435-do-not-create-namespace
branch
2 times, most recently
from
8a43107 to
6b7a2f6
Compare
…T_ARGOCD_INSTANCE=true Signed-off-by: Rizwana777 <rizwananaaz177@gmail.com> Assisted-by: Claude
Rizwana777
force-pushed
the
issue-10435-do-not-create-namespace
branch
from
6b7a2f6 to
55084fa
Compare
|
/retest |
|
This will also disable console-plugin deployment, is that expected ? |
2 participants
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
This PR prevents the openshift-gitops namespace from being created when DISABLE_DEFAULT_ARGOCD_INSTANCE=true is set. Previously, the flag only prevented the ArgoCD instance creation but still created the namespace.
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
https://redhat.atlassian.net/browse/GITOPS-10435
Test acceptance criteria:
How to test changes / Special notes to the reviewer:
Fresh install with flag enabled
Set environment variable in operator Subscription
DISABLE_DEFAULT_ARGOCD_INSTANCE=true
verify
oc get namespace openshift-gitops
Expected: Error from server (NotFound)
test with make
make install
export DISABLE_DEFAULT_ARGOCD_INSTANCE=true
make run
verify
oc get namespace openshift-gitops
Expected: Error from server (NotFound)