verify VMs and delete old templates

main.go

@@ -19,7 +19,6 @@ func main() {
 	setupLogging()
 	cfg := config.New()
 	builder := cloudbuilder.New(cfg)
-
 	if err := builder.Start(); err != nil {
 		panic(err)
 	}

pkg/cloudbuilder/cloudbuilder.go

@@ -99,7 +99,7 @@ func (c *CloudBuilder) CreateVM(
 	if err != nil {
 		return
 	}
-	const pollInterval = 10 * time.Second
+	const pollInterval = 20 * time.Second
 	// Due to clock drift, we may end up waiting a bit longer than this
 	const maxTimeToWait = 30 * time.Minute
 	var timeWaited time.Duration = 0
@@ -132,9 +132,14 @@ type DistroInfo struct {
 	user          string
 }
 
-func (c *CloudBuilder) getExistingTemplateVersions() map[string]string {
+type TemplateVersionInfo struct {
+	TemplateId string
+	Version    string
+}
+
+func (c *CloudBuilder) getExistingTemplateVersions() map[string]TemplateVersionInfo {
 	templatePattern := regexp.MustCompile("^CSC (?P<distro>[A-Za-z][A-Za-z ]+[A-Za-z]) (?P<version>\\d+(\\.\\d+)?)$")
-	mostRecentVersions := make(map[string]string)
+	mostRecentVersions := make(map[string]TemplateVersionInfo)
 	templates := c.client.ListTemplates()
 	for _, template := range templates {
 		submatches := templatePattern.FindStringSubmatch(template.Name)
@@ -146,23 +151,22 @@ func (c *CloudBuilder) getExistingTemplateVersions() map[string]string {
 		version := submatches[2]
 		log.Debug().Str("distro", distro).Str("version", version).Msg("Found template")
 		otherVersion, ok := mostRecentVersions[distro]
-		if !ok || version > otherVersion {
+		if !ok || version > otherVersion.Version {
-			mostRecentVersions[distro] = version
+			mostRecentVersions[distro] = TemplateVersionInfo{
+				TemplateId: template.Id,
+				Version:    version,
+			}
 		}
 	}
 	return mostRecentVersions
 }
 
-func (c *CloudBuilder) sendEmailNotification(
+func (c *CloudBuilder) sendEmailNotification(templateName string) (err error) {
-	templateName string, vm *cloudstack.VirtualMachine, vmUser string,
-) (err error) {
 	tmpl := template.Must(template.New("email-message").Parse(emailMessageTemplate))
 	data := map[string]interface{}{
 		"cfg":          c.cfg,
 		"date":         time.Now().Format(time.RFC1123Z),
 		"templateName": templateName,
-		"vm":           vm,
-		"vmUser":       vmUser,
 	}
 	var buf bytes.Buffer
 	if err = tmpl.Execute(&buf, data); err != nil {
@@ -200,6 +204,15 @@ func (c *CloudBuilder) createNewTemplate(
 	return
 }
 
+func (c *CloudBuilder) destroyVirtualMachine(vmID string) error {
+	log.Info().Str("id", vmID).Msg("Deleting VM")
+	deletionJobID, err := c.client.DestroyVirtualMachine(vmID)
+	if err != nil {
+		return err
+	}
+	return c.client.WaitForJobToComplete(deletionJobID)
+}
+
 func (c *CloudBuilder) versionStringCompare(version1, version2 string) int {
 	f1, err := strconv.ParseFloat(version1, 32)
 	if err != nil {
@@ -247,7 +260,8 @@ func (c *CloudBuilder) Start() (err error) {
 			user:          "almalinux",
 		},
 	}
-	existingVersions := c.getExistingTemplateVersions()
+	existingTemplates := c.getExistingTemplateVersions()
+	newVersions := make(map[string]string)
 	for _, distroLower := range c.cfg.DistrosToCheck {
 		distroInfo, ok := distrosInfo[distroLower]
 		if !ok {
@@ -262,7 +276,8 @@ func (c *CloudBuilder) Start() (err error) {
 		}
 		log.Debug().Str("newVersion", newVersion).Str("codename", codename).Msg(distro)
 
-		curVersion, ok := existingVersions[distro]
+		curTemplate, ok := existingTemplates[distro]
+		curVersion := curTemplate.Version
 		if ok && c.versionStringCompare(newVersion, curVersion) <= 0 {
 			log.Debug().
 				Str("distro", distro).
@@ -270,9 +285,11 @@ func (c *CloudBuilder) Start() (err error) {
 				Msg("Existing version is up to date, skipping")
 			continue
 		}
+
+		newVersions[distro] = newVersion
 		log.Info().
 			Str("distro", distroLower).
-			Msg("Existing template is out of date, creating a new one")
+			Msg("Template is nonexistent or out of date, creating a new one")
 
 		var template *cloudstack.Template
 		template, err = c.createNewTemplate(newVersion, codename, &distroInfo)
@@ -280,6 +297,7 @@ func (c *CloudBuilder) Start() (err error) {
 			return
 		}
 
+		// Create a VM using the new template
 		vmName := strings.Join([]string{
 			strings.ReplaceAll(distroLower, " ", "-"),
 			strings.ReplaceAll(newVersion, ".", "-"),
@@ -291,7 +309,30 @@ func (c *CloudBuilder) Start() (err error) {
 			return
 		}
 
-		if err = c.sendEmailNotification(template.Name, vm, distroInfo.user); err != nil {
+		// Make sure that everything is working properly in the VM
+		verifier := NewVMVerifier(c.cfg, distroInfo.user, vm.Nic[0].IpAddress, vmName, distroInfo.manager)
+		if err = verifier.Verify(); err != nil {
+			return
+		}
+
+		// Since we don't need the VM anymore, delete it
+		if err = c.destroyVirtualMachine(vm.Id); err != nil {
+			return
+		}
+		vm = nil
+
+		// When we originally created the template, it wasn't public,
+		// so make it public now
+		if err = c.client.MakeTemplatePublicAndFeatured(template.Id); err != nil {
+			return
+		}
+
+		if err = c.sendEmailNotification(template.Name); err != nil {
+			return
+		}
+	}
+	if c.cfg.DeleteOldTemplates {
+		if err = c.DeleteOldTemplates(existingTemplates, newVersions); err != nil {
 			return
 		}
 	}

pkg/cloudbuilder/email-message.txt

@@ -10,24 +10,12 @@ This is an automated message from cloudbuild, the CSC VM template
 builder.
 
 A new VM template, {{ .templateName }}, has been uploaded to CloudStack.
-It is not public. A new VM, {{ .vm.Name }}, has been created from this
+It is public and featured. If you have a chance, please create a new VM
-template. You can SSH into this VM from biloba or chamomile by running
+from this template and test it out to make sure everything is working
-the following:
+properly.
 
-  ssh -i /var/lib/cloudstack/management/.ssh/id_rsa {{ .vmUser }}@{{ (index .vm.Nic 0).IpAddress }}
+If you have any issues with cloudbuild, please report them here:
-
+https://git.csclub.uwaterloo.ca/cloud/cloudbuild/issues
-Please login to the VM and verify that everything is working correctly.
-Once you have done this, please login to CloudStack with the admin account
-and perform the following:
-
-  1. Delete the VM (enable the "Expunge" option too).
-  2. Make the template public:
-     From the web page for the template, click the "Update Template Sharing"
-     circular button in the top right corner, and toggle the "Public"
-     and "Featured" sliders.
-  3. Delete the old template (if it is not being used by any VMs):
-     From the web page for the old template, click the "Zones" tab, then
-     press the red circular "Delete" button beside "{{ .cfg.CloudstackZoneName }}".
 
 Sincerely,
 cloudbuild

pkg/cloudbuilder/template_deletion.go

@@ -0,0 +1,24 @@
+package cloudbuilder
+
+import "github.com/rs/zerolog/log"
+
+func (c *CloudBuilder) DeleteOldTemplates(oldTemplates map[string]TemplateVersionInfo, newVersions map[string]string) error {
+	for distro, _ := range newVersions {
+		oldTemplate, ok := oldTemplates[distro]
+		if !ok {
+			continue
+		}
+		log.Info().
+			Str("oldVersion", oldTemplate.Version).
+			Str("distro", distro).
+			Msg("Deleting template")
+		jobID, err := c.client.DeleteTemplate(oldTemplate.TemplateId)
+		if err != nil {
+			return err
+		}
+		if err = c.client.WaitForJobToComplete(jobID); err != nil {
+			return err
+		}
+	}
+	return nil
+}

pkg/cloudbuilder/vm_verifier.go

@@ -0,0 +1,161 @@
+package cloudbuilder
+
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"net"
+	"os/exec"
+	"strings"
+	"time"
+
+	"github.com/rs/zerolog/log"
+
+	"git.csclub.uwaterloo.ca/cloud/cloudbuild/pkg/config"
+	"git.csclub.uwaterloo.ca/cloud/cloudbuild/pkg/distros"
+)
+
+type VMVerifier struct {
+	cfg             *config.Config
+	user            string
+	ipAddress       string
+	hostname        string
+	templateManager distros.ITemplateManager
+}
+
+func NewVMVerifier(
+	cfg *config.Config,
+	user string,
+	ipAddress string,
+	hostname string,
+	templateManager distros.ITemplateManager,
+) *VMVerifier {
+	return &VMVerifier{
+		cfg:             cfg,
+		user:            user,
+		ipAddress:       ipAddress,
+		hostname:        hostname,
+		templateManager: templateManager,
+	}
+}
+
+func waitUntilPort22IsOpen(ipAddress string) error {
+	const maxTimeToWait = 5 * time.Minute
+	const retryInterval = 5 * time.Second
+	const maxTries = int(maxTimeToWait / retryInterval)
+	address := ipAddress + ":22"
+	connected := false
+	for i := 0; i < maxTries; i++ {
+		_, err := net.DialTimeout("tcp", address, retryInterval)
+		if err == nil {
+			connected = true
+			break
+		}
+		if err.(*net.OpError).Timeout() {
+			log.Debug().Str("address", address).Msg("TCP connection timed out")
+		} else {
+			log.Debug().Str("address", address).Msg(err.Error())
+			time.Sleep(retryInterval)
+		}
+	}
+	if !connected {
+		return fmt.Errorf("Could not connect to %s", address)
+	}
+	return nil
+}
+
+func (v *VMVerifier) prepareSSHCommand(args ...string) *exec.Cmd {
+	log.Debug().
+		Str("user", v.user).
+		Str("address", v.ipAddress).
+		Msg("Running `" + strings.Join(args, " ") + "`")
+	args = append(
+		[]string{
+			"-i", v.cfg.SSHKeyPath,
+			"-o", "IdentitiesOnly=yes",
+			"-o", "StrictHostKeyChecking=no",
+			"-o", "CheckHostIP=no",
+			"-o", "UserKnownHostsFile=/dev/null",
+			"-o", "LogLevel=ERROR",
+			v.user + "@" + v.ipAddress,
+		},
+		args...,
+	)
+	return exec.Command("ssh", args...)
+}
+
+func (v *VMVerifier) runSSHCommand(args ...string) error {
+	return v.prepareSSHCommand(args...).Run()
+}
+
+func (v *VMVerifier) verifyThatVMCanResolveItsOwnHostname() error {
+	return v.runSSHCommand("ping", "-c", "1", "-w", "1", v.hostname)
+}
+
+func (v *VMVerifier) verifyThatSLAACandRAareDisabled() error {
+	output, err := v.prepareSSHCommand("ip", "-6", "addr", "show").Output()
+	if err != nil {
+		return err
+	}
+	if bytes.Contains(output, []byte("dynamic")) {
+		log.Debug().
+			Str("address", v.ipAddress).
+			Msg("IPv6 SLAAC address detected:\n" + string(output))
+		return fmt.Errorf("IPv6 SLAAC address detected")
+	}
+	output, err = v.prepareSSHCommand("ip", "-6", "route", "show").Output()
+	if err != nil {
+		return err
+	}
+	if bytes.Contains(output, []byte("proto ra")) {
+		log.Debug().
+			Str("address", v.ipAddress).
+			Msg("IPv6 RA route detected:\n" + string(output))
+		return fmt.Errorf("IPv6 RA route detected")
+	}
+	return nil
+}
+
+func (v *VMVerifier) verifyThatPackageCacheCanBeUpdated() error {
+	return v.runSSHCommand(v.templateManager.CommandToUpdatePackageCache()...)
+}
+
+func (v *VMVerifier) verifyThatCloudInitServicesDidNotFail() error {
+	output, err := v.prepareSSHCommand("systemctl", "list-units", "--state=failed", "-o", "json").Output()
+	if err != nil {
+		return err
+	}
+	var data []struct {
+		Unit   string `json:"unit"`
+		Active string `json:"active"`
+	}
+	if err = json.Unmarshal(output, &data); err != nil {
+		return err
+	}
+	for _, unit := range data {
+		if strings.HasPrefix(unit.Unit, "cloud-") && unit.Active == "failed" {
+			return fmt.Errorf("unit %s failed", unit.Unit)
+		}
+	}
+	return nil
+}
+
+func (v *VMVerifier) Verify() (err error) {
+	log.Debug().Str("user", v.user).Str("address", v.ipAddress).Msg("Verifying VM")
+	if err = waitUntilPort22IsOpen(v.ipAddress); err != nil {
+		return
+	}
+	if err = v.verifyThatVMCanResolveItsOwnHostname(); err != nil {
+		return
+	}
+	if err = v.verifyThatSLAACandRAareDisabled(); err != nil {
+		return
+	}
+	if err = v.verifyThatPackageCacheCanBeUpdated(); err != nil {
+		return
+	}
+	if err = v.verifyThatCloudInitServicesDidNotFail(); err != nil {
+		return
+	}
+	return
+}

pkg/cloudstack/cloudstack.go

@@ -19,6 +19,9 @@ import (
 	"net/url"
 	"sort"
 	"strings"
+	"time"
+
+	"github.com/rs/zerolog/log"
 
 	"git.csclub.uwaterloo.ca/cloud/cloudbuild/pkg/config"
 )
@@ -330,6 +333,49 @@ func (client *CloudstackClient) RegisterTemplate(name string, downloadUrl string
 	return &data.Template[0], nil
 }
 
+type UpdateTemplatePermissionsResponse struct {
+	ErrorInfo
+	Success bool `json:"success"`
+}
+
+func (client *CloudstackClient) MakeTemplatePublicAndFeatured(templateID string) error {
+	url := client.createURL(map[string]string{
+		"command":    "updateTemplatePermissions",
+		"id":         templateID,
+		"ispublic":   "true",
+		"isfeatured": "true",
+	})
+	responseWrapper := struct {
+		Response UpdateTemplatePermissionsResponse `json:"updatetemplatepermissionsresponse"`
+	}{}
+	getDeserializedResponse(url, &responseWrapper)
+	data := &responseWrapper.Response
+	checkErrorInfo(&data.ErrorInfo)
+	if !data.Success {
+		return fmt.Errorf("updateTemplatePermissions failed")
+	}
+	return nil
+}
+
+type DeleteTemplateResponse struct {
+	ErrorInfo
+	JobId string `json:"jobid"`
+}
+
+func (client *CloudstackClient) DeleteTemplate(templateID string) (jobID string, err error) {
+	url := client.createURL(map[string]string{
+		"command": "deleteTemplate",
+		"id":      templateID,
+	})
+	responseWrapper := struct {
+		Response DeleteTemplateResponse `json:"deletetemplateresponse"`
+	}{}
+	getDeserializedResponse(url, &responseWrapper)
+	data := &responseWrapper.Response
+	checkErrorInfo(&data.ErrorInfo)
+	return data.JobId, nil
+}
+
 type VirtualMachine struct {
 	Id          string `json:"id"`
 	DisplayName string `json:"displayname"`
@@ -377,6 +423,66 @@ func (client *CloudstackClient) DeployVirtualMachine(
 	return data.Id, nil
 }
 
+type DestroyVirtualMachineResponse struct {
+	ErrorInfo
+	JobId string `json:"jobid"`
+}
+
+// DestroyVirtualMachine returns the job ID of the job which deletes the
+// VM asynchronously.
+func (client *CloudstackClient) DestroyVirtualMachine(vmID string) (jobID string, err error) {
+	url := client.createURL(map[string]string{
+		"command": "destroyVirtualMachine",
+		"id":      vmID,
+		"expunge": "true",
+	})
+	responseWrapper := struct {
+		Response DestroyVirtualMachineResponse `json:"destroyvirtualmachineresponse"`
+	}{}
+	getDeserializedResponse(url, &responseWrapper)
+	data := &responseWrapper.Response
+	checkErrorInfo(&data.ErrorInfo)
+	return data.JobId, nil
+}
+
+type QueryAsyncJobResultResponse struct {
+	ErrorInfo
+	Created   string `json:"created"`
+	Completed string `json:"completed"`
+	JobStatus int    `json:"jobstatus"`
+}
+
+func (client *CloudstackClient) WaitForJobToComplete(jobID string) error {
+	url := client.createURL(map[string]string{
+		"command": "queryAsyncJobResult",
+		"jobid":   jobID,
+	})
+	const maxTimeToWait = 30 * time.Minute
+	const retryInterval = 5 * time.Second
+	const maxTries = int(maxTimeToWait / retryInterval)
+	for i := 0; i < maxTries; i++ {
+		responseWrapper := struct {
+			Response QueryAsyncJobResultResponse `json:"queryasyncjobresultresponse"`
+		}{}
+		getDeserializedResponse(url, &responseWrapper)
+		data := &responseWrapper.Response
+		checkErrorInfo(&data.ErrorInfo)
+		if data.JobStatus == 0 {
+			log.Debug().Str("id", jobID).Msg("job is still pending")
+			time.Sleep(retryInterval)
+			continue
+		} else if data.JobStatus == 1 {
+			log.Debug().Str("id", jobID).Msg("job completed successfully")
+			return nil
+		} else {
+			// either an error, or an unknown status code
+			// see https://github.com/apache/cloudstack-cloudmonkey/blob/main/cmd/network.go
+			return fmt.Errorf("Job %s has status code %d", jobID, data.JobStatus)
+		}
+	}
+	return fmt.Errorf("Job %s took too long", jobID)
+}
+
 type ListVirtualMachinesResponse struct {
 	ErrorInfo
 	Count          int              `json:"count"`

pkg/config/config.go

@@ -7,6 +7,11 @@ import (
 	"strings"
 )
 
+func isFalsy(s string) bool {
+	s = strings.ToLower(s)
+	return s == "false" || s == "no" || s == "0"
+}
+
 // A Config holds all of the configuration values needed for the program.
 type Config struct {
 	CloudstackApiKey              string
@@ -15,6 +20,8 @@ type Config struct {
 	CloudstackZoneName            string
 	CloudstackServiceOfferingName string
 	CloudstackKeypairName         string
+	SSHKeyPath                    string
+	DeleteOldTemplates            bool
 	// These must be keys of the distrosInfo map in cloudbuilder.go
 	DistrosToCheck  []string
 	UploadDirectory string
@@ -33,9 +40,14 @@ func New() *Config {
 	cfg := &Config{
 		CloudstackApiKey:    os.Getenv("CLOUDSTACK_API_KEY"),
 		CloudstackSecretKey: os.Getenv("CLOUDSTACK_SECRET_KEY"),
+		SSHKeyPath:          os.Getenv("SSH_KEY_PATH"),
 		UploadDirectory:     os.Getenv("UPLOAD_DIRECTORY"),
 		UploadBaseUrl:       os.Getenv("UPLOAD_BASE_URL"),
 		EmailRecipient:      os.Getenv("EMAIL_RECIPIENT"),
+		DeleteOldTemplates:  true,
+	}
+	if isFalsy(os.Getenv("DELETE_OLD_TEMPLATES")) {
+		cfg.DeleteOldTemplates = false
 	}
 	if cfg.CloudstackApiKey == "" {
 		panic("CLOUDSTACK_API_KEY is empty or not set")
@@ -43,6 +55,9 @@ func New() *Config {
 	if cfg.CloudstackSecretKey == "" {
 		panic("CLOUDSTACK_SECRET_KEY is empty or not set")
 	}
+	if cfg.SSHKeyPath == "" {
+		panic("SSH_KEY_PATH is empty or not set")
+	}
 	if val, ok := os.LookupEnv("DISTROS_TO_CHECK"); ok {
 		cfg.DistrosToCheck = strings.Split(val, ",")
 	} else {

pkg/distros/almalinux.go

@@ -82,6 +82,10 @@ func (mgr *AlmaLinuxTemplateManager) HasSELinuxEnabled() bool {
 	return true
 }
 
+func (mgr *AlmaLinuxTemplateManager) CommandToUpdatePackageCache() []string {
+	return fedoraCommandToUpdatePackageCache()
+}
+
 var almaLinuxYumRepoBaseUrlPattern *regexp.Regexp = regexp.MustCompile(
 	"^(?P<scheme>https?://)[A-Za-z0-9./-]+(?P<path>/almalinux/\\$releasever/[A-Za-z0-9./$-]+)$",
 )
@@ -105,7 +109,7 @@ func (mgr *AlmaLinuxTemplateManager) PerformDistroSpecificModifications(handle *
 	if err = mgr.addCloudInitSnippet(handle); err != nil {
 		return
 	}
-	if err = mgr.replaceYumRepoMirrorUrls(handle, mgr.transformAlmaLinuxYumRepoBaseUrl); err != nil {
+	if err = mgr.replaceYumMirrorUrls(handle, mgr.transformAlmaLinuxYumRepoBaseUrl); err != nil {
 		return
 	}
 	return

pkg/distros/debian.go

@@ -80,6 +80,10 @@ func (mgr *DebianTemplateManager) UsesNetworkManager() bool {
 	return false
 }
 
+func (mgr *DebianTemplateManager) CommandToUpdatePackageCache() []string {
+	return debianCommandToUpdatePackageCache()
+}
+
 func (mgr *DebianTemplateManager) PerformDistroSpecificModifications(handle *guestfs.Guestfs) (err error) {
 	if err = mgr.setChronyOptions(handle, "/etc/chrony/chrony.conf"); err != nil {
 		return

pkg/distros/fedora.go

@@ -127,6 +127,10 @@ func (mgr *FedoraTemplateManager) HasSELinuxEnabled() bool {
 	return true
 }
 
+func (mgr *FedoraTemplateManager) CommandToUpdatePackageCache() []string {
+	return fedoraCommandToUpdatePackageCache()
+}
+
 var fedoraYumRepoBaseUrlPattern *regexp.Regexp = regexp.MustCompile(
 	"^(?P<scheme>https?://)[A-Za-z0-9./-]+(?P<path>/fedora/linux/[A-Za-z0-9./$-]+)$",
 )
@@ -150,7 +154,7 @@ func (mgr *FedoraTemplateManager) PerformDistroSpecificModifications(handle *gue
 	if err = mgr.addCloudInitSnippet(handle); err != nil {
 		return
 	}
-	if err = mgr.replaceYumRepoMirrorUrls(handle, mgr.transformFedoraYumRepoBaseUrl); err != nil {
+	if err = mgr.replaceYumMirrorUrls(handle, mgr.transformFedoraYumRepoBaseUrl); err != nil {
 		return
 	}
 	return

pkg/distros/template_manager.go

@@ -45,6 +45,7 @@ type TemplateManager struct {
 }
 
 type ITemplateManager interface {
+	IDistroSpecificTemplateManager
 	// 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)
@@ -66,6 +67,10 @@ type IDistroSpecificTemplateManager interface {
 	PerformDistroSpecificModifications(handle *guestfs.Guestfs) error
 	// HasSELinuxEnabled returns whether SELinux is enabled for this distro
 	HasSELinuxEnabled() bool
+	// CommandToUpdatePackageCache returns the command which updates
+	// (but does not upgrade) the packages for this distro, e.g.
+	// "sudo apt update".
+	CommandToUpdatePackageCache() []string
 }
 
 func (mgr *TemplateManager) DownloadTemplateGeneric(filename, url string) (path string, err error) {
@@ -86,6 +91,14 @@ func (mgr *TemplateManager) DownloadTemplateGeneric(filename, url string) (path
 	return
 }
 
+func debianCommandToUpdatePackageCache() []string {
+	return []string{"sudo", "apt", "update"}
+}
+
+func fedoraCommandToUpdatePackageCache() []string {
+	return []string{"sudo", "dnf", "makecache"}
+}
+
 func (mgr *TemplateManager) performDistroAgnosticModifications(handle *guestfs.Guestfs) (err error) {
 	if err = mgr.setupIpv6Scripts(handle); err != nil {
 		return
@@ -347,7 +360,24 @@ func getNamedRegexGroup(re *regexp.Regexp, submatches []string, groupName string
 	return value
 }
 
-func (mgr *TemplateManager) addAugeasComment(
+func getNumAugeasComments(handle *guestfs.Guestfs, parentNode string) (numComments int, err error) {
+	keyPaths, err := handle.Aug_ls(parentNode)
+	if err != nil {
+		return 0, fmt.Errorf("aug_ls(%s) failed: %w", parentNode, err)
+	}
+	for _, keyPath := range keyPaths {
+		key, err := handle.Aug_label(keyPath)
+		if err != nil {
+			return 0, fmt.Errorf("aug_label(%s) failed: %w", keyPath, err)
+		}
+		if strings.HasPrefix(key, "#comment") {
+			numComments += 1
+		}
+	}
+	return
+}
+
+func addAugeasComment(
 	handle *guestfs.Guestfs, numExistingComments int, parentNode string, comment string,
 ) error {
 	return handle.Aug_set(
@@ -356,17 +386,35 @@ func (mgr *TemplateManager) addAugeasComment(
 	)
 }
 
-// replaceYumRepoMirrorUrls comments out the metalink URLs and uncomments and
+func (mgr *TemplateManager) commentOutAugeasPath(
+	handle *guestfs.Guestfs, numExistingComments int, augeasPath string, commentedValue string,
+) error {
+	lastSlashIndex := strings.LastIndex(augeasPath, "/")
+	if lastSlashIndex == -1 {
+		// sanity check
+		panic("augeasPath must have a slash")
+	}
+	parentNode := augeasPath[:lastSlashIndex]
+	mgr.logger.Debug().Msg("Commenting out " + augeasPath)
+	if _, err := handle.Aug_rm(augeasPath); err != nil {
+		return fmt.Errorf("Could not remove %s: %w", augeasPath, err)
+	}
+	if err := addAugeasComment(handle, numExistingComments, parentNode, commentedValue); err != nil {
+		return fmt.Errorf("Could not insert comment in %s: %w", parentNode, err)
+	}
+	return nil
+}
+
+// replaceYumRepoMirrorUrls comments out the metalink and mirrorlist URLs and
 // replaces the baseurl URLs for each repo in /etc/yum.repos.d.
-// It assumes that the baseurl will be a commented line.
+// It assumes that the baseurl will be present or commented.
 //
 // transformBaseUrl accepts a repo base URL and replaces the host (and
 // optionally parts of the path) as appropriate. It should return the input
 // if the URL should not be modified.
-func (mgr *TemplateManager) replaceYumRepoMirrorUrls(
+func (mgr *TemplateManager) replaceYumMirrorUrls(
 	handle *guestfs.Guestfs, transformBaseurl func(string) string,
-) (err error) {
+) error {
-	log := mgr.logger
 	repoPaths, err := handle.Aug_ls("/files/etc/yum.repos.d")
 	if err != nil {
 		return fmt.Errorf("Could not enumerate yum repos: %w", err)
@@ -379,6 +427,19 @@ func (mgr *TemplateManager) replaceYumRepoMirrorUrls(
 		}
 		// A subrepoPath looks like e.g. /files/etc/yum.repos.d/almalinux-appstream.repo/appstream
 		for _, subrepoPath := range subrepoPaths {
+			if err = mgr.replaceYumMirrorUrlsForSingleSubrepo(handle, transformBaseurl, subrepoPath); err != nil {
+				return fmt.Errorf("Could not replace mirror URLs for %s: %w", subrepoPath, err)
+			}
+		}
+	}
+	return nil
+}
+
+func (mgr *TemplateManager) replaceYumMirrorUrlsForSingleSubrepo(
+	handle *guestfs.Guestfs, transformBaseurl func(string) string, subrepoPath string,
+) (err error) {
+	log := mgr.logger
+	// subrepoPath looks like e.g. /files/etc/yum.repos.d/almalinux-appstream.repo/appstream
 	keyPaths, err := handle.Aug_ls(subrepoPath)
 	if err != nil {
 		return fmt.Errorf("Could not enumerate keys for %s: %w", subrepoPath, err)
@@ -387,6 +448,7 @@ func (mgr *TemplateManager) replaceYumRepoMirrorUrls(
 		numComments        int
 		commentedBaseurl   string
 		mirrorlist         string
+		metalink           string
 		uncommentedBaseurl string
 	)
 	// A keyPath looks like e.g. /files/etc/yum.repos.d/almalinux-appstream.repo/appstream/mirrorlist
@@ -407,6 +469,8 @@ func (mgr *TemplateManager) replaceYumRepoMirrorUrls(
 			}
 		} else if key == "mirrorlist" {
 			mirrorlist = value
+		} else if key == "metalink" {
+			metalink = value
 		} else if key == "baseurl" {
 			uncommentedBaseurl = value
 		}
@@ -414,32 +478,27 @@ func (mgr *TemplateManager) replaceYumRepoMirrorUrls(
 	var baseurl string
 	if uncommentedBaseurl != "" {
 		baseurl = transformBaseurl(uncommentedBaseurl)
-				if baseurl == uncommentedBaseurl {
-					baseurl = ""
-				}
 	} else if commentedBaseurl != "" {
 		baseurl = transformBaseurl(commentedBaseurl)
-				if baseurl == commentedBaseurl {
-					baseurl = ""
 	}
+	if baseurl == "" {
+		return
 	}
-			if baseurl != "" {
 	log.Debug().Msg(fmt.Sprintf("Setting %s to %s", subrepoPath+"/baseurl", baseurl))
 	if err = handle.Aug_set(subrepoPath+"/baseurl", baseurl); err != nil {
 		return fmt.Errorf("Could not set baseurl for %s: %w", subrepoPath, err)
 	}
 	if mirrorlist != "" {
-					// comment out the mirrorlist line
+		if err = mgr.commentOutAugeasPath(handle, numComments, subrepoPath+"/mirrorlist", "mirrorlist="+mirrorlist); err != nil {
-					log.Debug().Msg("Commenting out " + subrepoPath + "/mirrorlist")
+			return
-					if _, err = handle.Aug_rm(subrepoPath + "/mirrorlist"); err != nil {
-						return fmt.Errorf("Could not remove %s: %w", subrepoPath+"/mirrorlist", err)
-					}
-					if err = mgr.addAugeasComment(handle, numComments, subrepoPath, "mirrorlist="+mirrorlist); err != nil {
-						return fmt.Errorf("Could not insert comment in %s: %w", subrepoPath, err)
-					}
 		}
+		numComments += 1
 	}
+	if metalink != "" {
+		if err = mgr.commentOutAugeasPath(handle, numComments, subrepoPath+"/metalink", "metalink="+metalink); err != nil {
+			return
 		}
+		numComments += 1
 	}
 	return
 }

pkg/distros/ubuntu.go

@@ -92,7 +92,7 @@ func (mgr *UbuntuTemplateManager) disableNoisyMotdMessages(handle *guestfs.Guest
 		"00-header",
 		"10-help-text",
 		"50-landscape-sysinfo",
-		"50-motd-news",
+		//"50-motd-news",
 		"88-esm-announce",
 	}
 	for _, filename := range filesToDisable {
@@ -113,6 +113,10 @@ func (mgr *UbuntuTemplateManager) UsesNetworkManager() bool {
 	return false
 }
 
+func (mgr *UbuntuTemplateManager) CommandToUpdatePackageCache() []string {
+	return debianCommandToUpdatePackageCache()
+}
+
 func (mgr *UbuntuTemplateManager) PerformDistroSpecificModifications(handle *guestfs.Guestfs) (err error) {
 	if err = mgr.setTimesyncdConf(handle); err != nil {
 		return