run systemctl commands in VM

pkg/distros/opensuse_tumbleweed.go

@@ -2,7 +2,7 @@ package distros
 
 import (
 	"errors"
-	"strings"
+	"fmt"
 
 	"github.com/rs/zerolog/log"
 	"libguestfs.org/guestfs"
@@ -39,51 +39,27 @@ func (mgr *OpensuseTumbleweedTemplateManager) DownloadTemplate(version, codename
 	)
 }
 
-func (mgr *OpensuseTumbleweedTemplateManager) setSysconfigNetworkParams(handle *guestfs.Guestfs) error {
-	path := "/etc/sysconfig/network/config"
-	// Make sure you don't override NETCONFIG_DNS_POLICY; the DNS server IP address
-	// obtained from DHCP is needed for cloud-init to work.
-	params := map[string]string{
-		"NETCONFIG_NTP_POLICY": "",
-		"AUTO6_WAIT_AT_BOOT":   "0",
-		"AUTO6_UPDATE":         "none",
-	}
-
-	// Unfortunately this causes Aug_save to fail later. I'm not sure why.
-	// The error message just says "aug_save: No error".
-	//for key, value := range params {
-	//	augPath := "/files" + path + "/ + key
-	//	if err := handle.Aug_set(augPath, value); err != nil {
-	//		return err
-	//	}
-	//}
-
-	content, err := handle.Cat(path)
+func (mgr *OpensuseTumbleweedTemplateManager) InstallNetworkManager(handle *guestfs.Guestfs) error {
+	args := []string{"zypper", "--non-interactive", "install", "NetworkManager"}
+	_, err := mgr.logAndRunCommand(handle, args)
 	if err != nil {
-		return err
+		return fmt.Errorf("Could not install NetworkManager: %w", err)
 	}
-	lines := strings.Split(content, "\n")
-	replacedLines := make([]string, len(lines))
-	for i, line := range lines {
-		for key, value := range params {
-			if strings.HasPrefix(line, key+"=") {
-				mgr.logger.Debug().
-					Str("path", path).
-					Msg("Setting " + key + "=" + value)
-				replacedLines[i] = key + "=" + value
-			} else {
-				replacedLines[i] = line
-			}
-		}
+	// Make sure dhcp-client is installed because we use it as the
+	// DHCP backend
+	dhclientIsPresent, err := handle.Is_file("/sbin/dhclient", nil)
+	if err != nil {
+		return fmt.Errorf("Could not determine if /sbin/dhclient is a file: %w", err)
 	}
-	replacedContent := strings.Join(replacedLines, "\n")
-	return handle.Write(path, []byte(replacedContent))
-}
-
-func (mgr *OpensuseTumbleweedTemplateManager) NeedsDynamicResolvConf() bool {
-	// cloud-init won't work if we overwrite /etc/resolv.conf.
-	// I'm not sure why it works on other distros but not on this one.
-	return true
+	if !dhclientIsPresent {
+		return errors.New("dhclient is not present")
+	}
+	// Workaround for bug in cloud-init which prevents the NetworkManager
+	// backend from being selected if NetworkManager.conf does not exist
+	// https://github.com/canonical/cloud-init/blob/23.1.x/cloudinit/net/network_manager.py#L381
+	const cfgFilePath = "/etc/NetworkManager/NetworkManager.conf"
+	mgr.logger.Debug().Msg("Creating " + cfgFilePath)
+	return handle.Touch(cfgFilePath)
 }
 
 func (mgr *OpensuseTumbleweedTemplateManager) removeWelcomeMessage(handle *guestfs.Guestfs) error {
@@ -93,18 +69,17 @@ func (mgr *OpensuseTumbleweedTemplateManager) removeWelcomeMessage(handle *guest
 }
 
 func (mgr *OpensuseTumbleweedTemplateManager) CommandToUpdatePackageCache() []string {
-	return []string{"sudo", "zypper", "refresh"}
+	return []string{"sudo", "zypper", "refresh", "--force"}
 }
 
-func (mgr *OpensuseTumbleweedTemplateManager) PerformDistroSpecificModifications(handle *guestfs.Guestfs) (err error) {
-	if err = mgr.maskSystemdUnit(handle, "wickedd-dhcp6.service"); err != nil {
-		return
+func (mgr *OpensuseTumbleweedTemplateManager) PerformDistroSpecificModifications(handle *guestfs.Guestfs) error {
+	// Newer Tumbleweed images shouldn't be using wickedd anymore
+	wickeddExists, err := handle.Is_file("/lib/systemd/system/wickedd.service", nil)
+	if err != nil {
+		return err
 	}
-	if err = mgr.setSysconfigNetworkParams(handle); err != nil {
-		return
+	if wickeddExists {
+		return errors.New("wickedd.service was unexpectedly found")
 	}
-	if err = mgr.removeWelcomeMessage(handle); err != nil {
-		return
-	}
-	return
+	return mgr.removeWelcomeMessage(handle)
 }

pkg/distros/resources/97_csclub_resolv_conf.sh

@@ -1,12 +0,0 @@
-#!/bin/bash
-
-set -ex
-
-RESOLV_CONF=/run/netconfig/resolv.conf
-
-sed -i '/^search .*/d' $RESOLV_CONF
-sed -i '/^options .*/d' $RESOLV_CONF
-cat <<EOF >> $RESOLV_CONF
-search      csclub.uwaterloo.ca uwaterloo.ca
-options     ndots:2
-EOF

pkg/distros/template_manager.go

@@ -52,10 +52,6 @@ type ITemplateManager interface {
 // An IDistroSpecificTemplateManager performs the distro-specific tasks
 // when modifying a VM template. It is used by the generic TemplateManager.
 type IDistroSpecificTemplateManager interface {
-	// On some distros, like OpenSUSE Tumbleweed, we cannot overwrite
-	// /etc/resolv.conf or else cloud-init will fail. Such distros should
-	// override this method to return true.
-	NeedsDynamicResolvConf() bool
 	// GetLatestVersion returns the version number and codename of the
 	// latest version of particular OS (e.g. version="22.04", codename="jammy")
 	GetLatestVersion() (version string, codename string, err error)
@@ -70,12 +66,14 @@ type IDistroSpecificTemplateManager interface {
 	// (but does not upgrade) the packages for this distro, e.g.
 	// "sudo apt update".
 	CommandToUpdatePackageCache() []string
+	// InstallNetworkManager installs the OS-specific package for
+	// NetworkManager.
+	InstallNetworkManager(handle *guestfs.Guestfs) error
 }
 
-func (mgr *TemplateManager) NeedsDynamicResolvConf() bool {
-	// Assume false by default. Individual distros should override this
-	// method if necessary.
-	return false
+func (mgr *TemplateManager) InstallNetworkManager(handle *guestfs.Guestfs) error {
+	// Individual distros should override this method if necessary
+	return errors.New("InstallNetworkManager: not implemented for this distro")
 }
 
 func (mgr *TemplateManager) DownloadTemplateGeneric(filename, url string) (path string, err error) {
@@ -166,6 +164,12 @@ func (mgr *TemplateManager) getGuestfsMountedHandle(filename string) (handle *gu
 	if err != nil {
 		return
 	}
+	// Needed to install packages in VM
+	log.Debug().Msg("Enabling network access")
+	err = handle.Set_network(true)
+	if err != nil {
+		return
+	}
 	log.Debug().Msg("Adding drive " + filename)
 	if err = handle.Add_drive(filename, nil); err != nil {
 		return
@@ -268,60 +272,39 @@ func isSameFile(handle *guestfs.Guestfs, path1, path2 string) (bool, error) {
 	return stat1.Ino == stat2.Ino, nil
 }
 
-func systemdServiceIsEnabled(handle *guestfs.Guestfs, unit string) (bool, error) {
-	dirPrefixes := []string{"/etc", "/lib"}
-	unitPath := ""
-	for _, dirPrefix := range dirPrefixes {
-		possibleUnitPath := dirPrefix + "/systemd/system/" + unit
-		exists, err := handle.Is_file(possibleUnitPath, nil)
-		if err != nil {
-			return false, fmt.Errorf("could not determine if %s is a file: %w", possibleUnitPath, err)
-		}
-		if exists {
-			unitPath = possibleUnitPath
-			break
-		}
+var systemdDirPrefixes = []string{"/etc", "/lib"}
+
+func (mgr *TemplateManager) logAndRunCommand(handle *guestfs.Guestfs, args []string) (string, error) {
+	mgr.logger.Debug().Msg("Running command `" + strings.Join(args, " ") + "`")
+	return handle.Command(args)
+}
+
+func (mgr *TemplateManager) enableSystemdUnit(handle *guestfs.Guestfs, unit string) error {
+	_, err := mgr.logAndRunCommand(handle, []string{
+		"systemctl", "enable", unit,
+	})
+	return err
+}
+
+func (mgr *TemplateManager) systemdServiceIsEnabled(handle *guestfs.Guestfs, unit string) (bool, error) {
+	// Use '|| true' because the exit code will be non-zero if the unit
+	// is not enabled or does not exist
+	command := "systemctl is-enabled " + unit + " || true"
+	mgr.logger.Debug().Msg(`Running sh -c "` + command + `"`)
+	output, err := handle.Sh(command)
+	if err != nil {
+		return false, err
 	}
-	if unitPath == "" {
+	// Strip newline, if there is one
+	if len(output) > 0 && output[len(output)-1] == '\n' {
+		output = output[:len(output)-1]
+	}
+	if output == "enabled" {
+		return true, nil
+	} else if output == "disabled" || output == "not-found" {
 		return false, nil
 	}
-	content, err := handle.Cat(unitPath)
-	if err != nil {
-		return false, fmt.Errorf("could not read %s: %w", unitPath, err)
-	}
-	lines := strings.Split(content, "\n")
-	wantedBy := ""
-	for _, line := range lines {
-		if strings.HasPrefix(line, "WantedBy=") {
-			wantedBy = line[len("WantedBy="):]
-			break
-		}
-	}
-	if wantedBy == "" {
-		return false, fmt.Errorf("could not find WantedBy property for %s", unitPath)
-	}
-	linkName := ""
-	for _, dirPrefix := range dirPrefixes {
-		possibleLinkName := dirPrefix + "/systemd/system/" + wantedBy + ".wants/" + unit
-		isSymlink, err := handle.Is_symlink(possibleLinkName)
-		if err != nil {
-			return false, fmt.Errorf("could not determine if %s is a symlink: %w", possibleLinkName, err)
-		}
-		if isSymlink {
-			linkName = possibleLinkName
-			break
-		}
-	}
-	if linkName == "" {
-		return false, nil
-	}
-	target, err := handle.Readlink(linkName)
-	if err != nil {
-		return false, fmt.Errorf("could not read link at %s: %w", linkName, err)
-	}
-	// The target might be in /usr/lib, which will likely be a symlink
-	// to /lib. So we can't compare the paths directly.
-	return isSameFile(handle, target, unitPath)
+	return false, errors.New("Unexpected output '" + output + "'")
 }
 
 // setChronyOptions sets custom NTP server URLs in a chrony config file.
@@ -423,28 +406,28 @@ func (mgr *TemplateManager) setTimesyncdConf(handle *guestfs.Guestfs) error {
 	return handle.Write("/etc/systemd/timesyncd.conf.d/csclub.conf", getResource("timesyncd.conf"))
 }
 
-func usesChrony(handle *guestfs.Guestfs) (bool, error) {
+func (mgr *TemplateManager) usesChrony(handle *guestfs.Guestfs) (bool, error) {
 	// The systemd service for chrony is called on chrony.service on e.g. Debian,
 	// but is called chronyd.service on e.g. Fedora
-	chronyIsEnabled, err := systemdServiceIsEnabled(handle, "chrony.service")
+	chronyIsEnabled, err := mgr.systemdServiceIsEnabled(handle, "chrony.service")
 	if err != nil {
 		return false, err
 	}
 	if chronyIsEnabled {
 		return true, nil
 	}
-	return systemdServiceIsEnabled(handle, "chronyd.service")
+	return mgr.systemdServiceIsEnabled(handle, "chronyd.service")
 }
 
 func (mgr *TemplateManager) setupCommonNTPConfigs(handle *guestfs.Guestfs) error {
-	usesChrony, err := usesChrony(handle)
+	usesChrony, err := mgr.usesChrony(handle)
 	if err != nil {
 		return err
 	}
 	if usesChrony {
 		return mgr.setChronyOptions(handle)
 	}
-	usesSystemdTimesyncd, err := systemdServiceIsEnabled(handle, "systemd-timesyncd.service")
+	usesSystemdTimesyncd, err := mgr.systemdServiceIsEnabled(handle, "systemd-timesyncd.service")
 	if err != nil {
 		return err
 	}
@@ -463,11 +446,11 @@ func (mgr *TemplateManager) setNetworkManagerOptions(handle *guestfs.Guestfs) (e
 	return handle.Write(path, getResource("network-manager-snippet"))
 }
 
-func usesNetworkManager(handle *guestfs.Guestfs) (bool, error) {
-	return systemdServiceIsEnabled(handle, "NetworkManager.service")
+func (mgr *TemplateManager) usesNetworkManager(handle *guestfs.Guestfs) (bool, error) {
+	return mgr.systemdServiceIsEnabled(handle, "NetworkManager.service")
 }
 
-func usesEtcNetworkInterfaces(handle *guestfs.Guestfs) (bool, error) {
+func (mgr *TemplateManager) usesEtcNetworkInterfaces(handle *guestfs.Guestfs) (bool, error) {
 	configPath := "/etc/network/interfaces"
 	configExists, err := handle.Is_file(configPath, nil)
 	if err != nil {
@@ -476,16 +459,7 @@ func usesEtcNetworkInterfaces(handle *guestfs.Guestfs) (bool, error) {
 	if !configExists {
 		return false, nil
 	}
-	return systemdServiceIsEnabled(handle, "networking.service")
-}
-
-func usesSysconfigNetwork(handle *guestfs.Guestfs) (bool, error) {
-	configPath := "/etc/sysconfig/network/config"
-	configExists, err := handle.Is_file(configPath, nil)
-	if err != nil {
-		err = fmt.Errorf("Could not determine if "+configPath+" is a file: %w", err)
-	}
-	return configExists, err
+	return mgr.systemdServiceIsEnabled(handle, "networking.service")
 }
 
 func (mgr *TemplateManager) addSystemdNetworkdCloudInitSnippet(handle *guestfs.Guestfs) error {
@@ -495,43 +469,54 @@ func (mgr *TemplateManager) addSystemdNetworkdCloudInitSnippet(handle *guestfs.G
 	return handle.Write(path, getResource("systemd-networkd-cloud-init"))
 }
 
+func (mgr *TemplateManager) installAndEnableNetworkManager(handle *guestfs.Guestfs) error {
+	isInstalled, err := handle.Is_file("/lib/systemd/system/NetworkManager.service", nil)
+	if err != nil {
+		return err
+	}
+	if !isInstalled {
+		err = mgr.impl.InstallNetworkManager(handle)
+		if err != nil {
+			return err
+		}
+	}
+	return mgr.enableSystemdUnit(handle, "NetworkManager.service")
+}
+
 func (mgr *TemplateManager) setupCommonNetworkConfigs(handle *guestfs.Guestfs) error {
-	usesNetworkManager, err := usesNetworkManager(handle)
+	usesNetworkManager, err := mgr.usesNetworkManager(handle)
 	if err != nil {
 		return err
 	}
 	if usesNetworkManager {
 		return mgr.setNetworkManagerOptions(handle)
 	}
-	usesSystemdNetworkd, err := systemdServiceIsEnabled(handle, "systemd-networkd.service")
+	usesSystemdNetworkd, err := mgr.systemdServiceIsEnabled(handle, "systemd-networkd.service")
 	if err != nil {
 		return err
 	}
 	if usesSystemdNetworkd {
 		return mgr.addSystemdNetworkdCloudInitSnippet(handle)
 	}
-	usesEtcNetworkInterfaces, err := usesEtcNetworkInterfaces(handle)
+	usesEtcNetworkInterfaces, err := mgr.usesEtcNetworkInterfaces(handle)
 	if err != nil {
 		return err
 	}
 	if usesEtcNetworkInterfaces {
 		return mgr.setDhclientOptions(handle)
 	}
-	usesSysconfigNetwork, err := usesSysconfigNetwork(handle)
+	mgr.logger.Info().Msg("Could not determine network backend, resorting to installing NetworkManager")
+	err = mgr.installAndEnableNetworkManager(handle)
 	if err != nil {
 		return err
 	}
-	if usesSysconfigNetwork {
-		// Handled separately in opensuse_tumbleweed.go
-		return nil
-	}
-	return fmt.Errorf("Could not determine network backend")
+	return mgr.setNetworkManagerOptions(handle)
 }
 
 func (mgr *TemplateManager) setupIpv6Scripts(handle *guestfs.Guestfs) (err error) {
 	log := mgr.logger
 	scripts := []string{"99_csclub_ipv6_addr.sh"}
-	usesNetworkManager, err := usesNetworkManager(handle)
+	usesNetworkManager, err := mgr.usesNetworkManager(handle)
 	if err != nil {
 		return
 	}
@@ -560,25 +545,11 @@ func (mgr *TemplateManager) setupIpv6Scripts(handle *guestfs.Guestfs) (err error
 }
 
 func (mgr *TemplateManager) setResolvConf(handle *guestfs.Guestfs) (err error) {
-	if mgr.impl.NeedsDynamicResolvConf() {
-		scriptDir := "/var/lib/cloud/scripts/per-boot"
-		if err = handle.Mkdir_p(scriptDir); err != nil {
-			return
-		}
-		filename := "97_csclub_resolv_conf.sh"
-		path := scriptDir + "/" + filename
-		mgr.logger.Debug().Msg("Writing to " + path)
-		if err = handle.Write(path, getResource(filename)); err != nil {
-			return
-		}
-		return handle.Chmod(0755, path)
-	} else {
-		mgr.logger.Debug().Msg("Writing to /etc/resolv.conf")
-		if err = handle.Rm_f("/etc/resolv.conf"); err != nil {
-			return
-		}
-		return handle.Write("/etc/resolv.conf", getResource("resolv.conf"))
+	mgr.logger.Debug().Msg("Writing to /etc/resolv.conf")
+	if err = handle.Rm_f("/etc/resolv.conf"); err != nil {
+		return
 	}
+	return handle.Write("/etc/resolv.conf", getResource("resolv.conf"))
 }
 
 func (mgr *TemplateManager) createAugeasHandle(handle *guestfs.Guestfs) (err error) {
@@ -795,33 +766,32 @@ func (mgr *TemplateManager) dnfRemoveUnnecessaryPackages(handle *guestfs.Guestfs
 	// auditd spams the system log and uses lots of disk IO.
 	// bluez is unnecessary on servers.
 	args := []string{"dnf", "-C", "remove", "-y", "sssd-common", "audit", "bluez"}
-	mgr.logger.Debug().Msg("Running '" + strings.Join(args, " ") + "'")
-	_, err = handle.Command(args)
+	_, err = mgr.logAndRunCommand(handle, args)
 	if err != nil {
 		return
 	}
 	// Now that we removed SSSD, we also have to make sure that it's not being used in PAM.
 	// The way to do this on Fedora (and likely other RHEL-based distros) is with authselect.
 	args = []string{"authselect", "select", "minimal"}
-	mgr.logger.Debug().Msg("Running '" + strings.Join(args, " ") + "'")
-	_, err = handle.Command(args)
+	_, err = mgr.logAndRunCommand(handle, args)
 	return err
 }
 
 // requires an Augeas handle to be open
 func (mgr *TemplateManager) updateSshdConfig(handle *guestfs.Guestfs) error {
-	sshdConfigExists, err := handle.Is_file("/etc/ssh/sshd_config", nil)
+	// WARNING: do NOT create /etc/ssh/sshd_config if it does not exist!
+	// On some distros, like OpenSUSE Tumbleweed, sshd_config does not exist,
+	// and creating a new file will actually prevent cloud-init from copying
+	// the user's SSH key to the default user's authorized_keys.
+	const dropinDir = "/etc/ssh/sshd_config.d"
+	mgr.logger.Debug().Msg("Creating " + dropinDir)
+	err := handle.Mkdir_p(dropinDir)
 	if err != nil {
 		return err
 	}
-	if !sshdConfigExists {
-		// On some distros, like OpenSUSE Tumbleweed, sshd_config does not exist,
-		// and creating a new file will actually prevent cloud-init from copying
-		// the user's SSH key to the default user's authorized_keys.
-		return nil
-	}
-	mgr.logger.Debug().Msg("Setting PrintLastLog=no in sshd_config")
-	return handle.Aug_set("/files/etc/ssh/sshd_config/PrintLastLog", "no")
+	filepath := dropinDir + "/csclub.conf"
+	mgr.logger.Debug().Msg("Writing to " + filepath)
+	return handle.Write(filepath, []byte("PrintLastLog no\n"))
 }
 
 func (mgr *TemplateManager) setJournaldConf(handle *guestfs.Guestfs) (err error) {

scripts/guestfish.sh

@@ -9,4 +9,4 @@ if ! [ -e /usr/bin/qemu-img ]; then
     export PATH="$DEPS_DIR/usr/bin:$PATH"
 fi
 set -x
-exec $GUESTFISH
+exec $GUESTFISH --network