Merge pull request #3856 from ori-amizur/MCO-708

MCO-708: Extract and merge kernel arguments from /proc/cmdline
openshift · Sep 18, 2023 · 7aacc8a · 7aacc8a
2 parents 39f12c8 + f2877d6
commit 7aacc8a
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 0 deletions.
diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go
@@ -1072,6 +1072,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
@@ -11,6 +11,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"
@@ -320,6 +322,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) {
 	t.Parallel()
 

diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go
@@ -123,6 +123,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