ci(cnv-validation-checkup): set ACCEPT_WINDOWS_EULA=true and add TEST_FOCUS for disconnected - #82706
ci(cnv-validation-checkup): set ACCEPT_WINDOWS_EULA=true and add TEST_FOCUS for disconnected#82706rlobillo wants to merge 2 commits into
Conversation
…_FOCUS for disconnected - Set ACCEPT_WINDOWS_EULA back to "true" in e2e-azure now that ocp-virt-validation-checkup#87 and openshift#88 are merged. - Add TEST_FOCUS to run-validation-checkup-disconnected with one fast test per suite to reduce job duration: - compute: test_id:1650 (~25s) - network: should report PodIP as its own on interface status (~38s) - ssp: test_id:4777 (~0.004s) - storage: should use existing ControllerRevisions for an existing VM restore (~18s) - tier2: test_machine_config_pool_health (~0.11s) Resolves: CNV-94166 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rlobillo 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 |
openshift-ci
Bot
added
the
approved
|
/pj-rehearse periodic-ci-openshift-cnv-ocp-virt-validation-checkup-main-periodics-e2e-azure |
|
@rlobillo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe validation checkup configuration accepts the Windows EULA by default for Azure runs. The disconnected checkup adds focused tests for PodIP interface status, ControllerRevision reuse during VM restore, and MachineConfigPool health. ChangesValidation checkup configuration
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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-cnv/ocp-virt-validation-checkup/openshift-cnv-ocp-virt-validation-checkup-main__periodics.yaml`:
- Around line 129-131: Update TEST_FOCUS in all periodic definitions so every
intended test is preserved as a separate focus entry, including
test_machine_config_pool_health and the complete test_id:4777 case; do not rely
on a multiline or runner-incompatible value that truncates after “should”.
🪄 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: Pro Plus
Run ID: 3b1cac19-6cc7-4a06-97e3-a81e4ef18a2b
📒 Files selected for processing (1)
ci-operator/config/openshift-cnv/ocp-virt-validation-checkup/openshift-cnv-ocp-virt-validation-checkup-main__periodics.yaml
| - default: test_id:1650|should report PodIP as its own on interface status|test_id:4777|should | ||
| use existing ControllerRevisions for an existing VM restore|test_machine_config_pool_health | ||
| name: TEST_FOCUS |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\bTEST_FOCUS\b|test_id:1650|test_id:4777|test_machine_config_pool_health' .Repository: openshift/release
Length of output: 3369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced CI job and any sibling env usages.
sed -n '1,190p' ci-operator/config/openshift-cnv/ocp-virt-validation-checkup/openshift-cnv-ocp-virt-validation-checkup-main__periodics.yaml
printf '\n--- TEST_FOCUS references in repository ---\n'
rg -n --hidden --glob '!_tmp/**' --glob '!.git/**' '\bTEST_FOCUS\b|TEST_SKIPS\b|TEST_SUITES\b|OCP_VIRT_VALIDATION' . | head -200
printf '\n--- YAML env value after fold/continuation expansion probe ---\n'
python3 - <<'PY'
from pathlib import Path
import re
path = Path('ci-operator/config/openshift-cnv/ocp-virt-validation-checkup/openshift-cnv-ocp-virt-validation-checkup-main__periodics.yaml')
text = path.read_text()
m = re.search(r'name:\s*TEST_FOCUS\s+[\S\s]*?default:\s*([^\n][\s\S]*?)(?:\n\s*name:|\n\s*resources:|$)', text)
print('raw match')
print(m.group(1) if m else 'NO MATCH')
# Very lightweight YAML folding model for the exact | block shown in the context:
# YAML treats non-indented continuation lines as separate scalar values unless the scalar is inside a mapping value
# that has explicit folded/continued indentation. Instead, identify the indented default content lines.
default_pat = re.compile(r'^ - default:(.*)$', re.M)
envs = []
cur = None
for line in text.splitlines():
if default_pat.match(line):
cur = {'name': None, 'default': [default_pat.match(line).group(1)], 'raw_default': line}
envs.append(cur)
elif cur is not None and line.startswith(' name:'):
cur['name'] = line.split(':', 1)[1].strip()
elif cur is not None and line.startswith(' resources:'):
cur = None
print('parsed envs with name', [(e.get('name'), e.get('raw_default'), e.get('default')) for e in envs if e.get('name')])
default_env = next((e for e in envs if e.get('name') == 'TEST_FOCUS'), {})
default_lines = default_env.get('default')
print('joined by space:')
print(' '.join(default_lines or []))
print('joined by newline:')
print('\n'.join(default_lines or []))
PYRepository: openshift/release
Length of output: 48261
Fix the TEST_FOCUS value across all periodic definitions.
TEST_FOCUS is passed unchanged to the remote checkup image, so the value only keeps the first half of test_id:4777|should use existing ControllerRevisions.... Add test_machine_config_pool_health as a separate focused test or move it into a runner-aware focused-test variable.
🤖 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-cnv/ocp-virt-validation-checkup/openshift-cnv-ocp-virt-validation-checkup-main__periodics.yaml`
around lines 129 - 131, Update TEST_FOCUS in all periodic definitions so every
intended test is preserved as a separate focus entry, including
test_machine_config_pool_health and the complete test_id:4777 case; do not rely
on a multiline or runner-incompatible value that truncates after “should”.
…connected TEST_FOCUS pytest's -k expression parser rejects multi-word entries with spaces (e.g. 'should report PodIP as its own on interface status'). These ginkgo-style test names work for --ginkgo.focus but break pytest -k, causing tier2 to fail with: ERROR: Wrong expression passed to '-k': ... at column 25: expected right parenthesis; got identifier Remove the two space-containing entries from the disconnected TEST_FOCUS, keeping only valid pytest/ginkgo identifiers: test_id:1650 (compute), test_id:4777 (ssp), test_machine_config_pool_health (tier2) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
/pj-rehearse periodic-ci-openshift-cnv-ocp-virt-validation-checkup-main-periodics-e2e-azure |
|
@rlobillo: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
1 participant
Summary
ACCEPT_WINDOWS_EULAback to"true"ine2e-azurenow that ocp-virt-validation-checkup#87 and #88 are merged.TEST_FOCUStorun-validation-checkup-disconnectedwith one fast test per suite to reduce job duration.More details
TEST_FOCUSselects the fastest passing test from each suite based on results from this run:test_id:1650should report PodIP as its own on interface statustest_id:4777should use existing ControllerRevisions for an existing VM restoretest_machine_config_pool_healthFor ginkgo suites (compute/network/storage/ssp),
TEST_FOCUStranslates to--ginkgo.focus=(A)|(B)|...combined with the existing per-suite label filter (sig-compute&&conformance, etc.), so cross-suite matching is not an issue. For tier2 (pytest), it translates to-k "A or B or ..."where onlytest_machine_config_pool_healthmatches a real pytest function name.What this PR does / why we need it
ACCEPT_WINDOWS_EULA.Resolves: CNV-94166
Summary by CodeRabbit
ACCEPT_WINDOWS_EULAto"true".test_id:1650,test_id:4777, andtest_machine_config_pool_health.-kexpressions.