Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MCO-694: revert from layered pool to non-layered pool #4284

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
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"time"

daemonconsts "github.com/openshift/machine-config-operator/pkg/daemon/constants"

daemon "github.com/openshift/machine-config-operator/pkg/daemon"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand All @@ -21,11 +23,14 @@ var firstbootCompleteMachineconfig = &cobra.Command{

var persistNics bool

var machineConfigFile string

// init executes upon import
func init() {
rootCmd.AddCommand(firstbootCompleteMachineconfig)
firstbootCompleteMachineconfig.PersistentFlags().StringVar(&startOpts.rootMount, "root-mount", "/rootfs", "where the nodes root filesystem is mounted for chroot and file manipulation.")
firstbootCompleteMachineconfig.PersistentFlags().BoolVar(&persistNics, "persist-nics", false, "Run nmstatectl persist-nic-names")
firstbootCompleteMachineconfig.PersistentFlags().StringVar(&machineConfigFile, "machineconfig-file", daemonconsts.MachineConfigEncapsulatedPath, "The MachineConfig file on-disk to use as the source of truth.")
pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
}

Expand Down Expand Up @@ -55,7 +60,7 @@ func runFirstBootCompleteMachineConfig(_ *cobra.Command, _ []string) error {
return err
}

return dn.RunFirstbootCompleteMachineconfig()
return dn.RunFirstbootCompleteMachineconfig(machineConfigFile)
}

func executeFirstbootCompleteMachineConfig(cmd *cobra.Command, args []string) {
Expand Down
29 changes: 14 additions & 15 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -1207,8 +1207,8 @@ func (dn *Daemon) RunOnceFrom(onceFrom string, skipReboot bool) error {

// RunFirstbootCompleteMachineconfig is run via systemd on the first boot
// to complete processing of the target MachineConfig.
func (dn *Daemon) RunFirstbootCompleteMachineconfig() error {
data, err := os.ReadFile(constants.MachineConfigEncapsulatedPath)
func (dn *Daemon) RunFirstbootCompleteMachineconfig(machineConfigFile string) error {
data, err := os.ReadFile(machineConfigFile)
if err != nil {
return err
}
Expand Down Expand Up @@ -1287,9 +1287,18 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error {
return err
}

// Removing this file signals completion of the initial MC processing.
if err := os.Rename(constants.MachineConfigEncapsulatedPath, constants.MachineConfigEncapsulatedBakPath); err != nil {
return fmt.Errorf("failed to rename encapsulated MachineConfig after processing on firstboot: %w", err)
if machineConfigFile == constants.MachineConfigEncapsulatedPath {
// Removing this file signals completion of the initial MC processing.
if err := os.Rename(constants.MachineConfigEncapsulatedPath, constants.MachineConfigEncapsulatedBakPath); err != nil {
return fmt.Errorf("failed to rename encapsulated MachineConfig after processing on firstboot: %w", err)
}
}

// If we re-bootstrapped the node, we should disable and remove the systemd
// unit that we used to do that. This function will no-op and return nil if
// the systemd unit is not present.
if err := dn.disableRevertSystemdUnit(); err != nil {
return err
}

dn.skipReboot = false
Expand Down Expand Up @@ -2456,16 +2465,6 @@ func (dn *Daemon) triggerUpdate(currentConfig, desiredConfig *mcfgv1.MachineConf
return dn.triggerUpdateWithMachineConfig(currentConfig, desiredConfig, true)
}

// If the desired image annotation is empty, but the current image is not
// empty, this should be a regular MachineConfig update.
//
// However, the node will not roll back from a layered config to a
// non-layered config without admin intervention; so we should emit an error
// for now.
if desiredImage == "" && currentImage != "" {
return fmt.Errorf("rolling back from a layered to non-layered configuration is not currently supported")
}

// Shut down the Config Drift Monitor since we'll be performing an update
// and the config will "drift" while the update is occurring.
dn.stopConfigDriftMonitor()
Expand Down
25 changes: 25 additions & 0 deletions pkg/daemon/runtimeassets/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# runtimeassets

The intent behind this package is to provide a place for assets that the MCD
must render at runtime. Generally speaking, these are used to work around edge
cases where it may not be desirable to have a config file or systemd unit
rendered as part of the normal MachineConfig flow.

These could be static files or they could be runtime-rendered templates based
upon the status of the MCD and/or objects the MCD can access, such as
ControllerConfig.

The RuntimeAsset interface specifies that any additional types or files added
to this package should be rendered as Ignition. This will allow the MCD to
write the files to the nodes' filesystem using the standard Ignition file
writing paths that currently exist.

In the future, items in this package could be expanded and used in the
certificate-writer path as well as other similar paths.

## RevertService

This is used for reverting from a layeed MachineConfigPool to a non-layered
MachineConfigPool. This is because the systemd unit that performs this function
should not be part of the default MachineConfig. Instead, it should be rendered
and applied on an as-needed basis.
29 changes: 29 additions & 0 deletions pkg/daemon/runtimeassets/machine-config-daemon-revert.service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# This systemd unit is intended to help revert from a layered state to a
# non-layered state. It is heavily based upon the
# machine-config-daemon-firstboot.service.yaml found in:
# templates/common/_base/units/machine-config-daemon-firstboot.service.yaml
name: {{ .ServiceName }}
enabled: true
contents: |
[Unit]
Description=Machine Config Daemon Revert
# Make sure it runs only on OSTree booted system
ConditionPathExists=/run/ostree-booted
# Removal of this file signals firstboot completion
ConditionPathExists={{ .RevertServiceMachineConfigFile }}
After=network.target

[Service]
Type=oneshot
RemainAfterExit=yes
# Disable existing repos (if any) so that OS extensions would use embedded RPMs only
ExecStartPre=-/usr/bin/sh -c "sed -i 's/enabled=1/enabled=0/' /etc/yum.repos.d/*.repo"
# Run this via podman because we want to use the nmstatectl binary in our container
ExecStart=/usr/bin/podman run --authfile=/var/lib/kubelet/config.json --rm --privileged --net=host -v /:/rootfs --entrypoint machine-config-daemon '{{ .MCOImage }}' firstboot-complete-machineconfig --persist-nics --machineconfig-file {{ .RevertServiceMachineConfigFile }}
ExecStart=/usr/bin/podman run --authfile=/var/lib/kubelet/config.json --rm --privileged --pid=host --net=host -v /:/rootfs --entrypoint machine-config-daemon '{{ .MCOImage }}' firstboot-complete-machineconfig --machineconfig-file {{ .RevertServiceMachineConfigFile }}
{{if .Proxy -}}
EnvironmentFile=/etc/mco/proxy.env
{{end -}}

[Install]
RequiredBy=multi-user.target
112 changes: 112 additions & 0 deletions pkg/daemon/runtimeassets/revertservice.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package runtimeassets

import (
"bytes"
_ "embed"
"fmt"
"html/template"

ign3types "github.com/coreos/ignition/v2/config/v3_4/types"
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
ctrlcommon "github.com/openshift/machine-config-operator/pkg/controller/common"
"sigs.k8s.io/yaml"
)

var _ RuntimeAsset = &revertService{}

const (
RevertServiceName string = "machine-config-daemon-revert.service"
RevertServiceMachineConfigFile string = "/etc/mco/machineconfig-revert.json"
)

//go:embed machine-config-daemon-revert.service.yaml
var mcdRevertServiceIgnYAML string

type revertService struct {
// The MCO image pullspec that should be used.
MCOImage string
// Whether the proxy file exists and should be considered.
Proxy bool
}

// Constructs a revertService instance from a ControllerConfig. Returns an
// error if the provided ControllerConfig cannot be used.
func NewRevertService(ctrlcfg *mcfgv1.ControllerConfig) (RuntimeAsset, error) {
mcoImage, ok := ctrlcfg.Spec.Images["machineConfigOperator"]
if !ok {
return nil, fmt.Errorf("controllerconfig Images does not have machineConfigOperator image")
}

if mcoImage == "" {
return nil, fmt.Errorf("controllerconfig Images has machineConfigOperator but it is empty")
}

hasProxy := false
if ctrlcfg.Spec.Proxy != nil {
hasProxy = true
}

return &revertService{
MCOImage: mcoImage,
Proxy: hasProxy,
}, nil
}

// Returns an Ignition config containing the
// machine-config-daemon-revert.service systemd unit.
func (r *revertService) Ignition() (*ign3types.Config, error) {
rendered, err := r.render()
if err != nil {
return nil, err
}

out := &ign3types.Unit{}

if err := yaml.Unmarshal(rendered, out); err != nil {
return nil, err
}

ignConfig := ctrlcommon.NewIgnConfig()
ignConfig.Systemd = ign3types.Systemd{
Units: []ign3types.Unit{
*out,
},
}

return &ignConfig, nil
}

// Renders the embedded template with the provided values.
func (r *revertService) render() ([]byte, error) {
if r.MCOImage == "" {
return nil, fmt.Errorf("MCOImage field must be provided")
}

tmpl := template.New(RevertServiceName)

tmpl, err := tmpl.Parse(mcdRevertServiceIgnYAML)
if err != nil {
return nil, err
}

// Golang templates must be rendered using exported fields. However, we want
// to manage the exported field for this service, so we create an anonymous
// struct which embeds RevertService and the ServiceName before we render
// the template.
data := struct {
ServiceName string
RevertServiceMachineConfigFile string
revertService
}{
ServiceName: RevertServiceName,
RevertServiceMachineConfigFile: RevertServiceMachineConfigFile,
revertService: *r,
}

buf := bytes.NewBuffer([]byte{})
if err := tmpl.Execute(buf, data); err != nil {
return nil, err
}

return buf.Bytes(), nil
}
96 changes: 96 additions & 0 deletions pkg/daemon/runtimeassets/revertservice_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package runtimeassets

import (
"fmt"
"testing"

configv1 "github.com/openshift/api/config/v1"
mcfgv1 "github.com/openshift/api/machineconfiguration/v1"
"github.com/stretchr/testify/assert"
)

func TestRevertService(t *testing.T) {
mcoImagePullspec := "mco.image.pullspec"

proxyContents := "EnvironmentFile=/etc/mco/proxy.env"

alwaysExpectedContents := []string{
fmt.Sprintf("ConditionPathExists=%s", RevertServiceMachineConfigFile),
fmt.Sprintf("podman run --authfile=/var/lib/kubelet/config.json --rm --privileged --net=host -v /:/rootfs --entrypoint machine-config-daemon '%s' firstboot-complete-machineconfig --persist-nics --machineconfig-file %s", mcoImagePullspec, RevertServiceMachineConfigFile),
fmt.Sprintf("podman run --authfile=/var/lib/kubelet/config.json --rm --privileged --pid=host --net=host -v /:/rootfs --entrypoint machine-config-daemon '%s' firstboot-complete-machineconfig --machineconfig-file %s", mcoImagePullspec, RevertServiceMachineConfigFile),
}

testCases := []struct {
name string
ctrlcfg *mcfgv1.ControllerConfig
expectedContents []string
unexpectedContents []string
errExpected bool
}{
{
name: "no proxy",
ctrlcfg: &mcfgv1.ControllerConfig{
Spec: mcfgv1.ControllerConfigSpec{
Images: map[string]string{
"machineConfigOperator": mcoImagePullspec,
},
},
},
expectedContents: alwaysExpectedContents,
unexpectedContents: []string{
proxyContents,
},
},
{
name: "with proxy",
ctrlcfg: &mcfgv1.ControllerConfig{
Spec: mcfgv1.ControllerConfigSpec{
Proxy: &configv1.ProxyStatus{},
Images: map[string]string{
"machineConfigOperator": mcoImagePullspec,
},
},
},
expectedContents: append(alwaysExpectedContents, proxyContents),
},
{
name: "no mco image found",
ctrlcfg: &mcfgv1.ControllerConfig{},
errExpected: true,
},
}

for _, testCase := range testCases {
testCase := testCase
t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

rs, err := NewRevertService(testCase.ctrlcfg)
if testCase.errExpected {
assert.Error(t, err)
assert.Nil(t, rs)
return
} else {
assert.NoError(t, err)
assert.NotNil(t, rs)
}

ign, err := rs.Ignition()
assert.NoError(t, err)
assert.NotNil(t, ign)

unit := ign.Systemd.Units[0]

for _, item := range testCase.expectedContents {
assert.Contains(t, *unit.Contents, item)
}

for _, item := range testCase.unexpectedContents {
assert.NotContains(t, *unit.Contents, item)
}

assert.Equal(t, unit.Name, RevertServiceName)
assert.True(t, *unit.Enabled)
})
}
}
9 changes: 9 additions & 0 deletions pkg/daemon/runtimeassets/runtimeassets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package runtimeassets

import (
ign3types "github.com/coreos/ignition/v2/config/v3_4/types"
)

type RuntimeAsset interface {
Ignition() (*ign3types.Config, error)
}
Loading