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 Sep 8, 2023
1 parent 2513389 commit c8e1191
Show file tree
Hide file tree
Showing 3 changed files with 80 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
27 changes: 27 additions & 0 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit c8e1191

Please sign in to comment.