Add rhel-coreos extensions image in configmap, propogate to

controllerconfig

The extensions for our layered/bootable `rhel-coreos` images are no
longer going to be contained within the image themselves. They will be a
separate container. See: openshift/os#763

This makes sure that, if present, the new extensions image gets pushed
through to controllerconfig and is available when merging
machineconfigs.

install/0000_80_machine-config-operator_01_machineconfig.crd.yaml

@@ -339,3 +339,11 @@ spec:
                 description: OSImageURL specifies the remote location that will be used
                   to fetch the OS
                 type: string
+              baseOperatingSystemContainer:
+                description: baseOperatingSystemContainer specifies the remote location that will be used
+                  to fetch the new-format OS image
+                type: string
+              baseOperatingSystemExtensionsContainer:
+                description: baseOperatingSystemExtensionContainer specifies the remote location that will be used
+                  to fetch the extensions container matching the new-format OS image
+                type: string

install/0000_80_machine-config-operator_05_osimageurl.yaml

@@ -11,7 +11,8 @@ data:
   releaseVersion: 0.0.1-snapshot
   # This (will eventually) replace the below when https://github.com/openshift/enhancements/pull/1032
   # progresses towards the default.
-  baseOperatingSystemContainer: "registry.ci.openshift.org/openshift:rhel-coreos"
+  baseOperatingSystemContainer: "registry.ci.openshift.org/openshift:rhel-coreos-8"
+  baseOperatingSystemExtensionscontaier: "registry.ci.openshift.org/openshift:rhel-coreos-8-extensions"
   # The OS payload used for 4.10 and below; more information in
   # https://github.com/openshift/machine-config-operator/blob/master/docs/OSUpgrades.md
   # (The original issue was  https://github.com/openshift/machine-config-operator/issues/183 )

install/image-references

@@ -23,10 +23,14 @@ spec:
     from:
       kind: DockerImage
       name: registry.svc.ci.openshift.org/openshift:machine-os-content
-  - name: rhel-coreos
+  - name: rhel-coreos-8
     from:
       kind: DockerImage
-      name: registry.ci.openshift.org/openshift:rhel-coreos
+      name: registry.svc.ci.openshift.org/openshift:rhel-coreos-8
+  - name: rhel-coreos-8-extensions
+    from:
+      kind: DockerImage
+      name: registry.svc.ci.openshift.org/openshift:rhel-coreos-8-extensions
   - name: keepalived-ipfailover
     from:
       kind: DockerImage

lib/resourcemerge/machineconfig.go

@@ -47,6 +47,8 @@ func EnsureMachineConfigPool(modified *bool, existing *mcfgv1.MachineConfigPool,
 func ensureMachineConfigSpec(modified *bool, existing *mcfgv1.MachineConfigSpec, required mcfgv1.MachineConfigSpec) {
 	resourcemerge.SetStringIfSet(modified, &existing.OSImageURL, required.OSImageURL)
 	resourcemerge.SetStringIfSet(modified, &existing.KernelType, required.KernelType)
+	resourcemerge.SetStringIfSet(modified, &existing.BaseOperatingSystemContainer, required.BaseOperatingSystemContainer)
+	resourcemerge.SetStringIfSet(modified, &existing.BaseOperatingSystemExtensionsContainer, required.BaseOperatingSystemExtensionsContainer)
 
 	if !equality.Semantic.DeepEqual(existing.KernelArguments, required.KernelArguments) {
 		*modified = true
@@ -72,6 +74,8 @@ func ensureControllerConfigSpec(modified *bool, existing *mcfgv1.ControllerConfi
 	resourcemerge.SetStringIfSet(modified, &existing.Platform, required.Platform)
 	resourcemerge.SetStringIfSet(modified, &existing.EtcdDiscoveryDomain, required.EtcdDiscoveryDomain)
 	resourcemerge.SetStringIfSet(modified, &existing.OSImageURL, required.OSImageURL)
+	resourcemerge.SetStringIfSet(modified, &existing.BaseOperatingSystemContainer, required.BaseOperatingSystemContainer)
+	resourcemerge.SetStringIfSet(modified, &existing.BaseOperatingSystemExtensionsContainer, required.BaseOperatingSystemExtensionsContainer)
 	resourcemerge.SetStringIfSet(modified, &existing.NetworkType, required.NetworkType)
 
 	setBytesIfSet(modified, &existing.AdditionalTrustBundle, required.AdditionalTrustBundle)

manifests/controllerconfig.crd.yaml

@@ -922,6 +922,10 @@ spec:
                 description: baseOperatingSystemContainer is the new format operating system update image.
                   See https://github.com/openshift/enhancements/pull/1032
                 type: string
+              baseOperatingSystemExtensionsContainer:
+                description: baseOperatingSystemExtensionsContainer is the extensions container matching new format operating system update image.
+                  See https://github.com/openshift/enhancements/pull/1032
+                type: string
               platform:
                 description: platform is deprecated, use Infra.Status.PlatformStatus.Type
                   instead

pkg/apis/machineconfiguration.openshift.io/v1/types.go

@@ -75,14 +75,17 @@ type ControllerConfigSpec struct {
 	// BaseOperatingSystemContainer is the new-format container image for operating system updates.
 	BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"`
 
+	// BaseOperatingSystemExtensionsContainer is the matching extensions container for the new-format container
+	BaseOperatingSystemExtensionsContainer string `json:"baseOperatingSystemExtensionsContainer"`
+
 	// OSImageURL is the old-format container image that contains the OS update payload.
 	OSImageURL string `json:"osImageURL"`
 
 	// releaseImage is the image used when installing the cluster
 	ReleaseImage string `json:"releaseImage"`
 
 	// proxy holds the current proxy configuration for the nodes
-	// +nullable
+	// +nullables
 	Proxy *configv1.ProxyStatus `json:"proxy"`
 
 	// infra holds the infrastructure details
@@ -197,6 +200,14 @@ type MachineConfigSpec struct {
 	// OSImageURL specifies the remote location that will be used to
 	// fetch the OS.
 	OSImageURL string `json:"osImageURL"`
+
+	// BaseOperatingSystemContainer specifies the remote location that will be used
+	// to fetch the new-format OS image
+	BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"`
+	// BaseOperatingSystemExtensionContainer specifies the remote location that will be used
+	// to fetch the extensions container matching the new-format OS image
+	BaseOperatingSystemExtensionsContainer string `json:"baseOperatingSystemExtensionsContainer"`
+
 	// Config is a Ignition Config object.
 	Config runtime.RawExtension `json:"config"`
 

pkg/controller/common/helpers.go

@@ -55,7 +55,7 @@ func strToPtr(s string) *string {
 // 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.
-func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*mcfgv1.MachineConfig, error) {
+func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) {
 	if len(configs) == 0 {
 		return nil, nil
 	}
@@ -127,7 +127,11 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m
 
 	return &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
+			// TODO(jkyros): for the cconfig fields, sure seems to me like this is just a hacky way to attach these to the pool
+			OSImageURL:                             cconfig.Spec.OSImageURL,
+			BaseOperatingSystemContainer:           cconfig.Spec.BaseOperatingSystemContainer,
+			BaseOperatingSystemExtensionsContainer: cconfig.Spec.BaseOperatingSystemExtensionsContainer,
+
 			KernelArguments: kargs,
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,

pkg/controller/common/helpers_test.go

@@ -233,7 +233,8 @@ func TestParseAndConvert(t *testing.T) {
 
 func TestMergeMachineConfigs(t *testing.T) {
 	// variable setup
-	osImageURL := "testURL"
+	cconfig := &mcfgv1.ControllerConfig{}
+	cconfig.Spec.OSImageURL = "testURL"
 	fips := true
 	kargs := []string{"testKarg"}
 	extensions := []string{"testExtensions"}
@@ -245,7 +246,7 @@ func TestMergeMachineConfigs(t *testing.T) {
 		},
 	}
 	inMachineConfigs := []*mcfgv1.MachineConfig{machineConfigFIPS}
-	mergedMachineConfig, err := MergeMachineConfigs(inMachineConfigs, osImageURL)
+	mergedMachineConfig, err := MergeMachineConfigs(inMachineConfigs, cconfig)
 	require.Nil(t, err)
 
 	// check that the outgoing config does have the version string set,
@@ -259,7 +260,7 @@ func TestMergeMachineConfigs(t *testing.T) {
 	require.Nil(t, err)
 	expectedMachineConfig := &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
+			OSImageURL:      cconfig.Spec.OSImageURL,
 			KernelArguments: []string{},
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,
@@ -323,12 +324,12 @@ func TestMergeMachineConfigs(t *testing.T) {
 		machineConfigIgn,
 		machineConfigFIPS,
 	}
-	mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, osImageURL)
+	mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig)
 	require.Nil(t, err)
 
 	expectedMachineConfig = &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
+			OSImageURL:      cconfig.Spec.OSImageURL,
 			KernelArguments: kargs,
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,

pkg/controller/render/render_controller.go

@@ -558,7 +558,7 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc
 		}
 	}
 
-	merged, err := ctrlcommon.MergeMachineConfigs(configs, cconfig.Spec.OSImageURL)
+	merged, err := ctrlcommon.MergeMachineConfigs(configs, cconfig)
 	if err != nil {
 		return nil, err
 	}

pkg/daemon/update.go

@@ -338,25 +338,49 @@ func (dn *CoreOSDaemon) applyOSChanges(mcDiff machineConfigDiff, oldConfig, newC
 	}
 
 	var osImageContentDir string
-	if mcDiff.osUpdate || mcDiff.extensions || mcDiff.kernelType {
-		// When we're going to apply an OS update, switch the block
-		// scheduler to BFQ to apply more fairness between etcd
-		// and the OS update. Only do this on masters since etcd
-		// only operates on masters, and RHEL compute nodes can't
-		// do this.
-		// Add nil check since firstboot also goes through this path,
-		// which doesn't have a node object yet.
-		if dn.node != nil {
-			if _, isControlPlane := dn.node.Labels[ctrlcommon.MasterLabel]; isControlPlane {
-				if err := setRootDeviceSchedulerBFQ(); err != nil {
-					return err
+	// TODO(jkyros): what about where we go backwards? Like when osImageURL is the same, but baseOSUpdate got blanked out
+	if newConfig.Spec.BaseOperatingSystemContainer != "" {
+		if mcDiff.baseOSUpdate || mcDiff.baseOSExtensionsUpdate || mcDiff.extensions || mcDiff.kernelType {
+			// When we're going to apply an OS update, switch the block
+			// scheduler to BFQ to apply more fairness between etcd
+			// and the OS update. Only do this on masters since etcd
+			// only operates on masters, and RHEL compute nodes can't
+			// do this.
+			// Add nil check since firstboot also goes through this path,
+			// which doesn't have a node object yet.
+			if dn.node != nil {
+				if _, isControlPlane := dn.node.Labels[ctrlcommon.MasterLabel]; isControlPlane {
+					if err := setRootDeviceSchedulerBFQ(); err != nil {
+						return err
+					}
 				}
 			}
+			// We emitted this event before, so keep it
+			if dn.nodeWriter != nil {
+				dn.nodeWriter.Eventf(corev1.EventTypeNormal, "InClusterUpgrade", fmt.Sprintf("Updating from oscontainer %s", newConfig.Spec.OSImageURL))
+			}
+
+			// If this is a new format/ bootable image
+			client := NewNodeUpdaterClient()
+			isBootable, retErr := client.IsBootableImage(newConfig.Spec.OSImageURL)
+			if retErr != nil {
+				// TODO(jkyros): to reduce impact to existing workflows, in the event that we get an error while checking, should we
+				// just fall back to the "regular" image path instead of erroring?
+				return retErr
+			}
+
+			// If this is a "new format"/bootable image, do things the "new" way, instead of the old way where we
+			// extract the image to disk and install the extensions
+			if isBootable {
+				return dn.applyLayeredOSImage(mcDiff, oldConfig, newConfig)
+			}
+
 		}
-		// We emitted this event before, so keep it
-		if dn.nodeWriter != nil {
-			dn.nodeWriter.Eventf(corev1.EventTypeNormal, "InClusterUpgrade", fmt.Sprintf("Updating from oscontainer %s", newConfig.Spec.OSImageURL))
-		}
+	}
+
+	// If we made it here, we know it's the "old way" and we have to extract the image
+	if mcDiff.osUpdate || mcDiff.extensions || mcDiff.kernelType {
+
 		var err error
 		if osImageContentDir, err = ExtractOSImage(newConfig.Spec.OSImageURL); err != nil {
 			return err
@@ -739,14 +763,16 @@ func (dn *Daemon) removeRollback() error {
 // and the MCO would just operate on that.  For now we're just doing this to get
 // improved logging.
 type machineConfigDiff struct {
-	osUpdate   bool
-	kargs      bool
-	fips       bool
-	passwd     bool
-	files      bool
-	units      bool
-	kernelType bool
-	extensions bool
+	osUpdate               bool
+	baseOSUpdate           bool
+	baseOSExtensionsUpdate bool
+	kargs                  bool
+	fips                   bool
+	passwd                 bool
+	files                  bool
+	units                  bool
+	kernelType             bool
+	extensions             bool
 }
 
 // isEmpty returns true if the machineConfigDiff has no changes, or
@@ -799,14 +825,16 @@ func newMachineConfigDiff(oldConfig, newConfig *mcfgv1.MachineConfig) (*machineC
 	extensionsEmpty := len(oldConfig.Spec.Extensions) == 0 && len(newConfig.Spec.Extensions) == 0
 
 	return &machineConfigDiff{
-		osUpdate:   oldConfig.Spec.OSImageURL != newConfig.Spec.OSImageURL,
-		kargs:      !(kargsEmpty || reflect.DeepEqual(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments)),
-		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),
-		kernelType: canonicalizeKernelType(oldConfig.Spec.KernelType) != canonicalizeKernelType(newConfig.Spec.KernelType),
-		extensions: !(extensionsEmpty || reflect.DeepEqual(oldConfig.Spec.Extensions, newConfig.Spec.Extensions)),
+		osUpdate:               oldConfig.Spec.OSImageURL != newConfig.Spec.OSImageURL,
+		baseOSUpdate:           oldConfig.Spec.BaseOperatingSystemContainer != newConfig.Spec.BaseOperatingSystemContainer,
+		baseOSExtensionsUpdate: oldConfig.Spec.BaseOperatingSystemContainer != newConfig.Spec.BaseOperatingSystemExtensionsContainer,
+		kargs:                  !(kargsEmpty || reflect.DeepEqual(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments)),
+		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),
+		kernelType:             canonicalizeKernelType(oldConfig.Spec.KernelType) != canonicalizeKernelType(newConfig.Spec.KernelType),
+		extensions:             !(extensionsEmpty || reflect.DeepEqual(oldConfig.Spec.Extensions, newConfig.Spec.Extensions)),
 	}, nil
 }
 
@@ -1217,6 +1245,84 @@ func (dn *CoreOSDaemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig)
 	return nil
 }
 
+// applyBootableOSImage performs an os update/rebase using
+func (dn *CoreOSDaemon) applyLayeredOSImage(mcDiff machineConfigDiff, oldConfig, newConfig *mcfgv1.MachineConfig) (retErr error) {
+
+	newURL := newConfig.Spec.BaseOperatingSystemContainer
+	glog.Infof("Updating OS to layered image %s", newURL)
+	client := NewNodeUpdaterClient()
+
+	isBootable, err := client.IsBootableImage(newURL)
+	if err != nil {
+		return err
+	}
+
+	if !isBootable {
+		return fmt.Errorf("Image %s is NOT a bootable container image", newURL)
+	}
+
+	booted, staged, err := client.GetBootedAndStagedDeployment()
+	if err != nil {
+		return err
+	}
+
+	// TODO(jkyros): this is ignoring live-apply for now, but since MCD behavior is "the usual", we don't need it yet
+	onDesired, _ := onDesiredImage(newURL, booted, staged)
+
+	if !onDesired {
+		if err := client.RebaseLayered(newURL); err != nil {
+			nodeName := ""
+			if dn.node != nil {
+				nodeName = dn.node.Name
+			}
+			MCDPivotErr.WithLabelValues(nodeName, newURL, err.Error()).SetToCurrentTime()
+			return err
+		}
+		if dn.nodeWriter != nil {
+			dn.nodeWriter.Eventf(corev1.EventTypeNormal, "OSUpgradeApplied", "OS upgrade applied; new MachineConfig (%s) has new OS image (%s)", newConfig.Name, newConfig.Spec.OSImageURL)
+		}
+
+		defer func() {
+			// Operations performed by rpm-ostree on the booted system are available
+			// as staged deployment. It gets applied only when we reboot the system.
+			// In case of an error during any rpm-ostree transaction, removing pending deployment
+			// should be sufficient to discard any applied changes.
+			if retErr != nil {
+				// Print out the error now so that if we fail to cleanup -p, we don't lose it.
+				glog.Infof("Rolling back applied changes to OS due to error: %v", retErr)
+				if err := removePendingDeployment(); err != nil {
+					errs := kubeErrs.NewAggregate([]error{err, retErr})
+					retErr = fmt.Errorf("error removing staged deployment: %w", errs)
+					return
+				}
+			}
+		}()
+
+	} else {
+		glog.Infof("Already on desired image %s", newURL)
+	}
+
+	// Apply kargs, we can still do this the "old" way since they aren't packed into the image yet
+	if mcDiff.kargs {
+		if err := dn.updateKernelArguments(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments); err != nil {
+			return err
+		}
+	}
+
+	// Switch to real time kernel
+	if err := dn.switchKernel(oldConfig, newConfig); err != nil {
+		return err
+	}
+
+	// TODO(jkyros): This is where we will handle Joseph's extensions container
+	glog.Infof("Can't apply extensions, not supported yet")
+
+	if dn.nodeWriter != nil {
+		dn.nodeWriter.Eventf(corev1.EventTypeNormal, "OSUpdateStaged", "Changes to OS staged")
+	}
+	return nil
+}
+
 // updateFiles writes files specified by the nodeconfig to disk. it also writes
 // systemd units. there is no support for multiple filesystems at this point.
 //

pkg/operator/bootstrap.go

@@ -140,6 +140,7 @@ func RenderBootstrap(
 	spec.PullSecret = nil
 	spec.OSImageURL = imgs.MachineOSContent
 	spec.BaseOperatingSystemContainer = imgs.BaseOperatingSystemContainer
+	spec.BaseOperatingSystemExtensionsContainer = imgs.BaseOperatingSystemExtensionsContainer
 	spec.ReleaseImage = releaseImage
 	spec.Images = map[string]string{
 		templatectrl.MachineConfigOperatorKey: imgs.MachineConfigOperator,

pkg/operator/images.go

@@ -19,6 +19,8 @@ type RenderConfigImages struct {
 	MachineOSContent      string `json:"machineOSContent"`
 	// The new format image
 	BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"`
+	// The matching extensions container for the new format image
+	BaseOperatingSystemExtensionsContainer string `json:"baseOperatingSystemExtensionsContainer"`
 	// These have to be named differently from the ones in ControllerConfigImages
 	// or we get errors about ambiguous selectors because both structs are
 	// combined in the Images struct.