OAPE-693:Add changes for checking E2E coverage with lgtm based approach for External-Secrets-Operator#81660
OAPE-693:Add changes for checking E2E coverage with lgtm based approach for External-Secrets-Operator#81660siddhibhor-56 wants to merge 2 commits into
Conversation
siddhibhor-56
changed the title
|
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:
WalkthroughThe CI configuration separates External Secrets Operator end-to-end testing from coverage execution and adds the repository to OpenShift LGTM configuration for the ChangesExternal Secrets Operator CI
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
core-services/pipeline-controller/lgtm-config.yaml (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStray blank line in repos list.
An empty line was inserted into the
openshiftorg'srepos:list right whereexternal-secrets-operatorwould alphabetically sort. This looks like an editing artifact rather than an intentional change — the actual entry was correctly added separately at Lines 35-37.🧹 Proposed cleanup
- egress-router-cni - - gcp-workload-identity-federation-webhook🤖 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 `@core-services/pipeline-controller/lgtm-config.yaml` at line 17, Remove the stray blank line from the openshift org repos list in lgtm-config.yaml so the list stays alphabetically contiguous; keep the external-secrets-operator entry in its correct sorted position and do not add any empty separator line in that section.
🤖 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.
Nitpick comments:
In `@core-services/pipeline-controller/lgtm-config.yaml`:
- Line 17: Remove the stray blank line from the openshift org repos list in
lgtm-config.yaml so the list stays alphabetically contiguous; keep the
external-secrets-operator entry in its correct sorted position and do not add
any empty separator line in that section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 65acbb1c-b1dc-459c-8c57-59f44bd48d7b
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/external-secrets-operator/openshift-external-secrets-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (2)
ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yamlcore-services/pipeline-controller/lgtm-config.yaml
01762cb to
3e3d004
Compare
|
/pj-rehearse max |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator-coverage |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator |
|
@bharath-b-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-external-secrets-operator-main-e2e-operator |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-ci
Bot
added
the
needs-rebase
|
@siddhibhor-56, |
|
@siddhibhor-56, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
openshift-ci
Bot
removed
the
needs-rebase
|
/pj-rehearse max |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@siddhibhor-56, |
Co-authored-by: Cursor <cursoragent@cursor.com>
f0de491 to
d12b1b4
Compare
d12b1b4 to
6685cec
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml`:
- Around line 81-84: Update all three oc wait invocations in the
external-secrets operator installation steps to include --timeout=10m, while
preserving their existing conditions, namespaces, and resources.
🪄 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), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 56ab862c-c1cc-492c-b83b-4abc77324198
📒 Files selected for processing (1)
ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml
| commands: | | ||
| oc create namespace external-secrets-operator | ||
| operator-sdk run bundle --timeout=10m --security-context-config=restricted --install-mode=AllNamespaces -n external-secrets-operator "$OO_BUNDLE" --verbose | ||
| oc wait --for condition=Available -n external-secrets-operator deployment external-secrets-operator-controller-manager |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml'
echo '--- relevant lines ---'
nl -ba "$file" | sed -n '70,140p'
echo
echo '--- search for oc wait / timeout ---'
rg -n 'oc wait|--timeout|operator-sdk run bundle' "$file"Repository: openshift/release
Length of output: 218
🌐 Web query:
kubectl wait default timeout oc wait default timeout docs
💡 Result:
For both kubectl wait and oc wait, the default timeout duration is 30 seconds [1][2]. The --timeout parameter specifies the length of time to wait before giving up on the condition being met [1][3]. The following behaviors apply to the timeout value: - Default: 30s [1][2] - Zero: The command checks the condition once and exits immediately without waiting [1][3]. - Negative: The command will wait for one week (effectively an indefinite wait) [3][4]. Since oc is a wrapper around kubectl (specifically for OpenShift clusters), it inherits this behavior and default configuration from the underlying kubectl implementation [2].
Citations:
- 1: https://kubernetes.io/docs/reference/kubectl/generated/kubectl_wait/
- 2: https://chrisphillips-cminion.github.io/openshift/2022/08/14/oc-waait.html
- 3: https://manpages.debian.org/testing/kubernetes-client/kubectl-wait.1.en.html
- 4: https://man.archlinux.org/man/kubectl-wait.1.en
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file='ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml'
echo '--- relevant lines with numbers ---'
sed -n '70,140p' "$file" | cat -n
echo
echo '--- matches ---'
rg -n 'oc wait|operator-sdk run bundle|--timeout' "$file"Repository: openshift/release
Length of output: 3808
Add --timeout=10m to all three oc wait calls. oc wait defaults to 30s, so the deployment can still be coming up when these install steps give up and make the jobs flaky.
🤖 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
`@ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml`
around lines 81 - 84, Update all three oc wait invocations in the
external-secrets operator installation steps to include --timeout=10m, while
preserving their existing conditions, namespaces, and resources.
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse max |
|
@siddhibhor-56: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@siddhibhor-56: all tests passed! 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bharath-b-rh, siddhibhor-56 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 |
siddhibhor-56
changed the title
openshift-ci-robot
added
the
jira/valid-reference
|
@siddhibhor-56: This pull request references OAPE-693 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
3 participants
Adds Separates E2E coverage instrumentation from the main e2e-operator presubmit into a standalone e2e-operator-coverage test with
always_run: false, gated onLGTMvia pipeline-controllerSummary by CodeRabbit
This PR updates the OpenShift CI configuration for
openshift/external-secrets-operatorto decouple E2E coverage instrumentation from the main E2E presubmit.ci-operator/config/openshift/external-secrets-operator/openshift-external-secrets-operator-main.yaml, the existinge2e-operatorpresubmit now only installs the operator bundle (restricted install mode) and runsmake test-e2ewithcpu: 100m, removing the coverage setup/collection flow from that job.e2e-operator-coveragepresubmit job is added withalways_run: false. It performs coverage collection viahack/e2e-coverage.sh(setupduring the test flow andcollectinpost), while running the samemake test-e2everification and using the samecpu: 100mconstraint.core-services/pipeline-controller/lgtm-config.yaml, the LGTM configuration for theopenshiftorg is extended to includeexternal-secrets-operatoron themainbranch, gating when the coverage job runs.Net effect: standard presubmits stay lighter, while coverage is still exercised through an LGTM-triggered, dedicated coverage presubmit for
main.