Skip to content

fix(e2e): TEST_VIRT excludes kdm specs, enable split-job e2e (issue #2413 option B) - #2423

Open
kaovilai wants to merge 2 commits into
openshift:oadp-devfrom
kaovilai:worktree-test-virt-kdm-filter
Open

fix(e2e): TEST_VIRT excludes kdm specs, enable split-job e2e (issue #2413 option B)#2423
kaovilai wants to merge 2 commits into
openshift:oadp-devfrom
kaovilai:worktree-test-virt-kdm-filter

Conversation

@kaovilai

@kaovilai kaovilai commented Aug 31, 2026

Copy link
Copy Markdown
Member

Summary

  • TEST_VIRT=true now excludes kdm-labeled specs from TEST_FILTER
  • TEST_VIRT_KDM=true still isolates kdm specs regardless of TEST_VIRT/TEST_VIRT_GA, and takes precedence

This enables option B from #2413: splitting e2e-test-kubevirt-aws into a non-kdm CSI job (TEST_VIRT=true) and a separate kdm-only job (TEST_VIRT_KDM=true), so each comfortably clears the 2h Prow step timeout instead of one job running both suites serially.

Fixes #2413

Test plan

  • TEST_VIRT_KDM=false TEST_VIRT=true → filter resolves to ... && (virt) && (! kdm) && ...
  • TEST_VIRT_KDM=true TEST_VIRT=false → filter resolves to ... && (kdm) && ...
  • openshift/release job config update to add the split kdm job (follow-up)

Note

Responses generated with Claude

Summary by CodeRabbit

  • Tests
    • Improved virtualized test selection with clearer handling for KDM-specific coverage.
    • When the KDM option is enabled, only KDM tests run.
    • When disabled, KDM tests are excluded from general virtualized testing.
    • Leaving the option unset preserves the existing virtualized test selection behavior.

TEST_VIRT=true now excludes kdm-labeled specs so the non-kdm CSI virt
job and a new kdm-only job (TEST_VIRT_KDM=true) can run as separate
parallel CI jobs, per option B of
openshift#2413.

TEST_VIRT_KDM=true takes precedence and still isolates kdm specs
regardless of TEST_VIRT/TEST_VIRT_GA.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4462a72b-1d8a-48e0-8944-166042660ae5

📥 Commits

Reviewing files that changed from the base of the PR and between 6d342bf and f20738d.

📒 Files selected for processing (1)
  • Makefile

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The Makefile adds tri-state handling for TEST_VIRT_KDM. An unset value preserves kdm inclusion under TEST_VIRT=true; false excludes kdm; true selects only kdm.

Changes

Virtualized E2E selection

Layer / File(s) Summary
Test filter precedence
Makefile
The Makefile records the origin of TEST_VIRT_KDM and applies the updated filter rules for TEST_VIRT_KDM, TEST_VIRT, and TEST_VIRT_GA.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to f2073

This change separates kdm and non-kdm end-to-end test selection through Makefile filters; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: joeavaikath, shubham-pampattiwar, weshayutin

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and directly relates to the TEST_VIRT filtering change and issue #2413 option B. It also identifies the intended split-job support, although the release configuration remains a fo…
Description check ✅ Passed The description provides the change summary, rationale, linked issue, test cases, and follow-up scope. It does not use the exact template headings, but it contains the required information and is most…
Linked Issues check ✅ Passed The Makefile changes implement the filtering behavior required to support option B from issue #2413. They allow explicit non-kdm selection with TEST_VIRT_KDM=false and kdm-only selection with TEST_VIR…
Out of Scope Changes check ✅ Passed The pull request changes only Makefile test-filter logic. The changes are directly related to the TEST_VIRT and TEST_VIRT_KDM behavior required for issue #2413. No unrelated code changes are present.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Stable And Deterministic Test Names ✅ Passed PASS. The pull request changes only Makefile. The diff adds TEST_VIRT_KDM origin handling and changes TEST_FILTER logic. It does not add or modify any Ginkgo It, Describe, Context, or `Whe…
Test Structure And Quality ✅ Passed PASS: The PR changes only Makefile test-filter logic. The diff from origin/oadp-dev contains no Ginkgo test files and no It, BeforeEach, AfterEach, Eventually, Consistently, or Expect changes. Therefo…
Microshift Test Compatibility ✅ Passed PASS — The pull request changes only Makefile. The diff adds no Ginkgo It, Describe, Context, or When tests and modifies no test files. Therefore, the MicroShift compatibility check has no n…
Single Node Openshift (Sno) Test Compatibility ✅ Passed PASS: The complete pull-request diff changes only Makefile; it adds no Ginkgo It, Describe, Context, or When tests and changes no test body. Therefore, the SNO multi-node compatibility check…
Topology-Aware Scheduling Compatibility ✅ Passed PASS: The pull request changes only Makefile. The diff adds tri-state TEST_VIRT_KDM handling and updates Ginkgo label filters. It adds or modifies no deployment manifests, operator code, controlle…
Ote Binary Stdout Contract ✅ Passed PASS: The PR changes only Makefile. Its added TEST_VIRT_KDM_ORIGIN logic changes the Ginkgo label filter and does not add code in main(), init(), TestMain(), suite setup, or top-level initia…
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PASS — The pull request changes only Makefile test-filter logic. The exact diff adds no Ginkgo declarations or e2e test files, and it introduces no IPv4 address handling or external connectivity. Th…
No-Weak-Crypto ✅ Passed PASS. The complete PR range changes only Makefile test-filter logic. Added lines define TEST_VIRT_KDM_ORIGIN and select virt/kdm labels. No changed line introduces MD5, SHA1, DES, RC4, 3DES, Blowfish,…
Container-Privileges ✅ Passed PASS: The PR changes only Makefile filter logic. The added lines introduce no container or Kubernetes manifest settings for privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, `allowPriv…
No-Sensitive-Data-In-Logs ✅ Passed PASS: The PR changes only Makefile test-selection logic and adds TEST_VIRT_KDM_ORIGIN. The exact diff adds no logging or output of passwords, tokens, API keys, PII, session IDs, hostnames, or custom…
Full details: Title check

Explanation

The title is concise and directly relates to the TEST_VIRT filtering change and issue #2413 option B. It also identifies the intended split-job support, although the release configuration remains a follow-up.

Full details: Description check

Explanation

The description provides the change summary, rationale, linked issue, test cases, and follow-up scope. It does not use the exact template headings, but it contains the required information and is mostly complete.

Full details: Linked Issues check

Explanation

The Makefile changes implement the filtering behavior required to support option B from issue #2413. They allow explicit non-kdm selection with TEST_VIRT_KDM=false and kdm-only selection with TEST_VIRT_KDM=true, while preserving existing behavior when the variable is unset. The release job configuration is identified as a follow-up and is not required for this filtering change. [#2413]

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

Full details: Stable And Deterministic Test Names

Explanation

PASS. The pull request changes only Makefile. The diff adds TEST_VIRT_KDM origin handling and changes TEST_FILTER logic. It does not add or modify any Ginkgo It, Describe, Context, or When test title, and it introduces no dynamic test-name value.

Full details: Test Structure And Quality

Explanation

PASS: The PR changes only Makefile test-filter logic. The diff from origin/oadp-dev contains no Ginkgo test files and no It, BeforeEach, AfterEach, Eventually, Consistently, or Expect changes. Therefore, this custom check introduces no test-structure, cleanup, timeout, assertion-message, or test-pattern failure.

Full details: Microshift Test Compatibility

Explanation

PASS — The pull request changes only Makefile. The diff adds no Ginkgo It, Describe, Context, or When tests and modifies no test files. Therefore, the MicroShift compatibility check has no new test to assess.

Full details: Single Node Openshift (Sno) Test Compatibility

Explanation

PASS: The complete pull-request diff changes only Makefile; it adds no Ginkgo It, Describe, Context, or When tests and changes no test body. Therefore, the SNO multi-node compatibility check is not applicable.

Full details: Topology-Aware Scheduling Compatibility

Explanation

PASS: The pull request changes only Makefile. The diff adds tri-state TEST_VIRT_KDM handling and updates Ginkgo label filters. It adds or modifies no deployment manifests, operator code, controllers, or scheduling constraints. Therefore the topology-aware scheduling check is not applicable.

Full details: Ote Binary Stdout Contract

Explanation

PASS: The PR changes only Makefile. Its added TEST_VIRT_KDM_ORIGIN logic changes the Ginkgo label filter and does not add code in main(), init(), TestMain(), suite setup, or top-level initializers. The diff contains no new fmt.Print*, logging, klog, or stdout writes. The Makefile $(shell echo ...) is build-time filter construction, not stdout from the OTE binary process. Existing suite logging is unchanged and is not PR-caused.

Full details: Ipv6 And Disconnected Network Test Compatibility

Explanation

PASS — The pull request changes only Makefile test-filter logic. The exact diff adds no Ginkgo declarations or e2e test files, and it introduces no IPv4 address handling or external connectivity. The custom check is therefore not applicable.

Full details: No-Weak-Crypto

Explanation

PASS. The complete PR range changes only Makefile test-filter logic. Added lines define TEST_VIRT_KDM_ORIGIN and select virt/kdm labels. No changed line introduces MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB mode, custom cryptography, or non-constant-time secret/token comparison.

Full details: Container-Privileges

Explanation

PASS: The PR changes only Makefile filter logic. The added lines introduce no container or Kubernetes manifest settings for privileged, hostPID, hostNetwork, hostIPC, SYS_ADMIN, allowPrivilegeEscalation, or root execution.

Full details: No-Sensitive-Data-In-Logs

Explanation

PASS: The PR changes only Makefile test-selection logic and adds TEST_VIRT_KDM_ORIGIN. The exact diff adds no logging or output of passwords, tokens, API keys, PII, session IDs, hostnames, or customer data. TEST_FILTER is passed only as a Ginkgo label filter; existing credential paths and test logging are unchanged.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci

openshift-ci Bot commented Aug 31, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kaovilai

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

@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2026
@kaovilai

Copy link
Copy Markdown
Member Author

/hold aligning pr to existing release to not break current configs.

@openshift-ci openshift-ci Bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2026
Use \$(origin) to tell "unset" from "explicitly false" so existing
openshift/release jobs that don't set TEST_VIRT_KDM at all keep today's
kdm coverage under TEST_VIRT=true (no regression), while a future split
non-kdm job can opt out explicitly with TEST_VIRT_KDM=false.

Signed-off-by: Tiger Kaovilai <tkaovila@redhat.com>
@kaovilai

Copy link
Copy Markdown
Member Author

/hold cancel

Reworked to distinguish unset vs explicit TEST_VIRT_KDM=false via $(origin). Existing openshift/release jobs that don't set TEST_VIRT_KDM at all keep today's kdm coverage under TEST_VIRT=true unchanged — this no longer breaks any release job's coverage until that job's config is updated to explicitly adopt TEST_VIRT_KDM=false for the split.

Note

Responses generated with Claude

@openshift-ci openshift-ci Bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Poll: how to fix e2e-test-kubevirt-aws hitting the 2h Prow step timeout

1 participant