config, docs, completions: enable NRI by default.

[klihub]

What type of PR is this?

/kind other

What this PR does / why we need it:

Flip NRI on in the default configuration.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

As discussed in the last community meeting, this PR flips NRI on in the default configuration.

For maintaining command line backward compatibility this PR leaves the original --enable-nri command line option intact, only changing its default to true from false (as opposed to flipping it to --disable-nri=[false]). Hence, NRI can now be disabled on the command line using the --enable-nri=false option.

Does this PR introduce a user-facing change?

Flip NRI on by default.
To revert to a configuration with NRI disabled, either either pass the `--enable-nri=false` option to crio
on the command line, or set the `enable_nri = false` in the `[crio.nri]` section of the configuration.

[saschagrunert]

/hold

for other @cri-o/cri-o-maintainers to chime in

[codecov]

Codecov Report

Merging #7790 (474fc77) into main (b9a3d4d) will increase coverage by 0.81%.
Report is 139 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7790      +/-   ##
==========================================
+ Coverage   48.56%   49.37%   +0.81%     
==========================================
  Files         148      148              
  Lines       16187    16187              
==========================================
+ Hits         7861     7993     +132     
+ Misses       7365     7204     -161     
- Partials      961      990      +29     

[haircommander]

/retest

LGTM

[saschagrunert]

/retest

[klihub]

/hold

@saschagrunert @haircommander
I think I know what's wrong. Some of the tests (that used to run with NRI turned off) now clash on the stock NRI socket path when run in parallel... I'll take a look at it.

[sohankunkerkar]

/retest

[sohankunkerkar]

/retest
/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: klihub, 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 [saschagrunert]

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

[github-actions]

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

[saschagrunert]

/unhold

[saschagrunert]

/retest

[saschagrunert]

/override ci/prow/ci-cgroupv2-e2e
/override ci/prow/ci-e2e-conmonrs

[openshift-ci]

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/ci-cgroupv2-e2e, ci/prow/ci-e2e-conmonrs

In response to this:

/override ci/prow/ci-cgroupv2-e2e
/override ci/prow/ci-e2e-conmonrs

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/test-infra repository.

[haircommander]

/retest

[klihub]

@haircommander I recall having seen occasional failures of the timezone tests in images.bat, and IIRC either with this local timezone test failing or the previous one with the explicit/specific timezone test.

I cannot prove it at the moment without doubt (unfortunately the details of the failed test verification are not dumped on failure), but looking at the actual test case implementation, I suspect the reason for flakes is that it implicitly assumes that running foo=$(date ...) twice, once on the host then in the container and with second resolution will always produce the same timestamp. IOW, it is assumed that running the first date command will never happen close enough to the 'end' of a second with the second date run taking long enough for the time to flip over to the next second... in which case the test is guaranteed to fail.

I have a change (I'd like to call it a fix but in lack of definite proof I won't), which takes a single timestamp in seconds and reuses that in both date formatting to verify that the timezones are correctly taken into account... which is what the test really tries to test.

One of the test failures in the previous /retest-cycle was that test case failing, which is why I took a closer look at the test case itself and I noticed it to be a bit iffy...

@haircommander Do you think it is worth a PR/fix for me to file ?

[haircommander]

@haircommander Do you think it is worth a PR/fix for me to file

absolutely!