From 85b297c1a063a7e130e82afb2fd2230ffd5c5fcc Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 4 Mar 2023 08:33:03 -0500 Subject: [PATCH] daemon: Remove old legacy OS update path This must be dead code in 4.14. There's no going back. Everything in 4.12 really should have transitioned, and certainly 4.13 and very much most definitely 4.14. Deleting code is good on general principle, but very specifically this cost me hours of debugging because I modified the legacy update path when trying something, not the new one. --- pkg/daemon/daemon.go | 24 +------ pkg/daemon/rpm-ostree.go | 14 ---- pkg/daemon/update.go | 136 ++------------------------------------- 3 files changed, 7 insertions(+), 167 deletions(-) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index 92d0f75fb9..e96553ad6d 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -1677,30 +1677,10 @@ func (dn *Daemon) checkStateOnFirstRun() error { if !osMatch { logSystem("Bootstrap pivot required to: %s", targetOSImageURL) - // Check to see if we have a layered/new format image - isLayeredImage, err := dn.NodeUpdaterClient.IsBootableImage(targetOSImageURL) - if err != nil { - return fmt.Errorf("error checking type of target image: %w", err) + if err := dn.updateLayeredOS(state.currentConfig); err != nil { + return err } - if isLayeredImage { - // If this is a new format image, we don't have to extract it, - // we can just update it the proper way - if err := dn.updateLayeredOS(state.currentConfig); err != nil { - return err - } - } else { - osImageContentDir, err := ExtractOSImage(targetOSImageURL) - if err != nil { - return err - } - if err := dn.updateOS(state.currentConfig, osImageContentDir); err != nil { - return err - } - if err := os.RemoveAll(osImageContentDir); err != nil { - return err - } - } return dn.reboot(fmt.Sprintf("Node will reboot into config %v", state.currentConfig.GetName())) } logSystem("No bootstrap pivot required; unlinking bootstrap node annotations") diff --git a/pkg/daemon/rpm-ostree.go b/pkg/daemon/rpm-ostree.go index 82db50789e..0664fadd36 100644 --- a/pkg/daemon/rpm-ostree.go +++ b/pkg/daemon/rpm-ostree.go @@ -282,20 +282,6 @@ func (r *RpmOstreeClient) Rebase(imgURL, osImageContentDir string) (changed bool return } -// IsBootableImage determines if the image is a bootable (new container formet) image, or a wrapper (old container format) -func (r *RpmOstreeClient) IsBootableImage(imgURL string) (bool, error) { - - var isBootableImage string - var imageData *types.ImageInspectInfo - var err error - if imageData, _, err = imageInspect(imgURL); err != nil { - return false, err - } - isBootableImage = imageData.Labels["ostree.bootable"] - - return isBootableImage == "true", nil -} - // 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. diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index 594598ed2d..70202a0845 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -39,7 +39,6 @@ const ( // 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/" osExtensionsContentBaseDir = "/run/mco-extensions/" // These are the actions for a node to take after applying config changes. (e.g. a new machineconfig is applied) @@ -160,17 +159,8 @@ func (dn *Daemon) compareMachineConfig(oldConfig, newConfig *mcfgv1.MachineConfi } // addExtensionsRepo adds a repo into /etc/yum.repos.d/ which we use later to -// install extensions and rt-kernel -func addExtensionsRepo(osImageContentDir string) error { - repoContent := "[coreos-extensions]\nenabled=1\nmetadata_expire=1m\nbaseurl=" + osImageContentDir + "/extensions/\ngpgcheck=0\nskip_if_unavailable=False\n" - return writeFileAtomicallyWithDefaults(extensionsRepo, []byte(repoContent)) -} - -// 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 { +// install extensions (additional packages). +func addExtensionsRepo(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" return writeFileAtomicallyWithDefaults(extensionsRepo, []byte(repoContent)) } @@ -228,40 +218,6 @@ func podmanCopy(imgURL, osImageContentDir string) (err error) { return } -// ExtractOSImage extracts OS image content in a temporary directory under /run/machine-os-content/ -// and returns the path on successful extraction. -// Note that since we do this in the MCD container, cluster proxy configuration must also be injected -// into the container. See the MCD daemonset. -func ExtractOSImage(imgURL string) (osImageContentDir string, err error) { - var registryConfig []string - if _, err := os.Stat(kubeletAuthFile); err == nil { - registryConfig = append(registryConfig, "--registry-config", kubeletAuthFile) - } - if err = os.MkdirAll(osImageContentBaseDir, 0o755); err != nil { - err = fmt.Errorf("error creating directory %s: %w", osImageContentBaseDir, err) - return - } - - if osImageContentDir, err = os.MkdirTemp(osImageContentBaseDir, "os-content-"); err != nil { - return - } - - // Extract the image - args := []string{"image", "extract", "-v", "10", "--path", "/:" + osImageContentDir} - 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 - klog.Infof("Falling back to using podman cp to fetch OS image content") - if err = podmanCopy(imgURL, osImageContentDir); err != nil { - return - } - } - - 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) { @@ -325,24 +281,10 @@ func (dn *CoreOSDaemon) applyOSChanges(mcDiff machineConfigDiff, oldConfig, newC dn.nodeWriter.Eventf(corev1.EventTypeNormal, "OSUpdateStarted", mcDiff.osChangesString()) } - // The steps from here on are different depending on the image type, so check the image type - isLayeredImage, err := dn.NodeUpdaterClient.IsBootableImage(newConfig.Spec.OSImageURL) - if err != nil { - return fmt.Errorf("error checking type of update image: %w", err) + if err := dn.applyLayeredOSChanges(mcDiff, oldConfig, newConfig); err != nil { + return err } - // TODO(jkyros): we can remove the format check and simplify this once we retire the "old format" images - if isLayeredImage { - // If it's a layered/bootable image, then apply it the "new" way - if err := dn.applyLayeredOSChanges(mcDiff, oldConfig, newConfig); err != nil { - return err - } - } else { - // Otherwise fall back to the old way -- we can take this out someday when it goes away - if err := dn.applyLegacyOSChanges(mcDiff, oldConfig, newConfig); err != nil { - return err - } - } if dn.nodeWriter != nil { var nodeName string var nodeObjRef corev1.ObjectReference @@ -1997,7 +1939,7 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi // Delete extracted OS image once we are done. defer os.RemoveAll(osExtensionsContentDir) - if err := addLayeredExtensionsRepo(osExtensionsContentDir); err != nil { + if err := addExtensionsRepo(osExtensionsContentDir); err != nil { return err } defer os.Remove(extensionsRepo) @@ -2069,71 +2011,3 @@ func (dn *CoreOSDaemon) applyLayeredOSChanges(mcDiff machineConfigDiff, oldConfi // Apply extensions return dn.applyExtensions(oldConfig, newConfig) } - -func (dn *CoreOSDaemon) applyLegacyOSChanges(mcDiff machineConfigDiff, oldConfig, newConfig *mcfgv1.MachineConfig) (retErr error) { - var osImageContentDir string - var err error - if mcDiff.osUpdate || mcDiff.extensions || mcDiff.kernelType { - - if osImageContentDir, err = ExtractOSImage(newConfig.Spec.OSImageURL); err != nil { - return err - } - // Delete extracted OS image once we are done. - defer os.RemoveAll(osImageContentDir) - - if err := addExtensionsRepo(osImageContentDir); err != nil { - return err - } - defer os.Remove(extensionsRepo) - } - - // Update OS - if mcDiff.osUpdate { - if err := dn.updateOS(newConfig, osImageContentDir); err != nil { - mcdPivotErr.Inc() - return err - } - if dn.nodeWriter != nil { - dn.nodeWriter.Eventf(corev1.EventTypeNormal, "OSUpgradeApplied", "OS upgrade applied; new MachineConfig (%s) has new OS image (%s)", newConfig.Name, newConfig.Spec.OSImageURL) - } - } else { //nolint:gocritic // The nil check for dn.nodeWriter has nothing to do with an OS update being unavailable. - // An OS upgrade is not available - if dn.nodeWriter != nil { - dn.nodeWriter.Eventf(corev1.EventTypeNormal, "OSUpgradeSkipped", "OS upgrade skipped; new MachineConfig (%s) has same OS image (%s) as old MachineConfig (%s)", newConfig.Name, newConfig.Spec.OSImageURL, oldConfig.Name) - } - } - - // if we're here, we've successfully pivoted, or pivoting wasn't necessary, so we reset the error gauge - mcdPivotErr.Set(0) - - defer func() { - // Operations performed by rpm-ostree on the booted system are available - // as staged deployment. It gets applied only when we reboot the system. - // In case of an error during any rpm-ostree transaction, removing pending deployment - // should be sufficient to discard any applied changes. - if retErr != nil { - // Print out the error now so that if we fail to cleanup -p, we don't lose it. - klog.Infof("Rolling back applied changes to OS due to error: %v", retErr) - if err := removePendingDeployment(); err != nil { - errs := kubeErrs.NewAggregate([]error{err, retErr}) - retErr = fmt.Errorf("error removing staged deployment: %w", errs) - return - } - } - }() - - // Apply kargs - if mcDiff.kargs { - if err := dn.updateKernelArguments(oldConfig.Spec.KernelArguments, newConfig.Spec.KernelArguments); err != nil { - return err - } - } - - // Switch to real time kernel - if err := dn.switchKernel(oldConfig, newConfig); err != nil { - return err - } - - // Apply extensions - return dn.applyExtensions(oldConfig, newConfig) -}