AGENT-1533: Increase time for agent ha5 test - #82635
Conversation
|
@bfournie: This pull request references AGENT-1533 which is a valid 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. |
|
/cc @andfasano |
|
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: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
WalkthroughDev-scripts setup now accepts configurable timeouts, defaults to 130 minutes, and adds a 45-minute SSH buffer. The nightly e2e-agent job overrides the setup timeout to 150 minutes, while an adjacent four-hour job timeout is removed. ChangesDev-scripts setup timeout
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant NightlyJob
participant DevscriptsSetup
participant SSHSession
participant MakeTarget
NightlyJob->>DevscriptsSetup: Pass DEVSCRIPTS_SETUP_TIMEOUT=150m
DevscriptsSetup->>SSHSession: Apply setup timeout plus 45m
SSHSession->>MakeTarget: Execute make target with 150m timeout
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-ha5-dualstack-iso-no-registry-techpreview |
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/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh`:
- Around line 196-198: Update the timeout handling around MAKE_TIMEOUT and
SSH_TIMEOUT to enforce or document that DEVSCRIPTS_SETUP_TIMEOUT must use
minute-based values. Prefer normalizing supported duration inputs before adding
the 45-minute SSH buffer, while preserving the existing MAKE_TIMEOUT value
passed to make.
🪄 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: c4a5670e-69f0-4551-be96-ee060f89631e
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/release/openshift-release-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (3)
ci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.shci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-ref.yaml
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
51a198a to
9f19344
Compare
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-ha5-dualstack-iso-no-registry-techpreview |
|
@bfournie: your |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-ha5-dualstack-iso-no-registry-techpreview |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-compact-iso-no-registry-techpreview |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse ack |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
9f19344 to
b3955b8
Compare
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-compact-iso-no-registry-techpreview |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-agent-ha5-dualstack-iso-no-registry-techpreview |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
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/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh`:
- Around line 196-203: Update the MAKE_TIMEOUT validation in the setup timeout
block to reject zero-minute values by requiring a positive integer minute format
such as ^[1-9][0-9]*m$. Preserve the existing error message and SSH_TIMEOUT
calculation for valid positive values.
In
`@ci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-ref.yaml`:
- Line 5: Increase the step timeout configured by the baremetalds-devscripts
setup ref from 3h to at least 3h15m, preferably with additional headroom, so it
exceeds the maximum computed internal SSH timeout of 195m.
🪄 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: a06ac167-25b5-4279-b758-f885affb1e41
⛔ Files ignored due to path filters (1)
ci-operator/jobs/openshift/release/openshift-release-main-periodics.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (3)
ci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.shci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-ref.yaml
The agent-ha5-dualstack-iso-no-registry occasionally times out before the installation is complete. This job takes longer for the 5 control plane nodes to resolve. Increase the timeout.
b3955b8 to
1d098a0
Compare
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
A total of 4515 jobs have been affected by this change. The above listing is non-exhaustive and limited to 25 jobs. A full list of affected jobs can be found here Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
/lgtm |
|
/pj-rehearse ack |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
Changes look good to me, especially adding an explicit step timeout greater than the older default of 2h: previously the ci-operator step timeout always overrode the internal dev-scripts script timeouts. With the new default there will be more room for the internal script timeout to fail. /pj-rehearse |
|
@andfasano: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-ipv4 |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
@bfournie: 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. |
|
The periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-ipv4 test failed for an unrelated reason. The installation and the test time was fine: |
|
@elfosardo @tdomnesc could you please have a look at this patch? We need to increase the timeout for one of our key jobs (5 control planes), and currently is failing due the step timeout. The changes should be fine and pretty transparent, but I'd like to get your feedback as well |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-4.22-e2e-metal-ipi-ovn-ipv6 |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-virtualmedia-1of2 |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-virtualmedia-1of2 |
|
@bfournie: your |
|
/pj-rehearse periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-serial-virtualmedia-1of2 |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bfournie, deepsm007, pawanpinjarkar 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 |
a1efeb2
into
openshift:main
The agent-ha5-dualstack-iso-no-registry occasionally times out before the installation is complete. This job takes longer for the 5 control plane nodes to resolve. Increase the timeout.
The agent-ha5-dualstack-iso-no-registry occasionally times out before the installation is complete. This job takes longer for the 5 control plane nodes to resolve. Increase the timeout.
Summary by CodeRabbit
e2e-agent-ha5-dualstack-iso-no-registry-techpreviewinci-operator/config/openshift/release/openshift-release-main__nightly-5.0.yamlby settingDEVSCRIPTS_SETUP_TIMEOUT: 150m, to reduce intermittent installation timeouts when resolving five control plane nodes takes longer than usual.timeout: 4h0m0sstanza immediately before thee2e-agent-ha5-dualstack-iso-no-registry-techpreviewjob entry to avoid conflicting/overlapping timeout behavior.ci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh) to derive timeouts fromDEVSCRIPTS_SETUP_TIMEOUT(default130m):MAKE_TIMEOUTcontrols themake ${DEVSCRIPTS_TARGET}step timeoutSSH_TIMEOUTis computed asMAKE_TIMEOUT + 45mand used to bound the SSH session (replacing the previous fixed SSH/heredoc timeout).ci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-ref.yaml) by documentingDEVSCRIPTS_SETUP_TIMEOUT(default130m) and setting the step-reftimeout: 3h15m.