Skip to content

Commit

Permalink
daemon: Completely remove imageInspect code
Browse files Browse the repository at this point in the history
Actually, I don't believe any of this logic is necessary anymore.

- We can now unambiguously rely on the container image being bootable;
  if it's not, it should be rpm-ostree that errors out!  There's
  no value in us "second guessing" it and having a duplicate fetch
  path; it just creates more problems
- Now crucially, `checkOS` had a special case for comparing the
  ostree commit digest, I think to try to deal with the firstboot
  problem.  But actually, what we've been doing for a while for
  scaleup of old bootimages is a double reboot, and hence we don't
  need special case logic for this!

This entirely deletes a lot of unnecessary code, more fully
deferring OS updates to rpm-ostree.
  • Loading branch information
cgwalters committed Jul 25, 2023
1 parent f2a11a5 commit d97af29
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 162 deletions.
15 changes: 1 addition & 14 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -2090,8 +2090,7 @@ func (dn *Daemon) validateOnDiskState(currentConfig *mcfgv1.MachineConfig) error
// checkOS determines whether the booted system matches the target
// osImageURL and if not whether we need to take action. This function
// returns `true` if no action is required, which is the case if we're
// not running RHCOS or FCOS, or if the target osImageURL is "" (unspecified),
// or if the digests match.
// not running RHCOS or FCOS, or if the target osImageURL is "" (unspecified).
// Otherwise if `false` is returned, then we need to perform an update.
func (dn *Daemon) checkOS(osImageURL string) bool {
// Nothing to do if we're not on RHCOS or FCOS
Expand All @@ -2100,18 +2099,6 @@ 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....
inspection, _, err := imageInspect(osImageURL)
if err != nil {
klog.Warningf("Unable to check manifest for matching hash: %s", err)
} else if ostreeCommit, ok := inspection.Labels["ostree.commit"]; ok {
if ostreeCommit == dn.bootedOSCommit {
klog.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
}

Expand Down
97 changes: 0 additions & 97 deletions pkg/daemon/image-inspect.go

This file was deleted.

51 changes: 0 additions & 51 deletions pkg/daemon/rpm-ostree.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
package daemon

import (
"encoding/json"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"
"time"

rpmostreeclient "github.com/coreos/rpmostree-client-go/pkg/client"
"github.com/opencontainers/go-digest"
pivotutils "github.com/openshift/machine-config-operator/pkg/daemon/pivot/utils"
"gopkg.in/yaml.v2"
"k8s.io/klog/v2"
)
Expand All @@ -24,21 +20,6 @@ const (
kubeletAuthFile = "/var/lib/kubelet/config.json"
)

// imageInspection is a public implementation of
// https://github.com/containers/skopeo/blob/82186b916faa9c8c70cfa922229bafe5ae024dec/cmd/skopeo/inspect.go#L20-L31
type imageInspection struct {
Name string `json:",omitempty"`
Tag string `json:",omitempty"`
Digest digest.Digest
RepoDigests []string
Created *time.Time
DockerVersion string
Labels map[string]string
Architecture string
Os string
Layers []string
}

// RpmOstreeClient provides all RpmOstree related methods in one structure.
// This structure implements DeploymentClient
//
Expand Down Expand Up @@ -157,38 +138,6 @@ func (r *RpmOstreeClient) GetBootedOSImageURL() (string, string, string, error)
return osImageURL, bootedDeployment.Version, baseChecksum, nil
}

func podmanInspect(imgURL string) (imgdata *imageInspection, err error) {
// Pull the container image if not already available
var authArgs []string
if _, err := os.Stat(kubeletAuthFile); err == nil {
authArgs = append(authArgs, "--authfile", kubeletAuthFile)
}
args := []string{"pull", "-q"}
args = append(args, authArgs...)
args = append(args, imgURL)
_, err = pivotutils.RunExt(numRetriesNetCommands, "podman", args...)
if err != nil {
return
}

inspectArgs := []string{"inspect", "--type=image"}
inspectArgs = append(inspectArgs, fmt.Sprintf("%s", imgURL))
var output []byte
output, err = runGetOut("podman", inspectArgs...)
if err != nil {
return
}
var imagedataArray []imageInspection
err = json.Unmarshal(output, &imagedataArray)
if err != nil {
err = fmt.Errorf("unmarshaling podman inspect: %w", err)
return
}
imgdata = &imagedataArray[0]
return

}

// 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
3 changes: 3 additions & 0 deletions pkg/daemon/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ const (
extensionsRepo = "/etc/yum.repos.d/coreos-extensions.repo"
osExtensionsContentBaseDir = "/run/mco-extensions/"

// cmdRetriesCount is an arbitrary number of times we will retry
cmdRetriesCount = 2

// 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
// This happens for example when ssh keys or the pull secret (/var/lib/kubelet/config.json) is changed
Expand Down

0 comments on commit d97af29

Please sign in to comment.