From 2203d88d81d22f581294461009f9bf165f61aa15 Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Wed, 24 Feb 2021 16:43:57 -0500 Subject: [PATCH] Revert "pkg/daemon: Add IgnitionVersion to Daemon" b823087109a9 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 b823087109a95108f2bed14f419822d4d5015555. --- pkg/daemon/daemon.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index bcaa1fb40c..18d50ee257 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -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 @@ -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 @@ -202,7 +197,6 @@ func New( var ( osImageURL string osVersion string - ignVersion string err error ) @@ -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 := "" @@ -257,7 +244,6 @@ func New( return &Daemon{ mock: mock, booting: true, - IgnitionVersion: ignVersion, os: os, NodeUpdaterClient: nodeUpdaterClient, bootedOSImageURL: osImageURL,