enhancements: support real-time kernels by miabbott · Pull Request #166 · openshift/enhancements

miabbott · 2019-12-19T17:05:53Z

This proposal covers adding support for real-time kernels to RHCOS and
the ability to select those kernels with the MCO.

miabbott · 2019-12-19T17:06:42Z

cc: @cgwalters @ashcrow @runcom @sinnykumari @imcleod @darkmuggle @mrguitar

darkmuggle

A few questions, but otherwise it looks good to me. The prose is clear and concise.

darkmuggle · 2019-12-19T17:14:12Z

+If there are additional changes required across the OCP product to properly support
+real-time kernel (i.e. container runtime, etc) which cannot be delivered at the
+same time, the real-time kernel packages can still be included as part of the
+`machine-os-content` image.  Additionally, exposing the tunable in the MachineConfig


Mind clarifying this? The part about lacking full support pivots to removing the tunable.

My logic was:

we prep RT kernel support in RHCOS + MCO

we discover there are other components that are not ready

mitigation: keep the RT kernel in the machine-os-content and backout the MCO change, effectively hiding the knob from users

Does that help?

darkmuggle · 2019-12-19T17:20:29Z

+
+1.  Include the `kernel-rt` packages in the `machine-os-content` image
+2.  Provide tunable in MachineConfig that selects the type of kernel to use
+3.  Do initial tuning of RHCOS node after real-time kernel is applied


I think we need to clarify what "tuning" is affected. For example, FIPS and encryption?

I believe this falls into the bucket of tuned profiles. @cgwalters can you provide some details?

sinnykumari

Overall it looks good to me. One inline comment

sinnykumari · 2019-12-19T17:59:40Z

+- Test removal of `kernel-rt` packages on single RHCOS node
+- Test `kernelType: realtime` on OCP cluster with RHCOS nodes using default kernel
+- Test `kernelType: default` on OCP cluster with RHCOS nodes using default kernel
+- Test `kernelType: default` on OCP cluster with RHCOS nodes already using real-time kernel


Specifying one MC with setting kerneType to realtime and another one to default can lead to uncertain behavior because rendered MC will pick one of them. Instead of creating another MC with kernelType: default, We might want to test here deleting MC which adds kernelType: realtime .

Got it; will update that test case.

ashcrow · 2019-12-19T19:42:54Z

+
+### Providing the Packages
+
+The proposal is to include the `kernel-rt` packages in the `machine-os-content`


nit: In the standard machine-os-content. Basically denote it's not a special or different one.

ashcrow · 2019-12-19T19:44:09Z

+
+When the MCO parses a MachineConfig with `kernelType: default`, it shall instruct `rpm-ostree`
+on the RHCOS node to remove any `kernel-rt` packages and use the default kernel.  If the
+`kernel-rt` packages are not present, it should be a no-op.


Any logging/warning?

I wasn't going to get into those details, but no harm in calling it out explicitly.

ashcrow

Added a couple minor items but it looks quite good!

ericavonb · 2019-12-19T23:22:59Z

+for things like nVidia GPUs will need to rebuild the kernel modules
+to support the real-time kernel.
+
+- Customers requiring FIPS support **SHOULD NOT** use the real-time kernel.


Can we handle this situation explicitly in the MCO? Like, the MCO can refuse to enable it (e.g. by setting the kerneltype=default on the rendered MC when fips=true)?

Seems feasible. @sinnykumari WDYT?

yes should be possible. MCO can print some message saying Not supported when FIPS is already enabled.

This proposal covers adding support for real-time kernels to RHCOS and the ability to select those kernels with the MCO.

miabbott · 2020-01-27T18:05:33Z

Finally got around to making the updates ⬆️

ashcrow

LGTM with the latest updates

ashcrow · 2020-01-30T14:59:28Z

Anyone object to merging?

miabbott · 2020-02-18T20:58:19Z

Can someone give a /lgtm here?

It's just a formality at this point; the machine-os-content already contains the RT kernel as described and the MCO has the functionality to switch kernels

sdodson · 2020-02-18T20:59:48Z

/lgtm

openshift-ci-robot · 2020-02-18T21:01:27Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashcrow, cgwalters, miabbott, mike-nguyen, sdodson

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [ashcrow,sdodson]

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

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 19, 2019

openshift-ci-robot requested review from jwforres and sdodson December 19, 2019 17:06

miabbott force-pushed the realtime branch from c1d89ac to 47fcf73 Compare December 19, 2019 17:23

darkmuggle reviewed Dec 19, 2019

View reviewed changes

sinnykumari reviewed Dec 19, 2019

View reviewed changes

ashcrow reviewed Dec 19, 2019

View reviewed changes

ericavonb reviewed Dec 19, 2019

View reviewed changes

mike-nguyen reviewed Dec 20, 2019

View reviewed changes

Comment thread enhancements/support-for-realtime-kernel.md

Comment thread enhancements/support-for-realtime-kernel.md

sinnykumari mentioned this pull request Jan 24, 2020

Bump RHCOS to 44.81.202001240222.0 openshift/installer#2974

Closed

enhancements: support real-time kernels

98cb9d8

This proposal covers adding support for real-time kernels to RHCOS and the ability to select those kernels with the MCO.

miabbott force-pushed the realtime branch from 47fcf73 to 98cb9d8 Compare January 27, 2020 18:04

ashcrow approved these changes Jan 29, 2020

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 29, 2020

mike-nguyen approved these changes Jan 30, 2020

View reviewed changes

cgwalters approved these changes Jan 30, 2020

View reviewed changes

slintes mentioned this pull request Feb 13, 2020

Add periodic job for OCP with realtime workers openshift/release#7154

Merged

4 tasks

openshift-ci-robot assigned sdodson Feb 18, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 18, 2020

openshift-merge-robot merged commit 6d25129 into openshift:master Feb 18, 2020


		### Providing the Packages

		The proposal is to include the `kernel-rt` packages in the `machine-os-content`

Conversation

miabbott commented Dec 19, 2019

Uh oh!

miabbott commented Dec 19, 2019

Uh oh!

darkmuggle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sinnykumari left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashcrow left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

miabbott commented Jan 27, 2020

Uh oh!

ashcrow left a comment

Choose a reason for hiding this comment

Uh oh!

ashcrow commented Jan 30, 2020

Uh oh!

miabbott commented Feb 18, 2020

Uh oh!

sdodson commented Feb 18, 2020

Uh oh!

openshift-ci-robot commented Feb 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

sinnykumari left a comment •

edited

Loading