test/metrics: simplify OOM test

[kolyshkin]

What type of PR is this?

/kind ci

What this PR does / why we need it:

Currently, this test case keeps waiting for the container that was
already killed in case OOM detection has failed. This is a huge (15
minutes) waste of time.

To fix, change the logic to wait for container exit, and then check the
OOM. This also makes the test code more readable.

Also, add some debug info in case the test has failed.

Also, add a 1 second sleep to the container command, which apparently
improves the chances for the test to succeed.

Which issue(s) this PR fixes:

None

Related to: #7035

Special notes for your reviewer:

Does this PR introduce a user-facing change?

NONE

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[codecov]

Codecov Report

Merging #7036 (538870a) into main (1c2ea67) will decrease coverage by 0.10%.
The diff coverage is n/a.

❗ Current head 538870a differs from pull request most recent head 927843e. Consider uploading reports for the commit 927843e to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7036      +/-   ##
==========================================
- Coverage   49.12%   49.03%   -0.10%     
==========================================
  Files         136      133       -3     
  Lines       15496    15471      -25     
==========================================
- Hits         7613     7586      -27     
+ Misses       6981     6978       -3     
- Partials      902      907       +5     

[littlejawa]

/test kata-containers

[kolyshkin]

@haircommander were you able to make a new test image, as noted in #6950 (comment)? I still see failures from bats in fedora-integration job, which is full of:

/usr/go/src/github.com/cri-o/cri-o/test/./helpers.bash: line 4: bats_require_minimum_version: command not found

[haircommander]

the images will automatically update once this merges. the update happens daily at some point. I think @sohankunkerkar knows how to manually triggger it

[sohankunkerkar]

The images will automatically update once this merges. the update happens daily at some point. I think @sohankunkerkar knows how to manually trigger it

Hmm... it looks like that PR was merged a couple of weeks ago. Ideally, that change should have been picked up by the periodic job on that day itself.

[sohankunkerkar]

/retest

[sohankunkerkar]

Built new images for CI. Let's see if this run passes the integration tests.

[sohankunkerkar]

need to address: https://github.com/cri-o/cri-o/actions/runs/5261831100/jobs/9510391799?pr=7036

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[haircommander]

@kolyshkin given opencontainers/runc#3932 is in 1.8, instead of this should we update CRI-O to have 1.8 and drop the inactive-or-failed?

[kolyshkin]

@kolyshkin given opencontainers/runc#3932 is in 1.8, instead of this should we update CRI-O to have 1.8 and drop the inactive-or-failed?

@haircommander I think that would be a second step. This PR still makes lots of sense because without it the test waits 15 minutes for the condition that is not going to happen).

I will change sleep 1 to sleep 5 now.

[kolyshkin]

This code (together with the sleep 10 below) waits for the OOMKilled condition that may never happen. In case there's no OOM, the test wastes 100 * 10 seconds.

The correct logic, as implemented in this PR, is to wait until container has exited, and check if OOMKilled is set:

	# Wait for container to OOM.
	EXPECTED_EXIT_STATUS=137 wait_until_exit "$CTR_ID"
	crictl inspect "$CTR_ID" | jq -e '.status.reason == "OOMKilled"'

[sohankunkerkar]

https://github.com/cri-o/cri-o/actions/runs/5658658156/job/15330451849?pr=7036
this needs to be fixed.

[kolyshkin]

https://github.com/cri-o/cri-o/actions/runs/5658658156/job/15330451849?pr=7036 this needs to be fixed.

fixed

[sohankunkerkar]

LGTM

[sohankunkerkar]

/retest

[sohankunkerkar]

/retest

[saschagrunert]

Thanks!

/retest

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kolyshkin, saschagrunert, sohankunkerkar

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

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [kolyshkin,saschagrunert]

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