Skip to content

Commit

Permalink
daemon: Remove old legacy OS update path
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@cgwalters
cgwalters committed Jul 10, 2023
1 parent 2069e55 commit 85b297c
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 167 deletions.
24 changes: 2 additions & 22 deletions pkg/daemon/daemon.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
14 changes: 0 additions & 14 deletions pkg/daemon/rpm-ostree.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
136 changes: 5 additions & 131 deletions pkg/daemon/update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

0 comments on commit 85b297c

Please sign in to comment.