kola/switch-kernel: add a test for switching between rt and default kernel #1218

zonggen · 2020-03-05T20:52:23Z

Adds a sub-command that uploads the RT Kernel RPMs to a machine and switch between kernels using rpm-ostree. The logic used to switch between Default Kernel and RT Kernel is defined in https://github.com/openshift/machine-config-operator/blob/f363c7be6d2d506d900e196fa2e2d05ca08b93b6/pkg/daemon/update.go#L651.

Closes: https://issues.redhat.com/browse/GRPA-1439
Signed-off-by: Allen Bai abai@redhat.com

mantle/kola/tests/rhcos/switch-kernel.go

zonggen · 2020-03-06T20:55:33Z

cc @arithx
Could you take a second look to see it's reasonable?

EDIT: *if it's reasonable

mantle/cmd/kola/switchkernel.go

cgwalters

Overall seems sane! That said, I am hoping to push #1215 over the line soon into supporting exactly this type of thing. Basically it needs:

Support for injecting a directory of files too

And I didn't yet test handling reboots, but the idea is to do so.

cgwalters · 2020-03-06T21:05:03Z

mantle/cmd/kola/switchkernel.go

+
+	err := dropRpmFilesAll(m, rtKernelRpmDir)
+	if err != nil {
+		return fmt.Errorf("Failed dropping Kernel RT RPM files: %v", err)


In new code let's use errors.Wrapf().

zonggen · 2020-03-06T21:26:05Z

sample run & output

[coreos-assembler]$ /coreos-assembler/mantle/bin/kola switch-kernel --kernel-rt ./kernel-rt -b rhcos --ignition-version v2 --qemu-image rhcos-44.81.202003021844-0-qemu.x86_64.qcow2 
Dropping RT Kernel RPMs...
Switching from Default to RT Kernel...
+ FROM_KERNEL=default
+ TO_KERNEL=rt-kernel
+ DEFAULT_KERNEL_PKG='kernel kernel-core kernel-modules kernel-modules-extra'
+ RT_KERNEL_PKG='kernel-rt-core kernel-rt-modules kernel-rt-modules-extra'
+ [[ default == \d\e\f\a\u\l\t ]]
+ [[ rt-kernel == \r\t\-\k\e\r\n\e\l ]]
+ RT_KERNEL_REPO=/var/home/core/kernel-rt-rpms/
++ ls /var/home/core/kernel-rt-rpms/
+ [[ -z kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm
kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm
kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm ]]
+ ARGS='override remove kernel kernel-core kernel-modules kernel-modules-extra'
++ ls /var/home/core/kernel-rt-rpms/
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ rpm-ostree override remove kernel kernel-core kernel-modules kernel-modules-extra --install /var/home/core/kernel-rt-rpms//kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm
Checking out tree a35c3a6...done
Enabled rpm-md repositories: rhel8-baseos rhel8-appstream rhel8-rt
Updating metadata for 'rhel8-baseos'...done
rpm-md repo 'rhel8-baseos'; generated: 2020-02-27T15:31:54Z
Updating metadata for 'rhel8-appstream'...done
rpm-md repo 'rhel8-appstream'; generated: 2020-03-04T17:20:00Z
Updating metadata for 'rhel8-rt'...done
rpm-md repo 'rhel8-rt'; generated: 2020-02-25T05:36:45Z
Importing rpm-md...done
Resolving dependencies...done
Applying 4 overrides and 3 overlays
Processing packages...done
Running pre scripts...done
Running post scripts...done
Running posttrans scripts...done
Writing rpmdb...done
Generating initramfs...done
Writing OSTree commit...done
Staging deployment...done
Removed:
  kernel-4.18.0-147.5.1.el8_1.x86_64
  kernel-core-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-extra-4.18.0-147.5.1.el8_1.x86_64
Added:
  kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64
Run "systemctl reboot" to start a reboot
Rebooting machine...
Checking kernel type...
Switched to RT Kernel successfully!
Switching from RT to Default Kernel...
+ FROM_KERNEL=rt-kernel
+ TO_KERNEL=default
+ DEFAULT_KERNEL_PKG='kernel kernel-core kernel-modules kernel-modules-extra'
+ RT_KERNEL_PKG='kernel-rt-core kernel-rt-modules kernel-rt-modules-extra'
+ [[ rt-kernel == \d\e\f\a\u\l\t ]]
+ [[ rt-kernel == \r\t\-\k\e\r\n\e\l ]]
+ [[ default == \d\e\f\a\u\l\t ]]
+ ARGS='override reset kernel kernel-core kernel-modules kernel-modules-extra'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-core'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-modules'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-modules-extra'
+ rpm-ostree override reset kernel kernel-core kernel-modules kernel-modules-extra --uninstall kernel-rt-core --uninstall kernel-rt-modules --uninstall kernel-rt-modules-extra
Staging deployment...done
Removed:
  kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64
Added:
  kernel-4.18.0-147.5.1.el8_1.x86_64
  kernel-core-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-extra-4.18.0-147.5.1.el8_1.x86_64
Run "systemctl reboot" to start a reboot
Rebooting machine...
Checking kernel type...
Switched back to Default Kernel successfully!

cgwalters · 2020-03-06T21:28:08Z

mantle/cmd/kola/switchkernel.go

+		}
+	}`, base64.StdEncoding.EncodeToString([]byte(switchKernelScript))))
+
+	flight, err := kola.NewFlight("qemu-unpriv")


There's nothing intrinsically specific to qemu about this, right? I think you can use kola.NewFlight(kolaPlatform) and drop the kolaPlatform check above.

Ah, yes that makes sense.

arithx · 2020-03-07T02:28:01Z

Seems reasonable to me; I agree with @cgwalters' comments but after those are fixed should be fine.

zonggen · 2020-03-09T13:48:01Z

Switched to errors.Wrapf and de-hardcoded qemu-unpriv.

Rebased and squashed into one commit.

cgwalters · 2020-03-09T14:06:21Z

mantle/cmd/kola/switchkernel.go

+
+	flight, err := kola.NewFlight(kolaPlatform)
+	if err != nil {
+		return errors.Wrapf(err, "Flight failed: %v", err)


The nice thing about errors.Wrapf is you don't need to do the : %v", err); it does that automatically. And it also preserves the underlying error so it's not turned into a string.

(Search for other places in the code that use it)

zonggen · 2020-03-09T15:17:45Z

Fixed : %v", err in errors.Wrapf and updated the error messages to be consistent with other error messages.

cgwalters · 2020-03-09T15:21:27Z

mantle/cmd/kola/switchkernel.go

+	fmt.Println("Checking kernel type...")
+	cmd = "uname -v | grep -q 'PREEMPT RT'"
+	_, _, err = m.SSH(cmd)
+	if err == nil {


This should be err != nil right?

Hmm...I think this test is passing because you're missing a _ in PREEMPT_RT so both tests are inverted?

Double checked inside a rhcos machine..

[core@ibm-p8-kvm-03-guest-02 ~]$ uname -v #1 SMP PREEMPT RT Tue Jan 14 16:03:46 UTC 2020

Ah OK. But then I think we should invert these checks, because if err == nil is a HUGE trap in Go, it just looks like a typo.

And specifically here the error.Wrapf() is wrong for this one because err is nil, so it will return nil, and the rest of the code will think no error occurred.

The make check ran by openshift-ci-rebot didn't run switchkernel.go at all, it only ran kola --help for sanity check.

Yes, it should be if err != nil, sorry should've mentioned earlier.. and I confirmed on RHCOS the original test did not produce expected result (because error.Wrapf() was hiding the nil error)

cgwalters · 2020-03-09T15:23:05Z

mantle/cmd/kola/switchkernel.go

+
+	// check if the kernel has switched back to default kernel
+	fmt.Println("Checking kernel type...")
+	cmd = "! $(uname -v | grep -q 'PREEMPT RT')"


I think it's better to use grep -v then ! grep - it more clearly expresses intent (and ensures that you still do get an error if e.g. an input file doesn't exist or grep got OOM killed or whatever).

And see above too about the _.

Updated with grep -v and tested on a RHCOS machine, worked without issue

zonggen · 2020-03-09T15:33:45Z

mantle/cmd/kola/switchkernel.go

+	cmd := "sudo " + homeDir + "/switch-kernel.sh rt-kernel default"
+	stdout, stderr, err := m.SSH(cmd)
+	if err != nil {
+		return errors.Wrapf(err, "failed to run %", cmd)


cmd/kola/switchkernel.go:206:10: Wrapf format % is missing verb at end of string

You're missing the s of the %s in the format string.

Yep I wanted to make a note I missed that

Supports --rpm-dir option for uploading the rt kernel rpms into the cluster machine and adds a test that switches to target kernel with rpm-ostree, using logic defined in https://github.com/openshift/machine-config-operator/blob/f363c7be6d2d506d900e196fa2e2d05ca08b93b6/pkg/daemon/update.go#L651. Closes: https://issues.redhat.com/browse/GRPA-1439 Signed-off-by: Allen Bai <abai@redhat.com> kola: add a switch-kernel sub-command Instead of adding a direct test under kola/tests, adds a sub-command that uploads the specified kernel rt rpms and switch to RT Kernel and switch back with rpm-ostree. Closes: https://issues.redhat.com/browse/GRPA-1439 Signed-off-by: Allen Bai <abai@redhat.com> kola/switchkernel: use wrapf and drop hardcoded platform id Drops hardcoded id "qemu-unpriv" and sets to global kolaPlatform. Also switches to "errors.Wrapf" for better error context and stack trace info. Signed-off-by: Allen Bai <abai@redhat.com> kola/switchkernel: remove use of '%v, err' Signed-off-by: Allen Bai <abai@redhat.com>

zonggen · 2020-03-09T19:28:08Z

Updated and tested locally

Output

[coreos-assembler]$ /coreos-assembler/mantle/bin/kola switch-kernel -b rhcos --ignition-version v2 --qemu-image rhcos-44.81.202003021844-0-qemu.x86_64.qcow2 --kernel-rt kernel-/
Dropping RT Kernel RPMs...
Switching from Default to RT Kernel...
+ FROM_KERNEL=default
+ TO_KERNEL=rt-kernel
+ DEFAULT_KERNEL_PKG='kernel kernel-core kernel-modules kernel-modules-extra'
+ RT_KERNEL_PKG='kernel-rt-core kernel-rt-modules kernel-rt-modules-extra'
+ [[ default == \d\e\f\a\u\l\t ]]
+ [[ rt-kernel == \r\t\-\k\e\r\n\e\l ]]
+ RT_KERNEL_REPO=/var/home/core/kernel-rt-rpms/
++ ls /var/home/core/kernel-rt-rpms/
+ [[ -z kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm
kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm
kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm ]]
+ ARGS='override remove kernel kernel-core kernel-modules kernel-modules-extra'
++ ls /var/home/core/kernel-rt-rpms/
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ for RPM in $(ls ${RT_KERNEL_REPO})
+ ARGS+=' --install /var/home/core/kernel-rt-rpms//kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64.rpm'
+ rpm-ostree override remove kernel kernel-core kernel-modules kernel-modules-extra --install /var/home/core/kernel-rt-rpms//kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64.m
Checking out tree a35c3a6...done
Enabled rpm-md repositories: rhel8-baseos rhel8-appstream rhel8-rt
Updating metadata for 'rhel8-baseos'...done
rpm-md repo 'rhel8-baseos'; generated: 2020-02-27T15:31:54Z
Updating metadata for 'rhel8-appstream'...done
rpm-md repo 'rhel8-appstream'; generated: 2020-03-04T17:20:00Z
Updating metadata for 'rhel8-rt'...done
rpm-md repo 'rhel8-rt'; generated: 2020-02-25T05:36:45Z
Importing rpm-md...done
Resolving dependencies...done
Applying 4 overrides and 3 overlays
Processing packages...done
Running pre scripts...done
Running post scripts...done
Running posttrans scripts...done
Writing rpmdb...done
Generating initramfs...done
Writing OSTree commit...done
Staging deployment...done
Removed:
  kernel-4.18.0-147.5.1.el8_1.x86_64
  kernel-core-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-extra-4.18.0-147.5.1.el8_1.x86_64
Added:
  kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64
Run "systemctl reboot" to start a reboot
Rebooting machine...
Checking kernel type...
Switched to RT Kernel successfully!
Switching from RT to Default Kernel...
+ FROM_KERNEL=rt-kernel
+ TO_KERNEL=default
+ DEFAULT_KERNEL_PKG='kernel kernel-core kernel-modules kernel-modules-extra'
+ RT_KERNEL_PKG='kernel-rt-core kernel-rt-modules kernel-rt-modules-extra'
+ [[ rt-kernel == \d\e\f\a\u\l\t ]]
+ [[ rt-kernel == \r\t\-\k\e\r\n\e\l ]]
+ [[ default == \d\e\f\a\u\l\t ]]
+ ARGS='override reset kernel kernel-core kernel-modules kernel-modules-extra'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-core'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-modules'
+ for PKG in $RT_KERNEL_PKG
+ ARGS+=' --uninstall kernel-rt-modules-extra'
+ rpm-ostree override reset kernel kernel-core kernel-modules kernel-modules-extra --uninstall kernel-rt-core --uninstall kernel-rt-modules --uninstall kernel-rt-modules-extra
Staging deployment...done
Removed:
  kernel-rt-core-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-4.18.0-147.5.1.rt24.98.el8_1.x86_64
  kernel-rt-modules-extra-4.18.0-147.5.1.rt24.98.el8_1.x86_64
Added:
  kernel-4.18.0-147.5.1.el8_1.x86_64
  kernel-core-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-4.18.0-147.5.1.el8_1.x86_64
  kernel-modules-extra-4.18.0-147.5.1.el8_1.x86_64
Run "systemctl reboot" to start a reboot
Rebooting machine...
Checking kernel type...
Switched back to Default Kernel successfully!

cgwalters · 2020-03-09T19:28:29Z

/lgtm

zonggen · 2020-03-09T19:29:26Z

Thank you @cgwalters, merging

openshift-ci-robot · 2020-03-09T19:29:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, zonggen

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~~ [cgwalters,zonggen]

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

cgwalters · 2020-03-09T20:09:58Z

Thank you @cgwalters, merging

Currently this repo uses Prow, so the bot will merge.

openshift-ci-robot added do-not-merge/work-in-progress size/L labels Mar 5, 2020

openshift-ci-robot requested review from arithx and miabbott March 5, 2020 20:53

openshift-ci-robot added the approved label Mar 5, 2020

zonggen force-pushed the test-switch-kernel branch 2 times, most recently from a3b8e49 to 8fa71b0 Compare March 5, 2020 21:44

arithx reviewed Mar 5, 2020

View reviewed changes

mantle/kola/tests/rhcos/switch-kernel.go Outdated Show resolved Hide resolved

zonggen commented Mar 6, 2020

View reviewed changes

mantle/cmd/kola/switchkernel.go Show resolved Hide resolved

cgwalters approved these changes Mar 6, 2020

View reviewed changes

zonggen changed the title ~~[WIP] kola/switch-kernel: add a test for switching between rt and default kernel~~ kola/switch-kernel: add a test for switching between rt and default kernel Mar 6, 2020

openshift-ci-robot removed the do-not-merge/work-in-progress label Mar 6, 2020

cgwalters reviewed Mar 6, 2020

View reviewed changes

zonggen force-pushed the test-switch-kernel branch from 75c1a41 to b5750ca Compare March 9, 2020 13:47

cgwalters reviewed Mar 9, 2020

View reviewed changes

zonggen force-pushed the test-switch-kernel branch from b5750ca to 9700e4a Compare March 9, 2020 15:16

cgwalters reviewed Mar 9, 2020

View reviewed changes

zonggen commented Mar 9, 2020

View reviewed changes

zonggen force-pushed the test-switch-kernel branch from 9700e4a to 3a48a44 Compare March 9, 2020 18:18

cgwalters approved these changes Mar 9, 2020

View reviewed changes

zonggen merged commit 28b6aaa into master Mar 9, 2020

zonggen deleted the test-switch-kernel branch March 9, 2020 19:29

openshift-ci-robot assigned cgwalters Mar 9, 2020

openshift-ci-robot added the lgtm label Mar 9, 2020

cgwalters mentioned this pull request Mar 11, 2020

kola: Add run-ext-bin command #1215

Merged

zonggen mentioned this pull request Mar 13, 2020

kola/switch-kernel: rpm-ostree fails to switch from Default to RT Kernel #1245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

kola/switch-kernel: add a test for switching between rt and default kernel #1218

kola/switch-kernel: add a test for switching between rt and default kernel #1218

zonggen commented Mar 5, 2020 •

edited

Loading

zonggen commented Mar 6, 2020 •

edited

Loading

cgwalters left a comment

cgwalters Mar 6, 2020

zonggen commented Mar 6, 2020

cgwalters Mar 6, 2020

zonggen Mar 6, 2020

arithx commented Mar 7, 2020

zonggen commented Mar 9, 2020

cgwalters Mar 9, 2020

cgwalters Mar 9, 2020

zonggen commented Mar 9, 2020 •

edited

Loading

cgwalters Mar 9, 2020

zonggen Mar 9, 2020

cgwalters Mar 9, 2020

zonggen Mar 9, 2020 •

edited

Loading

cgwalters Mar 9, 2020

zonggen Mar 9, 2020

zonggen Mar 9, 2020

arithx Mar 9, 2020

zonggen Mar 9, 2020

zonggen commented Mar 9, 2020 •

edited

Loading

cgwalters commented Mar 9, 2020

zonggen commented Mar 9, 2020

openshift-ci-robot commented Mar 9, 2020

cgwalters commented Mar 9, 2020

kola/switch-kernel: add a test for switching between rt and default kernel #1218

kola/switch-kernel: add a test for switching between rt and default kernel #1218

Conversation

zonggen commented Mar 5, 2020 • edited Loading

zonggen commented Mar 6, 2020 • edited Loading

cgwalters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zonggen commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arithx commented Mar 7, 2020

zonggen commented Mar 9, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zonggen commented Mar 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zonggen Mar 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zonggen commented Mar 9, 2020 • edited Loading

cgwalters commented Mar 9, 2020

zonggen commented Mar 9, 2020

openshift-ci-robot commented Mar 9, 2020

cgwalters commented Mar 9, 2020

zonggen commented Mar 5, 2020 •

edited

Loading

zonggen commented Mar 6, 2020 •

edited

Loading

zonggen commented Mar 9, 2020 •

edited

Loading

zonggen Mar 9, 2020 •

edited

Loading

zonggen commented Mar 9, 2020 •

edited

Loading