From c8e1191318d4406c194a2fa079ad623de933083e 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 | 27 +++++++++++++++++++++++ 3 files changed, 80 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..b374b9799d 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -122,6 +122,32 @@ 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 { + logSystem("rpm-ostree failed: %w", err) + return err + } + cmdline, _ := os.ReadFile(CmdLineFile) + logSystem("rpm-ostree: %s, cmdline: %s", string(rpmostreeKargsBytes), string(cmdline)) + + return setRunningKargsWithCmdline(config, requestedKargs, rpmostreeKargsBytes) +} + func canonicalizeEmptyMC(config *mcfgv1.MachineConfig) *mcfgv1.MachineConfig { if config != nil { return config @@ -155,6 +181,7 @@ func (dn *Daemon) compareMachineConfig(oldConfig, newConfig *mcfgv1.MachineConfi klog.Infof("No changes from %s to %s", oldConfigName, newConfigName) return false, nil } + logSystem("diff: %+v", *mcDiff) return true, nil }