Unify --recover and --boot-entry upgrade options in code

Signed-off-by: Dimitris Karakasilis <dimitris@karakasilis.me>

internal/agent/upgrade.go

@@ -65,18 +65,18 @@ func ListNewerReleases(includePrereleases bool) ([]string, error) {
 }
 
 func Upgrade(
-	source string, force, strictValidations bool, dirs []string, singleEntry string, preReleases, upgradeRecovery bool) error {
+	source string, force, strictValidations bool, dirs []string, upgradeEntry string, preReleases bool) error {
 	bus.Manager.Initialize()
 
 	if internalutils.UkiBootMode() == internalutils.UkiHDD {
-		return upgradeUki(source, dirs, singleEntry, strictValidations, upgradeRecovery)
+		return upgradeUki(source, dirs, upgradeEntry, strictValidations)
 	} else {
-		return upgrade(source, force, strictValidations, dirs, preReleases, upgradeRecovery)
+		return upgrade(source, force, strictValidations, dirs, upgradeEntry, preReleases)
 	}
 }
 
-func upgrade(source string, force, strictValidations bool, dirs []string, preReleases, upgradeRecovery bool) error {
+func upgrade(source string, force, strictValidations bool, dirs []string, upgradeEntry string, preReleases bool) error {
-	upgradeSpec, c, err := generateUpgradeSpec(source, force, strictValidations, dirs, "", preReleases, upgradeRecovery)
+	upgradeSpec, c, err := generateUpgradeSpec(source, force, strictValidations, dirs, upgradeEntry, preReleases)
 	if err != nil {
 		return err
 	}
@@ -135,16 +135,10 @@ func newerReleases() (versioneer.TagList, error) {
 
 // generateUpgradeConfForCLIArgs creates a kairos configuration for `--source` and `--recovery`
 // command line arguments. It will be added to the rest of the configurations.
-func generateUpgradeConfForCLIArgs(source, singleEntry string, upgradeRecovery bool) (string, error) {
+func generateUpgradeConfForCLIArgs(source, upgradeEntry string) (string, error) {
 	upgradeConfig := ExtraConfigUpgrade{}
 
-	if upgradeRecovery {
+	upgradeConfig.Upgrade.Entry = upgradeEntry
-		upgradeConfig.Upgrade.Recovery = true
-	}
-
-	// if upgradeSingleEntry {
-	// 	upgradeConfig.Upgrade.Recovery = true
-	// }
 
 	// Set uri both for active and recovery because we don't know what we are
 	// actually upgrading. The "upgradeRecovery" is just the command line argument.
@@ -161,8 +155,8 @@ func generateUpgradeConfForCLIArgs(source, singleEntry string, upgradeRecovery b
 	return string(d), err
 }
 
-func generateUpgradeSpec(sourceImageURL string, force, strictValidations bool, dirs []string, singleEntry string, preReleases, upgradeRecovery bool) (*v1.UpgradeSpec, *config.Config, error) {
+func generateUpgradeSpec(sourceImageURL string, force, strictValidations bool, dirs []string, upgradeEntry string, preReleases bool) (*v1.UpgradeSpec, *config.Config, error) {
-	cliConf, err := generateUpgradeConfForCLIArgs(sourceImageURL, singleEntry, upgradeRecovery)
+	cliConf, err := generateUpgradeConfForCLIArgs(sourceImageURL, upgradeEntry)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -205,8 +199,8 @@ func getReleasesFromProvider(includePrereleases bool) ([]string, error) {
 	return result, nil
 }
 
-func upgradeUki(source string, dirs []string, singleEntry string, strictValidations, upgradeRecovery bool) error {
+func upgradeUki(source string, dirs []string, upgradeEntry string, strictValidations bool) error {
-	cliConf, err := generateUpgradeConfForCLIArgs(source, singleEntry, upgradeRecovery)
+	cliConf, err := generateUpgradeConfForCLIArgs(source, upgradeEntry)
 	if err != nil {
 		return err
 	}
@@ -244,8 +238,7 @@ func upgradeUki(source string, dirs []string, singleEntry string, strictValidati
 // ExtraConfigUpgrade is the struct that holds the upgrade options that come from flags and events
 type ExtraConfigUpgrade struct {
 	Upgrade struct {
-		Recovery       bool   `json:"recovery,omitempty"`
+		Entry          string `json:"entry,omitempty"`
-		SingleEntry    string `json:"single-entry,omitempty"`
 		RecoverySystem struct {
 			URI string `json:"uri,omitempty"`
 		} `json:"recovery-system,omitempty"`

main.go

@@ -69,7 +69,7 @@ var cmds = []*cli.Command{
 				Usage: "[DEPRECATED] Specify a full image reference, e.g.: quay.io/some/image:tag",
 			},
 			&sourceFlag,
-			&cli.StringFlag{Name: "single-entry", Usage: "Specify a \"single\" systemd-boot entry to upgrade (other than active/passive/recovery)"},
+			&cli.StringFlag{Name: "boot-entry", Usage: "Specify a systemd-boot entry to upgrade (other than active/passive/recovery). The value should match the name of the '.efi' file."},
 			&cli.BoolFlag{Name: "pre", Usage: "Include pre-releases (rc, beta, alpha)"},
 			&cli.BoolFlag{Name: "recovery", Usage: "Upgrade recovery"},
 		},
@@ -186,12 +186,20 @@ See https://kairos.io/docs/upgrade/manual/ for documentation.
 				source = fmt.Sprintf("oci:%s", image)
 			}
 
-			// TODO: Too many flags? Merge --recovery and --single-entry somehow?
+			if c.Bool("recovery") && c.String("boot-entry") != "" {
-			// Does the new flag make sense in non-uki? (No)
+				return fmt.Errorf("only one of '--recovery' and '--boot-entry' can be set")
+			}
+
+			upgradeEntry := ""
+			if c.Bool("recovery") {
+				upgradeEntry = constants.BootEntryRecovery
+			} else if c.String("boot-entry") != "" {
+				upgradeEntry = c.String("boot-entry")
+			}
+
 			return agent.Upgrade(source, c.Bool("force"),
 				c.Bool("strict-validation"), constants.GetConfigScanDirs(),
-				c.String("single-entry"),
+				upgradeEntry, c.Bool("pre"),
-				c.Bool("pre"), c.Bool("recovery"),
 			)
 		},
 	},

pkg/action/upgrade.go

@@ -26,7 +26,7 @@ import (
 	"github.com/kairos-io/kairos-agent/v2/pkg/elemental"
 	v1 "github.com/kairos-io/kairos-agent/v2/pkg/types/v1"
 	"github.com/kairos-io/kairos-agent/v2/pkg/utils"
-	"github.com/kairos-io/kairos-agent/v2/pkg/utils/fs"
+	fsutils "github.com/kairos-io/kairos-agent/v2/pkg/utils/fs"
 	"github.com/kairos-io/kairos-sdk/state"
 )
 
@@ -90,7 +90,7 @@ func (u *UpgradeAction) upgradeInstallStateYaml(meta interface{}, img v1.Image)
 		Label:          img.Label,
 		FS:             img.FS,
 	}
-	if u.spec.RecoveryUpgrade {
+	if u.spec.RecoveryUpgrade() {
 		recoveryPart := u.spec.State.Partitions[constants.RecoveryPartName]
 		if recoveryPart == nil {
 			recoveryPart = &v1.PartitionState{
@@ -137,7 +137,7 @@ func (u *UpgradeAction) Run() (err error) {
 
 	e := elemental.NewElemental(u.config)
 
-	if u.spec.RecoveryUpgrade {
+	if u.spec.RecoveryUpgrade() {
 		upgradeImg = u.spec.Recovery
 		if upgradeImg.FS == constants.SquashFs {
 			finalImageFile = filepath.Join(u.spec.Partitions.Recovery.MountPoint, "cOS", constants.RecoverySquashFile)
@@ -227,7 +227,7 @@ func (u *UpgradeAction) Run() (err error) {
 	}
 
 	// Only apply rebrand stage for system upgrades
-	if !u.spec.RecoveryUpgrade {
+	if !u.spec.RecoveryUpgrade() {
 		u.Info("rebranding")
 		if rebrandingErr := e.SetDefaultGrubEntry(u.spec.Partitions.State.MountPoint, upgradeImg.MountPoint, u.spec.GrubDefEntry); rebrandingErr != nil {
 			u.config.Logger.Warn("failure while rebranding GRUB default entry (ignoring), run with --debug to see more details")
@@ -244,7 +244,7 @@ func (u *UpgradeAction) Run() (err error) {
 	// If not upgrading recovery and booting from non passive, backup active into passive
 	// We dont want to overwrite passive if we are booting from passive as it could mean that active is broken and we would
 	// be overriding a working passive with a broken/unknown  active
-	if u.spec.RecoveryUpgrade == false && bootedFrom != state.Passive {
+	if !u.spec.RecoveryUpgrade() && bootedFrom != state.Passive {
 		// backup current active.img to passive.img before overwriting the active.img
 		u.Info("Backing up current active image")
 		source := filepath.Join(u.spec.Partitions.State.MountPoint, "cOS", constants.ActiveImgFile)
@@ -289,7 +289,7 @@ func (u *UpgradeAction) Run() (err error) {
 	}
 
 	u.Info("Upgrade completed")
-	if !u.spec.RecoveryUpgrade {
+	if !u.spec.RecoveryUpgrade() {
 		u.config.Logger.Warn("Remember that recovery is upgraded separately by passing the --recovery flag to the upgrade command!\n" +
 			"See more info about this on https://kairos.io/docs/upgrade/")
 	}

pkg/config/spec.go

@@ -346,7 +346,7 @@ func setUpgradeSourceSize(cfg *Config, spec *v1.UpgradeSpec) error {
 	var err error
 
 	var targetSpec *v1.Image
-	if spec.RecoveryUpgrade {
+	if spec.RecoveryUpgrade() {
 		targetSpec = &(spec.Recovery)
 	} else {
 		targetSpec = &(spec.Active)

pkg/constants/constants.go

@@ -88,6 +88,7 @@ const (
 	GPT                          = "gpt"
 	UsrLocalPath                 = "/usr/local"
 	OEMPath                      = "/oem"
+	BootEntryRecovery            = "recovery"
 
 	// SELinux targeted policy paths
 	SELinuxTargetedPath        = "/etc/selinux/targeted"

pkg/types/v1/config.go

@@ -170,7 +170,7 @@ func (r *ResetSpec) ShouldReboot() bool   { return r.Reboot }
 func (r *ResetSpec) ShouldShutdown() bool { return r.PowerOff }
 
 type UpgradeSpec struct {
-	RecoveryUpgrade bool     `yaml:"recovery,omitempty" mapstructure:"recovery"`
+	Entry           string   `yaml:"entry,omitempty" mapstructure:"entry"`
 	Active          Image    `yaml:"system,omitempty" mapstructure:"system"`
 	Recovery        Image    `yaml:"recovery-system,omitempty" mapstructure:"recovery-system"`
 	GrubDefEntry    string   `yaml:"grub-entry-name,omitempty" mapstructure:"grub-entry-name"`
@@ -182,10 +182,14 @@ type UpgradeSpec struct {
 	State           *InstallState
 }
 
+func (u *UpgradeSpec) RecoveryUpgrade() bool {
+	return u.Entry == constants.BootEntryRecovery
+}
+
 // Sanitize checks the consistency of the struct, returns error
 // if unsolvable inconsistencies are found
 func (u *UpgradeSpec) Sanitize() error {
-	if u.RecoveryUpgrade {
+	if u.RecoveryUpgrade() {
 		if u.Recovery.Source.IsEmpty() {
 			return fmt.Errorf(constants.UpgradeNoSourceError)
 		}
@@ -528,12 +532,15 @@ func (i *InstallUkiSpec) GetPartitions() ElementalPartitions { return i.Partitio
 func (i *InstallUkiSpec) GetExtraPartitions() PartitionList  { return i.ExtraPartitions }
 
 type UpgradeUkiSpec struct {
-	RecoveryUpgrade    bool       `yaml:"recovery,omitempty" mapstructure:"recovery"`
+	Entry        string     `yaml:"entry,omitempty" mapstructure:"entry"`
-	UpgradeSingleEntry string     `yaml:"upgrade-single-entry,omitempty" mapstructure:"upgrade-single-entry"`
+	Active       Image      `yaml:"system,omitempty" mapstructure:"system"`
-	Active             Image      `yaml:"system,omitempty" mapstructure:"system"`
+	Reboot       bool       `yaml:"reboot,omitempty" mapstructure:"reboot"`
-	Reboot             bool       `yaml:"reboot,omitempty" mapstructure:"reboot"`
+	PowerOff     bool       `yaml:"poweroff,omitempty" mapstructure:"poweroff"`
-	PowerOff           bool       `yaml:"poweroff,omitempty" mapstructure:"poweroff"`
+	EfiPartition *Partition `yaml:"efi-partition,omitempty" mapstructure:"efi-partition"`
-	EfiPartition       *Partition `yaml:"efi-partition,omitempty" mapstructure:"efi-partition"`
+}
+
+func (i *UpgradeUkiSpec) RecoveryUpgrade() bool {
+	return i.Entry == constants.BootEntryRecovery
 }
 
 func (i *UpgradeUkiSpec) Sanitize() error {

pkg/uki/upgrade.go

@@ -51,17 +51,20 @@ func (i *UpgradeAction) Run() (err error) {
 	// If we decide to first copy and then rotate, we need ~4 times the size of
 	// the artifact set [TBD]
 
-	// When upgrading recovery, we don't want to replace loader.conf or any other
+	// When upgrading recovery or single entries, we don't want to replace loader.conf or any other
 	// files, thus we take a simpler approach and only install the new efi file
 	// and the relevant conf
-	if i.spec.RecoveryUpgrade {
+	if i.spec.RecoveryUpgrade() {
+		i.cfg.Logger.Infof("installing entry: recovery")
 		return i.installRecovery()
 	}
 
-	if i.spec.UpgradeSingleEntry != "" {
+	if i.spec.Entry != "" { // single entry upgrade
-		return i.installEntry(i.spec.UpgradeSingleEntry)
+		i.cfg.Logger.Infof("installing entry: %s", i.spec.Entry)
+		return i.installEntry(i.spec.Entry)
 	}
 
+	i.cfg.Logger.Infof("installing entry: active")
 	// Dump artifact to efi dir
 	_, err = e.DumpSource(constants.UkiEfiDir, i.spec.Active.Source)
 	if err != nil {
@@ -127,6 +130,11 @@ func (i *UpgradeAction) Run() (err error) {
 }
 
 func (i *UpgradeAction) installEntry(entry string) error {
+	targetEntryFile := filepath.Join(constants.UkiEfiDir, "EFI", "kairos", fmt.Sprintf("%s.efi", entry))
+	if _, err := os.Stat(targetEntryFile); err != nil {
+		return fmt.Errorf("could not stat target efi file for entry %s: %s", entry, err)
+	}
+
 	tmpDir, err := os.MkdirTemp("", "")
 	if err != nil {
 		i.cfg.Logger.Errorf("creating a tmp dir: %s", err.Error())
@@ -142,9 +150,7 @@ func (i *UpgradeAction) installEntry(entry string) error {
 		return err
 	}
 
-	err = copyFile(
+	err = copyFile(filepath.Join(tmpDir, "EFI", "kairos", UnassignedArtifactRole+".efi"), targetEntryFile)
-		filepath.Join(tmpDir, "EFI", "kairos", UnassignedArtifactRole+".efi"),
-		filepath.Join(constants.UkiEfiDir, "EFI", "kairos", fmt.Sprintf("%s.efi", entry)))
 	if err != nil {
 		i.cfg.Logger.Errorf("copying efi files: %s", err.Error())
 		return err