Allow overriding OSImageURL with a layered image

pkg/controller/common/helpers.go

@@ -54,7 +54,7 @@ func strToPtr(s string) *string {
 // It sorts all the configs in increasing order of their name.
 // It uses the Ignition config from first object as base and appends all the rest.
 // Kernel arguments are concatenated.
-// It uses only the OSImageURL provided by the CVO and ignores any MC provided OSImageURL.
+// It defaults to the OSImageURL provided by the CVO but allows a MC provided OSImageURL to take precedence.
 func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*mcfgv1.MachineConfig, error) {
 	if len(configs) == 0 {
 		return nil, nil
@@ -125,6 +125,18 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m
 		}
 	}
 
+	// For layering, we want to let the user override OSImageURL again
+	overriddenOSImageURL := ""
+	for _, cfg := range configs {
+		if cfg.Spec.OSImageURL != "" {
+			overriddenOSImageURL = cfg.Spec.OSImageURL
+		}
+	}
+	// Make sure it's obvious in the logs that it was overridden
+	if overriddenOSImageURL != "" && overriddenOSImageURL != osImageURL {
+		osImageURL = overriddenOSImageURL
+	}
+
 	return &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
 			OSImageURL:      osImageURL,

pkg/controller/common/helpers_test.go

@@ -273,11 +273,12 @@ func TestMergeMachineConfigs(t *testing.T) {
 
 	// Test that all other configs can also be set properly
 
-	// osImageURL should be set from the passed variable, make sure that
-	// setting it via a MachineConfig doesn't do anything
+	// we previously prevented OSImageURL from being overridden via
+	// machineconfig, but now that we're doing layering, we want to
+	// give that functionality back, so make sure we can override it
 	machineConfigOSImageURL := &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL: "badURL",
+			OSImageURL: "overriddenURL",
 		},
 	}
 	machineConfigKernelArgs := &mcfgv1.MachineConfig{
@@ -328,7 +329,7 @@ func TestMergeMachineConfigs(t *testing.T) {
 
 	expectedMachineConfig = &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
+			OSImageURL:      "overriddenURL",
 			KernelArguments: kargs,
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,

pkg/controller/render/render_controller.go

@@ -487,6 +487,11 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo
 		return err
 	}
 
+	// Emit an event so it's more visible that OSImageURL was overridden.
+	if generated.Spec.OSImageURL != cc.Spec.OSImageURL {
+		ctrl.eventRecorder.Eventf(generated, corev1.EventTypeNormal, "OSImageURLOverridden", "OSImageURL was overridden via machineconfig in %s (was: %s is: %s)", generated.Name, cc.Spec.OSImageURL, generated.Spec.OSImageURL)
+	}
+
 	source := []corev1.ObjectReference{}
 	for _, cfg := range configs {
 		source = append(source, corev1.ObjectReference{Kind: machineconfigKind.Kind, Name: cfg.GetName(), APIVersion: machineconfigKind.GroupVersion().String()})
@@ -570,6 +575,12 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc
 	merged.Annotations[ctrlcommon.GeneratedByControllerVersionAnnotationKey] = version.Hash
 	merged.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey] = cconfig.Annotations[ctrlcommon.ReleaseImageVersionAnnotationKey]
 
+	// Make it obvious that the OSImageURL has been overridden. If we log this in MergeMachineConfigs, we don't know the name yet, so we're
+	// logging out here instead so it's actually helpful.
+	if merged.Spec.OSImageURL != cconfig.Spec.OSImageURL {
+		glog.Infof("OSImageURL has been overridden via machineconfig in %s (was: %s is: %s)", merged.Name, cconfig.Spec.OSImageURL, merged.Spec.OSImageURL)
+	}
+
 	return merged, nil
 }
 

pkg/controller/render/render_controller_test.go

@@ -370,7 +370,7 @@ func TestUpdatesGeneratedMachineConfig(t *testing.T) {
 	f.run(getKey(mcp, t))
 }
 
-func TestGenerateMachineConfigNoOverrideOSImageURL(t *testing.T) {
+func TestGenerateMachineConfigOverrideOSImageURL(t *testing.T) {
 	mcp := helpers.NewMachineConfigPool("test-cluster-master", helpers.MasterSelector, nil, "")
 	mcs := []*mcfgv1.MachineConfig{
 		helpers.NewMachineConfig("00-test-cluster-master", map[string]string{"node-role/master": ""}, "dummy-test-1", []ign3types.File{}),
@@ -383,7 +383,7 @@ func TestGenerateMachineConfigNoOverrideOSImageURL(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	assert.Equal(t, "dummy", gmc.Spec.OSImageURL)
+	assert.Equal(t, "dummy-change", gmc.Spec.OSImageURL)
 }
 
 func TestVersionSkew(t *testing.T) {

pkg/daemon/daemon.go

@@ -1410,16 +1410,30 @@ func (dn *Daemon) checkStateOnFirstRun() error {
 		osMatch := dn.checkOS(targetOSImageURL)
 		if !osMatch {
 			glog.Infof("Bootstrap pivot required to: %s", targetOSImageURL)
-			// This only returns on error
-			osImageContentDir, err := ExtractOSImage(targetOSImageURL)
+
+			// Check to see if we have a layered/new format image
+			isLayeredImage, err := dn.NodeUpdaterClient.IsBootableImage(targetOSImageURL)
 			if err != nil {
-				return err
-			}
-			if err := updateOS(state.currentConfig, osImageContentDir); err != nil {
-				return err
+				return fmt.Errorf("Error checking type of target image: %w", err)
 			}
-			if err := os.RemoveAll(osImageContentDir); err != nil {
-				return err
+
+			if isLayeredImage {
+				// If this is a new format image, we don't have to extract it,
+				// we can just update it the proper way
+				if err := updateLayeredOS(state.currentConfig); err != nil {
+					return err
+				}
+			} else {
+				osImageContentDir, err := ExtractOSImage(targetOSImageURL)
+				if err != nil {
+					return err
+				}
+				if err := updateOS(state.currentConfig, osImageContentDir); err != nil {
+					return err
+				}
+				if err := os.RemoveAll(osImageContentDir); err != nil {
+					return err
+				}
 			}
 			if err := dn.finalizeBeforeReboot(state.currentConfig); err != nil {
 				return err

pkg/daemon/rpm-ostree.go

@@ -2,6 +2,7 @@ package daemon
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -31,15 +32,18 @@ type rpmOstreeState struct {
 
 // RpmOstreeDeployment represents a single deployment on a node
 type RpmOstreeDeployment struct {
-	ID           string   `json:"id"`
-	OSName       string   `json:"osname"`
-	Serial       int32    `json:"serial"`
-	Checksum     string   `json:"checksum"`
-	Version      string   `json:"version"`
-	Timestamp    uint64   `json:"timestamp"`
-	Booted       bool     `json:"booted"`
-	Origin       string   `json:"origin"`
-	CustomOrigin []string `json:"custom-origin"`
+	ID                      string   `json:"id"`
+	OSName                  string   `json:"osname"`
+	Serial                  int32    `json:"serial"`
+	Checksum                string   `json:"checksum"`
+	Version                 string   `json:"version"`
+	Timestamp               uint64   `json:"timestamp"`
+	Booted                  bool     `json:"booted"`
+	Staged                  bool     `json:"staged"`
+	LiveReplaced            string   `json:"live-replaced,omitempty"`
+	Origin                  string   `json:"origin"`
+	CustomOrigin            []string `json:"custom-origin"`
+	ContainerImageReference string   `json:"container-image-reference"`
 }
 
 // imageInspection is a public implementation of
@@ -64,7 +68,9 @@ type NodeUpdaterClient interface {
 	GetStatus() (string, error)
 	GetBootedOSImageURL() (string, string, error)
 	Rebase(string, string) (bool, error)
-	GetBootedDeployment() (*RpmOstreeDeployment, error)
+	RebaseLayered(string) error
+	IsBootableImage(string) (bool, error)
+	GetBootedAndStagedDeployment() (*RpmOstreeDeployment, *RpmOstreeDeployment, error)
 }
 
 // RpmOstreeClient provides all RpmOstree related methods in one structure.
@@ -105,20 +111,16 @@ func (r *RpmOstreeClient) Initialize() error {
 }
 
 // GetBootedDeployment returns the current deployment found
-func (r *RpmOstreeClient) GetBootedDeployment() (*RpmOstreeDeployment, error) {
+func (r *RpmOstreeClient) GetBootedAndStagedDeployment() (booted, staged *RpmOstreeDeployment, err error) {
 	status, err := r.loadStatus()
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	for _, deployment := range status.Deployments {
-		if deployment.Booted {
-			deployment := deployment
-			return &deployment, nil
-		}
-	}
+	booted, err = status.getBootedDeployment()
+	staged = status.getStagedDeployment()
 
-	return nil, fmt.Errorf("not currently booted in a deployment")
+	return
 }
 
 // GetStatus returns multi-line human-readable text describing system status
@@ -135,7 +137,7 @@ func (r *RpmOstreeClient) GetStatus() (string, error) {
 // Returns the empty string if the host doesn't have a custom origin that matches pivot://
 // (This could be the case for e.g. FCOS, or a future RHCOS which comes not-pivoted by default)
 func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, error) {
-	bootedDeployment, err := r.GetBootedDeployment()
+	bootedDeployment, _, err := r.GetBootedAndStagedDeployment()
 	if err != nil {
 		return "", "", err
 	}
@@ -148,6 +150,14 @@ func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, error) {
 		}
 	}
 
+	// we have container images now, make sure we can parse those too
+	if bootedDeployment.ContainerImageReference != "" {
+		// right now they start with "ostree-unverified-registry:", so scrape that off
+		tokens := strings.SplitN(bootedDeployment.ContainerImageReference, ":", 2)
+		if len(tokens) > 1 {
+			osImageURL = tokens[1]
+		}
+	}
 	return osImageURL, bootedDeployment.Version, nil
 }
 
@@ -189,7 +199,7 @@ func (r *RpmOstreeClient) Rebase(imgURL, osImageContentDir string) (changed bool
 		ostreeCsum    string
 		ostreeVersion string
 	)
-	defaultDeployment, err := r.GetBootedDeployment()
+	defaultDeployment, _, err := r.GetBootedAndStagedDeployment()
 	if err != nil {
 		return
 	}
@@ -275,6 +285,66 @@ func (r *RpmOstreeClient) Rebase(imgURL, osImageContentDir string) (changed bool
 	return
 }
 
+// IsBootableImage determines if the image is a bootable (new container formet) image, or a wrapper (old container format)
+func (r *RpmOstreeClient) IsBootableImage(imgURL string) (bool, error) {
+
+	// TODO(jkyros): This is duplicated-ish from Rebase(), do we still need to carry this around?
+	var isBootableImage string
+	var imageData *types.ImageInspectInfo
+	var err error
+	if imageData, err = imageInspect(imgURL); err != nil {
+		if err != nil {
+			var podmanImgData *imageInspection
+			glog.Infof("Falling back to using podman inspect")
+
+			if podmanImgData, err = podmanInspect(imgURL); err != nil {
+				return false, err
+			}
+			isBootableImage = podmanImgData.Labels["ostree.bootable"]
+		}
+	} else {
+		isBootableImage = imageData.Labels["ostree.bootable"]
+	}
+	// We may have pulled in OSContainer image as fallback during podmanCopy() or podmanInspect()
+	defer exec.Command("podman", "rmi", imgURL).Run()
+
+	return isBootableImage == "true", nil
+}
+
+// RebaseLayered rebases system or errors if already rebased
+func (r *RpmOstreeClient) RebaseLayered(imgURL string) (err error) {
+	glog.Infof("Executing rebase to %s", imgURL)
+
+	// For now, just let ostree use the kublet config.json,
+	err = useKubeletConfigSecrets()
+	if err != nil {
+		return fmt.Errorf("Error while ensuring access to kublet config.json pull secrets: %w", err)
+	}
+
+	return runRpmOstree("rebase", "--experimental", "ostree-unverified-registry:"+imgURL)
+}
+
+// useKubeletConfigSecrets gives the rpm-ostree client access to secrets in the kubelet config.json by symlinking so that
+// rpm-ostree can use those secrets to pull images. It does this by symlinking the kubelet's config.json into /run/ostree.
+func useKubeletConfigSecrets() error {
+	if _, err := os.Stat("/run/ostree/auth.json"); err != nil {
+
+		if errors.Is(err, os.ErrNotExist) {
+
+			err := os.MkdirAll("/run/ostree", 0o544)
+			if err != nil {
+				return err
+			}
+
+			err = os.Symlink(kubeletAuthFile, "/run/ostree/auth.json")
+			if err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
 // truncate a string using runes/codepoints as limits.
 // This specifically will avoid breaking a UTF-8 value.
 func truncate(input string, limit int) string {
@@ -303,3 +373,23 @@ func runGetOut(command string, args ...string) ([]byte, error) {
 	}
 	return rawOut, nil
 }
+
+func (state *rpmOstreeState) getBootedDeployment() (*RpmOstreeDeployment, error) {
+	for num := range state.Deployments {
+		deployment := state.Deployments[num]
+		if deployment.Booted {
+			return &deployment, nil
+		}
+	}
+	return &RpmOstreeDeployment{}, fmt.Errorf("not currently booted in a deployment")
+}
+
+func (state *rpmOstreeState) getStagedDeployment() *RpmOstreeDeployment {
+	for num := range state.Deployments {
+		deployment := state.Deployments[num]
+		if deployment.Staged {
+			return &deployment
+		}
+	}
+	return &RpmOstreeDeployment{}
+}

pkg/daemon/rpm-ostree_test.go

@@ -46,3 +46,15 @@ func (r RpmOstreeClientMock) GetStatus() (string, error) {
 func (r RpmOstreeClientMock) GetBootedDeployment() (*RpmOstreeDeployment, error) {
 	return &RpmOstreeDeployment{}, nil
 }
+
+func (r RpmOstreeClientMock) GetBootedAndStagedDeployment() (booted, staged *RpmOstreeDeployment, err error) {
+	return nil, nil, nil
+}
+
+func (r RpmOstreeClientMock) IsBootableImage(string) (bool, error) {
+	return false, nil
+}
+
+func (r RpmOstreeClientMock) RebaseLayered(string) error {
+	return nil
+}