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 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bfournie 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 |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ 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. |
|
@bfournie: all tests passed! 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. |
|
/pj-rehearse ack |
|
@bfournie: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
| ref: | ||
| as: baremetalds-devscripts-setup | ||
| from: dev-scripts | ||
| grace_period: 10m |
There was a problem hiding this comment.
The step doesn't define an explicit timeout, so as per the current docs it's defaulted to 2h. This means that any internal timeout (in the script) will be clipped to the external step timeout by ci-operator.
So, to keep supporting the internal script timeout, I think it will be anyhow required to define a larger step timeout, ie 3h, so that the internal timeout could be properly honored (right now, it is not, because currently the step timeout overrides it completely).
Not sure if such change may introduce any regression (different error codes?) in case of timeout, it may be worth trying
There was a problem hiding this comment.
Good point @andfasano. Added a explicit 3h timeout to baremetalds-devscripts-setup
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.
9f19344 to
b3955b8
Compare
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.yamlto setDEVSCRIPTS_SETUP_TIMEOUT: 150m, giving the5CONTROL_IPV4V6install more time to complete during ISO no-registry setup.ci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-commands.sh) to use a configurableDEVSCRIPTS_SETUP_TIMEOUT(default130m) by deriving:MAKE_TIMEOUTfor the dev-scriptsmaketarget, andSSH_TIMEOUTfor the SSH session asMAKE_TIMEOUT + 45m.DEVSCRIPTS_SETUP_TIMEOUTknob inci-operator/step-registry/baremetalds/devscripts/setup/baremetalds-devscripts-setup-ref.yaml.