Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Install RT kernel from RPMs included in RHCOS #34

Merged
merged 3 commits into from
Jan 10, 2020
Merged

Install RT kernel from RPMs included in RHCOS #34

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
82d7cbe
Install RT kernel from RPMs included in RHCOS
slintes Jan 9, 2020
3ed78b6
Added functest
slintes Jan 9, 2020
b7dfb10
Added comment about what is happening in the rt-kernel.sh script
slintes Jan 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ OPERATOR_DEV_CSV="0.0.1"
# Export GO111MODULE=on to enable project to be built from within GOPATH/src
export GO111MODULE=on

build: gofmt golint
build: gofmt golint govet
@echo "Building operator binary"
mkdir -p build/_output/bin
env GOOS=$(TARGET_GOOS) GOARCH=$(TARGET_GOARCH) go build -i -ldflags="-s -w" -mod=vendor -o build/_output/bin/performance-addon-operators ./cmd/manager
Expand Down Expand Up @@ -112,6 +112,10 @@ cluster-deploy: kustomize
@echo "Deploying operator"
FULL_REGISTRY_IMAGE=$(FULL_REGISTRY_IMAGE) KUSTOMIZE=$(KUSTOMIZE) hack/deploy.sh

cluster-label-worker-rt:
@echo "Adding worker-rt label to worker nodes"
hack/label-worker-rt.sh

cluster-clean:
@echo "Deleting operator"
FULL_REGISTRY_IMAGE=$(FULL_REGISTRY_IMAGE) hack/clean-deploy.sh
Expand Down
82 changes: 41 additions & 41 deletions build/assets/scripts/rt-kernel.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,55 +1,55 @@
#!/usr/bin/env bash

######################################################################################
## NOTE: ##
## THIS IS A TEMPORARY WORKAROUND UNTIL THIS FEATURE IS AVAILABLE VIA MACHINECONFIG ##
## IT ONLY WORKS WITH RT KERNEL VERSION 4.* ##
######################################################################################

set -euo pipefail

REPO_DIR="/etc/yum.repos.d"
RT_REPO="${REPO_DIR}/rt-kernel.repo"
echo "Mounting OS image"

# Enable yum repo
if [[ -f $RT_REPO ]]
then
# The env var might have been changed, so always create new rt repo
rm $RT_REPO
fi
# remove old container
osContainerName="osContainer"
set +e
podman rm -f "$osContainerName" >/dev/null 2>&1
set -e

mkdir -p $REPO_DIR
cat > $RT_REPO <<EOF
[rt]
baseurl=${RT_REPO_URL}
gpgcheck=0
EOF

# update cache
rpm-ostree refresh-md -f

exit_handler () {
exit_code=$?
if [[ ${exit_code} -eq 77 ]]; then
echo "No update available, nothing to do";
exit 0;
elif [[ ${exit_code} -eq 100 ]]; then
echo "Initiate reboot, touch /var/reboot"
touch /var/reboot
exit 0;
else
exit ${exit_code}
fi
}
# find booted image sha
sha=$(rpm-ostree status --json | jq -r '.deployments[] | select(.booted == true) | .["custom-origin"][0]' | sed -E "s|.*@(.*)|\1|")
imageID=$(podman images --digests | grep $sha | awk '{print $4}')

trap exit_handler EXIT
# create and mount new container
podman create --net=none --name "$osContainerName" "$imageID" > /dev/null
kernelDir=$(podman mount "$osContainerName")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, so we're delivering kernel pkgs in containers? interesting.

Copy link
Member

@ffromani ffromani Jan 10, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIU this is used to access the packages already shipped in the system image (so we are not shipping anything anymore). But I may have out of date information.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is complicated, let me try to summarize:

  • the installer's bootstrap node installs an "old" os bootimage on cluster nodes, which is just new enough to be able to do everything which is needed on first boot
  • then the MCO installs a newer version of the os ("early pivot"). That one is delivered via container image. Maybe you noticed that OCP comes with a machine-os-content image: that one contains the os
  • with RHCOS version 44 the image contains the RT kernel RPMs. They were just put into the root directory of the image
  • so what I do here (stolen from MCO code): mount the image (which is not possible directly, so create (but don't run) a container from it) which contains the os version which is currently booted, and install the RPMs from the mount path.

longer version: https://github.com/openshift/machine-config-operator/blob/master/docs/OSUpgrades.md and others

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @slintes great summary. I think it would be great to record this as comment in the rt-kernel.sh script itself. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought REPO_URL was a workaround in 4.3 until we have machineconfig in 4.4 and I see you have a note here as "TEMPORARY WORKAROUND".

Are we having 2 workarounds now for 4.3 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The operator is for 4.4 only. There we have the RT kernel RPMs already available in the RHCOS image, so no need for a yum repo anymore, but the MCO is not ready to use it yet, see WIP PR [0]. So this is a new temporary workaround for 4.4, unrelated to 4.3.

[0] openshift/machine-config-operator#1330

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fromanirh ah missed your comment
will do

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fromanirh done


# Swap to RT kernel
# Swap to or update RT kernel
kernel=$(uname -a)
if [[ $kernel =~ "PREEMPT RT" ]]
if [[ "$kernel" =~ "PREEMPT RT" ]]
then
echo "RT kernel already installed, checking for updates"
# if no upgrade is available the script will exit with code 77, and we will trap it
rpm-ostree upgrade --unchanged-exit-77
echo "RT kernel updated"
exit 100

installedVersion=$(rpm -qa | grep kernel-rt-core)
# filename without rpm suffix is available version
availableVersion=$(ls ${kernelDir}/kernel-rt-core-4*.rpm | xargs basename | xargs -i bash -c 'f="{}" && echo "${f%.*}"')

if [[ "$installedVersion" == "$availableVersion" ]]
then
echo "No update available, nothing to do";
exit 0
else
echo "Updating RT kernel"
rpm-ostree override replace ${kernelDir}/kernel-rt-core-4*.rpm ${kernelDir}/kernel-rt-modules-4*.rpm ${kernelDir}/kernel-rt-modules-extra-4*.rpm
echo "RT kernel updated, trigger reboot by touching /var/reboot"
touch /var/reboot
exit 0
fi

else
echo "Installing RT kernel"
rpm-ostree override remove kernel{,-core,-modules,-modules-extra} --install kernel-rt-core --install kernel-rt-modules --install kernel-rt-modules-extra
echo "RT kernel installed"
exit 100
rpm-ostree override remove kernel{,-core,-modules,-modules-extra} --install ${kernelDir}/kernel-rt-core-4*.rpm --install ${kernelDir}/kernel-rt-modules-4*.rpm --install ${kernelDir}/kernel-rt-modules-extra-4*.rpm
echo "RT kernel installed, trigger reboot by touching /var/reboot"
touch /var/reboot
exit 0
fi
2 changes: 1 addition & 1 deletion cluster-setup/base/performance/performance_profile.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ spec:
- size: "1G"
count: 1
realTimeKernel:
repoURL: "http://download-node-02.eng.bos.redhat.com/rhel-8/nightly/RHEL-8/latest-RHEL-8.1.1/compose/RT/x86_64/os"
enabled: true
8 changes: 4 additions & 4 deletions deploy/crds/performance.openshift.io_performanceprofiles_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ spec:
description: RealTimeKernel defines set of real time kernel related
parameters.
properties:
repoURL:
description: RepoURL defines the URL to the repository with real
time kernel packages
type: string
enabled:
description: Enabled defines if the real time kernel packages should
be installed
type: boolean
type: object
type: object
status:
Expand Down
8 changes: 4 additions & 4 deletions ...g/performance-addon-operators/0.0.1/performance.openshift.io_performanceprofiles_crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ spec:
description: RealTimeKernel defines set of real time kernel related
parameters.
properties:
repoURL:
description: RepoURL defines the URL to the repository with real
time kernel packages
type: string
enabled:
description: Enabled defines if the real time kernel packages should
be installed
type: boolean
type: object
type: object
status:
Expand Down
12 changes: 12 additions & 0 deletions hack/label-worker-rt.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who calls this? only humans or also the operator proper? I guess the former, asking to be sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we use a MachineConfig for installing this script and a systemd unit, and systemd will execute the script then.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry, just noticed I was looking at the wrong script
I called this one manually on test clusters


set -e

# expect oc to be in PATH by default
OC_TOOL="${OC_TOOL:-oc}"

# Label worker nodes
echo "[INFO]:labeling worker nodes"
for node in $(${OC_TOOL} get nodes --selector='!node-role.kubernetes.io/master' -o name); do
${OC_TOOL} label $node node-role.kubernetes.io/worker-rt=""
done
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: new line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

4 changes: 2 additions & 2 deletions pkg/apis/performance/v1alpha1/performanceprofile_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ type HugePage struct {

// RealTimeKernel defines the set of parameters relevant for the real time kernel.
type RealTimeKernel struct {
// RepoURL defines the URL to the repository with real time kernel packages
RepoURL *string `json:"repoURL,omitempty"`
// Enabled defines if the real time kernel packages should be installed
Enabled *bool `json:"enabled,omitempty"`
}

// PerformanceProfileStatus defines the observed state of PerformanceProfile.
Expand Down
6 changes: 3 additions & 3 deletions pkg/apis/performance/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 12 additions & 9 deletions pkg/controller/performanceprofile/components/machineconfig/machineconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const (
)

const (
environmentRTRepoURL = "RT_REPO_URL"
environmentNonIsolatedCpus = "NON_ISOLATED_CPUS"
)

Expand All @@ -73,7 +72,7 @@ func New(assetsDir string, profile *performancev1alpha1.PerformanceProfile) (*ma
Spec: machineconfigv1.MachineConfigSpec{},
}

ignitionConfig, err := getIgnitionConfig(assetsDir, *profile.Spec.RealTimeKernel.RepoURL, profile.Spec.CPU.NonIsolated)
ignitionConfig, err := getIgnitionConfig(assetsDir, profile)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -118,7 +117,8 @@ func getKernelArgs(hugePages *performancev1alpha1.HugePages, isolatedCPUs *perfo
return kargs
}

func getIgnitionConfig(assetsDir string, rtRepoURL string, nonIsolatedCpus *performancev1alpha1.CPUSet) (*igntypes.Config, error) {
func getIgnitionConfig(assetsDir string, profile *performancev1alpha1.PerformanceProfile) (*igntypes.Config, error) {

mode := 0700
ignitionConfig := &igntypes.Config{
Ignition: igntypes.Ignition{
Expand Down Expand Up @@ -149,11 +149,12 @@ func getIgnitionConfig(assetsDir string, rtRepoURL string, nonIsolatedCpus *perf
})
}

rtKernelService, err := getSystemdContent(getRTKernelUnitOptions(rtRepoURL))
rtKernelService, err := getSystemdContent(getRTKernelUnitOptions())
if err != nil {
return nil, err
}

nonIsolatedCpus := profile.Spec.CPU.NonIsolated
preBootTuningService, err := getSystemdContent(
getPreBootTuningUnitOptions(string(*nonIsolatedCpus)),
)
Expand All @@ -166,11 +167,15 @@ func getIgnitionConfig(assetsDir string, rtRepoURL string, nonIsolatedCpus *perf
return nil, err
}

enableRTKernel := profile.Spec.RealTimeKernel != nil &&
profile.Spec.RealTimeKernel.Enabled != nil &&
*profile.Spec.RealTimeKernel.Enabled

ignitionConfig.Systemd = igntypes.Systemd{
Units: []igntypes.Unit{
{
Contents: rtKernelService,
Enabled: pointer.BoolPtr(true),
Enabled: &enableRTKernel,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we create at all RT kernel unit if the RT disabled, also pre-tunning also depends on the RT kernel unit, so I am interesting if it will work as expected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah yes, good point
I'm wondering if we need the option to not install the RT kernel at all, when other performance features depend on it (no clue if all of them do?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other than that, pre-tuning uses wants and not requires, so it would be started without rt-kernel (to my best understanding)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK it is correct, if an unit wants another unit and the latter can't be started, the former should run anyway (see: https://unix.stackexchange.com/questions/388586/systemd-requires-vs-wants - which in turn quotes manpages).
Question is however is if makes sense to do pretuning without rt-kernel, like Marc mentionedin on his grandparent comment.

Name: getSystemdService(rtKernel),
},
{
Expand Down Expand Up @@ -209,7 +214,7 @@ func getSystemdContent(options []*unit.UnitOption) (string, error) {
return string(outBytes), nil
}

func getRTKernelUnitOptions(rtRepoURL string) []*unit.UnitOption {
func getRTKernelUnitOptions() []*unit.UnitOption {
return []*unit.UnitOption{
// [Unit]
// Description
Expand All @@ -222,8 +227,6 @@ func getRTKernelUnitOptions(rtRepoURL string) []*unit.UnitOption {
unit.NewUnitOption(systemdSectionUnit, systemdBefore, systemdServiceKubelet),
unit.NewUnitOption(systemdSectionUnit, systemdBefore, getSystemdService(preBootTuning)),
// [Service]
// Environment
unit.NewUnitOption(systemdSectionService, systemdEnvironment, getSystemdEnvironment(environmentRTRepoURL, rtRepoURL)),
// Type
unit.NewUnitOption(systemdSectionService, systemdType, systemdServiceTypeOneshot),
// RemainAfterExit
Expand Down Expand Up @@ -264,7 +267,7 @@ func getPreBootTuningUnitOptions(nonIsolatedCpus string) []*unit.UnitOption {
return []*unit.UnitOption{
// [Unit]
// Description
unit.NewUnitOption(systemdSectionUnit, systemdDescription, "Reboot initiated by rt-kernel and pre-boot-tuning"),
unit.NewUnitOption(systemdSectionUnit, systemdDescription, "Preboot tuning patch"),
// Wants
unit.NewUnitOption(systemdSectionUnit, systemdWants, getSystemdService(rtKernel)),
// After
Expand Down
3 changes: 1 addition & 2 deletions pkg/controller/performanceprofile/components/machineconfig/machineconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ const expectedSystemdUnits = `
Before=pre-boot-tuning.service

[Service]
Environment=RT_REPO_URL=https://test.test
Type=oneshot
RemainAfterExit=true
ExecStart=/usr/local/bin/rt-kernel.sh
Expand All @@ -33,7 +32,7 @@ const expectedSystemdUnits = `
name: rt-kernel.service
- contents: |
[Unit]
Description=Reboot initiated by rt-kernel and pre-boot-tuning
Description=Preboot tuning patch
Wants=rt-kernel.service
After=rt-kernel.service
Before=kubelet.service
Expand Down
7 changes: 0 additions & 7 deletions pkg/controller/performanceprofile/performanceprofile_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,13 +447,6 @@ func (r *ReconcilePerformanceProfile) validatePerformanceProfileParameters(perfo
return fmt.Errorf("you should provide non isolated CPU set")
}

if performanceProfile.Spec.RealTimeKernel == nil {
return fmt.Errorf("you should provide real time kernel section")
}

if performanceProfile.Spec.RealTimeKernel.RepoURL == nil {
return fmt.Errorf("you should provide real time kernel repository URL")
}
return nil
}

Expand Down
4 changes: 1 addition & 3 deletions pkg/utils/testing/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ const (
NonIsolateCPUs = performancev1alpha1.CPUSet("2-3")
// ReservedCPUs defines the reserved CPU set used for tests
ReservedCPUs = performancev1alpha1.CPUSet("0-3")
// RepoURL defines the real-time kernel repository URL used for tests
RepoURL = "https://test.test"
)

// NewPerformanceProfile returns new performance profile object that used for tests
Expand Down Expand Up @@ -50,7 +48,7 @@ func NewPerformanceProfile(name string) *performancev1alpha1.PerformanceProfile
},
},
RealTimeKernel: &performancev1alpha1.RealTimeKernel{
RepoURL: pointer.StringPtr(RepoURL),
Enabled: pointer.BoolPtr(true),
},
NodeSelector: map[string]string{
"test": "test",
Expand Down
Loading