WIP: Add draft s390x VPN connectivity test - #82562
Conversation
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
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 daily s390x CI workflow that validates HTTP connectivity to port 7001 through external and internal cluster-profile IPs. Changess390x VPN connectivity CI
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ScheduledCI
participant VPNWorkflow
participant VPNStep
participant ClusterProfile
participant VPNEndpoint
ScheduledCI->>VPNWorkflow: Start e2e-s390x-vpn-connectivity
VPNWorkflow->>VPNStep: Run kubevirt-libvirt-vpn
VPNStep->>ClusterProfile: Read external and internal IPs
VPNStep->>VPNEndpoint: Curl port 7001 on each IP
VPNEndpoint-->>VPNStep: Return response body
VPNStep-->>ScheduledCI: Report aggregate status
Possibly related PRs
Suggested labels: Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (12 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: 3
🤖 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/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-commands.sh`:
- Around line 34-37: Remove the four diagnostic echo statements for
LEASED_RESOURCE, EXTERNAL_IP, HOSTNAME, and REMOTE_LIBVIRT_URI from the command
script. Do not print these sensitive lease or connectivity values; retain only
non-sensitive status logging if needed.
- Around line 46-52: Bound each network probe in the retry condition within the
attempt loop: configure both curl calls and the mock-nss.sh virsh invocation
using the repository’s established timeout options or variables, with limits
covering connection and operation time. Preserve the existing success condition
and retry behavior while ensuring a black-holed endpoint cannot block an
iteration indefinitely.
In
`@ci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-workflow.yaml`:
- Around line 3-7: Update the workflow definition containing
allow_best_effort_post_steps and the kubevirt-libvirt-vpn test reference to
declare all required workflow phases, not only test. Follow the established
step-registry phase structure, then run make validate-step-registry and resolve
any missing-phase or registry-reference validation errors.
🪄 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: 4ae1a7fd-6c0e-420e-9f9b-8789d6f7534d
📒 Files selected for processing (7)
ci-operator/config/kubevirt/ssp-operator/kubevirt-ssp-operator-main.yamlci-operator/step-registry/kubevirt/libvirt/vpn/OWNERSci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-commands.shci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-ref.metadata.jsonci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-ref.yamlci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-workflow.metadata.jsonci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-workflow.yaml
There was a problem hiding this comment.
Not sure if this variable is defined.
There was a problem hiding this comment.
It's not yet, should be in the vault I think
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
c425720 to
c826e24
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
/pj-rehearse pull-ci-kubevirt-ssp-operator-main-e2e-functests |
|
@Davo911: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse abort |
|
@Davo911: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
Davo911
changed the title
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Davo911 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 |
|
@Davo911, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
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/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-commands.sh`:
- Around line 52-55: Remove the unconditional 20-minute sleep following the
debug-window message, or gate it behind an explicit rehearsal-only variable in
the surrounding script. Ensure normal successful and failed runs proceed without
delay while preserving the optional sleep for deliberate rehearsals.
🪄 Autofix
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: 1fd7d8c5-b2a5-4787-bfaf-08616617de60
⛔ Files ignored due to path filters (1)
ci-operator/jobs/kubevirt/ssp-operator/kubevirt-ssp-operator-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (4)
ci-operator/config/kubevirt/ssp-operator/kubevirt-ssp-operator-main.yamlci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-commands.shci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-ref.yamlci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-workflow.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- ci-operator/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-workflow.yaml
| # Temporary debug window so we can `oc rsh` / inspect the pod during rehearsals | ||
| # Remove once the lane is stable | ||
| echo "Sleeping 20m for debug window..." | ||
| sleep 20m |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove or gate the 20-minute debug sleep.
Line 55 delays every successful and failed run by 20 minutes. The daily job in ci-operator/config/kubevirt/ssp-operator/kubevirt-ssp-operator-main.yaml can retain CI resources and its cluster lease during this delay. Gate the sleep behind an explicit rehearsal-only variable or remove it before merge.
Proposed fix
-echo "Sleeping 20m for debug window..."
-sleep 20m
+if [[ "${VPN_DEBUG_SLEEP:-false}" == "true" ]]; then
+ echo "Sleeping 20m for debug window..."
+ sleep 20m
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Temporary debug window so we can `oc rsh` / inspect the pod during rehearsals | |
| # Remove once the lane is stable | |
| echo "Sleeping 20m for debug window..." | |
| sleep 20m | |
| # Temporary debug window so we can `oc rsh` / inspect the pod during rehearsals | |
| # Remove once the lane is stable | |
| if [[ "${VPN_DEBUG_SLEEP:-false}" == "true" ]]; then | |
| echo "Sleeping 20m for debug window..." | |
| sleep 20m | |
| fi |
🤖 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/step-registry/kubevirt/libvirt/vpn/kubevirt-libvirt-vpn-commands.sh`
around lines 52 - 55, Remove the unconditional 20-minute sleep following the
debug-window message, or gate it behind an explicit rehearsal-only variable in
the surrounding script. Ensure normal successful and failed runs proceed without
delay while preserving the optional sleep for deliberate rehearsals.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@Davo911, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/pj-rehearse list |
|
@Davo911: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-kubevirt-ssp-operator-main-e2e-s390x-vpn-connectivity |
|
@Davo911, Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@Davo911: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@Davo911, |
Rebases the SSP orange-zone smoke lane onto current main as a single commit so pj-rehearse can rebase cleanly. Co-authored-by: Cursor <cursoragent@cursor.com>
ab7232d to
9fcc630
Compare
|
/pj-rehearse periodic-ci-kubevirt-ssp-operator-main-e2e-s390x-vpn-connectivity |
|
@Davo911: 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: |
|
/pj-rehearse periodic-ci-kubevirt-ssp-operator-main-e2e-s390x-vpn-connectivity |
|
@Davo911: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@Davo911: 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. |
2 participants
Summary
libvirt-s390x-vpn-virte2e-s390x-vpn-connectivityon ssp-operator mainBlocked by
Summary by CodeRabbit
Adds draft s390x VPN connectivity coverage to the
ssp-operatorrepository’s periodic CI jobs.The daily
e2e-s390x-vpn-connectivityjob uses thelibvirt-s390x-vpn-virtcluster profile and thekubevirt-libvirt-vpnworkflow. The test checks port 7001 on external and internal cluster-profile IPs and validates the expected response.The change adds the CI workflow, test reference, ownership metadata, and a 20-minute debugging delay. Rehearsal depends on Boskos lease and secret bootstrap prerequisites.