From 2b598afee9376d73c8e12d18dcb161f79e3c45e7 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 3 Mar 2022 18:21:22 -0500 Subject: [PATCH 1/6] Add `rhel-coreos` image in configmap, propagate to controllerconfig Part of https://github.com/openshift/enhancements/pull/1032 We'll add the new-format image into the payload alongside the old one until we can complete the transition. (There may actually be a separate `rhel-coreos-extensions` image e.g. too, so this is just the start) Note this PR is just laying groundwork; the new format container will not be used by default. --- ...machine-config-operator_05_osimageurl.yaml | 8 +++++-- install/image-references | 4 ++++ manifests/controllerconfig.crd.yaml | 5 ++++ .../v1/types.go | 6 +++-- pkg/controller/render/render_controller.go | 6 +++++ pkg/operator/bootstrap.go | 1 + pkg/operator/images.go | 2 ++ pkg/operator/sync.go | 23 ++++++++++++++----- 8 files changed, 45 insertions(+), 10 deletions(-) diff --git a/install/0000_80_machine-config-operator_05_osimageurl.yaml b/install/0000_80_machine-config-operator_05_osimageurl.yaml index 612cf74762..d1fe31e2a0 100644 --- a/install/0000_80_machine-config-operator_05_osimageurl.yaml +++ b/install/0000_80_machine-config-operator_05_osimageurl.yaml @@ -9,6 +9,10 @@ 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 + # 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" + # 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: "registry.svc.ci.openshift.org/openshift:machine-os-content" diff --git a/install/image-references b/install/image-references index b186e81c1f..85605021b8 100644 --- a/install/image-references +++ b/install/image-references @@ -23,6 +23,10 @@ spec: from: kind: DockerImage name: registry.svc.ci.openshift.org/openshift:machine-os-content + - name: rhel-coreos + from: + kind: DockerImage + name: registry.ci.openshift.org/openshift:rhel-coreos - name: keepalived-ipfailover from: kind: DockerImage diff --git a/manifests/controllerconfig.crd.yaml b/manifests/controllerconfig.crd.yaml index d26b059bba..29cb49da3c 100644 --- a/manifests/controllerconfig.crd.yaml +++ b/manifests/controllerconfig.crd.yaml @@ -918,6 +918,10 @@ 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 platform: description: platform is deprecated, use Infra.Status.PlatformStatus.Type instead @@ -992,6 +996,7 @@ spec: - ipFamilies - kubeAPIServerServingCAData - osImageURL + - baseOperatingSystemContainer - proxy - releaseImage - rootCAData diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index 8107f0090d..986bcaef68 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/types.go @@ -72,8 +72,10 @@ 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"` + + // 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 diff --git a/pkg/controller/render/render_controller.go b/pkg/controller/render/render_controller.go index dd836b4a50..e359985df8 100644 --- a/pkg/controller/render/render_controller.go +++ b/pkg/controller/render/render_controller.go @@ -550,6 +550,12 @@ 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") + } else { + glog.Infof("BaseOperatingSystemContainer=%s", cconfig.Spec.BaseOperatingSystemContainer) + } + // Before merging all MCs for a specific pool, let's make sure MachineConfigs are valid for _, config := range configs { if err := ctrlcommon.ValidateMachineConfig(config.Spec); err != nil { diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go index 608aefc4f0..eb7188c24d 100644 --- a/pkg/operator/bootstrap.go +++ b/pkg/operator/bootstrap.go @@ -139,6 +139,7 @@ func RenderBootstrap( spec.RootCAData = bundle spec.PullSecret = nil spec.OSImageURL = imgs.MachineOSContent + spec.BaseOperatingSystemContainer = imgs.BaseOperatingSystemContainer 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 2f24746831..f9790f7b42 100644 --- a/pkg/operator/images.go +++ b/pkg/operator/images.go @@ -17,6 +17,8 @@ type Images struct { type RenderConfigImages struct { MachineConfigOperator string `json:"machineConfigOperator"` MachineOSContent string `json:"machineOSContent"` + // The new format image + BaseOperatingSystemContainer string `json:"baseOperatingSystemContainer"` // 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/sync.go b/pkg/operator/sync.go index 488819f8de..680aa2da0b 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -255,11 +255,12 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { // sync up os image url // TODO: this should probably be part of the imgs - osimageurl, err := optr.getOsImageURL(optr.namespace) + oscontainer, osimageurl, err := optr.getOsImageURLs(optr.namespace) if err != nil { return err } imgs.MachineOSContent = osimageurl + imgs.BaseOperatingSystemContainer = oscontainer // sync up the ControllerConfigSpec infra, network, proxy, dns, err := optr.getGlobalConfig() @@ -308,6 +309,7 @@ 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.Images = map[string]string{ templatectrl.MachineConfigOperatorKey: imgs.MachineConfigOperator, @@ -688,7 +690,7 @@ func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error { _, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey] if hasRequiredPoolLabel { - opURL, err := optr.getOsImageURL(optr.namespace) + _, opURL, err := optr.getOsImageURLs(optr.namespace) if err != nil { glog.Errorf("Error getting configmap osImageURL: %q", err) return false, nil @@ -859,17 +861,26 @@ func (optr *Operator) waitForControllerConfigToBeCompleted(resource *mcfgv1.Cont return nil } -func (optr *Operator) getOsImageURL(namespace string) (string, error) { +// getOsImageURLs returns (new type, old type) for operating system update images. +func (optr *Operator) getOsImageURLs(namespace string) (string, string, error) { cm, err := optr.mcoCmLister.ConfigMaps(namespace).Get(osImageConfigMapName) if err != nil { - return "", err + return "", "", err } releaseVersion := cm.Data["releaseVersion"] optrVersion, _ := optr.vStore.Get("operator") if releaseVersion != optrVersion { - return "", fmt.Errorf("refusing to read osImageURL version %q, operator version %q", releaseVersion, optrVersion) + return "", "", fmt.Errorf("refusing to read osImageURL version %q, operator version %q", releaseVersion, optrVersion) + } + newformat, ok := cm.Data["baseOperatingSystemContainer"] + if !ok { + return "", "", fmt.Errorf("Missing baseOperatingSystemContainer from configmap") + } + oldformat := cm.Data["osImageURL"] + if !ok { + return "", "", fmt.Errorf("Missing osImageURL from configmap") } - return cm.Data["osImageURL"], nil + return newformat, oldformat, nil } func (optr *Operator) getCAsFromConfigMap(namespace, name, key string) ([]byte, error) { From f52fa42c032e68d0e7b72ecad1d602155448afbb Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Fri, 8 Jul 2022 18:48:38 -0500 Subject: [PATCH 2/6] Add `rhel-coreos` extensions 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: https://github.com/openshift/os/pull/763 This makes sure that, if present, the new extensions image gets pushed through to controllerconfig and is available when merging machineconfigs. --- ...machine-config-operator_05_osimageurl.yaml | 3 -- install/image-references | 4 --- lib/resourcemerge/machineconfig.go | 2 ++ manifests/controllerconfig.crd.yaml | 4 +++ .../v1/types.go | 3 ++ pkg/operator/bootstrap.go | 1 + pkg/operator/images.go | 2 ++ pkg/operator/sync.go | 36 ++++++++++++------- 8 files changed, 35 insertions(+), 20 deletions(-) diff --git a/install/0000_80_machine-config-operator_05_osimageurl.yaml b/install/0000_80_machine-config-operator_05_osimageurl.yaml index d1fe31e2a0..f0eb977480 100644 --- a/install/0000_80_machine-config-operator_05_osimageurl.yaml +++ b/install/0000_80_machine-config-operator_05_osimageurl.yaml @@ -9,9 +9,6 @@ metadata: include.release.openshift.io/single-node-developer: "true" 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" # 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 85605021b8..b186e81c1f 100644 --- a/install/image-references +++ b/install/image-references @@ -23,10 +23,6 @@ spec: from: kind: DockerImage name: registry.svc.ci.openshift.org/openshift:machine-os-content - - name: rhel-coreos - from: - kind: DockerImage - name: registry.ci.openshift.org/openshift:rhel-coreos - name: keepalived-ipfailover from: kind: DockerImage diff --git a/lib/resourcemerge/machineconfig.go b/lib/resourcemerge/machineconfig.go index 34ff0c12fe..c3aed344a1 100644 --- a/lib/resourcemerge/machineconfig.go +++ b/lib/resourcemerge/machineconfig.go @@ -72,6 +72,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) diff --git a/manifests/controllerconfig.crd.yaml b/manifests/controllerconfig.crd.yaml index 29cb49da3c..e323c05abb 100644 --- a/manifests/controllerconfig.crd.yaml +++ b/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 diff --git a/pkg/apis/machineconfiguration.openshift.io/v1/types.go b/pkg/apis/machineconfiguration.openshift.io/v1/types.go index 986bcaef68..183f310ad2 100644 --- a/pkg/apis/machineconfiguration.openshift.io/v1/types.go +++ b/pkg/apis/machineconfiguration.openshift.io/v1/types.go @@ -75,6 +75,9 @@ 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"` diff --git a/pkg/operator/bootstrap.go b/pkg/operator/bootstrap.go index eb7188c24d..d83cd6a586 100644 --- a/pkg/operator/bootstrap.go +++ b/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, diff --git a/pkg/operator/images.go b/pkg/operator/images.go index f9790f7b42..33a997705b 100644 --- a/pkg/operator/images.go +++ b/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. diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 680aa2da0b..fdedceacc9 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -255,12 +255,13 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { // sync up os image url // TODO: this should probably be part of the imgs - oscontainer, osimageurl, err := optr.getOsImageURLs(optr.namespace) + oscontainer, osextensionscontainer, osimageurl, err := optr.getOsImageURLs(optr.namespace) if err != nil { return err } imgs.MachineOSContent = osimageurl imgs.BaseOperatingSystemContainer = oscontainer + imgs.BaseOperatingSystemExtensionsContainer = osextensionscontainer // sync up the ControllerConfigSpec infra, network, proxy, dns, err := optr.getGlobalConfig() @@ -310,6 +311,7 @@ func (optr *Operator) syncRenderConfig(_ *renderConfig) error { spec.PullSecret = &corev1.ObjectReference{Namespace: "openshift-config", Name: "pull-secret"} spec.OSImageURL = imgs.MachineOSContent spec.BaseOperatingSystemContainer = imgs.BaseOperatingSystemContainer + spec.BaseOperatingSystemExtensionsContainer = imgs.BaseOperatingSystemExtensionsContainer spec.Images = map[string]string{ templatectrl.MachineConfigOperatorKey: imgs.MachineConfigOperator, @@ -690,7 +692,7 @@ func (optr *Operator) syncRequiredMachineConfigPools(_ *renderConfig) error { _, hasRequiredPoolLabel := pool.Labels[requiredForUpgradeMachineConfigPoolLabelKey] if hasRequiredPoolLabel { - _, opURL, err := optr.getOsImageURLs(optr.namespace) + _, _, opURL, err := optr.getOsImageURLs(optr.namespace) if err != nil { glog.Errorf("Error getting configmap osImageURL: %q", err) return false, nil @@ -861,26 +863,34 @@ func (optr *Operator) waitForControllerConfigToBeCompleted(resource *mcfgv1.Cont return nil } -// getOsImageURLs returns (new type, old type) for operating system update images. -func (optr *Operator) getOsImageURLs(namespace string) (string, string, error) { +// getOsImageURLs returns (new type, new extensions, old type) for operating system update images. +func (optr *Operator) getOsImageURLs(namespace string) (string, string, string, error) { cm, err := optr.mcoCmLister.ConfigMaps(namespace).Get(osImageConfigMapName) if err != nil { - return "", "", err + return "", "", "", err } releaseVersion := cm.Data["releaseVersion"] optrVersion, _ := optr.vStore.Get("operator") if releaseVersion != optrVersion { - return "", "", fmt.Errorf("refusing to read osImageURL version %q, operator version %q", releaseVersion, optrVersion) + return "", "", "", fmt.Errorf("refusing to read osImageURL version %q, operator version %q", releaseVersion, optrVersion) } - newformat, ok := cm.Data["baseOperatingSystemContainer"] - if !ok { - return "", "", fmt.Errorf("Missing baseOperatingSystemContainer from configmap") + + newextensions, hasNewExtensions := cm.Data["baseOperatingSystemExtensionsContainer"] + + newformat, hasNewFormat := cm.Data["baseOperatingSystemContainer"] + + oldformat, hasOldFormat := cm.Data["osImageURL"] + + if hasNewFormat && !hasNewExtensions { + // TODO(jkyros): This is okay for now because it's not required, but someday this will be bad } - oldformat := cm.Data["osImageURL"] - if !ok { - return "", "", fmt.Errorf("Missing osImageURL from configmap") + + // 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 newformat, oldformat, nil + + return newformat, newextensions, oldformat, nil } func (optr *Operator) getCAsFromConfigMap(namespace, name, key string) ([]byte, error) { From 76c17f8ac946025be8236ddb4fafb0e348d2e217 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 14 Mar 2022 16:47:58 -0400 Subject: [PATCH 3/6] operator/bootstrap: Accept new `--image-references` argument The MCO orchestrates the management/setup of a big pile of different containers today, from `machine-os-content` to `haproxy` and configuring cri-o to use the correct `pause`/`pod` container. These images get set up at both "bootstrap" time and cluster time. In bootstrap today we manually scrape each image out of the CVO in shell script, and then pass them as CLI arguments inside `bootkube.sh`. This means adding new images requires a tedious "ratcheting" process where the CLI argument is added to the MCO, then we patch the installer to pass it in. Instead, add a new CLI argument which accepts a serialized imagestream object, which is exactly what the CVO carries. Prep for adding a new `rhel-coreos` image into the payload, so I can avoid that ratcheting process. --- cmd/machine-config-operator/bootstrap.go | 46 ++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/cmd/machine-config-operator/bootstrap.go b/cmd/machine-config-operator/bootstrap.go index cad72352bb..2179f17b18 100644 --- a/cmd/machine-config-operator/bootstrap.go +++ b/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" ) @@ -41,6 +45,7 @@ var ( proxyConfigFile string additionalTrustBundleFile string dnsConfigFile string + imageReferences string } ) @@ -72,10 +77,32 @@ 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() @@ -83,6 +110,25 @@ func runBootstrapCmd(cmd *cobra.Command, args []string) { // 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") + } + imgs := operator.Images{ RenderConfigImages: operator.RenderConfigImages{ MachineConfigOperator: bootstrapOpts.mcoImage, From 611c5dcb6591a51109b51647f6488fd82844d8ad Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Fri, 15 Jul 2022 15:44:12 -0500 Subject: [PATCH 4/6] Get the base OS container into controllerconfig --- cmd/machine-config-operator/bootstrap.go | 70 ++++++++++++++---------- pkg/operator/sync.go | 1 + 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/cmd/machine-config-operator/bootstrap.go b/cmd/machine-config-operator/bootstrap.go index 2179f17b18..c6418322ed 100644 --- a/cmd/machine-config-operator/bootstrap.go +++ b/cmd/machine-config-operator/bootstrap.go @@ -23,29 +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 + 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 } ) @@ -127,16 +129,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, "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, diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index fdedceacc9..3ad87870f4 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -883,6 +883,7 @@ func (optr *Operator) getOsImageURLs(namespace string) (string, string, string, if hasNewFormat && !hasNewExtensions { // TODO(jkyros): This is okay for now because it's not required, but someday this will be bad + glog.Warningf("New format image specified, but no matching extensions image present") } // If we don't have a new format image, and we can't fall back to the old one From 1f19e2f33e5d4de5fe69a364279becbb287c3761 Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Mon, 18 Jul 2022 14:17:35 -0500 Subject: [PATCH 5/6] Replace image prefixes with obvious placeholder values So once upon a time, our image references in our manifests, the ones that look like: `registry.svc.ci.openshift.org/openshift:something` probably pointed at real images. But: - that registry has long since retired and - the openshift client (`oc`) rewrites these values as though they were templated when an openshift release gets packed by `oc adm release new` This was not immediately apparent to someone who was not familiar with the process, and an outside observer could be forgiven for thinking that these were real values. This tries to alleviate any confusion or belief that these values are 'real' by making them obviously fake, but also descriptive of their function. --- ...hine-config-operator_02_images.configmap.yaml | 14 +++++++------- ...80_machine-config-operator_04_deployment.yaml | 2 +- ...80_machine-config-operator_05_osimageurl.yaml | 2 +- install/image-references | 16 ++++++++-------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/install/0000_80_machine-config-operator_02_images.configmap.yaml b/install/0000_80_machine-config-operator_02_images.configmap.yaml index 85da7d1672..7e9473b932 100644 --- a/install/0000_80_machine-config-operator_02_images.configmap.yaml +++ b/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" } diff --git a/install/0000_80_machine-config-operator_04_deployment.yaml b/install/0000_80_machine-config-operator_04_deployment.yaml index 74b543a4c1..f5026f7318 100644 --- a/install/0000_80_machine-config-operator_04_deployment.yaml +++ b/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" diff --git a/install/0000_80_machine-config-operator_05_osimageurl.yaml b/install/0000_80_machine-config-operator_05_osimageurl.yaml index f0eb977480..d86c7af8bd 100644 --- a/install/0000_80_machine-config-operator_05_osimageurl.yaml +++ b/install/0000_80_machine-config-operator_05_osimageurl.yaml @@ -12,4 +12,4 @@ data: # 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: "registry.svc.ci.openshift.org/openshift:machine-os-content" + osImageURL: "placeholder.url.oc.will.replace.this.org/placeholdernamespace:machine-os-content" diff --git a/install/image-references b/install/image-references index b186e81c1f..3cae1298a3 100644 --- a/install/image-references +++ b/install/image-references @@ -7,35 +7,35 @@ 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: 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 From 954ce11485a4cf534d9c25841ed242dd420f7c5a Mon Sep 17 00:00:00 2001 From: John Kyros <79665180+jkyros@users.noreply.github.com> Date: Wed, 20 Jul 2022 23:21:44 -0500 Subject: [PATCH 6/6] Add new format images to image-references The way things work with the openshift client, once we add an image to image-references, `oc` expects it to be in the release, and `oc adm release new` will fail if it's not present. Also, in order for the placeholder image location in our `machine-config-osimageurl` configmap to get rewritten with a real value (rather than the bogus 'template' value it has by default), it has to be referenced in image-references. The desired outcome was "if it's there use it, otherwise ignore it", but that's just not possible with regard to the payload the way things are set up. This groups the addition of the new format os container and extensions container together into sort of an "on switch" commit that will break the builds if merged before https://issues.redhat.com/browse/ART-3883, but will make them work with new images if merged afterwards. --- .../0000_80_machine-config-operator_05_osimageurl.yaml | 4 ++++ install/image-references | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/install/0000_80_machine-config-operator_05_osimageurl.yaml b/install/0000_80_machine-config-operator_05_osimageurl.yaml index d86c7af8bd..6dd8866443 100644 --- a/install/0000_80_machine-config-operator_05_osimageurl.yaml +++ b/install/0000_80_machine-config-operator_05_osimageurl.yaml @@ -9,6 +9,10 @@ metadata: include.release.openshift.io/single-node-developer: "true" 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" # 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 3cae1298a3..4170492251 100644 --- a/install/image-references +++ b/install/image-references @@ -23,6 +23,14 @@ spec: from: kind: DockerImage 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