Merge pull request #3317 from jkyros/mco-289-extensions-container

MCO-289: Teach the MCO to use new format image

Dockerfile

@@ -11,7 +11,14 @@ ARG TAGS=""
 COPY --from=builder /go/src/github.com/openshift/machine-config-operator/instroot.tar /tmp/instroot.tar
 RUN cd / && tar xf /tmp/instroot.tar && rm -f /tmp/instroot.tar
 COPY install /manifests
-RUN if [[ "${TAGS}" == "fcos" ]]; then sed -i 's/rhel-coreos-8/fedora-coreos/g' /manifests/*; \
+
+RUN if [[ "${TAGS}" == "fcos" ]]; then \
+    # comment out non-base/extensions image-references entirely for fcos
+    sed -i '/- name: rhel-coreos-8-/,+3 s/^/#/' /manifests/image-references && \
+    # also remove extensions from the osimageurl configmap (if we don't, oc won't rewrite it, and the placeholder value will survive and get used)
+    sed -i '/baseOSExtensionsContainerImage:/ s/^/#/' /manifests/0000_80_machine-config-operator_05_osimageurl.yaml && \
+    # rewrite image names for fcos 
+    sed -i 's/rhel-coreos-8/fedora-coreos/g' /manifests/* ; \
     elif [[ "${TAGS}" == "scos" ]]; then sed -i 's/rhel-coreos-8/centos-stream-coreos-9/g' /manifests/*; fi && \
     if ! rpm -q util-linux; then yum install -y util-linux && yum clean all && rm -rf /var/cache/yum/*; fi
 COPY templates /etc/mcc/templates

cmd/machine-config-operator/bootstrap.go

@@ -23,31 +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
-		imageReferences                        string
-		baseOperatingSystemContainer           string
-		baseOperatingSystemExtensionsContainer 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
+		baseOSContainerImage           string
+		baseOSExtensionsContainerImage string
 	}
 )
 
@@ -136,26 +136,26 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) {
 		bootstrapOpts.oauthProxyImage = findImageOrDie(imgstream, "oauth-proxy")
 		bootstrapOpts.infraImage = findImageOrDie(imgstream, "pod")
 		bootstrapOpts.haproxyImage = findImageOrDie(imgstream, "haproxy-router")
-		bootstrapOpts.baseOperatingSystemContainer, err = findImage(imgstream, baseOSContainerImageTag)
+		bootstrapOpts.baseOSContainerImage, err = findImage(imgstream, baseOSContainerImageTag)
 		if err != nil {
 			glog.Warningf("Base OS container not found: %s", err)
 		}
-		bootstrapOpts.baseOperatingSystemExtensionsContainer, err = findImage(imgstream, fmt.Sprintf("%s-extensions", baseOSContainerImageTag))
+		bootstrapOpts.baseOSExtensionsContainerImage, err = findImage(imgstream, fmt.Sprintf("%s-extensions", baseOSContainerImageTag))
 		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,
-			BaseOperatingSystemContainer:           bootstrapOpts.baseOperatingSystemContainer,
-			BaseOperatingSystemExtensionsContainer: bootstrapOpts.baseOperatingSystemExtensionsContainer,
+			MachineConfigOperator:          bootstrapOpts.mcoImage,
+			MachineOSContent:               bootstrapOpts.oscontentImage,
+			KeepalivedBootstrap:            bootstrapOpts.keepalivedImage,
+			CorednsBootstrap:               bootstrapOpts.corednsImage,
+			BaremetalRuntimeCfgBootstrap:   bootstrapOpts.baremetalRuntimeCfgImage,
+			OauthProxy:                     bootstrapOpts.oauthProxyImage,
+			BaseOSContainerImage:           bootstrapOpts.baseOSContainerImage,
+			BaseOSExtensionsContainerImage: bootstrapOpts.baseOSExtensionsContainerImage,
 		},
 		ControllerConfigImages: operator.ControllerConfigImages{
 			InfraImage:          bootstrapOpts.infraImage,

go.mod

@@ -285,7 +285,7 @@ require (
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/ini.v1 v1.66.6 // indirect
 	gopkg.in/square/go-jose.v2 v2.6.0 // indirect
-	gopkg.in/yaml.v2 v2.4.0 // indirect
+	gopkg.in/yaml.v2 v2.4.0
 	gopkg.in/yaml.v3 v3.0.1 // indirect
 	honnef.co/go/tools v0.3.3 // indirect
 	k8s.io/apiserver v0.25.1 // indirect

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

@@ -66,6 +66,10 @@ spec:
             description: MachineConfigSpec is the spec for MachineConfig
             type: object
             properties:
+              baseOSExtensionsContainerImage:
+                description: baseOSExtensionsContainerImage specifies the remote location that will be used
+                  to fetch the extensions container matching a new-format OS image
+                type: string
               config:
                 description: Config is a Ignition Config object.
                 type: object

install/0000_80_machine-config-operator_05_osimageurl.yaml

@@ -11,8 +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: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8"
-  #baseOperatingSystemExtensionsContainer: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8-extensions"
+  baseOSContainerImage: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8"
+  baseOSExtensionsContainerImage: "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 )

install/image-references

@@ -27,11 +27,10 @@ spec:
     from:
       kind: DockerImage
       name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8
-  # Uncomment this after https://issues.redhat.com/browse/COS-1646 lands
-  # - name: rhel-coreos-8-extensions
-  #   from:
-  #     kind: DockerImage
-  #     name: placeholder.url.oc.will.replace.this.org/placeholdernamespace:rhel-coreos-8-extensions
+  - 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

lib/resourcemerge/machineconfig.go

@@ -47,6 +47,7 @@ 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.BaseOSExtensionsContainerImage, required.BaseOSExtensionsContainerImage)
 
 	if !equality.Semantic.DeepEqual(existing.KernelArguments, required.KernelArguments) {
 		*modified = true
@@ -72,8 +73,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.BaseOSContainerImage, required.BaseOSContainerImage)
+	resourcemerge.SetStringIfSet(modified, &existing.BaseOSExtensionsContainerImage, required.BaseOSExtensionsContainerImage)
 	resourcemerge.SetStringIfSet(modified, &existing.NetworkType, required.NetworkType)
 
 	setBytesIfSet(modified, &existing.AdditionalTrustBundle, required.AdditionalTrustBundle)

manifests/controllerconfig.crd.yaml

@@ -968,12 +968,12 @@ 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.
+              baseOSContainerImage:
+                description: baseOSContainerImage 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.
+              baseOSExtensionsContainerImage:
+                description: baseOSExtensionsContainerImage is the extensions container matching new format operating system update image.
                   See https://github.com/openshift/enhancements/pull/1032
                 type: string
               platform:
@@ -1050,7 +1050,7 @@ spec:
             - ipFamilies
             - kubeAPIServerServingCAData
             - osImageURL
-            - baseOperatingSystemContainer
+            - baseOSContainerImage
             - proxy
             - releaseImage
             - rootCAData

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

@@ -72,11 +72,11 @@ 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"`
 
-	// BaseOperatingSystemContainer is the new-format container image for operating system updates.
-	BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"`
+	// BaseOSContainerImage is the new-format container image for operating system updates.
+	BaseOSContainerImage string `json:"baseOSContainerImage"`
 
-	// BaseOperatingSystemExtensionsContainer is the matching extensions container for the new-format container
-	BaseOperatingSystemExtensionsContainer string `json:"baseOperatingSystemExtensionsContainer"`
+	// BaseOSExtensionsContainerImage is the matching extensions container for the new-format container
+	BaseOSExtensionsContainerImage string `json:"baseOSExtensionsContainerImage"`
 
 	// OSImageURL is the old-format container image that contains the OS update payload.
 	OSImageURL string `json:"osImageURL"`
@@ -200,6 +200,11 @@ type MachineConfigSpec struct {
 	// OSImageURL specifies the remote location that will be used to
 	// fetch the OS.
 	OSImageURL string `json:"osImageURL"`
+
+	// BaseOSExtensionsContainerImage specifies the remote location that will be used
+	// to fetch the extensions container matching a new-format OS image
+	BaseOSExtensionsContainerImage string `json:"baseOSExtensionsContainerImage"`
+
 	// Config is a Ignition Config object.
 	Config runtime.RawExtension `json:"config"`
 

pkg/controller/common/constants.go

@@ -10,6 +10,9 @@ const (
 	// ReleaseImageVersionAnnotationKey is used to tag the rendered machineconfigs & controller config with the release image version.
 	ReleaseImageVersionAnnotationKey = "machineconfiguration.openshift.io/release-image-version"
 
+	// OSImageURLOverriddenKey is used to tag a rendered machineconfig when OSImageURL has been overridden from default using machineconfig
+	OSImageURLOverriddenKey = "machineconfiguration.openshift.io/os-image-url-overridden"
+
 	// ControllerConfigName is the name of the ControllerConfig object that controllers use
 	ControllerConfigName = "machine-config-controller"
 

pkg/controller/common/helpers.go

@@ -48,6 +48,9 @@ import (
 	mcfgclientset "github.com/openshift/machine-config-operator/pkg/generated/clientset/versioned"
 )
 
+// Gates whether or not the MCO uses the new format base OS container image by default
+var UseNewFormatImageByDefault = true
+
 // strToPtr converts the input string to a pointer to itself
 func strToPtr(s string) *string {
 	return &s
@@ -58,7 +61,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 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) {
+func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, cconfig *mcfgv1.ControllerConfig) (*mcfgv1.MachineConfig, error) {
 	if len(configs) == 0 {
 		return nil, nil
 	}
@@ -129,21 +132,28 @@ func MergeMachineConfigs(configs []*mcfgv1.MachineConfig, osImageURL string) (*m
 	}
 
 	// For layering, we want to let the user override OSImageURL again
-	overriddenOSImageURL := ""
+	// The template configs always match what's in controllerconfig because they get rendered from there,
+	// so the only way we get an override here is if the user adds something different
+	osImageURL := GetDefaultBaseImageContainer(&cconfig.Spec)
 	for _, cfg := range configs {
 		if cfg.Spec.OSImageURL != "" {
-			overriddenOSImageURL = cfg.Spec.OSImageURL
+			osImageURL = cfg.Spec.OSImageURL
 		}
 	}
-	// Make sure it's obvious in the logs that it was overridden
-	if overriddenOSImageURL != "" && overriddenOSImageURL != osImageURL {
-		osImageURL = overriddenOSImageURL
+
+	// Allow overriding the extensions container
+	baseOSExtensionsContainerImage := cconfig.Spec.BaseOSExtensionsContainerImage
+	for _, cfg := range configs {
+		if cfg.Spec.BaseOSExtensionsContainerImage != "" {
+			baseOSExtensionsContainerImage = cfg.Spec.BaseOSExtensionsContainerImage
+		}
 	}
 
 	return &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
-			KernelArguments: kargs,
+			OSImageURL:                     osImageURL,
+			BaseOSExtensionsContainerImage: baseOSExtensionsContainerImage,
+			KernelArguments:                kargs,
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,
 			},
@@ -971,3 +981,12 @@ func GetLongestValidCertificate(certificateList []*x509.Certificate, subjectPref
 	}
 	return nil
 }
+
+// GetDefaultBaseImageContainer is kind of a "soft feature gate" for using the "new format" image by default, its behavior
+// is determined by the "UseNewFormatImageByDefault" boolean
+func GetDefaultBaseImageContainer(cconfigspec *mcfgv1.ControllerConfigSpec) string {
+	if UseNewFormatImageByDefault {
+		return cconfigspec.BaseOSContainerImage
+	}
+	return cconfigspec.OSImageURL
+}

pkg/controller/common/helpers_test.go

@@ -234,7 +234,9 @@ func TestParseAndConvert(t *testing.T) {
 
 func TestMergeMachineConfigs(t *testing.T) {
 	// variable setup
-	osImageURL := "testURL"
+	cconfig := &mcfgv1.ControllerConfig{}
+	cconfig.Spec.OSImageURL = "testURL"
+	cconfig.Spec.BaseOSContainerImage = "newformatURL"
 	fips := true
 	kargs := []string{"testKarg"}
 	extensions := []string{"testExtensions"}
@@ -246,7 +248,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,
@@ -260,7 +262,8 @@ func TestMergeMachineConfigs(t *testing.T) {
 	require.Nil(t, err)
 	expectedMachineConfig := &mcfgv1.MachineConfig{
 		Spec: mcfgv1.MachineConfigSpec{
-			OSImageURL:      osImageURL,
+			// TODO(jkyros): take this back out when we drop machine-os-content
+			OSImageURL:      GetDefaultBaseImageContainer(&cconfig.Spec),
 			KernelArguments: []string{},
 			Config: runtime.RawExtension{
 				Raw: rawOutIgn,
@@ -325,7 +328,7 @@ func TestMergeMachineConfigs(t *testing.T) {
 		machineConfigIgn,
 		machineConfigFIPS,
 	}
-	mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, osImageURL)
+	mergedMachineConfig, err = MergeMachineConfigs(inMachineConfigs, cconfig)
 	require.Nil(t, err)
 
 	expectedMachineConfig = &mcfgv1.MachineConfig{