[WIP] Teach MCO to use the new-format image by default

cmd/machine-config-operator/bootstrap.go

@@ -2,10 +2,14 @@ package main
 
 import (
 	"flag"
+	"fmt"
+	"io/ioutil"
 
 	"github.com/golang/glog"
 	"github.com/spf13/cobra"
 
+	imagev1 "github.com/openshift/api/image/v1"
+	"github.com/openshift/library-go/pkg/operator/resource/resourceread"
 	"github.com/openshift/machine-config-operator/pkg/operator"
 	"github.com/openshift/machine-config-operator/pkg/version"
 )
@@ -19,28 +23,31 @@ var (
 	}
 
 	bootstrapOpts struct {
-		baremetalRuntimeCfgImage  string
-		cloudConfigFile           string
-		configFile                string
-		cloudProviderCAFile       string
-		corednsImage              string
-		destinationDir            string
-		haproxyImage              string
-		imagesConfigMapFile       string
-		infraConfigFile           string
-		infraImage                string
-		releaseImage              string
-		keepalivedImage           string
-		kubeCAFile                string
-		mcoImage                  string
-		oauthProxyImage           string
-		networkConfigFile         string
-		oscontentImage            string
-		pullSecretFile            string
-		rootCAFile                string
-		proxyConfigFile           string
-		additionalTrustBundleFile string
-		dnsConfigFile             string
+		baremetalRuntimeCfgImage               string
+		cloudConfigFile                        string
+		configFile                             string
+		cloudProviderCAFile                    string
+		corednsImage                           string
+		destinationDir                         string
+		haproxyImage                           string
+		imagesConfigMapFile                    string
+		infraConfigFile                        string
+		infraImage                             string
+		releaseImage                           string
+		keepalivedImage                        string
+		kubeCAFile                             string
+		mcoImage                               string
+		oauthProxyImage                        string
+		networkConfigFile                      string
+		oscontentImage                         string
+		pullSecretFile                         string
+		rootCAFile                             string
+		proxyConfigFile                        string
+		additionalTrustBundleFile              string
+		dnsConfigFile                          string
+		imageReferences                        string
+		baseOperatingSystemContainer           string
+		baseOperatingSystemExtensionsContainer string
 	}
 )
 
@@ -72,25 +79,76 @@ func init() {
 	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.haproxyImage, "haproxy-image", "", "Image for haproxy.")
 	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.baremetalRuntimeCfgImage, "baremetal-runtimecfg-image", "", "Image for baremetal-runtimecfg.")
 	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.oauthProxyImage, "oauth-proxy-image", "", "Image for origin oauth proxy.")
+	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.imageReferences, "image-references", "", "File containing imagestreams (from cluster-version-operator)")
 	bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.cloudProviderCAFile, "cloud-provider-ca-file", "", "path to cloud provider CA certificate")
 
 }
 
+// findImage returns the image with a particular tag in an imagestream.
+func findImage(stream *imagev1.ImageStream, name string) (string, error) {
+	for _, tag := range stream.Spec.Tags {
+		if tag.Name == name {
+			// we found the short name in ImageStream
+			if tag.From != nil && tag.From.Kind == "DockerImage" {
+				return tag.From.Name, nil
+			}
+		}
+	}
+	return "", fmt.Errorf("could not find %s in images", name)
+}
+
+func findImageOrDie(stream *imagev1.ImageStream, name string) string {
+	img, err := findImage(stream, name)
+	if err != nil {
+		glog.Fatalf("Failed to find %s in image references", name)
+	}
+	return img
+}
+
 func runBootstrapCmd(cmd *cobra.Command, args []string) {
 	flag.Set("logtostderr", "true")
 	flag.Parse()
 
 	// To help debugging, immediately log version
 	glog.Infof("Version: %+v (%s)", version.Raw, version.Hash)
 
+	if bootstrapOpts.imageReferences != "" {
+		imageRefData, err := ioutil.ReadFile(bootstrapOpts.imageReferences)
+		if err != nil {
+			glog.Fatalf("failed to read %s: %v", bootstrapOpts.imageReferences, err)
+		}
+
+		imgstream := resourceread.ReadImageStreamV1OrDie(imageRefData)
+
+		bootstrapOpts.mcoImage = findImageOrDie(imgstream, "machine-config-operator")
+		bootstrapOpts.oscontentImage = findImageOrDie(imgstream, "machine-os-content")
+		bootstrapOpts.keepalivedImage = findImageOrDie(imgstream, "keepalived-ipfailover")
+		bootstrapOpts.corednsImage = findImageOrDie(imgstream, "coredns")
+		bootstrapOpts.baremetalRuntimeCfgImage = findImageOrDie(imgstream, "baremetal-runtimecfg")
+		// TODO: Hmm, this one doesn't actually seem to be passed right now at bootstrap time by the installer
+		bootstrapOpts.oauthProxyImage = findImageOrDie(imgstream, "oauth-proxy")
+		bootstrapOpts.infraImage = findImageOrDie(imgstream, "pod")
+		bootstrapOpts.haproxyImage = findImageOrDie(imgstream, "haproxy-router")
+		bootstrapOpts.baseOperatingSystemContainer, err = findImage(imgstream, "rhel-coreos-8")
+		if err != nil {
+			glog.Warningf("Base OS container not found: %s", err)
+		}
+		bootstrapOpts.baseOperatingSystemExtensionsContainer, err = findImage(imgstream, "rhel-coreos-8-extensions")
+		if err != nil {
+			glog.Warningf("Base OS extensions container not found: %s", err)
+		}
+	}
+
 	imgs := operator.Images{
 		RenderConfigImages: operator.RenderConfigImages{
-			MachineConfigOperator:        bootstrapOpts.mcoImage,
-			MachineOSContent:             bootstrapOpts.oscontentImage,
-			KeepalivedBootstrap:          bootstrapOpts.keepalivedImage,
-			CorednsBootstrap:             bootstrapOpts.corednsImage,
-			BaremetalRuntimeCfgBootstrap: bootstrapOpts.baremetalRuntimeCfgImage,
-			OauthProxy:                   bootstrapOpts.oauthProxyImage,
+			MachineConfigOperator:                  bootstrapOpts.mcoImage,
+			MachineOSContent:                       bootstrapOpts.oscontentImage,
+			KeepalivedBootstrap:                    bootstrapOpts.keepalivedImage,
+			CorednsBootstrap:                       bootstrapOpts.corednsImage,
+			BaremetalRuntimeCfgBootstrap:           bootstrapOpts.baremetalRuntimeCfgImage,
+			OauthProxy:                             bootstrapOpts.oauthProxyImage,
+			BaseOperatingSystemContainer:           bootstrapOpts.baseOperatingSystemContainer,
+			BaseOperatingSystemExtensionsContainer: bootstrapOpts.baseOperatingSystemExtensionsContainer,
 		},
 		ControllerConfigImages: operator.ControllerConfigImages{
 			InfraImage:          bootstrapOpts.infraImage,

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_02_images.configmap.yaml

@@ -11,11 +11,11 @@ data:
   images.json: >
     {
       "releaseVersion": "0.0.1-snapshot",
-      "machineConfigOperator": "registry.svc.ci.openshift.org/openshift:machine-config-operator",
-      "infraImage": "registry.svc.ci.openshift.org/openshift:pod",
-      "keepalivedImage": "registry.svc.ci.openshift.org/openshift:keepalived-ipfailover",
-      "corednsImage": "registry.svc.ci.openshift.org/openshift:coredns",
-      "haproxyImage": "registry.svc.ci.openshift.org/openshift:haproxy-router",
-      "baremetalRuntimeCfgImage": "registry.svc.ci.openshift.org/openshift:baremetal-runtimecfg",
-      "oauthProxy": "registry.svc.ci.openshift.org/openshift:oauth-proxy"
+      "machineConfigOperator": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-config-operator",
+      "infraImage": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:pod",
+      "keepalivedImage": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:keepalived-ipfailover",
+      "corednsImage": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:coredns",
+      "haproxyImage": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:haproxy-router",
+      "baremetalRuntimeCfgImage": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:baremetal-runtimecfg",
+      "oauthProxy": "placeholder.url.oc.will.replace.this.org/placeholdernamespace:oauth-proxy"
     }

install/0000_80_machine-config-operator_04_deployment.yaml

@@ -23,7 +23,7 @@ spec:
     spec:
       containers:
       - name: machine-config-operator
-        image: registry.svc.ci.openshift.org/openshift:machine-config-operator
+        image: placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-config-operator
         args:
         - "start"
         - "--images-json=/etc/mco/images/images.json"

install/0000_80_machine-config-operator_05_osimageurl.yaml

@@ -9,6 +9,11 @@ metadata:
     include.release.openshift.io/single-node-developer: "true"
 data:
   releaseVersion: 0.0.1-snapshot
-  # The OS payload, managed by the daemon + pivot + rpm-ostree
-  # https://github.com/openshift/machine-config-operator/issues/183
-  osImageURL: "registry.svc.ci.openshift.org/openshift:machine-os-content"
+  # This (will eventually) replace the below when https://github.com/openshift/enhancements/pull/1032
+  # progresses towards the default.
+  baseOperatingSystemContainer: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8"
+  baseOperatingSystemExtensionsContainer: "placeholder.url.oc.will.replace.this.org/placeholdernamespace: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 )
+  osImageURL: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-os-content"

install/image-references

@@ -7,35 +7,43 @@ spec:
   - name: machine-config-operator
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:machine-config-operator
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-config-operator
   - name: pod
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:pod
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:pod
   - name: oauth-proxy
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:oauth-proxy
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:oauth-proxy
   # This one is special, it's the OS payload
   # https://github.com/openshift/machine-config-operator/issues/183
   # See the machine-config-osimageurl configmap.
   - name: machine-os-content
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:machine-os-content
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-os-content
+  - name: rhel-coreos-8
+    from:
+      kind: DockerImage
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8
+  - name: rhel-coreos-8-extensions
+    from:
+      kind: DockerImage
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8-extensions
   - name: keepalived-ipfailover
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:keepalived-ipfailover
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:keepalived-ipfailover
   - name: coredns
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:coredns
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:coredns
   - name: haproxy-router
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:haproxy-router
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:haproxy-router
   - name: baremetal-runtimecfg
     from:
       kind: DockerImage
-      name: registry.svc.ci.openshift.org/openshift:baremetal-runtimecfg
+      name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:baremetal-runtimecfg

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

@@ -918,6 +918,14 @@ spec:
                   contains the OS update payload. Its value is taken from the data.osImageURL
                   field on the machine-config-osimageurl ConfigMap.
                 type: string
+              baseOperatingSystemContainer:
+                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
@@ -992,6 +1000,7 @@ spec:
             - ipFamilies
             - kubeAPIServerServingCAData
             - osImageURL
+            - baseOperatingSystemContainer
             - proxy
             - releaseImage
             - rootCAData

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

@@ -72,8 +72,13 @@ type ControllerConfigSpec struct {
 	// images is map of images that are used by the controller to render templates under ./templates/
 	Images map[string]string `json:"images"`
 
-	// osImageURL is the location of the container image that contains the OS update payload.
-	// Its value is taken from the data.osImageURL field on the machine-config-osimageurl ConfigMap.
+	// 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
@@ -195,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,