Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
95 changes: 54 additions & 41 deletions build/assets/scripts/rt-kernel.sh
Original file line number Diff line number Diff line change
@@ -1,55 +1,68 @@
#!/usr/bin/env bash

######################################################################################
## NOTE: ##
## THIS IS A TEMPORARY WORKAROUND UNTIL THIS FEATURE IS AVAILABLE VIA MACHINECONFIG ##
## SEE MCO WIP PR: https://github.com/openshift/machine-config-operator/pull/1330 ##
## IT ONLY WORKS ON OCP 4.4 AND WITH KERNEL VERSIONS 4.* ##
######################################################################################

set -euo pipefail

REPO_DIR="/etc/yum.repos.d"
RT_REPO="${REPO_DIR}/rt-kernel.repo"
# What we are doing here:
#
# - 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. See the
# "machine-os-content" image of OCP
# - With RHCOS version 44 (so OCP 4.4, what the operator is aiming at) the image contains the RT kernel RPMs. They were
# just put into the root directory of the image
# - So what we do here (borrowed 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.

# 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
echo "Mounting OS image"

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
}
# remove old container
osContainerName="osContainer"
set +e
podman rm -f "$osContainerName" >/dev/null 2>&1
set -e

# 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

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
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
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
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
61 changes: 61 additions & 0 deletions functests/performance/rt-kernel.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package performance

import (
"fmt"
"time"

. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

testutils "github.com/openshift-kni/performance-addon-operators/functests/utils"
testclient "github.com/openshift-kni/performance-addon-operators/functests/utils/client"
"github.com/openshift-kni/performance-addon-operators/functests/utils/pods"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

var _ = Describe("[performance]RT Kernel", func() {
var testpod *corev1.Pod

AfterEach(func() {
if err := testclient.Client.Pods(testutils.NamespaceTesting).Delete(testpod.Name, &metav1.DeleteOptions{}); err == nil {
pods.WaitForDeletion(testclient.Client, testpod, 60*time.Second)
}
})

It("should have RT kernel enabled", func() {

Eventually(func() string {

// run uname -a in a busybox pod and get logs
testpod = pods.GetBusybox()
testpod.Spec.Containers[0].Command = []string{"uname", "-a"}
testpod.Spec.RestartPolicy = corev1.RestartPolicyNever
testpod.Spec.NodeSelector = map[string]string{
fmt.Sprintf("%s/%s", testutils.LabelRole, testutils.RoleWorkerRT): "",
}

var err error
testpod, err = testclient.Client.Pods(testutils.NamespaceTesting).Create(testpod)
if err != nil {
return ""
}

err = pods.WaitForPhase(testclient.Client, testpod, corev1.PodSucceeded, 60*time.Second)
if err != nil {
return ""
}

logs, err := pods.GetLogs(testclient.Client, testpod)
if err != nil {
return ""
}

return logs

}, 15*time.Minute, 30*time.Second).Should(ContainSubstring("PREEMPT RT"))

})

})
35 changes: 35 additions & 0 deletions functests/utils/pods/pods.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package pods

import (
"bytes"
"io"
"time"

testutils "github.com/openshift-kni/performance-addon-operators/functests/utils"
testclient "github.com/openshift-kni/performance-addon-operators/functests/utils/client"

corev1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -59,3 +62,35 @@ func WaitForCondition(cs *testclient.ClientSet, pod *corev1.Pod, conditionType c
return false, nil
})
}

// WaitForPhase waits until the pod will have specified phase
func WaitForPhase(cs *testclient.ClientSet, pod *corev1.Pod, phase corev1.PodPhase, timeout time.Duration) error {
return wait.PollImmediate(time.Second, timeout, func() (bool, error) {
updatePod, err := cs.Pods(pod.Namespace).Get(pod.Name, metav1.GetOptions{})
if err != nil {
return false, nil
}

if updatePod.Status.Phase == phase {
return true, nil
}

return false, nil
})
}

// GetLogs returns logs of the specified pod
func GetLogs(cs *testclient.ClientSet, pod *corev1.Pod) (string, error) {
logStream, err := testclient.Client.Pods(testutils.NamespaceTesting).GetLogs(pod.Name, &corev1.PodLogOptions{}).Stream()
if err != nil {
return "", err
}
defer logStream.Close()

buf := new(bytes.Buffer)
if _, err := io.Copy(buf, logStream); err != nil {
return "", err
}

return buf.String(), nil
}
11 changes: 11 additions & 0 deletions hack/label-worker-rt.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#!/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 1 worker node
echo "[INFO]:labeling 1 worker node with worker-rt"
node=$(${OC_TOOL} get nodes --selector='node-role.kubernetes.io/worker' -o name | head -1)
${OC_TOOL} label $node node-role.kubernetes.io/worker-rt=""
4 changes: 2 additions & 2 deletions pkg/apis/performance/v1alpha1/performanceprofile_types.go
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

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

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
Loading