Make ContainerCreateTimeout configurable at runtime handler level #9499

snir911 · 2025-10-05T13:27:55Z

This allows different runtime handlers to have different container creation timeouts,
useful for VM-based runtimes that may need longer timeouts than OCI runtimes.

What type of PR is this?

/kind feature

What this PR does / why we need it:

Fixes #9151 and additional minor fixes

Which issue(s) this PR fixes:

Fixes #9151

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Added `container_create_timeout` option to control timeout duration of container creation

openshift-ci · 2025-10-05T13:28:08Z

Hi @snir911. Thanks for your PR.

I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 kubernetes-sigs/prow repository.

codecov · 2025-10-05T13:34:29Z

Codecov Report

❌ Patch coverage is 35.55556% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.54%. Comparing base (6e79482) to head (ea5fe0d).
⚠️ Report is 17 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (6e79482) and HEAD (ea5fe0d). Click for more details.

HEAD has 4 uploads less than BASE

Flag BASE (6e79482) HEAD (ea5fe0d)

15 11

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #9499       +/-   ##
===========================================
- Coverage   66.95%   55.54%   -11.42%     
===========================================
  Files         202      202               
  Lines       28247    28219       -28     
===========================================
- Hits        18914    15673     -3241     
- Misses       7736    10983     +3247     
+ Partials     1597     1563       -34

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

fidencio · 2025-10-09T11:21:37Z

@snir911, what happens when the ContainerCreateTimeout is set to, let's say, 600 (10 minutes), but kubelet's runtimeRequestTimeout is lower than that?

What I think that would happen is that kubelet's runtimeRequestTimeout would be the value respected in this case, which makes me think it's at least worth it adding to the documentation that the value set here depends also on the kubelet's value.

snir911 · 2025-10-09T12:45:24Z

That's right, I'll mention it, thanks @fidencio !

littlejawa

Thanks @snir911 !

openshift-ci · 2025-10-09T14:24:42Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: littlejawa, snir911
Once this PR has been reviewed and has the lgtm label, please assign mrunalp for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

bitoku · 2025-10-10T13:43:21Z

/ok-to-test

docs/crio.conf.5.md

internal/oci/runtime_vm.go

bitoku · 2025-10-10T14:54:05Z

/release-note-edit

Added `container_create_timeout` option to control timeout duration of container creation

openshift-ci · 2025-10-10T14:54:08Z

@bitoku: /release-note-edit must be used with a single release note block.

In response to this:

/release-note-edit

Added `container_create_timeout` option to control timeout duration of container creation

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 kubernetes-sigs/prow repository.

This allows different runtime handlers to have different container creation timeouts, useful for VM-based runtimes that may need longer timeouts than OCI runtimes. Signed-off-by: Snir Schreiber <ssheribe@redhat.com> Fixes: cri-o#9151

Previously, r.task.Create() used r.ctx (background context) which had no timeout, this may cause the goroutine to continue running even after the select timeout was reached. Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

This eliminates duplication and makes the struct cleaner with a single source of truth for configuration values from the handler. Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

- Add container_create_timeout to crio.conf.5.md man page documentation - Add container_create_timeout to configuration template in template.go Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

- Add ValidateContainerCreateTimeout unit tests covering: * Default timeout when not configured (240s) * Valid configured timeout values * Minimum timeout enforcement (30s) * Zero and negative value handling * Large timeout values * Different timeouts per runtime handler Fixes: cri-o#9151 Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

openshift-ci · 2025-10-15T12:47:15Z

New changes are detected. LGTM label has been removed.

internal/oci/runtime_vm.go

docs/crio.conf.5.md

bitoku

sorry one more thing. we want integration tests for this config.
See test/*.bats for other integration tests.

internal/oci/runtime_vm.go

littlejawa · 2025-10-17T08:25:51Z

test/container_create_timeout.bats

+
+@test "container create timeout with custom value" {
+	# Test that CRI-O starts with valid container_create_timeout configuration
+	cat > "$CRIO_CONFIG_DIR"/01-timeout.conf << EOF


I think you need to do:

setup_crio cat > [timeout.conf]... start_crio_no_setup

bitoku · 2025-10-17T11:35:45Z

test/container_create_timeout.bats

I think we want at least one test case that ensures it times out in the seconds we set. only shortest (30s) test is enough.
You can use --cancel-timeout or -T option to set a client (crictl) timeout.

Add comprehensive BATS integration tests for the container_create_timeout feature introduced in commit 63131b6. These tests verify: - Default timeout value (240s) is applied when not configured - Custom timeout values are properly set and respected - Minimum timeout enforcement (30s minimum) - Different runtime handlers can have different timeout values - Test verifies timeout works with slow container creation Assisted-by: Claude AI Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

openshift-ci · 2025-10-19T11:37:47Z

@snir911: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/ci-fedora-kata	`3a9f331`	link	true	`/test ci-fedora-kata`
ci/prow/ci-cgroupv2-integration	`ea5fe0d`	link	true	`/test ci-cgroupv2-integration`
ci/prow/ci-fedora-integration	`ea5fe0d`	link	true	`/test ci-fedora-integration`

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Oct 5, 2025

snir911 force-pushed the 9151 branch 2 times, most recently from f74dbd0 to a0408f0 Compare October 9, 2025 09:13

snir911 marked this pull request as ready for review October 9, 2025 10:33

snir911 requested review from fidencio and mrunalp as code owners October 9, 2025 10:33

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 9, 2025

openshift-ci bot requested review from QiWang19 and hasan4791 October 9, 2025 10:33

littlejawa approved these changes Oct 9, 2025

View reviewed changes

openshift-ci bot assigned littlejawa Oct 9, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2025

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Oct 10, 2025

bitoku reviewed Oct 10, 2025

View reviewed changes

docs/crio.conf.5.md Show resolved Hide resolved

internal/oci/runtime_vm.go Outdated Show resolved Hide resolved

openshift-ci bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Oct 10, 2025

snir911 added 5 commits October 15, 2025 15:46

Add context timeout to task.Create in runtime_vm.go

f48d19b

Previously, r.task.Create() used r.ctx (background context) which had no timeout, this may cause the goroutine to continue running even after the select timeout was reached. Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

Improve returned error text at CreateContainer failure

03d9b40

Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

Simplify runtimeVM struct by removing redundant fields

e85f514

This eliminates duplication and makes the struct cleaner with a single source of truth for configuration values from the handler. Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

Add documentation for container_create_timeout configuration option

3297c3f

- Add container_create_timeout to crio.conf.5.md man page documentation - Add container_create_timeout to configuration template in template.go Signed-off-by: Snir Schreiber <ssheribe@redhat.com>

snir911 force-pushed the 9151 branch from a0408f0 to 526bbdb Compare October 15, 2025 12:47

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2025

snir911 requested a review from bitoku October 15, 2025 13:49

bitoku reviewed Oct 15, 2025

View reviewed changes

internal/oci/runtime_vm.go Show resolved Hide resolved

docs/crio.conf.5.md Show resolved Hide resolved

bitoku reviewed Oct 15, 2025

View reviewed changes

internal/oci/runtime_vm.go Show resolved Hide resolved

littlejawa reviewed Oct 17, 2025

View reviewed changes

bitoku reviewed Oct 17, 2025

View reviewed changes

snir911 force-pushed the 9151 branch from 3a9f331 to ea5fe0d Compare October 19, 2025 11:04

Make ContainerCreateTimeout configurable at runtime handler level #9499

Are you sure you want to change the base?

Make ContainerCreateTimeout configurable at runtime handler level #9499

Conversation

snir911 commented Oct 5, 2025 • edited by bitoku Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Uh oh!

openshift-ci bot commented Oct 5, 2025

Uh oh!

codecov bot commented Oct 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fidencio commented Oct 9, 2025

Uh oh!

snir911 commented Oct 9, 2025

Uh oh!

littlejawa left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 9, 2025

Uh oh!

bitoku commented Oct 10, 2025

Uh oh!

Uh oh!

Uh oh!

bitoku commented Oct 10, 2025

Uh oh!

openshift-ci bot commented Oct 10, 2025

Uh oh!

openshift-ci bot commented Oct 15, 2025

Uh oh!

Uh oh!

Uh oh!

bitoku left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

littlejawa Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

bitoku Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Oct 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

snir911 commented Oct 5, 2025 •

edited by bitoku

Loading

codecov bot commented Oct 5, 2025 •

edited

Loading

bitoku Oct 17, 2025 •

edited

Loading

openshift-ci bot commented Oct 19, 2025 •

edited

Loading