OAPE-694: Add E2E coverage reporting for secrets-store-csi-driver-operator#79231
OAPE-694: Add E2E coverage reporting for secrets-store-csi-driver-operator#79231PillaiManish wants to merge 3 commits into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@PillaiManish: This pull request explicitly references no jira issue. 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. |
|
/pj-rehearse max |
|
@PillaiManish: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
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:
WalkthroughAdds a coverage-enabled operator image and CI steps to the ChangesCoverage Instrumentation and Collection for FIPS E2E Tests
sequenceDiagram
participant CI as CI Job
participant API as OpenShift API
participant Pod as Operator Pod
participant Codecov as Codecov
CI->>API: setup-coverage: patch CSV -> use COVERAGE_IMAGE, add GOCOVERDIR
API->>Pod: rollout new operator deployment
Pod-->>API: readiness
CI->>API: run operator-e2e-fips tests
API->>Pod: tests exercise operator (writes coverage to /tmp/e2e-cover)
CI->>Pod: collect-coverage: kubectl exec SIGTERM (flush coverage)
Pod-->>API: restart/ready
CI->>Pod: kubectl cp /tmp/e2e-cover -> workspace
CI->>CI: go tool covdata convert -> text/percent
CI->>Codecov: verify uploader SHA256
alt CODECOV_TOKEN present
CI->>Codecov: upload coverage profile
else
CI->>CI: preserve local coverage files
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 12✅ Passed checks (12 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
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
`@ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml`:
- Around line 225-228: The JSON patch that appends GOCOVERDIR to the container
env can fail if the container has no env array; modify the patching logic around
the oc patch csv "${csv}" -n "${NAMESPACE}" call so you first ensure the target
container has an env array before using the add with path ".../env/-". Implement
this by either (a) issuing a preparatory patch (e.g., an oc patch --type=merge
or a JSON patch that creates
/spec/install/spec/deployments/0/spec/template/spec/containers/0/env as an empty
array if missing) and then run the existing add for the GOCOVERDIR entry, or (b)
stop using a single JSON-patch append and instead perform a
merge/strategic-merge patch that injects the env entry (including COVERAGE_IMAGE
and GOCOVERDIR_PATH) in one safe operation so it works whether env exists or
not.
- Around line 142-151: The current flow sends SIGTERM to PID 1 then waits for
the pod to restart and runs oc cp from ${GOCOVERDIR_PATH}, but that path is on
the container ephemeral filesystem so the coverage files are lost when the
container restarts; update the job to either (A) ensure ${GOCOVERDIR_PATH} is
backed by a persistent volume (e.g., mount an emptyDir or hostPath to the same
path in the pod spec) so files survive the restart, or (B) copy the coverage
files out of the running container before sending kill -TERM 1 (use oc cp from
${NAMESPACE}/${pod}:${GOCOVERDIR_PATH} to ${coverage_dir} prior to the oc exec
'kill -TERM 1'), referencing the variables GOCOVERDIR_PATH, coverage_dir, oc
exec, and oc cp in your changes.
🪄 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: dc595cc8-a074-4cb4-966d-d7c7cea09e3e
📒 Files selected for processing (1)
ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml
PillaiManish
force-pushed
the
sscsi-e2e-coverage-integration
branch
from
4a11003 to
81f4d0c
Compare
|
/pj-rehearse max |
|
@PillaiManish: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-openshift-secrets-store-csi-driver-operator-main-operator-e2e-gcp |
|
@PillaiManish: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/cc: @rausingh-rh, @siddhibhor-56 please refer job: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_release/79231/rehearse-79231-pull-ci-openshift-secrets-store-csi-driver-operator-main-operator-e2e-fips/2054546102635466752 Do let me know if we can proceed with this approach. Thanks 😄 |
…ator Add coverage-instrumented image build and CI steps to collect E2E test coverage data and upload to Codecov. Co-authored-by: Cursor <cursoragent@cursor.com>
PillaiManish
force-pushed
the
sscsi-e2e-coverage-integration
branch
from
81f4d0c to
e1260f0
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml (1)
40-128:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegenerate CI-generated artifacts for this config change.
Since this modifies
ci-operator/config/..., please runmake updateand include any generated downstream config changes in this PR to keep CI job definitions in sync.As per coding guidelines, "After modifying CI configuration files in
ci-operator/config/, regenerate downstream artifacts by runningmake update."🤖 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/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml` around lines 40 - 128, The CI config change for the secrets-store-csi-driver-operator (changes to the operator bundles, substitutions, promotion/releases, and tests such as operator-e2e-fips) requires regenerating downstream CI artifacts; run make update in the repo, capture and stage all generated changes (downstream configs, job definitions, and any updated templates), and add those generated files to this PR so the CI job definitions stay in sync with the modified CI config.
🤖 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.
Outside diff comments:
In
`@ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml`:
- Around line 40-128: The CI config change for the
secrets-store-csi-driver-operator (changes to the operator bundles,
substitutions, promotion/releases, and tests such as operator-e2e-fips) requires
regenerating downstream CI artifacts; run make update in the repo, capture and
stage all generated changes (downstream configs, job definitions, and any
updated templates), and add those generated files to this PR so the CI job
definitions stay in sync with the modified CI config.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2499e51d-5a7f-4d44-a1bc-6c94339c34b4
📒 Files selected for processing (1)
ci-operator/config/openshift/secrets-store-csi-driver-operator/openshift-secrets-store-csi-driver-operator-main.yaml
PillaiManish
changed the title
|
@PillaiManish: This pull request references OAPE-694 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 sub-task 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. |
Move coverage instrumentation out of operator-e2e-fips into a new operator-e2e-coverage test that runs only after lgtm label via the pipeline controller. Enroll secrets-store-csi-driver-operator (main) in the LGTM pipeline config. Co-authored-by: Cursor <cursoragent@cursor.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: PillaiManish 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 |
|
/pj-rehearse max |
|
@rausingh-rh: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Add a postsubmit job that runs e2e coverage on main after merge, so Codecov has a baseline to compute PR coverage deltas against. Co-authored-by: Cursor <cursoragent@cursor.com>
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@PillaiManish: 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. |
3 participants
Summary
operator-e2e-coverageCI test for secrets-store-csi-driver-operator that provisions an Azure cluster, deploys a coverage-instrumented operator image, runs e2e tests, and collects/uploads Go coverage data to Codecovalways_run: false, not optional) — it only triggers in the second stage after thelgtmlabel is added to a PR, via the pipeline controlleropenshift/secrets-store-csi-driver-operator(main branch) in the LGTM pipeline controller configoperator-e2e-fipstest unchanged (no coverage logic)How It Works
unit,verify,operator-e2e-fips,operator-e2e-aws,operator-e2e-azure,operator-e2e-gcp, etc. run automaticallylgtm): Pipeline controller triggersoperator-e2e-coverageopenshift-e2e-azure-csi-secrets-storeworkflowsetup-coveragestep patches the CSV to swap in the coverage-instrumented image and injectGOCOVERDIRtest-e2estep runs the full e2e suite against the instrumented operatorcollect-coveragepost step (best_effort) sends SIGTERM to flush coverage, copies data viaoc cp, converts to Go profile, and uploads to CodecovCoverage collection failures never block PR merge (
best_effort: trueon the collect step). The test itself is merge-blocking but only runs after reviewer approval.Can also be triggered manually anytime with
/test operator-e2e-coverage.Changes
ci-operator/config/openshift/secrets-store-csi-driver-operator/...-main.yamloperator-e2e-coveragetest, keep coverage image build, revertoperator-e2e-fipsto originalcore-services/pipeline-controller/lgtm-config.yamlsecrets-store-csi-driver-operator(main branch)ci-operator/jobs/...(generated)make updateReferences