OCPVE-382: fix: add default state for crio config

[eggfoobar]

The original implementation of workload partitioning tried to leverage default behavior for CRIO to allow full use of CPU Sets when no Performance Profile is supplied by the user while still being a CPU partitioned cluster. This works fine for CPU affinity however because we don't supply a config and allow the default behavior to kick in, CRIO does not alter the CPU share and gives all pods 2 CPU Share value.

We need to supply a config for CRIO with an empty string for CPU Set to support both CPU share and CPU affinity behavior when NO performance profile is supplied, so that the resource.requests which get altered to CPU Share, are correctly being applied in a default state.

/assign @haircommander

[openshift-ci-robot]

@eggfoobar: This pull request references OCPVE-382 which is a valid jira issue.

In response to this:

The original implementation of workload partitioning tried to leverage default behavior for CRIO to allow full use of CPU Sets when no Performance Profile is supplied by the user while still being a CPU partitioned cluster. This works fine for CPU affinity however because we don't supply a config and allow the default behavior to kick in, CRIO does not alter the CPU share and gives all pods 2 CPU Share value.

We need to supply a config for CRIO with an empty string for CPU Set to support both CPU share and CPU affinity behavior when NO performance profile is supplied, so that the resource.requests which get altered to CPU Share, are correctly being applied in a default state.

/assign @haircommander

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.

[eggfoobar]

/assign @jmencak

[haircommander]

LGTM from CRI-O side

[eggfoobar]

Validated behavior is correct on release build with this PR.

Steps:

• validated the new MC contains both config files for kubelet and crio
• spun up debug pod in node
• checked process with taskset to make sure CPU affinity behavior was not effected by change
• checked with crictl inspect to validate cpu shares and cpu set were correctly being applied during default behavior

[jmencak]

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eggfoobar, jmencak

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 [jmencak]

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

[ffromani]

Validated behavior is correct on release build with this PR.

Steps:

* validated the new MC contains both config files for kubelet and crio

* spun up debug pod in node

* checked process with `taskset` to make sure CPU affinity behavior was not effected by change

* checked with `crictl inspect` to validate `cpu shares` and `cpu set` were correctly being applied during default behavior

thanks for the validation, appreciated. Do we have (at least one) functests covering this flow? If not we should probably add them.

[eggfoobar]

I don't think there are func tests in this repo for empty state. I'll be adding CPU set and CPU share checks on origin to accompany this API feature going GA. If it's okay with you @ffromani I can add functests here in a different PR as part of that effort, what do you think?

Currently tracking that here https://issues.redhat.com/browse/OCPVE-318

[openshift-ci]

@eggfoobar: all tests passed!

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/test-infra repository. I understand the commands that are listed here.

[MarSik]

Yeah, our test suite does not have a lane for checking the install bootstrapping of a WP SNO. I am ok with relying on the check of generated manifests.

/lgtm