diff --git a/Dockerfile b/Dockerfile index ff144d794b..ab4d4470d4 100644 --- a/Dockerfile +++ b/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 diff --git a/cmd/machine-config-operator/bootstrap.go b/cmd/machine-config-operator/bootstrap.go index 7cdf60abff..e092ff9d2f 100644 --- a/cmd/machine-config-operator/bootstrap.go +++ b/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,11 +136,11 @@ 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) } @@ -148,14 +148,14 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) { 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, diff --git a/go.mod b/go.mod index 961c871047..6deec1fd5d 100644 --- a/go.mod +++ b/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 diff --git a/install/0000_80_machine-config-operator_01_machineconfig.crd.yaml b/install/0000_80_machine-config-operator_01_machineconfig.crd.yaml index 10608f76e1..9161b821b8 100644 --- a/install/0000_80_machine-config-operator_01_machineconfig.crd.yaml +++ b/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 diff --git a/install/0000_80_machine-config-operator_05_osimageurl.yaml b/install/0000_80_machine-config-operator_05_osimageurl.yaml index 122359a5be..bf49ba613b 100644 --- a/install/0000_80_machine-config-operator_05_osimageurl.yaml +++ b/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 ) diff --git a/install/image-references b/install/image-references index 6211bbbeb3..4170492251 100644 --- a/install/image-references +++ b/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 diff --git a/lib/resourcemerge/machineconfig.go b/lib/resourcemerge/machineconfig.go index c3aed344a1..f4185d21fc 100644 --- a/lib/resourcemerge/machineconfig.go +++ b/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) diff --git a/manifests/controllerconfig.crd.yaml b/manifests/controllerconfig.crd.yaml index eeda4d94c8..56024d0c7f 100644 --- a/manifests/controllerconfig.crd.yaml +++ b/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 diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index cdc12b4eec..5d64a9de3f 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/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"` diff --git a/pkg/controller/common/constants.go b/pkg/controller/common/constants.go index aa1fa75961..12177c3618 100644 --- a/pkg/controller/common/constants.go +++ b/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" diff --git a/pkg/controller/common/helpers.go b/pkg/controller/common/helpers.go index c2b1428b27..3f0d32c792 100644 --- a/pkg/controller/common/helpers.go +++ b/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 +} diff --git a/pkg/controller/common/helpers_test.go b/pkg/controller/common/helpers_test.go index 156a926fd6..5089b5c1cb 100644 --- a/pkg/controller/common/helpers_test.go +++ b/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{ diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index 49fb33213f..fe4afe1bc3 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -488,7 +488,7 @@ func (ctrl *Controller) syncGeneratedMachineConfig(pool *mcfgv1.MachineConfigPoo } // Emit event and collect metric when OSImageURL was overridden. - if generated.Spec.OSImageURL != cc.Spec.OSImageURL { + if generated.Spec.OSImageURL != ctrlcommon.GetDefaultBaseImageContainer(&cc.Spec) { ctrlcommon.OSImageURLOverride.WithLabelValues(pool.Name).Set(1) 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) } else { @@ -554,10 +554,10 @@ func generateRenderedMachineConfig(pool *mcfgv1.MachineConfigPool, configs []*mc return nil, fmt.Errorf("Ignoring controller config generated without %s annotation (my version: %s)", daemonconsts.GeneratedByVersionAnnotationKey, version.Raw) } - if cconfig.Spec.BaseOperatingSystemContainer == "" { - glog.Warningf("No BaseOperatingSystemContainer set") + if cconfig.Spec.BaseOSContainerImage == "" { + glog.Warningf("No BaseOSContainerImage set") } else { - glog.Infof("BaseOperatingSystemContainer=%s", cconfig.Spec.BaseOperatingSystemContainer) + glog.Infof("BaseOSContainerImage=%s", cconfig.Spec.BaseOSContainerImage) } // Before merging all MCs for a specific pool, let's make sure MachineConfigs are valid @@ -567,7 +567,8 @@ 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 } @@ -585,10 +586,11 @@ 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) + // The operator needs to know the user overrode this, so it knows if it needs to skip the + // OSImageURL check during upgrade -- if the user took over managing OS upgrades this way, + // the operator shouldn't stop the rest of the upgrade from progressing/completing. + if merged.Spec.OSImageURL != ctrlcommon.GetDefaultBaseImageContainer(&cconfig.Spec) { + merged.Annotations[ctrlcommon.OSImageURLOverriddenKey] = "true" } return merged, nil diff --git a/pkg/controller/template/render.go b/pkg/controller/template/render.go index 35a1ea04a4..48368872a5 100644 --- a/pkg/controller/template/render.go +++ b/pkg/controller/template/render.go @@ -293,8 +293,14 @@ func generateMachineConfigForName(config *RenderConfig, role, name, templateDir, if err != nil { return nil, fmt.Errorf("error creating MachineConfig from Ignition config: %w", err) } + + // TODO(jkyros): you might think you can remove this since we override later when we merge + // config, but resourcemerge doesn't blank this field out once it's populated + // so if you end up on a cluster where it was ever populated in this machineconfig, it + // will keep that last value forever once you upgrade...which is a problen now that we allow OSImageURL overrides + // because it will look like an override when it shouldn't be. So don't take this out until you've solved that. // And inject the osimageurl here - mcfg.Spec.OSImageURL = config.OSImageURL + mcfg.Spec.OSImageURL = ctrlcommon.GetDefaultBaseImageContainer(config.ControllerConfigSpec) return mcfg, nil } diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 91cba1c1de..4b38d4c482 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -64,6 +64,9 @@ type Daemon struct { // bootedOSImageURL is the currently booted URL of the operating system bootedOSImageURL string + // bootedOScommit is the commit hash of the currently booted operating system + bootedOSCommit string + // kubeClient allows interaction with Kubernetes, including the node we are running on. kubeClient kubernetes.Interface @@ -210,6 +213,7 @@ func New( var ( osImageURL string osVersion string + osCommit string err error ) @@ -228,11 +232,11 @@ func New( if err != nil { return nil, fmt.Errorf("error initializing rpm-ostree: %w", err) } - osImageURL, osVersion, err = nodeUpdaterClient.GetBootedOSImageURL() + osImageURL, osVersion, osCommit, err = nodeUpdaterClient.GetBootedOSImageURL() if err != nil { return nil, fmt.Errorf("error reading osImageURL from rpm-ostree: %w", err) } - glog.Infof("Booted osImageURL: %s (%s)", osImageURL, osVersion) + glog.Infof("Booted osImageURL: %s (%s) %s", osImageURL, osVersion, osCommit) } bootID := "" @@ -264,6 +268,7 @@ func New( os: hostos, NodeUpdaterClient: nodeUpdaterClient, bootedOSImageURL: osImageURL, + bootedOSCommit: osCommit, bootID: bootID, exitCh: exitCh, currentConfigPath: currentConfigPath, @@ -884,6 +889,30 @@ func (dn *Daemon) RunFirstbootCompleteMachineconfig() error { if err != nil { return fmt.Errorf("failed to parse MachineConfig: %w", err) } + newEnough, err := RpmOstreeIsNewEnoughForLayering() + if err != nil { + return err + } + + // If the host isn't new enough to understand the new container model natively, run as a privileged container. + // See https://github.com/coreos/rpm-ostree/pull/3961 and https://issues.redhat.com/browse/MCO-356 + // This currently will incur a double reboot; see https://github.com/coreos/rpm-ostree/issues/4018 + if !newEnough { + dn.logSystem("rpm-ostree is not new enough for new-format image; forcing an update via container and queuing immediate reboot") + if err := dn.InplaceUpdateViaNewContainer(mc.Spec.OSImageURL); err != nil { + return err + } + rebootCmd := rebootCommand("extra reboot for in-place update") + if err := rebootCmd.Run(); err != nil { + dn.logSystem("failed to run reboot: %v", err) + return err + } + // wait to be killed via SIGTERM + time.Sleep(defaultRebootTimeout) + return fmt.Errorf("failed to reboot for secondary in-place update") + } + + glog.Info("rpm-ostree has container feature") // Start with an empty config, then add our *booted* osImageURL to // it, reflecting the current machine state. @@ -1456,7 +1485,7 @@ func (dn *Daemon) checkStateOnFirstRun() error { targetOSImageURL := state.currentConfig.Spec.OSImageURL osMatch := dn.checkOS(targetOSImageURL) if !osMatch { - glog.Infof("Bootstrap pivot required to: %s", targetOSImageURL) + dn.logSystem("Bootstrap pivot required to: %s", targetOSImageURL) // Check to see if we have a layered/new format image isLayeredImage, err := dn.NodeUpdaterClient.IsBootableImage(targetOSImageURL) @@ -1487,7 +1516,7 @@ func (dn *Daemon) checkStateOnFirstRun() error { } return dn.reboot(fmt.Sprintf("Node will reboot into config %v", state.currentConfig.GetName())) } - glog.Info("No bootstrap pivot required; unlinking bootstrap node annotations") + dn.logSystem("No bootstrap pivot required; unlinking bootstrap node annotations") // Rename the bootstrap node annotations; the // currentConfig's osImageURL should now be *truth*. @@ -1533,7 +1562,7 @@ func (dn *Daemon) checkStateOnFirstRun() error { } if forceFileExists() { - glog.Infof("Skipping on-disk validation; %s present", constants.MachineConfigDaemonForceFile) + dn.logSystem("Skipping on-disk validation; %s present", constants.MachineConfigDaemonForceFile) return dn.triggerUpdateWithMachineConfig(state.currentConfig, state.desiredConfig) } @@ -1543,7 +1572,7 @@ func (dn *Daemon) checkStateOnFirstRun() error { return wErr } - glog.Info("Validated on-disk state") + dn.logSystem("Validated on-disk state") // We've validated state. Now, ensure that node is in desired state var inDesiredConfig bool @@ -1837,7 +1866,7 @@ func (dn *Daemon) validateOnDiskState(currentConfig *mcfgv1.MachineConfig) error // Be sure we're booted into the OS we expect osMatch := dn.checkOS(currentConfig.Spec.OSImageURL) if !osMatch { - return fmt.Errorf("expected target osImageURL %q, have %q", currentConfig.Spec.OSImageURL, dn.bootedOSImageURL) + return fmt.Errorf("expected target osImageURL %q, have %q (%q)", currentConfig.Spec.OSImageURL, dn.bootedOSImageURL, dn.bootedOSCommit) } if dn.os.IsCoreOSVariant() { @@ -1863,6 +1892,20 @@ func (dn *Daemon) checkOS(osImageURL string) bool { return true } + // TODO(jkyros): the header for this functions says "if the digests match" + // so I'm wondering if at one point this used to work this way.... + // TODO(jkyros): and pulling it just to check is so expensive, skopeo works now + // if you use the --no-tags argument (doesn't pull tags) so we should probably just do that + inspection, err := imageInspect(osImageURL) + if err != nil { + glog.Warningf("Unable to check manifest for matching hash: %s", err) + } else if ostreeCommit, ok := inspection.Labels["ostree.commit"]; ok { + if ostreeCommit == dn.bootedOSCommit { + glog.Infof("We are technically in the right image even if the URL doesn't match (%s == %s)", ostreeCommit, osImageURL) + return true + } + } + return dn.bootedOSImageURL == osImageURL } diff --git a/pkg/daemon/rpm-ostree.go b/pkg/daemon/rpm-ostree.go index e64cf96dda..bca0ad4e17 100644 --- a/pkg/daemon/rpm-ostree.go +++ b/pkg/daemon/rpm-ostree.go @@ -14,6 +14,7 @@ import ( "github.com/golang/glog" "github.com/opencontainers/go-digest" pivotutils "github.com/openshift/machine-config-operator/pkg/daemon/pivot/utils" + "gopkg.in/yaml.v2" ) const ( @@ -37,6 +38,7 @@ type RpmOstreeDeployment struct { OSName string `json:"osname"` Serial int32 `json:"serial"` Checksum string `json:"checksum"` + BaseChecksum string `json:"base-checksum,omitempty"` Version string `json:"version"` Timestamp uint64 `json:"timestamp"` Booted bool `json:"booted"` @@ -67,7 +69,7 @@ type imageInspection struct { type NodeUpdaterClient interface { Initialize() error GetStatus() (string, error) - GetBootedOSImageURL() (string, string, error) + GetBootedOSImageURL() (string, string, string, error) Rebase(string, string) (bool, error) RebaseLayered(string) error IsBootableImage(string) (bool, error) @@ -135,6 +137,13 @@ func (r *RpmOstreeClient) Initialize() error { return err } + // Commands like update and rebase need the pull secrets to pull images and manifests, + // make sure we get access to them when we Initialize + err := useKubeletConfigSecrets() + if err != nil { + return fmt.Errorf("Error while ensuring access to kublet config.json pull secrets: %w", err) + } + return nil } @@ -161,16 +170,21 @@ func (r *RpmOstreeClient) GetStatus() (string, error) { return string(output), nil } -// GetBootedOSImageURL returns the image URL as well as the OSTree version (for logging) +// GetBootedOSImageURL returns the image URL as well as the OSTree version(for logging) and the ostree commit (for comparisons) // 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) { +func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, string, error) { bootedDeployment, _, err := r.GetBootedAndStagedDeployment() if err != nil { - return "", "", err + return "", "", "", err } + // TODO(jkyros): take this out, I just want to see when/why it's empty? + j, _ := json.MarshalIndent(bootedDeployment, "", " ") + glog.Infof("%s", j) + // the canonical image URL is stored in the custom origin field. + osImageURL := "" if len(bootedDeployment.CustomOrigin) > 0 { if strings.HasPrefix(bootedDeployment.CustomOrigin[0], "pivot://") { @@ -186,7 +200,15 @@ func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, error) { osImageURL = tokens[1] } } - return osImageURL, bootedDeployment.Version, nil + + // BaseChecksum is populated if the system has been modified in a way that changes the base checksum + // (like via an RPM install) so prefer BaseCheksum that if it's present, otherwise just use Checksum. + baseChecksum := bootedDeployment.Checksum + if bootedDeployment.BaseChecksum != "" { + baseChecksum = bootedDeployment.BaseChecksum + } + + return osImageURL, bootedDeployment.Version, baseChecksum, nil } func podmanInspect(imgURL string) (imgdata *imageInspection, err error) { @@ -339,16 +361,50 @@ func (r *RpmOstreeClient) IsBootableImage(imgURL string) (bool, error) { 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) +// RpmOstreeIsNewEnoughForLayering returns true if the version of rpm-ostree on the +// host system is new enough for layering. +// VersionData represents the static information about rpm-ostree. +type VersionData struct { + Version string `yaml:"Version"` + Features []string `yaml:"Features"` + Git string `yaml:"Git"` +} + +type RpmOstreeVersionData struct { + Root VersionData `yaml:"rpm-ostree"` +} - // For now, just let ostree use the kublet config.json, - err = useKubeletConfigSecrets() +// RpmOstreeVersion returns the running rpm-ostree version number +func rpmOstreeVersion() (*VersionData, error) { + buf, err := runGetOut("rpm-ostree", "--version") if err != nil { - return fmt.Errorf("Error while ensuring access to kublet config.json pull secrets: %w", err) + return nil, err + } + + var q RpmOstreeVersionData + if err := yaml.Unmarshal(buf, &q); err != nil { + return nil, fmt.Errorf("failed to parse `rpm-ostree --version` output: %w", err) } + return &q.Root, nil +} + +func RpmOstreeIsNewEnoughForLayering() (bool, error) { + verdata, err := rpmOstreeVersion() + if err != nil { + return false, err + } + for _, v := range verdata.Features { + if v == "container" { + return true, nil + } + } + return false, nil +} + +// RebaseLayered rebases system or errors if already rebased +func (r *RpmOstreeClient) RebaseLayered(imgURL string) (err error) { + glog.Infof("Executing rebase to %s", imgURL) return runRpmOstree("rebase", "--experimental", "ostree-unverified-registry:"+imgURL) } diff --git a/pkg/daemon/rpm-ostree_test.go b/pkg/daemon/rpm-ostree_test.go new file mode 100644 index 0000000000..31a8a10dc9 --- /dev/null +++ b/pkg/daemon/rpm-ostree_test.go @@ -0,0 +1,27 @@ +package daemon + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v2" +) + +func TestParseVersion(t *testing.T) { + verdata := `rpm-ostree: + Version: '2022.10' + Git: 6b302116c969397fd71899e3b9bb3b8c100d1af9 + Features: + - rust + - compose + - rhsm +` + var q RpmOstreeVersionData + if err := yaml.UnmarshalStrict([]byte(verdata), &q); err != nil { + panic(err) + } + + assert.Equal(t, "2022.10", q.Root.Version) + assert.Contains(t, q.Root.Features, "rust") + assert.NotContains(t, q.Root.Features, "container") +} diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 476178274f..8bfa1fc0cd 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -42,9 +42,10 @@ const ( // SSH Keys for user "core" will only be written at /home/core/.ssh coreUserSSHPath = "/home/core/.ssh/" // fipsFile is the file to check if FIPS is enabled - fipsFile = "/proc/sys/crypto/fips_enabled" - extensionsRepo = "/etc/yum.repos.d/coreos-extensions.repo" - osImageContentBaseDir = "/run/mco-machine-os-content/" + fipsFile = "/proc/sys/crypto/fips_enabled" + extensionsRepo = "/etc/yum.repos.d/coreos-extensions.repo" + osImageContentBaseDir = "/run/mco-machine-os-content/" + osExtensionsContentBaseDir = "/run/mco-extensions/" // These are the actions for a node to take after applying config changes. (e.g. a new machineconfig is applied) // "None" means no special action needs to be taken @@ -238,6 +239,18 @@ func addExtensionsRepo(osImageContentDir string) error { return nil } +// addLayeredExtensionsRepo adds a repo into /etc/yum.repos.d/ which we use later to +// install extensions and rt-kernel. This is separate from addExtensionsRepo because when we're +// extracting only the extensions container (because with the new format images they are packaged separately), +// we extract to a different location +func addLayeredExtensionsRepo(extensionsImageContentDir string) error { + repoContent := "[coreos-extensions]\nenabled=1\nmetadata_expire=1m\nbaseurl=" + extensionsImageContentDir + "/usr/share/rpm-ostree/extensions/\ngpgcheck=0\nskip_if_unavailable=False\n" + if err := writeFileAtomicallyWithDefaults(extensionsRepo, []byte(repoContent)); err != nil { + return err + } + return nil +} + // podmanRemove kills and removes a container func podmanRemove(cid string) { // Ignore errors here @@ -325,6 +338,38 @@ func ExtractOSImage(imgURL string) (osImageContentDir string, err error) { return } +// ExtractExtensionsImage extracts the OS extensions content in a temporary directory under /run/machine-os-extensions +// and returns the path on successful extraction +func ExtractExtensionsImage(imgURL string) (osExtensionsImageContentDir string, err error) { + var registryConfig []string + if _, err := os.Stat(kubeletAuthFile); err == nil { + registryConfig = append(registryConfig, "--registry-config", kubeletAuthFile) + } + if err = os.MkdirAll(osExtensionsContentBaseDir, 0o755); err != nil { + err = fmt.Errorf("error creating directory %s: %w", osExtensionsContentBaseDir, err) + return + } + + if osExtensionsImageContentDir, err = ioutil.TempDir(osExtensionsContentBaseDir, "os-extensions-content-"); err != nil { + return + } + + // Extract the image + args := []string{"image", "extract", "--path", "/:" + osExtensionsImageContentDir} + args = append(args, registryConfig...) + args = append(args, imgURL) + if _, err = pivotutils.RunExtBackground(cmdRetriesCount, "oc", args...); err != nil { + // Workaround fixes for the environment where oc image extract fails. + // See https://bugzilla.redhat.com/show_bug.cgi?id=1862979 + glog.Infof("Falling back to using podman cp to fetch OS image content") + if err = podmanCopy(imgURL, osExtensionsImageContentDir); err != nil { + return + } + } + + return +} + // Remove pending deployment on OSTree based system func removePendingDeployment() error { return runRpmOstree("cleanup", "-p") @@ -1859,11 +1904,31 @@ func (dn *Daemon) updateOS(config *mcfgv1.MachineConfig, osImageContentDir strin return nil } +// InplaceUpdateViaNewContainer runs rpm-ostree ex deploy-via-self +// via a privileged container. This is needed on firstboot of old +// nodes as well as temporarily for 4.11 -> 4.12 upgrades. +func (dn *Daemon) InplaceUpdateViaNewContainer(target string) error { + return runCmdSync("systemd-run", "--unit", "machine-config-daemon-update-rpmostree-via-container", "--collect", "--wait", "--", "podman", "run", "--authfile", "/var/lib/kubelet/config.json", "--privileged", "--pid=host", "--net=host", "--rm", "-v", "/:/run/host", target, "rpm-ostree", "ex", "deploy-from-self", "/run/host") +} + // updateLayeredOS updates the system OS to the one specified in newConfig func (dn *Daemon) updateLayeredOS(config *mcfgv1.MachineConfig) error { newURL := config.Spec.OSImageURL glog.Infof("Updating OS to layered image %s", newURL) - if err := dn.NodeUpdaterClient.RebaseLayered(newURL); err != nil { + + newEnough, err := RpmOstreeIsNewEnoughForLayering() + if err != nil { + return err + } + // If the host isn't new enough to understand the new container model natively, run as a privileged container. + // See https://github.com/coreos/rpm-ostree/pull/3961 and https://issues.redhat.com/browse/MCO-356 + // This currently will incur a double reboot; see https://github.com/coreos/rpm-ostree/issues/4018 + if !newEnough { + dn.logSystem("rpm-ostree is not new enough for layering; forcing an update via container") + if err := dn.InplaceUpdateViaNewContainer(newURL); err != nil { + return err + } + } else if err := dn.NodeUpdaterClient.RebaseLayered(newURL); err != nil { return fmt.Errorf("failed to update OS to %s : %w", newURL, err) } @@ -2074,6 +2139,26 @@ func (dn *Daemon) reboot(rationale string) error { } func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfig, newConfig *mcfgv1.MachineConfig) (retErr error) { + + var osExtensionsContentDir string + var err error + if mcDiff.extensions || mcDiff.kernelType { + + // TODO(jkyros): the original intent was that we use the extensions container as a service, but that currently results + // in a lot of complexity due to boostrap and firstboot where the service isn't easily available, so for now we are going + // to extract them to disk like we did previously. + if osExtensionsContentDir, err = ExtractExtensionsImage(newConfig.Spec.BaseOSExtensionsContainerImage); err != nil { + return err + } + // Delete extracted OS image once we are done. + defer os.RemoveAll(osExtensionsContentDir) + + if err := addLayeredExtensionsRepo(osExtensionsContentDir); err != nil { + return err + } + defer os.Remove(extensionsRepo) + } + // Update OS if mcDiff.osUpdate { if err := dn.updateLayeredOS(newConfig); err != nil { @@ -2116,15 +2201,14 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi } } - // TODO(jkyros): We can't currently switch kernels on layered images, so only allow it if they're both default. We'll come back for this when it's supported. - // If you did try to switch kernels when using layered image, you would get a "No enabled repositories" error. - if !(canonicalizeKernelType(oldConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault && canonicalizeKernelType(newConfig.Spec.KernelType) == ctrlcommon.KernelTypeDefault) { - return fmt.Errorf("Non-default kernels are not currently supported for layered images. (old: %s new %s)", oldConfig.Spec.KernelType, newConfig.Spec.KernelType) + // 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 - if len(newConfig.Spec.Extensions) > 0 { - return fmt.Errorf("Extensions are not currently supported with layered images, but extensions were supplied: %s", strings.Join(newConfig.Spec.Extensions, " ")) + // Apply extensions + if err := dn.applyExtensions(oldConfig, newConfig); err != nil { + return err } return nil diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go index b146b0ea1c..e167ed7716 100644 --- a/pkg/operator/bootstrap.go +++ b/pkg/operator/bootstrap.go @@ -139,8 +139,8 @@ func RenderBootstrap( spec.RootCAData = bundle spec.PullSecret = nil spec.OSImageURL = imgs.MachineOSContent - spec.BaseOperatingSystemContainer = imgs.BaseOperatingSystemContainer - spec.BaseOperatingSystemExtensionsContainer = imgs.BaseOperatingSystemExtensionsContainer + spec.BaseOSContainerImage = imgs.BaseOSContainerImage + spec.BaseOSExtensionsContainerImage = imgs.BaseOSExtensionsContainerImage spec.ReleaseImage = releaseImage spec.Images = map[string]string{ templatectrl.MachineConfigOperatorKey: imgs.MachineConfigOperator, diff --git a/pkg/operator/images.go b/pkg/operator/images.go index 33a997705b..1d7a4d48e6 100644 --- a/pkg/operator/images.go +++ b/pkg/operator/images.go @@ -18,9 +18,9 @@ type RenderConfigImages struct { MachineConfigOperator string `json:"machineConfigOperator"` MachineOSContent string `json:"machineOSContent"` // The new format image - BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"` + BaseOSContainerImage string `json:"baseOSContainerImage"` // The matching extensions container for the new format image - BaseOperatingSystemExtensionsContainer string `json:"baseOperatingSystemExtensionsContainer"` + BaseOSExtensionsContainerImage string `json:"baseOSExtensionsContainerImage"` // 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. diff --git a/pkg/operator/status.go b/pkg/operator/status.go index a3f9d81e86..d37e81ec56 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -612,8 +612,16 @@ func isMachineConfigPoolConfigurationValid(pool *mcfgv1.MachineConfigPool, versi if err != nil { return err } + + // TODO(jkyros): For "Phase 0" layering, we're going to allow this check to pass once the user has "taken the wheel" by overriding OSImageURL. + // We will find a way to make this more visible to the user somewhere since the MCO is kind of "lying" about completing the + // upgrade to the new os version otherwise. if renderedMC.Spec.OSImageURL != osURL { - return fmt.Errorf("osImageURL mismatch for %s in %s expected: %s got: %s", pool.GetName(), renderedMC.Name, osURL, renderedMC.Spec.OSImageURL) + // If we didn't override OSImageURL, this is still bad, because it means that we aren't on the proper OS image yet + _, ok := renderedMC.Annotations[ctrlcommon.OSImageURLOverriddenKey] + if !ok { + return fmt.Errorf("osImageURL mismatch for %s in %s expected: %s got: %s", pool.GetName(), renderedMC.Name, osURL, renderedMC.Spec.OSImageURL) + } } // check that the rendered config matches the OCP release version for cases where there is no OSImageURL change nor new MCO commit diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go index e9a00fce12..c9a6d30563 100644 --- a/pkg/operator/status_test.go +++ b/pkg/operator/status_test.go @@ -30,9 +30,10 @@ import ( func TestIsMachineConfigPoolConfigurationValid(t *testing.T) { configNotFound := errors.New("Config Not Found") type config struct { - name string - version string - releaseVersion string + name string + version string + releaseVersion string + osimageurlOverridden bool } tests := []struct { @@ -119,22 +120,42 @@ func TestIsMachineConfigPoolConfigurationValid(t *testing.T) { generated: "g", err: errors.New("osImageURL mismatch for dummy-pool in g expected: myurl got: wrongurl"), }, { + // This is specifically testing that we can make sure the operator status check + // allows a mismatched URL if it's a user override for Phase 0 layering knownConfigs: []config{{ - name: "g", - version: "v2", - releaseVersion: "rv1", + name: "g", + version: "v2", + releaseVersion: "rv2", + osimageurlOverridden: true, }, { name: "c-0", version: "v2", - releaseVersion: "rv1", + releaseVersion: "rv2", }, { name: "u-0", }}, source: []string{"c-0", "u-0"}, - testurl: "myurl", + testurl: "overriddenurl", generated: "g", - err: errors.New("release image version mismatch for dummy-pool in g expected: rv2 got: rv1"), - }} + err: nil, + }, + { + knownConfigs: []config{{ + name: "g", + version: "v2", + releaseVersion: "rv1", + }, { + name: "c-0", + version: "v2", + releaseVersion: "rv1", + }, { + name: "u-0", + }}, + source: []string{"c-0", "u-0"}, + testurl: "myurl", + generated: "g", + err: errors.New("release image version mismatch for dummy-pool in g expected: rv2 got: rv1"), + }} for idx, test := range tests { t.Run(fmt.Sprintf("case #%d", idx), func(t *testing.T) { @@ -148,6 +169,9 @@ func TestIsMachineConfigPoolConfigurationValid(t *testing.T) { if c.releaseVersion != "" { annos[ctrlcommon.ReleaseImageVersionAnnotationKey] = c.releaseVersion } + if c.osimageurlOverridden { + annos[ctrlcommon.OSImageURLOverriddenKey] = "true" + } return &mcfgv1.MachineConfig{ ObjectMeta: metav1.ObjectMeta{ Name: c.name, diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 6a7ff40a49..3c1f3d4811 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -260,8 +260,8 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { return err } imgs.MachineOSContent = osimageurl - imgs.BaseOperatingSystemContainer = oscontainer - imgs.BaseOperatingSystemExtensionsContainer = osextensionscontainer + imgs.BaseOSContainerImage = oscontainer + imgs.BaseOSExtensionsContainerImage = osextensionscontainer // sync up the ControllerConfigSpec infra, network, proxy, dns, err := optr.getGlobalConfig() @@ -310,8 +310,8 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { spec.RootCAData = bundle spec.PullSecret = &corev1.ObjectReference{Namespace: "openshift-config", Name: "pull-secret"} spec.OSImageURL = imgs.MachineOSContent - spec.BaseOperatingSystemContainer = imgs.BaseOperatingSystemContainer - spec.BaseOperatingSystemExtensionsContainer = imgs.BaseOperatingSystemExtensionsContainer + spec.BaseOSContainerImage = imgs.BaseOSContainerImage + spec.BaseOSExtensionsContainerImage = imgs.BaseOSExtensionsContainerImage spec.Images = map[string]string{ templatectrl.MachineConfigOperatorKey: imgs.MachineConfigOperator, @@ -692,12 +692,19 @@ func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error { _, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey] if hasRequiredPoolLabel { - _, _, opURL, err := optr.getOsImageURLs(optr.namespace) + newFormatOpURL, _, opURL, err := optr.getOsImageURLs(optr.namespace) if err != nil { glog.Errorf("Error getting configmap osImageURL: %q", err) return false, nil } releaseVersion, _ := optr.vStore.Get("operator") + + // TODO(jkyros): The operator looks at the osimageurl configmap directly, so we can't use + // our centralized default image selection helper, but we can still use the constant. + // This will come out once we drop machine-os-content. + if ctrlcommon.UseNewFormatImageByDefault { + opURL = newFormatOpURL + } if err := isMachineConfigPoolConfigurationValid(pool, version.Hash, releaseVersion, opURL, optr.mcLister.Get); err != nil { lastErr = fmt.Errorf("pool %s has not progressed to latest configuration: %w, retrying", pool.Name, err) syncerr := optr.syncUpgradeableStatus() @@ -875,9 +882,9 @@ func (optr *Operator) getOsImageURLs(namespace string) (string, string, string, return "", "", "", fmt.Errorf("refusing to read osImageURL version %q, operator version %q", releaseVersion, optrVersion) } - newextensions, hasNewExtensions := cm.Data["baseOperatingSystemExtensionsContainer"] + newextensions, hasNewExtensions := cm.Data["baseOSExtensionsContainerImage"] - newformat, hasNewFormat := cm.Data["baseOperatingSystemContainer"] + newformat, hasNewFormat := cm.Data["baseOSContainerImage"] oldformat, hasOldFormat := cm.Data["osImageURL"] @@ -888,7 +895,7 @@ func (optr *Operator) getOsImageURLs(namespace string) (string, string, string, // If we don't have a new format image, and we can't fall back to the old one if !hasOldFormat && !hasNewFormat { - return "", "", "", fmt.Errorf("Missing baseOperatingSystemContainer and osImageURL from configmap") + return "", "", "", fmt.Errorf("Missing baseOSContainerImage and osImageURL from configmap") } return newformat, newextensions, oldformat, nil