Skip to content

OSAC-4489: address storage e2e test review findings - #868

Merged
osac-ci-bot merged 1 commit into
mainfrom
autofix/osac-4489
Sep 25, 2026
Merged

osac-ci-bot merged 1 commit into
mainfrom
autofix/osac-4489

Conversation

@jira-autofix

@jira-autofix jira-autofix Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Address automated review findings on storage e2e test files from PR #492:

  • Extract duplicated K8s manifest templates into shared conftest constants
  • Wrap teardown steps individually to prevent cascading cleanup failures
  • Let AssertionError propagate from verification steps so they fail the test
  • Move namespace cleanup out of _verify_teardown into the finally block in test_tenant_storage_lifecycle to prevent namespace leaks
  • Normalize poll_until checked=False in poll lambdas for consistent error handling
  • Always verify ClusterOrder removal even on fast deletion path
  • Assert tenant-scoped secrets are cleaned up during teardown
  • Add docstrings to all functions and fixtures (coverage improvement from ~7.69%)

@openshift-ci-robot

openshift-ci-robot commented Sep 10, 2026 •

Copy link
Copy Markdown

@jira-autofix[bot]: This pull request references OSAC-4489 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 bug to target the "5.1.0" version, but no target version was set.

Details

In response to this:

Address automated review findings on storage e2e test files from PR #492:

  • Extract duplicated K8s manifest templates into shared conftest constants
  • Wrap teardown steps individually to prevent cascading cleanup failures
  • Let AssertionError propagate from verification steps so they fail the test
  • Move namespace cleanup out of _verify_teardown into the finally block in test_tenant_storage_lifecycle to prevent namespace leaks
  • Normalize poll_until checked=False in poll lambdas for consistent error handling
  • Always verify ClusterOrder removal even on fast deletion path
  • Assert tenant-scoped secrets are cleaned up during teardown
  • Add docstrings to all functions and fixtures (coverage improvement from ~7.69%)

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.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: a64cd96c-ac1a-4325-bed2-c5bcb287805e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

🧭 E2E Suite Selection (POC, informational only)

Suite Decision Source Reason
VMAAS skip deterministic No changed files matched a path rule for this suite
CAAS skip deterministic No changed files matched a path rule for this suite
BMAAS skip deterministic No changed files matched a path rule for this suite

No AI validation needed -- nothing in this PR was recognized as relevant to any E2E suite. This comment is informational only; nothing is gated on it yet.

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/e2e-ready

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/ok-to-test

@github-actions

Copy link
Copy Markdown

Labeled ok-to-test. Re-ran 2 failed run(s).

@github-actions

Copy link
Copy Markdown

Labeled e2e-ready on f6a2ceb. Starting expensive e2e (cleanup removes the label on next push).

@osac-ai

osac-ai Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

✅ E2E CaaS Full Install -- Passing

Previously failing; now passing as of this run.

✅ E2E BMaaS Full Install -- Passing

Previously failing; now passing as of this run.

✅ E2E VMaaS Full Install -- Passing

Previously failing; now passing as of this run.

Total AI diagnostic cost for this PR: $0.4661 (142765 input + 15044 output tokens across 4 diagnoses)

@github-actions

Copy link
Copy Markdown

E2E on e2e-ready

Label e2e-ready applied — starting expensive e2e (PR run replay).

  • Started: 2/3
  • Already active/green (skipped rerun): 1
  • Did not POST e2e-*-gate Checks API checks (native jobs report; required gates stay pending until then).

@github-actions

Copy link
Copy Markdown

🧭 Jobs Selection (informational only)

E2E Suites

Suite Decision Source Reason
VMAAS skip deterministic No changed files matched a path rule for this suite
CAAS skip deterministic No changed files matched a path rule for this suite
BMAAS skip deterministic No changed files matched a path rule for this suite

No AI validation needed -- nothing in this PR was recognized as relevant to any E2E suite.
Estimated cost: $0.0000 (0 input + 0 output tokens, gemini-3.1-pro-preview)

Unit Tests

Job Decision Reason
fulfillment-service run This workflow has no per-component scoping -- runs for any non-doc change
osac-metering run This workflow has no per-component scoping -- runs for any non-doc change
osac-metering/adapters run This workflow has no per-component scoping -- runs for any non-doc change
osac-metering/schema run This workflow has no per-component scoping -- runs for any non-doc change

Integration Tests

Job Decision Reason
fulfillment-service run This workflow has no per-component scoping -- runs for any non-doc change
osac-operator run This workflow has no per-component scoping -- runs for any non-doc change
bare-metal-fulfillment-operator run This workflow has no per-component scoping -- runs for any non-doc change
osac-aap run This workflow has no per-component scoping -- runs for any non-doc change
osac-installer run This workflow has no per-component scoping -- runs for any non-doc change

Helm Lint

Job Decision Reason
osac-operator skip No changed files matched this job's path filter
bare-metal-fulfillment-operator skip No changed files matched this job's path filter
fulfillment-service skip No changed files matched this job's path filter
osac-aap skip No changed files matched this job's path filter
osac-csi-driver skip No changed files matched this job's path filter
osac-metering skip No changed files matched this job's path filter
osac-installer skip No dependent component chart changed

Checks & Builds

Job Decision Reason
Check generated code (proto) skip No changed files matched this job's path filter
fulfillment-service checks skip No changed files matched this job's path filter
Build container image (osac-operator) skip No changed files matched this job's path filter
Build container image (bare-metal-fulfillment-operator) skip No changed files matched this job's path filter
ansible-lint (osac-aap) skip No changed files matched this job's path filter
Darwin keychain tests skip No changed files matched this job's path filter

Every table above is informational only -- nothing here gates whether a job actually runs. The E2E Suites table can use AI judgment for ambiguous files; every other table is deterministic-only (no AI).

Harden teardown exception handling in both storage E2E tests so that
AssertionError from verification steps propagates and fails the test
instead of being silently caught by the blanket except-Exception handler.
Assertion errors are collected during teardown and re-raised after all
cleanup steps complete, ensuring resource cleanup is never skipped.

Move namespace cleanup out of _verify_teardown in
test_tenant_storage_lifecycle into the finally block as a separate
wrapped step, matching the pattern already used by
test_caas_cluster_storage, so that namespace cleanup runs even when
verification raises.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: aipcc-bot <aipcc-bot@redhat.com>
@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/e2e-ready

@omer-vishlitzky

Copy link
Copy Markdown
Contributor

/ok-to-test

@github-actions

Copy link
Copy Markdown

Removed ok-to-test label due to new commits. An org member must re-approve with /ok-to-test.

@github-actions

Copy link
Copy Markdown

Labeled ok-to-test. Re-ran 1 failed run(s).

@github-actions

Copy link
Copy Markdown

Labeled e2e-ready on dc07bf1. Starting expensive e2e (cleanup removes the label on next push).

@github-actions

Copy link
Copy Markdown

E2E on e2e-ready

Label e2e-ready applied — not starting a new full-install run.

  • Started: 0/3
  • Already active/green (skipped rerun): 3
  • Skipped gate invalidation (full-install already active or in-flight).

@zszabo-rh

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci

openshift-ci Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jira-autofix[bot], zszabo-rh

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions

Copy link
Copy Markdown

E2E on lgtm

Label lgtm applied — starting expensive e2e (PR run replay).

  • Started: 1/3
  • Already active/green (skipped rerun): 2
  • Did not POST e2e-*-gate Checks API checks (native jobs report; required gates stay pending until then).

@osac-ci-bot
osac-ci-bot added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 24, 2026
@osac-ci-bot
osac-ci-bot added this pull request to the merge queue Sep 24, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 24, 2026
@osac-ci-bot
osac-ci-bot added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit eba2efe Sep 25, 2026
147 of 152 checks passed

This branch was successfully deployed

1 active deployment
e2e-test — dc07bf12 Deployed Sep 24, 2026 by omer-vishlitzky via e2e-vmaas-full-install / e2e #6616
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants