From f2877d6b31bc0b2295c648ffc9bcfb69b67f0524 Mon Sep 17 00:00:00 2001 From: Ori Amizur Date: Tue, 15 Aug 2023 11:52:02 +0300 Subject: [PATCH] MCO-708: Extract and merge kernel arguments from /proc/cmdline In firstboot MCO checks if reboot can be skipped. In order for reboot to be skipped, the kernel arguments of the current (booted) system and the expected system need to match. Currently, in firstboot the list of the current kargs is assumed to be empty. To reflect the actual list of arguments the system was booted with, this change extracts the set of booted kargs from /proc/cmdline to be used for comparison. Only kargs that appear both in the requested kargs and /proc/cmdline are used for comparison. --- pkg/daemon/daemon.go | 7 ++++++ pkg/daemon/daemon_test.go | 46 +++++++++++++++++++++++++++++++++++++++ pkg/daemon/update.go | 23 ++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 8247a123e0..cae71062e2 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -1054,6 +1054,13 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error { // it, reflecting the current machine state. oldConfig := canonicalizeEmptyMC(nil) oldConfig.Spec.OSImageURL = dn.bootedOSImageURL + + // Setting the Kernel Arguments is for comparison only with the desired MachineConfig. + // The resulting MC should not be for updating node configuration. + if err = setRunningKargs(oldConfig, mc.Spec.KernelArguments); err != nil { + return fmt.Errorf("failed to set kernel arguments from /proc/cmdline: %w", err) + } + // Currently, we generally expect the bootimage to be older, but in the special // case of having bootimage == machine-os-content, and no kernel arguments // specified, then we don't need to do anything here. diff --git a/pkg/daemon/daemon_test.go b/pkg/daemon/daemon_test.go index caab5ae39b..91fc3b3da3 100644 --- a/pkg/daemon/daemon_test.go +++ b/pkg/daemon/daemon_test.go @@ -10,6 +10,8 @@ import ( ign2types "github.com/coreos/ignition/config/v2_2/types" ign3types "github.com/coreos/ignition/v2/config/v3_4/types" + ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vincent-petithory/dataurl" corev1 "k8s.io/api/core/v1" @@ -319,6 +321,50 @@ func newNode(annotations map[string]string) *corev1.Node { } } +func TestSetRunningKargs(t *testing.T) { + oldIgnCfg := ctrlcommon.NewIgnConfig() + oldConfig := helpers.CreateMachineConfigFromIgnition(oldIgnCfg) + oldConfig.ObjectMeta = metav1.ObjectMeta{Name: "oldconfig"} + newIgnCfg := ctrlcommon.NewIgnConfig() + newConfig := helpers.CreateMachineConfigFromIgnition(newIgnCfg) + newConfig.ObjectMeta = metav1.ObjectMeta{Name: "newconfig"} + diff, err := newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.True(t, diff.isEmpty()) + + cmdline := "BOOT_IMAGE=(hd0,gpt3)/ostree/rhcos-c3b004db4/vmlinuz-5.14.0-284.23.1.el9_2.x86_64 systemd.unified_cgroup_hierarchy=0 systemd.legacy_systemd_cgroup_controller=1" + newConfig.Spec.KernelArguments = []string{"systemd.unified_cgroup_hierarchy=0", "systemd.legacy_systemd_cgroup_controller=1"} + _ = setRunningKargsWithCmdline(oldConfig, newConfig.Spec.KernelArguments, []byte(cmdline)) + diff, err = newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.True(t, diff.isEmpty()) + + newConfig.Spec.KernelArguments = []string{"systemd.legacy_systemd_cgroup_controller=1", "systemd.unified_cgroup_hierarchy=0"} + diff, err = newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.False(t, diff.isEmpty()) + assert.True(t, diff.kargs) + + newConfig.Spec.KernelArguments = []string{"systemd.unified_cgroup_hierarchy=0", "systemd.legacy_systemd_cgroup_controller=1", "systemd.unified_cgroup_hierarchy=0"} + diff, err = newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.False(t, diff.isEmpty()) + assert.True(t, diff.kargs) + + cmdline = "BOOT_IMAGE=(hd0,gpt3)/ostree/rhcos-c3b004db4/vmlinuz-5.14.0-284.23.1.el9_2.x86_64 systemd.unified_cgroup_hierarchy=0 systemd.unified_cgroup_hierarchy=0 systemd.legacy_systemd_cgroup_controller=1" + _ = setRunningKargsWithCmdline(oldConfig, newConfig.Spec.KernelArguments, []byte(cmdline)) + diff, err = newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.False(t, diff.isEmpty()) + assert.True(t, diff.kargs) + + cmdline = "BOOT_IMAGE=(hd0,gpt3)/ostree/rhcos-c3b004db4/vmlinuz-5.14.0-284.23.1.el9_2.x86_64 systemd.unified_cgroup_hierarchy=0 systemd.legacy_systemd_cgroup_controller=1 systemd.unified_cgroup_hierarchy=0" + _ = setRunningKargsWithCmdline(oldConfig, newConfig.Spec.KernelArguments, []byte(cmdline)) + diff, err = newMachineConfigDiff(oldConfig, newConfig) + assert.Nil(t, err) + assert.True(t, diff.isEmpty()) +} + func TestPrepUpdateFromClusterOnDiskDrift(t *testing.T) { tmpCurrentConfig, err := os.CreateTemp("", "currentconfig") require.Nil(t, err) diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 53b2eb36e8..1209c9fc1c 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -122,6 +122,29 @@ func (dn *Daemon) performPostConfigChangeAction(postConfigChangeActions []string return dn.triggerUpdateWithMachineConfig(state.currentConfig, state.desiredConfig, true) } +func setRunningKargsWithCmdline(config *mcfgv1.MachineConfig, requestedKargs []string, cmdline []byte) error { + splits := splitKernelArguments(strings.TrimSpace(string(cmdline))) + config.Spec.KernelArguments = nil + for _, split := range splits { + for _, reqKarg := range requestedKargs { + if reqKarg == split { + config.Spec.KernelArguments = append(config.Spec.KernelArguments, reqKarg) + break + } + } + } + return nil +} + +func setRunningKargs(config *mcfgv1.MachineConfig, requestedKargs []string) error { + rpmostreeKargsBytes, err := runGetOut("rpm-ostree", "kargs") + if err != nil { + return err + } + + return setRunningKargsWithCmdline(config, requestedKargs, rpmostreeKargsBytes) +} + func canonicalizeEmptyMC(config *mcfgv1.MachineConfig) *mcfgv1.MachineConfig { if config != nil { return config