clearer instruction to disable default argocd instance - #412
Conversation
|
Hi @makeittotop. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
|
ok-to-test |
|
/retest |
|
@makeittotop I know it is an old PR but can you rebase this ? |
|
Sure, doing it
…On Wed, Sep 4, 2024 at 10:49 AM Saumeya Katyal ***@***.***> wrote:
@makeittotop <https://github.com/makeittotop> I know it is an old PR but
can you rebase this ?
—
Reply to this email directly, view it on GitHub
<#412 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFYC7JEN7R53GHOBSCPNWDZU5BY5AVCNFSM6AAAAABNU2NLOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRZGY2TEMJSGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@makeittotop: The following tests 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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olivergondza The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Clearly mentioning the `env` resource within the `subscriptions.operators.coreos.com.spec.config` CRD to add the associated env. var. leading to disabling the default argocd instance launch
|
New changes are detected. LGTM label has been removed. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ROSA/OSD installation guidance now explains how to disable the default Argo CD instance through the GitOps Operator ChangesOpenShift GitOps documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Suggested reviewers: 🚥 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 |
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 `@docs/OpenShift` GitOps Usage Guide.md:
- Line 124: Update the fenced code block in the OpenShift GitOps Usage Guide to
use a yaml language identifier, changing the opening fence to ```yaml while
preserving the block contents and closing fence.
- Around line 125-130: Correct the Subscription YAML example by adding the
missing metadata mapping before name and namespace, keeping those fields nested
under metadata and preserving kind as its own top-level field.
🪄 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: c6dd3706-80ab-4f0c-9145-2b05a76abd0f
📒 Files selected for processing (1)
docs/OpenShift GitOps Usage Guide.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
| To disable the default ‘ready-to-use’ installation of Argo CD: as an admin, update the existing Subscription Object for Gitops Operator and add `DISABLE_DEFAULT_ARGOCD_INSTANCE = true` to the spec. | ||
| To disable the default ‘ready-to-use’ installation of Argo CD: as an admin, update the existing Subscription Object for Gitops Operator and add `DISABLE_DEFAULT_ARGOCD_INSTANCE = true` to the `subscriptions.operators.coreos.com.spec.config.env` resource as follows - | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Declare the fenced block as YAML.
Use ```yaml instead of an untyped fence to satisfy markdownlint MD040 and provide correct syntax highlighting.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 124-124: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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/OpenShift` GitOps Usage Guide.md at line 124, Update the fenced code
block in the OpenShift GitOps Usage Guide to use a yaml language identifier,
changing the opening fence to ```yaml while preserving the block contents and
closing fence.
Source: Linters/SAST tools
| apiVersion: operators.coreos.com/v1alpha1 | ||
| kind: Subscription | ||
| name: openshift-gitops-operator | ||
| namespace: openshift-operators | ||
| spec: | ||
| config: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add the missing metadata: key to the Subscription example.
name and namespace are currently indented beneath the scalar kind field, so this block is invalid YAML and cannot be applied.
Proposed fix
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
+metadata:
name: openshift-gitops-operator
namespace: openshift-operators📝 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.
| apiVersion: operators.coreos.com/v1alpha1 | |
| kind: Subscription | |
| name: openshift-gitops-operator | |
| namespace: openshift-operators | |
| spec: | |
| config: | |
| apiVersion: operators.coreos.com/v1alpha1 | |
| kind: Subscription | |
| metadata: | |
| name: openshift-gitops-operator | |
| namespace: openshift-operators | |
| spec: | |
| config: |
🤖 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/OpenShift` GitOps Usage Guide.md around lines 125 - 130, Correct the
Subscription YAML example by adding the missing metadata mapping before name and
namespace, keeping those fields nested under metadata and preserving kind as its
own top-level field.
What type of PR is this?
/kind documentation
What does this PR do / why we need it:
Specifically mentioning to edit the
envresource within thesubscriptions.operators.coreos.com.spec.configCRD to add the associated env. var. in order to disable the default argocd instance launch.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: