Skip to content

Commit

Permalink
Revert "pkg/daemon: Add IgnitionVersion to Daemon"
Browse files Browse the repository at this point in the history
b823087 introduced code to obtain the version of the Ignition binary
shipped in the OS root filesystem.  That commit contemplated that the
version would be used more widely in the MCD, but that hasn't occurred,
so its only current use is to log the Ignition version at MCD startup.
On balance, I think that code path introduces more risk than value, so
this PR backs it out.  My reasoning is:

1. By the time this code is invoked, Ignition will never run again, so
its version is useful for forensics and not much else.  In particular,
the version of the Ignition binary in machine-os-content probably doesn't
match the one in the bootimage.

2. MCD is invoking the Ignition binary via a private (non-contractual)
path in the root filesystem, but Ignition is designed and tested only for
use in the initramfs.  In practice, `ignition --version` _should_ be
harmless since it exits very early in Ignition startup, but see below.

3. If the MCD fails to run the binary, say because the path to Ignition
has changed, MCD considers that a fatal error.

4. And indeed, we now have a bug
(https://bugzilla.redhat.com/show_bug.cgi?id=1927731) where an
`ignition --version` segfault is apparently blocking an upgrade from 4.5
to 4.6, on exactly one customer node.  From the stack trace
(https://bugzilla.redhat.com/show_bug.cgi?id=1927731#c7), the problem
could be in Ignition itself, in the initialization code of a vendored
library, or in the Go runtime.  As a practical matter, the crash is
unlikely to be root-caused.

This reverts commit b823087.
  • Loading branch information
bgilbert committed Feb 24, 2021
1 parent 1402af2 commit 2203d88
Showing 1 changed file with 0 additions and 14 deletions.
14 changes: 0 additions & 14 deletions pkg/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,6 @@ type Daemon struct {
// os the operating system the MCD is running on
os OperatingSystem

// IgnitionVersion is the version of the installed Ignition binary on the system
IgnitionVersion string

// mock is set if we're running as non-root, probably under unit tests
mock bool

Expand Down Expand Up @@ -128,8 +125,6 @@ type Daemon struct {
}

const (
// pathIgnition is the path where the ignition binary resides
pathIgnition = "/usr/lib/dracut/modules.d/30ignition/ignition"
// pathSystemd is the path systemd modifiable units, services, etc.. reside
pathSystemd = "/etc/systemd/system"
// pathDevNull is the systems path to and endless blackhole
Expand Down Expand Up @@ -202,7 +197,6 @@ func New(
var (
osImageURL string
osVersion string
ignVersion string
err error
)

Expand All @@ -222,13 +216,6 @@ func New(
return nil, fmt.Errorf("error reading osImageURL from rpm-ostree: %v", err)
}
glog.Infof("Booted osImageURL: %s (%s)", osImageURL, osVersion)

ignVersionBytes, err := exec.Command(pathIgnition, "--version").Output()
if err != nil {
return nil, fmt.Errorf("error getting installed Ignition version: %v", err)
}
ignVersion = strings.SplitAfter(string(ignVersionBytes), " ")[1]
glog.Infof("Installed Ignition binary version: %s", ignVersion)
}

bootID := ""
Expand Down Expand Up @@ -257,7 +244,6 @@ func New(
return &Daemon{
mock: mock,
booting: true,
IgnitionVersion: ignVersion,
os: os,
NodeUpdaterClient: nodeUpdaterClient,
bootedOSImageURL: osImageURL,
Expand Down

0 comments on commit 2203d88

Please sign in to comment.