Hi @tariq1890. 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.

/ok-to-test

@tariq1890 do you mind adding an integration test for that use case?

@saschagrunert Done, ready for another round of review

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.00%. Comparing base (2f2b3ab) to head (dd38a18).
Report is 5 commits behind head on main.

@@           Coverage Diff           @@
##             main    #9129   +/-   ##
=======================================
  Coverage   46.99%   47.00%           
=======================================
  Files         161      161           
  Lines       23634    23635    +1     
=======================================
+ Hits        11107    11109    +2     
+ Misses      11427    11426    -1     
  Partials     1100     1100           

/retest

Looks like the [integration / integration / conmon / runc / amd64] fail in unrelated to the changes

/retest-required

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bitoku, saschagrunert, tariq1890

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

The pull request process is described here

/override ci/prow/ci-e2e-evented-pleg

@haircommander: Overrode contexts on behalf of haircommander: ci/prow/ci-e2e-evented-pleg

This is the failure I see in the ci-fedora-kata logs. Possible flake?

step fedora-kata failed: "fedora-kata" test steps failed: "fedora-kata" pod "fedora-kata-cri-o-fedora-kata-test" failed: could not watch pod: the pod ci-op-ip2vmvlp/fedora-kata-cri-o-fedora-kata-test failed after 1h19m14s (failed containers: test): ContainerFailed one or more containers exited

Is this required?

@tariq1890 unfortunately it's not flake.
https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/cri-o_cri-o/9129/pull-ci-cri-o-cri-o-main-ci-fedora-kata/1914574536410402816/artifacts/fedora-kata/cri-o-gather/artifacts/testout.txt

Apparently it failed here.

Thanks for pointing that out @bitoku. So it looks like I can't use the runtime() method as containerd-kata-shim-v2 does not have the --root flag :(. It means I probably need to modify the test

I am having a bit of trouble reading the logs, so I am not sure if these failures are related to my changes

@tariq1890

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/cri-o_cri-o/9129/pull-ci-cri-o-cri-o-main-ci-fedora-kata/1914740519007686656/artifacts/fedora-kata/cri-o-gather/artifacts/testout.txt

probably this. https://stackoverflow.com/questions/37344329/jq-not-working-on-tag-name-with-dashes-and-numbers

Looks like the e2e failure is unrelated to the changes this time.

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/cri-o_cri-o/9129/pull-ci-cri-o-cri-o-main-ci-cgroupv2-integration/1915224923299647488/artifacts/cgroupv2-integration/cri-o-gather/artifacts/testout.txt

/lgtm

/retest

/override ci/prow/ci-cgroupv2-integration
/override ci/prow/ci-e2e-evented-pleg

@sohankunkerkar: Overrode contexts on behalf of sohankunkerkar: ci/prow/ci-cgroupv2-integration, ci/prow/ci-e2e-evented-pleg

/retest

Thanks to everyone for helping with this PR :)

Is it possible to backport this to release-1.33?

1.33 hasn't released yet
were you referring to 1.32?

I was referring to 1.33. I was unsure if the release-1.33 was under code freeze or not. If we can backport this to 1.32, that would be great too

Since it's a feature, I'd wait for the new release and then we can backport it to the older releases.

That makes sense, thank you!