Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions cmd/machine-config-daemon/firstboot_complete_machineconfig.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package main

import (
"flag"
"fmt"
"os"

daemon "github.com/openshift/machine-config-operator/pkg/daemon"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

var firstbootCompleteMachineconfig = &cobra.Command{
Use: "firstboot-complete-machineconfig",
DisableFlagsInUseLine: true,
Short: "Complete the host's initial boot into a MachineConfig",
Args: cobra.MaximumNArgs(0),
Run: executeFirstbootCompleteMachineConfig,
}

// init executes upon import
func init() {
rootCmd.AddCommand(firstbootCompleteMachineconfig)
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
}

func runFirstBootCompleteMachineConfig(_ *cobra.Command, _ []string) error {
flag.Set("logtostderr", "true")
flag.Parse()

exitCh := make(chan error)
defer close(exitCh)

dn, err := daemon.New(daemon.NewNodeUpdaterClient(), exitCh)
if err != nil {
return err
}

return dn.RunFirstbootCompleteMachineconfig()
}

func executeFirstbootCompleteMachineConfig(cmd *cobra.Command, args []string) {
err := runFirstBootCompleteMachineConfig(cmd, args)
if err != nil {
fmt.Printf("error: %v\n", err)
os.Exit(1)
}
}
43 changes: 43 additions & 0 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,49 @@ func (dn *Daemon) RunOnceFrom(onceFrom string, skipReboot bool) error {
return errors.New("unsupported onceFrom type provided")
}

// RunFirstbootCompleteMachineconfig is run via systemd on the first boot
// to complete processing of the target MachineConfig.
func (dn *Daemon) RunFirstbootCompleteMachineconfig() error {
data, err := ioutil.ReadFile(constants.MachineConfigEncapsulatedPath)
if err != nil {
return err
}
var mc mcfgv1.MachineConfig
err = json.Unmarshal(data, &mc)
if err != nil {
return errors.Wrapf(err, "failed to parse MachineConfig")
}

// Start with an empty config, then add our *booted* osImageURL to
// it, reflecting the current machine state.
oldConfig := canonicalizeEmptyMC(nil)
oldConfig.Spec.OSImageURL = dn.bootedOSImageURL
// 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.
if !dn.compareMachineConfig(oldConfig, &mc) {
// Removing this file signals completion of the initial MC processing.
if err := os.Remove(constants.MachineConfigEncapsulatedPath); err != nil {
return errors.Wrapf(err, "failed to remove %s", constants.MachineConfigEncapsulatedPath)
}
return nil
}

dn.skipReboot = true
err = dn.update(nil, &mc)
if err != nil {
return err
}

// Removing this file signals completion of the initial MC processing.
if err := os.Remove(constants.MachineConfigEncapsulatedPath); err != nil {
return errors.Wrapf(err, "failed to remove %s", constants.MachineConfigEncapsulatedPath)
}

dn.skipReboot = false
return dn.reboot(fmt.Sprintf("Completing firstboot provisioning to %s", mc.GetName()))
}

// InstallSignalHandler installs the handler for the signals the daemon should act on
func (dn *Daemon) InstallSignalHandler(signaled chan struct{}) {
termChan := make(chan os.Signal, 2048)
Expand Down
109 changes: 76 additions & 33 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
errors "github.com/pkg/errors"
"github.com/vincent-petithory/dataurl"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
)

Expand Down Expand Up @@ -162,8 +163,35 @@ func (dn *Daemon) drainAndReboot(newConfig *mcfgv1.MachineConfig) (retErr error)

var errUnreconcilable = errors.New("unreconcilable")

func canonicalizeEmptyMC(config *mcfgv1.MachineConfig) *mcfgv1.MachineConfig {
if config != nil {
return config
}
return &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "mco-empty-mc"},
Spec: mcfgv1.MachineConfigSpec{
Config: ctrlcommon.NewIgnConfig(),
},
}
}

func (dn *Daemon) compareMachineConfig(oldConfig, newConfig *mcfgv1.MachineConfig) bool {
var diff *MachineConfigDiff
oldConfig = canonicalizeEmptyMC(oldConfig)
oldConfigName := oldConfig.GetName()
newConfigName := newConfig.GetName()
diff = NewMachineConfigDiff(oldConfig, newConfig)
if diff.IsEmpty() {
glog.Infof("No changes from %s to %s", oldConfigName, newConfigName)
return false
}
return true
}

// update the node to the provided node configuration.
func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr error) {
oldConfig = canonicalizeEmptyMC(oldConfig)

if dn.nodeWriter != nil {
state, err := getNodeAnnotationExt(dn.node, constants.MachineConfigDaemonStateAnnotationKey, true)
if err != nil {
Expand All @@ -183,36 +211,29 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr err
}
}()

oldConfigUnset := oldConfig == nil
if oldConfigUnset {
// Rather than change the rest of the code to deal
// with a nil oldConfig, just pass an empty one in
// most places.
emptyMC := mcfgv1.MachineConfig{}
oldConfig = &emptyMC
} else {
oldConfigName := oldConfig.GetName()
newConfigName := newConfig.GetName()
glog.Infof("Checking Reconcilable for config %v to %v", oldConfigName, newConfigName)
oldConfigName := oldConfig.GetName()
newConfigName := newConfig.GetName()

// make sure we can actually reconcile this state
diff, reconcilableError := Reconcilable(oldConfig, newConfig)
glog.Infof("Checking Reconcilable for config %v to %v", oldConfigName, newConfigName)

if reconcilableError != nil {
wrappedErr := fmt.Errorf("can't reconcile config %s with %s: %v", oldConfigName, newConfigName, reconcilableError)
if dn.recorder != nil {
mcRef := &corev1.ObjectReference{
Kind: "MachineConfig",
Name: newConfig.GetName(),
UID: newConfig.GetUID(),
}
dn.recorder.Eventf(mcRef, corev1.EventTypeWarning, "FailedToReconcile", wrappedErr.Error())
// make sure we can actually reconcile this state
diff, reconcilableError := Reconcilable(oldConfig, newConfig)

if reconcilableError != nil {
wrappedErr := fmt.Errorf("can't reconcile config %s with %s: %v", oldConfigName, newConfigName, reconcilableError)
if dn.recorder != nil {
mcRef := &corev1.ObjectReference{
Kind: "MachineConfig",
Name: newConfig.GetName(),
UID: newConfig.GetUID(),
}
return errors.Wrapf(errUnreconcilable, "%v", wrappedErr)
dn.recorder.Eventf(mcRef, corev1.EventTypeWarning, "FailedToReconcile", wrappedErr.Error())
}
dn.logSystem("Starting update from %s to %s: %+v", oldConfigName, newConfigName, diff)
return errors.Wrapf(errUnreconcilable, "%v", wrappedErr)
}

dn.logSystem("Starting update from %s to %s: %+v", oldConfigName, newConfigName, diff)

// update files on disk that need updating
if err := dn.updateFiles(oldConfig, newConfig); err != nil {
return err
Expand Down Expand Up @@ -244,7 +265,7 @@ func (dn *Daemon) update(oldConfig, newConfig *mcfgv1.MachineConfig) (retErr err
return err
}
defer func() {
if retErr != nil && !oldConfigUnset {
if retErr != nil {
if err := dn.storeCurrentConfigOnDisk(oldConfig); err != nil {
retErr = errors.Wrapf(retErr, "error rolling back current config on disk %v", err)
return
Expand Down Expand Up @@ -312,6 +333,35 @@ type MachineConfigDiff struct {
units bool
}

// NewMachineConfigDiff compares two MachineConfig objects.
func NewMachineConfigDiff(oldConfig, newConfig *mcfgv1.MachineConfig) *MachineConfigDiff {
oldIgn := oldConfig.Spec.Config
newIgn := newConfig.Spec.Config

// Both nil and empty slices are of zero length,
// consider them as equal while comparing KernelArguments in both MachineConfigs
kargsEmpty := len(oldConfig.Spec.KernelArguments) == 0 && len(newConfig.Spec.KernelArguments) == 0

return &MachineConfigDiff{
osUpdate: oldConfig.Spec.OSImageURL != newConfig.Spec.OSImageURL,
kargs: !(kargsEmpty || reflect.DeepEqual(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments)),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes perf! simple and clean.

fips: oldConfig.Spec.FIPS != newConfig.Spec.FIPS,
passwd: !reflect.DeepEqual(oldIgn.Passwd, newIgn.Passwd),
files: !reflect.DeepEqual(oldIgn.Storage.Files, newIgn.Storage.Files),
units: !reflect.DeepEqual(oldIgn.Systemd.Units, newIgn.Systemd.Units),
}
}

// IsEmpty returns true if the MachineConfigDiff has no changes, or
// in other words if the two MachineConfig objects are equivalent from
// the MCD's point of view. This is mainly relevant if e.g. two MC
// objects happen to have different Ignition versions but are otherwise
// the same. (Probably a better way would be to canonicalize)
func (d *MachineConfigDiff) IsEmpty() bool {
emptyDiff := MachineConfigDiff{}
return reflect.DeepEqual(d, &emptyDiff)
}

// Reconcilable checks the configs to make sure that the only changes requested
// are ones we know how to do in-place. If we can reconcile, (nil, nil) is returned.
// Otherwise, if we can't do it in place, the node is marked as degraded;
Expand Down Expand Up @@ -417,14 +467,7 @@ func Reconcilable(oldConfig, newConfig *mcfgv1.MachineConfig) (*MachineConfigDif

// we made it through all the checks. reconcile away!
glog.V(2).Info("Configs are reconcilable")
return &MachineConfigDiff{
osUpdate: oldConfig.Spec.OSImageURL != newConfig.Spec.OSImageURL,
kargs: !reflect.DeepEqual(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments),
fips: oldConfig.Spec.FIPS != newConfig.Spec.FIPS,
passwd: passwdChanged,
files: !reflect.DeepEqual(oldIgn.Storage.Files, newIgn.Storage.Files),
units: !reflect.DeepEqual(oldIgn.Systemd.Units, newIgn.Systemd.Units),
}, nil
return NewMachineConfigDiff(oldConfig, newConfig), nil
}

// verifyUserFields returns nil if the user Name = "core", if 1 or more SSHKeys exist for
Expand Down
30 changes: 30 additions & 0 deletions pkg/daemon/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
mcfgv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1"
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
"github.com/stretchr/testify/assert"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
k8sfake "k8s.io/client-go/kubernetes/fake"
)

Expand Down Expand Up @@ -175,6 +176,35 @@ func TestReconcilable(t *testing.T) {
checkIrreconcilableResults(t, "PasswdGroups", isReconcilable)
}

func TestMachineConfigDiff(t *testing.T) {
oldConfig := &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "oldconfig"},
Spec: mcfgv1.MachineConfigSpec{
Config: ctrlcommon.NewIgnConfig(),
},
}
newConfig := &mcfgv1.MachineConfig{
ObjectMeta: metav1.ObjectMeta{Name: "newconfig"},
Spec: mcfgv1.MachineConfigSpec{
Config: ctrlcommon.NewIgnConfig(),
},
}
diff := NewMachineConfigDiff(oldConfig, newConfig)
assert.True(t, diff.IsEmpty())

newConfig.Spec.OSImageURL = "quay.io/example/foo@sha256:b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"
diff = NewMachineConfigDiff(oldConfig, newConfig)
assert.False(t, diff.IsEmpty())
assert.True(t, diff.osUpdate)

emptyMc := canonicalizeEmptyMC(nil)
otherEmptyMc := canonicalizeEmptyMC(nil)
emptyMc.Spec.KernelArguments = nil
otherEmptyMc.Spec.KernelArguments = []string{}
diff = NewMachineConfigDiff(emptyMc, otherEmptyMc)
assert.True(t, diff.IsEmpty())
}

func newTestIgnitionFile(i uint) igntypes.File {
mode := 0644
return igntypes.File{Node: igntypes.Node{Path: fmt.Sprintf("/etc/config%d", i), Filesystem: "root"},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: "machine-config-daemon-firstboot.service"
enabled: true
contents: |
[Unit]
Description=Machine Config Daemon Firstboot
# Make sure it runs only on OSTree booted system
ConditionPathExists=/run/ostree-booted
# This effectively disables this unit unitl we get latest
# machine-config-daemon package into bootimage
ConditionPathExists=!/etc/pivot/image-pullspec
ConditionPathExists=/etc/ignition-machine-config-encapsulated.json
BindsTo=ignition-firstboot-complete.service
After=ignition-firstboot-complete.service
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I was testing this locally, noticed that machine-config-daemon-firstboot.service runs when a newly created cluster reboots after applying ignition configs and updates, which leads to another reboot. It's because /etc/pivot/image-pullspec gets deleted once cluster is updated to latest machine-os-content during first boot. Should we also add something like BindsTo=ignition-firstboot-complete.service here to avoid running it in next reboot or it's an acceptable behavior?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...we should be deleting /etc/pivot/image-pullspec during firstboot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we delete /etc/pivot/image-pullspec during firstboot which happens after OS is upgraded to latest machine-os-content but machine-config-daemon-firstboot.service runs early enough and at that time image-pullspec file still exist. Once system reboots after updating OS, /etc/pivot/image-pullspec doesn't exist and /etc/ignition-machine-config-encapsulated.json exist which leads machine-config-daemon-firstboot.servicerun. That's probably because systemd After=ignition-firstboot-complete.service is not enough to not run machine-config-daemon-firstboot.service service if ignition-firstboot-complete.service service failed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I see, though I think we should probably be deleting the /etc/ignition-machine-config-encapsulated.json file too.
But this BindsTo= approach is also fine.

Before=kubelet.service

[Service]
# Need oneshot to delay kubelet
Type=oneshot
ExecStart=/usr/libexec/machine-config-daemon firstboot-complete-machineconfig

[Install]
WantedBy=multi-user.target