Skip to content

Commit

Permalink
MCO-708: Extract and merge kernel arguments from /proc/cmdline
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ori-amizur committed Aug 31, 2023
1 parent 2513389 commit 279be2d
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 0 deletions.
7 changes: 7 additions & 0 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
46 changes: 46 additions & 0 deletions pkg/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 23 additions & 0 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 := strings.Split(strings.TrimSpace(string(cmdline)), " ")
config.Spec.KernelArguments = nil
for _, split := range splits {
for _, reqKarg := range requestedKargs {
if strings.ReplaceAll(reqKarg, "\"", "") == strings.ReplaceAll(split, "\"", "") {
config.Spec.KernelArguments = append(config.Spec.KernelArguments, reqKarg)
break
}
}
}
logSystem("requested kargs: %+v, merged kargs: %+v", requestedKargs, config.Spec.KernelArguments)
return nil
}

func setRunningKargs(config *mcfgv1.MachineConfig, requestedKargs []string) error {
b, err := os.ReadFile("/proc/cmdline")
if err != nil {
return err
}
return setRunningKargsWithCmdline(config, requestedKargs, b)
}

func canonicalizeEmptyMC(config *mcfgv1.MachineConfig) *mcfgv1.MachineConfig {
if config != nil {
return config
Expand Down

0 comments on commit 279be2d

Please sign in to comment.