From 1fd95f7673f761d6efd21d563f20e9372d1dc787 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Thu, 11 May 2023 16:01:24 +0200
Subject: [PATCH 01/27] microshift admin backup

---
 cmd/microshift/main.go |  1 +
 pkg/admin/backup.go    | 51 ++++++++++++++++++++++++++++++++++++++++++
 pkg/cmd/admin.go       | 36 +++++++++++++++++++++++++++++
 pkg/cmd/run.go         |  2 +-
 pkg/config/files.go    |  5 +++++
 5 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 pkg/admin/backup.go
 create mode 100644 pkg/cmd/admin.go

diff --git a/cmd/microshift/main.go b/cmd/microshift/main.go
index 89432b827b..a6b1702513 100644
--- a/cmd/microshift/main.go
+++ b/cmd/microshift/main.go
@@ -38,5 +38,6 @@ func newCommand() *cobra.Command {
 	cmd.AddCommand(cmds.NewRunMicroshiftCommand())
 	cmd.AddCommand(cmds.NewVersionCommand(ioStreams))
 	cmd.AddCommand(cmds.NewShowConfigCommand(ioStreams))
+	cmd.AddCommand(cmds.NewAdminCommand())
 	return cmd
 }
diff --git a/pkg/admin/backup.go b/pkg/admin/backup.go
new file mode 100644
index 0000000000..d0dbc31657
--- /dev/null
+++ b/pkg/admin/backup.go
@@ -0,0 +1,51 @@
+package admin
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/openshift/microshift/pkg/config"
+
+	"k8s.io/klog/v2"
+)
+
+var (
+	cpArgs = []string{
+		"--verbose",
+		"--recursive",
+		"--preserve",
+		"--reflink=auto",
+	}
+)
+
+func MakeBackup(name string) error {
+	if err := config.CreateDir(config.BackupsDir); err != nil {
+		klog.Errorf("Failed to create dir %s: %v", config.BackupsDir, err)
+		return err
+	}
+
+	dest := filepath.Join(config.BackupsDir, name)
+	tmp_dest := dest + "-tmp"
+
+	args := append(cpArgs, config.DataDir, tmp_dest)
+	out, err := exec.Command("cp", args...).CombinedOutput() //nolint:gosec
+	klog.Infof("Output of cp (%v): %v", args, string(out))
+	if err != nil {
+		klog.Errorf("Failed to make backup - copy failed: %v", err)
+		return err
+	}
+
+	if err := os.RemoveAll(dest); err != nil {
+		klog.Errorf("Failed to remove %s directory: %v", tmp_dest, err)
+		return err
+	}
+
+	if err := os.Rename(tmp_dest, dest); err != nil {
+		klog.Errorf("Failed to rename %s to %s: %v", tmp_dest, dest, err)
+		return err
+	}
+
+	klog.Infof("Backed up current data to %s", dest)
+	return nil
+}
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
new file mode 100644
index 0000000000..37ea06b4ca
--- /dev/null
+++ b/pkg/cmd/admin.go
@@ -0,0 +1,36 @@
+package cmd
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/openshift/microshift/pkg/admin"
+	"github.com/openshift/microshift/pkg/config"
+	"github.com/openshift/microshift/pkg/version"
+
+	"github.com/spf13/cobra"
+)
+
+func NewAdminCommand() *cobra.Command {
+	backup := &cobra.Command{
+		Use:   "backup",
+		Short: "Back up current data",
+		Long:  fmt.Sprintf("Backs up current MicroShift data to %s", config.BackupsDir),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return admin.MakeBackup(cmd.Flag("name").Value.String())
+		},
+	}
+
+	v := version.Get()
+	backup.PersistentFlags().String("name",
+		fmt.Sprintf("%s.%s__%s", v.Major, v.Minor, time.Now().UTC().Format("20060102_150405")),
+		fmt.Sprintf("Backup dir name in %s", config.BackupsDir))
+
+	cmd := &cobra.Command{
+		Use:   "admin",
+		Short: "Commands for managing MicroShift",
+	}
+	cmd.AddCommand(backup)
+
+	return cmd
+}
diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go
index 7e737f90a4..22a4b43e27 100644
--- a/pkg/cmd/run.go
+++ b/pkg/cmd/run.go
@@ -99,7 +99,7 @@ func RunMicroshift(cfg *config.Config) error {
 		klog.Fatal(err)
 	}
 
-	if err := os.MkdirAll(config.DataDir, 0700); err != nil {
+	if err := config.CreateDir(config.DataDir); err != nil {
 		return fmt.Errorf("failed to create dir %q: %w", config.DataDir, err)
 	}
 
diff --git a/pkg/config/files.go b/pkg/config/files.go
index ffe7ee6d33..1aa5e8f79f 100644
--- a/pkg/config/files.go
+++ b/pkg/config/files.go
@@ -10,6 +10,7 @@ import (
 const (
 	ConfigFile = "/etc/microshift/config.yaml"
 	DataDir    = "/var/lib/microshift"
+	BackupsDir = "/var/lib/microshift-backups"
 )
 
 func parse(contents []byte) (*Config, error) {
@@ -61,3 +62,7 @@ func ActiveConfig() (*Config, error) {
 	}
 	return getActiveConfigFromYAML(contents)
 }
+
+func CreateDir(path string) error {
+	return os.MkdirAll(path, 0700)
+}

From c505f6b295dbfc0b69009c764ce82c44ef030d24 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Thu, 18 May 2023 12:07:02 +0200
Subject: [PATCH 02/27] configurable backup dest dir

---
 pkg/admin/backup.go | 11 +++++------
 pkg/cmd/admin.go    | 29 +++++++++++++++++++----------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/pkg/admin/backup.go b/pkg/admin/backup.go
index d0dbc31657..cfa4816fd8 100644
--- a/pkg/admin/backup.go
+++ b/pkg/admin/backup.go
@@ -19,13 +19,13 @@ var (
 	}
 )
 
-func MakeBackup(name string) error {
-	if err := config.CreateDir(config.BackupsDir); err != nil {
-		klog.Errorf("Failed to create dir %s: %v", config.BackupsDir, err)
+func MakeBackup(target, name string) error {
+	if err := config.CreateDir(target); err != nil {
+		klog.Errorf("Failed to create dir %s: %v", target, err)
 		return err
 	}
 
-	dest := filepath.Join(config.BackupsDir, name)
+	dest := filepath.Join(target, name)
 	tmp_dest := dest + "-tmp"
 
 	args := append(cpArgs, config.DataDir, tmp_dest)
@@ -45,7 +45,6 @@ func MakeBackup(name string) error {
 		klog.Errorf("Failed to rename %s to %s: %v", tmp_dest, dest, err)
 		return err
 	}
-
-	klog.Infof("Backed up current data to %s", dest)
+	klog.Infof("Backed %s to %s", config.DataDir, dest)
 	return nil
 }
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 37ea06b4ca..b749d2371c 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -5,32 +5,41 @@ import (
 	"time"
 
 	"github.com/openshift/microshift/pkg/admin"
-	"github.com/openshift/microshift/pkg/config"
 	"github.com/openshift/microshift/pkg/version"
 
 	"github.com/spf13/cobra"
 )
 
-func NewAdminCommand() *cobra.Command {
+func newAdminBackupCommand() *cobra.Command {
 	backup := &cobra.Command{
 		Use:   "backup",
-		Short: "Back up current data",
-		Long:  fmt.Sprintf("Backs up current MicroShift data to %s", config.BackupsDir),
+		Short: "Makes a backup of MicroShift data",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return admin.MakeBackup(cmd.Flag("name").Value.String())
+			return admin.MakeBackup(
+				cmd.Flag("dest").Value.String(),
+				cmd.Flag("name").Value.String(),
+			)
 		},
 	}
-
 	v := version.Get()
-	backup.PersistentFlags().String("name",
+	backup.PersistentFlags().String(
+		"dest",
+		"/var/lib/microshift-backups",
+		"Directory with backups",
+	)
+	backup.PersistentFlags().String(
+		"name",
 		fmt.Sprintf("%s.%s__%s", v.Major, v.Minor, time.Now().UTC().Format("20060102_150405")),
-		fmt.Sprintf("Backup dir name in %s", config.BackupsDir))
+		"Backup name",
+	)
+	return backup
+}
 
+func NewAdminCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "admin",
 		Short: "Commands for managing MicroShift",
 	}
-	cmd.AddCommand(backup)
-
+	cmd.AddCommand(newAdminBackupCommand())
 	return cmd
 }

From da262e5b8fd826580bba131744e4a946fc8fec4d Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Thu, 18 May 2023 20:17:50 +0200
Subject: [PATCH 03/27] use config var for default value

---
 pkg/cmd/admin.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index b749d2371c..1b898b07c0 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/openshift/microshift/pkg/admin"
+	"github.com/openshift/microshift/pkg/config"
 	"github.com/openshift/microshift/pkg/version"
 
 	"github.com/spf13/cobra"
@@ -24,7 +25,7 @@ func newAdminBackupCommand() *cobra.Command {
 	v := version.Get()
 	backup.PersistentFlags().String(
 		"dest",
-		"/var/lib/microshift-backups",
+		config.BackupsDir,
 		"Directory with backups",
 	)
 	backup.PersistentFlags().String(

From 333200811c5aa554903a623a4462bf5a66b5f9f9 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Sat, 20 May 2023 16:30:29 +0200
Subject: [PATCH 04/27] more logs and logic for backup

---
 go.mod              |   2 +-
 pkg/admin/backup.go | 113 ++++++++++++++++++++++++++++++++++++++------
 pkg/cmd/admin.go    |   6 ++-
 pkg/cmd/run.go      |   2 +-
 pkg/config/files.go |   4 --
 pkg/util/util.go    |  16 +++++++
 6 files changed, 120 insertions(+), 23 deletions(-)

diff --git a/go.mod b/go.mod
index 39591a0835..8026ef4969 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/openshift/microshift
 
-go 1.19
+go 1.20
 
 require (
 	github.com/apparentlymart/go-cidr v1.1.0
diff --git a/pkg/admin/backup.go b/pkg/admin/backup.go
index cfa4816fd8..26103a4ad2 100644
--- a/pkg/admin/backup.go
+++ b/pkg/admin/backup.go
@@ -1,11 +1,13 @@
 package admin
 
 import (
+	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
 
 	"github.com/openshift/microshift/pkg/config"
+	"github.com/openshift/microshift/pkg/util"
 
 	"k8s.io/klog/v2"
 )
@@ -19,32 +21,113 @@ var (
 	}
 )
 
-func MakeBackup(target, name string) error {
-	if err := config.CreateDir(target); err != nil {
-		klog.Errorf("Failed to create dir %s: %v", target, err)
+type BackupConfig struct {
+	// Target is the base directory storing all backups
+	Target string
+
+	// Name is backup's directory name
+	Name string
+}
+
+// MakeBackup backs up MicroShift data (/var/lib/microshift) to
+// target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
+func MakeBackup(cfg BackupConfig) error {
+	if err := ensureDirExists(cfg.Target); err != nil {
 		return err
 	}
 
-	dest := filepath.Join(target, name)
-	tmp_dest := dest + "-tmp"
+	dest := filepath.Join(cfg.Target, cfg.Name)
+	dest_tmp := dest + ".tmp"
 
-	args := append(cpArgs, config.DataDir, tmp_dest)
-	out, err := exec.Command("cp", args...).CombinedOutput() //nolint:gosec
-	klog.Infof("Output of cp (%v): %v", args, string(out))
-	if err != nil {
-		klog.Errorf("Failed to make backup - copy failed: %v", err)
+	if err := dirShouldNotExist(dest_tmp); err != nil {
 		return err
 	}
 
-	if err := os.RemoveAll(dest); err != nil {
-		klog.Errorf("Failed to remove %s directory: %v", tmp_dest, err)
+	if err := copyDataDir(dest_tmp); err != nil {
 		return err
 	}
 
-	if err := os.Rename(tmp_dest, dest); err != nil {
-		klog.Errorf("Failed to rename %s to %s: %v", tmp_dest, dest, err)
+	if err := removeDirBeforeRenaming(dest); err != nil {
 		return err
 	}
-	klog.Infof("Backed %s to %s", config.DataDir, dest)
+
+	if err := renameDir(dest_tmp, dest); err != nil {
+		return err
+	}
+
+	klog.InfoS("Data backed up", "data", config.DataDir, "backup", dest)
+	return nil
+}
+
+func ensureDirExists(path string) error {
+	klog.InfoS("Making sure %s exists", path)
+
+	found, err := util.FileExists(path)
+	if err != nil {
+		return fmt.Errorf("failed checking if %s exists: %w", path, err)
+	}
+	if found {
+		klog.Infof("Directory %s already exists", path)
+		return nil
+	}
+
+	err = util.MakeDir(path)
+	if err != nil {
+		return fmt.Errorf("failed creating %s: %w", path, err)
+	}
+	return err
+}
+
+func dirShouldNotExist(path string) error {
+	klog.Infof("Making sure %s does not exist", path)
+
+	found, err := util.FileExists(path)
+	if err != nil {
+		return fmt.Errorf("failed to check if %s exists: %w", path, err)
+	}
+	if found {
+		return fmt.Errorf("directory %s already exists", path)
+	}
+	return nil
+}
+
+func removeDirBeforeRenaming(path string) error {
+	found, err := util.FileExists(path)
+	if err != nil {
+		return fmt.Errorf("failed to check if %s exists: %w", path, err)
+	}
+	if found {
+		klog.Infof("Removing %s before renaming tmp directory", path)
+		if err := os.RemoveAll(path); err != nil {
+			return fmt.Errorf("failed to remove %s: %w", path, err)
+		}
+	}
+	return nil
+}
+
+func copyDataDir(dest string) error {
+	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
+	klog.InfoS("Executing command", "cmd", cmd)
+	out, err := cmd.CombinedOutput()
+	klog.InfoS("Command finished running", "cmd", cmd, "output", out)
+
+	if err != nil {
+		return fmt.Errorf("failed to copy data: %w", err)
+	}
+	return nil
+}
+
+func renameDir(from, to string) error {
+	klog.InfoS("Renaming directory", "from", from, "to", to)
+	err := os.Rename(from, to)
+	if err == nil {
+		return nil
+	}
+
+	klog.ErrorS(err, "Renaming directory failed - deleting 'from' dir", "from", from, "to", to)
+	if rmErr := os.RemoveAll(from); rmErr != nil {
+		klog.ErrorS(rmErr, "Failed to remove directory", "dir", from)
+		return fmt.Errorf("failed to remove %s: %w: %w", from, err, rmErr)
+	}
 	return nil
 }
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 1b898b07c0..1b5a397d3f 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -17,8 +17,10 @@ func newAdminBackupCommand() *cobra.Command {
 		Short: "Makes a backup of MicroShift data",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return admin.MakeBackup(
-				cmd.Flag("dest").Value.String(),
-				cmd.Flag("name").Value.String(),
+				admin.BackupConfig{
+					Target: cmd.Flag("dest").Value.String(),
+					Name:   cmd.Flag("name").Value.String(),
+				},
 			)
 		},
 	}
diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go
index 22a4b43e27..71e0946a46 100644
--- a/pkg/cmd/run.go
+++ b/pkg/cmd/run.go
@@ -99,7 +99,7 @@ func RunMicroshift(cfg *config.Config) error {
 		klog.Fatal(err)
 	}
 
-	if err := config.CreateDir(config.DataDir); err != nil {
+	if err := util.MakeDir(config.DataDir); err != nil {
 		return fmt.Errorf("failed to create dir %q: %w", config.DataDir, err)
 	}
 
diff --git a/pkg/config/files.go b/pkg/config/files.go
index 1aa5e8f79f..ff6fb3af27 100644
--- a/pkg/config/files.go
+++ b/pkg/config/files.go
@@ -62,7 +62,3 @@ func ActiveConfig() (*Config, error) {
 	}
 	return getActiveConfigFromYAML(contents)
 }
-
-func CreateDir(path string) error {
-	return os.MkdirAll(path, 0700)
-}
diff --git a/pkg/util/util.go b/pkg/util/util.go
index e2ad95c78e..a48bb0e101 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -1,7 +1,9 @@
 package util
 
 import (
+	"errors"
 	"fmt"
+	"os"
 )
 
 func Must(err error) {
@@ -16,3 +18,17 @@ func Default(s string, defaultS string) string {
 	}
 	return s
 }
+
+func FileExists(path string) (bool, error) {
+	if _, err := os.Stat(path); err == nil {
+		return true, nil
+	} else if errors.Is(err, os.ErrNotExist) {
+		return false, nil
+	} else {
+		return false, err
+	}
+}
+
+func MakeDir(path string) error {
+	return os.MkdirAll(path, 0700)
+}

From 3fa1bd78535db3559a307113639677eb9d3ca30d Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Sat, 20 May 2023 16:42:07 +0200
Subject: [PATCH 05/27] ensure microshift is not running

---
 pkg/admin/backup.go    |  4 ++++
 pkg/admin/run_check.go | 25 +++++++++++++++++++++++++
 2 files changed, 29 insertions(+)
 create mode 100644 pkg/admin/run_check.go

diff --git a/pkg/admin/backup.go b/pkg/admin/backup.go
index 26103a4ad2..dc3d6d9969 100644
--- a/pkg/admin/backup.go
+++ b/pkg/admin/backup.go
@@ -32,6 +32,10 @@ type BackupConfig struct {
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
 func MakeBackup(cfg BackupConfig) error {
+	if err := microshiftShouldNotRun(); err != nil {
+		return err
+	}
+
 	if err := ensureDirExists(cfg.Target); err != nil {
 		return err
 	}
diff --git a/pkg/admin/run_check.go b/pkg/admin/run_check.go
new file mode 100644
index 0000000000..54f5f26532
--- /dev/null
+++ b/pkg/admin/run_check.go
@@ -0,0 +1,25 @@
+package admin
+
+import (
+	"fmt"
+	"os/exec"
+	"strings"
+
+	"k8s.io/klog/v2"
+)
+
+func microshiftShouldNotRun() error {
+	cmd := exec.Command("sh", "-c", "ps aux | grep  -v grep | grep 'microshift run'")
+	out, err := cmd.CombinedOutput()
+	sout := strings.TrimSpace(string(out))
+
+	if err == nil {
+		klog.InfoS("MicroShift is running", "process", sout)
+		return fmt.Errorf("command requires that MicroShift must not run")
+	}
+	if err != nil && sout != "" {
+		klog.ErrorS(err, "Unexpected error when checking if MicroShift is running already", "output", sout)
+		return fmt.Errorf("checking if MicroShift is running failed: %w", err)
+	}
+	return nil
+}

From c69fe38336b8bedf8e9bd7555413de3eabd21546 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 22 May 2023 14:08:47 +0200
Subject: [PATCH 06/27] move backup under data subcommand

---
 pkg/admin/backup.go |  2 +-
 pkg/cmd/admin.go    | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/pkg/admin/backup.go b/pkg/admin/backup.go
index dc3d6d9969..4cd0a53bd3 100644
--- a/pkg/admin/backup.go
+++ b/pkg/admin/backup.go
@@ -64,7 +64,7 @@ func MakeBackup(cfg BackupConfig) error {
 }
 
 func ensureDirExists(path string) error {
-	klog.InfoS("Making sure %s exists", path)
+	klog.Infof("Making sure %s exists", path)
 
 	found, err := util.FileExists(path)
 	if err != nil {
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 1b5a397d3f..56d2da5078 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -11,10 +11,10 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func newAdminBackupCommand() *cobra.Command {
+func newAdminDataCommand() *cobra.Command {
 	backup := &cobra.Command{
 		Use:   "backup",
-		Short: "Makes a backup of MicroShift data",
+		Short: "Backup MicroShift data",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return admin.MakeBackup(
 				admin.BackupConfig{
@@ -24,18 +24,25 @@ func newAdminBackupCommand() *cobra.Command {
 			)
 		},
 	}
+
+	data := &cobra.Command{
+		Use:   "data",
+		Short: "Commands for managing MicroShift data",
+	}
 	v := version.Get()
-	backup.PersistentFlags().String(
+	data.PersistentFlags().String(
 		"dest",
 		config.BackupsDir,
 		"Directory with backups",
 	)
-	backup.PersistentFlags().String(
+	data.PersistentFlags().String(
 		"name",
 		fmt.Sprintf("%s.%s__%s", v.Major, v.Minor, time.Now().UTC().Format("20060102_150405")),
 		"Backup name",
 	)
-	return backup
+
+	data.AddCommand(backup)
+	return data
 }
 
 func NewAdminCommand() *cobra.Command {
@@ -43,6 +50,6 @@ func NewAdminCommand() *cobra.Command {
 		Use:   "admin",
 		Short: "Commands for managing MicroShift",
 	}
-	cmd.AddCommand(newAdminBackupCommand())
+	cmd.AddCommand(newAdminDataCommand())
 	return cmd
 }

From f3cd8ca15ba4c08989ecd2f6a3fea0ca0e86b84c Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 22 May 2023 14:24:28 +0200
Subject: [PATCH 07/27] DataManager interface

---
 pkg/admin/{ => data}/backup.go    | 16 ++++------------
 pkg/admin/data/manager.go         | 30 ++++++++++++++++++++++++++++++
 pkg/admin/{ => data}/run_check.go |  2 +-
 pkg/cmd/admin.go                  | 10 +++++-----
 4 files changed, 40 insertions(+), 18 deletions(-)
 rename pkg/admin/{ => data}/backup.go (90%)
 create mode 100644 pkg/admin/data/manager.go
 rename pkg/admin/{ => data}/run_check.go (97%)

diff --git a/pkg/admin/backup.go b/pkg/admin/data/backup.go
similarity index 90%
rename from pkg/admin/backup.go
rename to pkg/admin/data/backup.go
index 4cd0a53bd3..c3580ede17 100644
--- a/pkg/admin/backup.go
+++ b/pkg/admin/data/backup.go
@@ -1,4 +1,4 @@
-package admin
+package data
 
 import (
 	"fmt"
@@ -21,26 +21,18 @@ var (
 	}
 )
 
-type BackupConfig struct {
-	// Target is the base directory storing all backups
-	Target string
-
-	// Name is backup's directory name
-	Name string
-}
-
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
-func MakeBackup(cfg BackupConfig) error {
+func makeBackup(cfg BackupConfig) error {
 	if err := microshiftShouldNotRun(); err != nil {
 		return err
 	}
 
-	if err := ensureDirExists(cfg.Target); err != nil {
+	if err := ensureDirExists(cfg.BackupsStorage); err != nil {
 		return err
 	}
 
-	dest := filepath.Join(cfg.Target, cfg.Name)
+	dest := filepath.Join(cfg.BackupsStorage, cfg.Name)
 	dest_tmp := dest + ".tmp"
 
 	if err := dirShouldNotExist(dest_tmp); err != nil {
diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
new file mode 100644
index 0000000000..27abaa54fd
--- /dev/null
+++ b/pkg/admin/data/manager.go
@@ -0,0 +1,30 @@
+package data
+
+import "fmt"
+
+type BackupConfig struct {
+	// BackupsStorage is the base directory storing all backups
+	BackupsStorage string
+
+	// Name is backup's directory name
+	Name string
+}
+
+type DataManager interface {
+	Backup(BackupConfig) error
+	Restore(BackupConfig) error
+}
+
+func NewDataManager() DataManager {
+	return &dataManager{}
+}
+
+type dataManager struct{}
+
+func (dm *dataManager) Backup(cfg BackupConfig) error {
+	return makeBackup(cfg)
+}
+
+func (dm *dataManager) Restore(cfg BackupConfig) error {
+	return fmt.Errorf("not implemented")
+}
diff --git a/pkg/admin/run_check.go b/pkg/admin/data/run_check.go
similarity index 97%
rename from pkg/admin/run_check.go
rename to pkg/admin/data/run_check.go
index 54f5f26532..f21c091cf3 100644
--- a/pkg/admin/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -1,4 +1,4 @@
-package admin
+package data
 
 import (
 	"fmt"
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 56d2da5078..4789ac3902 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/openshift/microshift/pkg/admin"
+	"github.com/openshift/microshift/pkg/admin/data"
 	"github.com/openshift/microshift/pkg/config"
 	"github.com/openshift/microshift/pkg/version"
 
@@ -16,10 +16,10 @@ func newAdminDataCommand() *cobra.Command {
 		Use:   "backup",
 		Short: "Backup MicroShift data",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return admin.MakeBackup(
-				admin.BackupConfig{
-					Target: cmd.Flag("dest").Value.String(),
-					Name:   cmd.Flag("name").Value.String(),
+			return data.NewDataManager().Backup(
+				data.BackupConfig{
+					BackupsStorage: cmd.Flag("dest").Value.String(),
+					Name:           cmd.Flag("name").Value.String(),
 				},
 			)
 		},

From 52fe5d387a08446ebcdc5fc39d9e43ffd386c205 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 22 May 2023 17:45:29 +0200
Subject: [PATCH 08/27] check if svc is inactive instead of `ps aux`

---
 pkg/admin/data/run_check.go | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/pkg/admin/data/run_check.go b/pkg/admin/data/run_check.go
index f21c091cf3..164f829fcb 100644
--- a/pkg/admin/data/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -8,18 +8,29 @@ import (
 	"k8s.io/klog/v2"
 )
 
+const (
+	expectedState = "inactive"
+)
+
+var (
+	services = []string{"microshift.service", "microshift-etcd.scope"}
+)
+
 func microshiftShouldNotRun() error {
-	cmd := exec.Command("sh", "-c", "ps aux | grep  -v grep | grep 'microshift run'")
-	out, err := cmd.CombinedOutput()
-	sout := strings.TrimSpace(string(out))
+	for _, service := range services {
+		cmd := exec.Command("systemctl", "show", "-p", "ActiveState", "--value", service)
+		out, err := cmd.CombinedOutput()
+		state := strings.TrimSpace(string(out))
+		if err != nil {
+			return fmt.Errorf("error when checking if %s is active: %w", service, err)
+		}
 
-	if err == nil {
-		klog.InfoS("MicroShift is running", "process", sout)
-		return fmt.Errorf("command requires that MicroShift must not run")
-	}
-	if err != nil && sout != "" {
-		klog.ErrorS(err, "Unexpected error when checking if MicroShift is running already", "output", sout)
-		return fmt.Errorf("checking if MicroShift is running failed: %w", err)
+		if state != expectedState {
+			return fmt.Errorf("%s is %s - expected to be %s", service, state, expectedState)
+		}
+
+		klog.Infof("Service %s is %s", service, state)
 	}
+
 	return nil
 }

From 59b7c7bcdbf2d465ccffb136d2949353ddf55983 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 22 May 2023 17:49:23 +0200
Subject: [PATCH 09/27] fix ireturn

---
 pkg/admin/data/manager.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index 27abaa54fd..7420d4a2fb 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -15,16 +15,16 @@ type DataManager interface {
 	Restore(BackupConfig) error
 }
 
-func NewDataManager() DataManager {
-	return &dataManager{}
+func NewDataManager() *IDataManager {
+	return &IDataManager{}
 }
 
-type dataManager struct{}
+type IDataManager struct{}
 
-func (dm *dataManager) Backup(cfg BackupConfig) error {
+func (dm *IDataManager) Backup(cfg BackupConfig) error {
 	return makeBackup(cfg)
 }
 
-func (dm *dataManager) Restore(cfg BackupConfig) error {
+func (dm *IDataManager) Restore(cfg BackupConfig) error {
 	return fmt.Errorf("not implemented")
 }

From 8548161707e4b54c192c8f5fbf939ada26359586 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Wed, 24 May 2023 11:24:37 +0200
Subject: [PATCH 10/27] rename `--dest` and make sure args are not empty

---
 pkg/admin/data/backup.go  | 11 +++++++++--
 pkg/admin/data/manager.go |  4 ++--
 pkg/cmd/admin.go          |  6 +++---
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index c3580ede17..fbedf9120e 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -24,15 +24,22 @@ var (
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
 func makeBackup(cfg BackupConfig) error {
+	if cfg.Storage == "" {
+		return fmt.Errorf("backup storage must not be empty")
+	}
+	if cfg.Name == "" {
+		return fmt.Errorf("backup name must not be empty")
+	}
+
 	if err := microshiftShouldNotRun(); err != nil {
 		return err
 	}
 
-	if err := ensureDirExists(cfg.BackupsStorage); err != nil {
+	if err := ensureDirExists(cfg.Storage); err != nil {
 		return err
 	}
 
-	dest := filepath.Join(cfg.BackupsStorage, cfg.Name)
+	dest := filepath.Join(cfg.Storage, cfg.Name)
 	dest_tmp := dest + ".tmp"
 
 	if err := dirShouldNotExist(dest_tmp); err != nil {
diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index 7420d4a2fb..cd439f2074 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -3,8 +3,8 @@ package data
 import "fmt"
 
 type BackupConfig struct {
-	// BackupsStorage is the base directory storing all backups
-	BackupsStorage string
+	// Storage is the base directory storing all backups
+	Storage string
 
 	// Name is backup's directory name
 	Name string
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 4789ac3902..9d4098a4ee 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -18,8 +18,8 @@ func newAdminDataCommand() *cobra.Command {
 		RunE: func(cmd *cobra.Command, args []string) error {
 			return data.NewDataManager().Backup(
 				data.BackupConfig{
-					BackupsStorage: cmd.Flag("dest").Value.String(),
-					Name:           cmd.Flag("name").Value.String(),
+					Storage: cmd.Flag("storage").Value.String(),
+					Name:    cmd.Flag("name").Value.String(),
 				},
 			)
 		},
@@ -31,7 +31,7 @@ func newAdminDataCommand() *cobra.Command {
 	}
 	v := version.Get()
 	data.PersistentFlags().String(
-		"dest",
+		"storage",
 		config.BackupsDir,
 		"Directory with backups",
 	)

From 6fa81a2133eb02f890e94e8999807bd4b31fe6aa Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Thu, 25 May 2023 10:02:26 +0200
Subject: [PATCH 11/27] rename DataManager to Manager

---
 pkg/admin/data/manager.go | 12 ++++++------
 pkg/cmd/admin.go          |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index cd439f2074..2deee0e008 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -10,21 +10,21 @@ type BackupConfig struct {
 	Name string
 }
 
-type DataManager interface {
+type Manager interface {
 	Backup(BackupConfig) error
 	Restore(BackupConfig) error
 }
 
-func NewDataManager() *IDataManager {
-	return &IDataManager{}
+func NewManager() *manager {
+	return &manager{}
 }
 
-type IDataManager struct{}
+type manager struct{}
 
-func (dm *IDataManager) Backup(cfg BackupConfig) error {
+func (dm *manager) Backup(cfg BackupConfig) error {
 	return makeBackup(cfg)
 }
 
-func (dm *IDataManager) Restore(cfg BackupConfig) error {
+func (dm *manager) Restore(cfg BackupConfig) error {
 	return fmt.Errorf("not implemented")
 }
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 9d4098a4ee..4e450518a3 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -16,7 +16,7 @@ func newAdminDataCommand() *cobra.Command {
 		Use:   "backup",
 		Short: "Backup MicroShift data",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return data.NewDataManager().Backup(
+			return data.NewManager().Backup(
 				data.BackupConfig{
 					Storage: cmd.Flag("storage").Value.String(),
 					Name:    cmd.Flag("name").Value.String(),

From d8a68cbc3a9de0bfd59c0d1b8edee1c3c8641a79 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 08:32:51 +0200
Subject: [PATCH 12/27] catch empty args early

---
 pkg/cmd/admin.go | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 4e450518a3..e62764bafb 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -28,6 +28,15 @@ func newAdminDataCommand() *cobra.Command {
 	data := &cobra.Command{
 		Use:   "data",
 		Short: "Commands for managing MicroShift data",
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			if cmd.Flag("storage").Value.String() == "" {
+				return fmt.Errorf("--storage must not be empty")
+			}
+			if cmd.Flag("name").Value.String() == "" {
+				return fmt.Errorf("--name must not be empty")
+			}
+			return nil
+		},
 	}
 	v := version.Get()
 	data.PersistentFlags().String(

From 8d86dc34c55fdebf6b380b535f50a0c3bc5483ed Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 08:34:19 +0200
Subject: [PATCH 13/27] log dir creation

---
 pkg/admin/data/backup.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index fbedf9120e..7376339b90 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -78,7 +78,8 @@ func ensureDirExists(path string) error {
 	if err != nil {
 		return fmt.Errorf("failed creating %s: %w", path, err)
 	}
-	return err
+	klog.Infof("Directory %s created", path)
+	return nil
 }
 
 func dirShouldNotExist(path string) error {

From 35a428a6dd75bec609f981f461302ce0d061d7f1 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 08:45:45 +0200
Subject: [PATCH 14/27] move logic around...

---
 pkg/admin/data/backup.go | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 7376339b90..c23281f587 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -50,11 +50,17 @@ func makeBackup(cfg BackupConfig) error {
 		return err
 	}
 
-	if err := removeDirBeforeRenaming(dest); err != nil {
+	klog.Infof("Removing %s before renaming %s", dest, dest_tmp)
+	if err := removePath(dest); err != nil {
 		return err
 	}
 
 	if err := renameDir(dest_tmp, dest); err != nil {
+		klog.Errorf("Renaming directory failed - deleting %s: %v", dest_tmp, err)
+		if rmErr := removePath(dest_tmp); rmErr != nil {
+			klog.ErrorS(rmErr, "Failed to remove directory", "dir", dest_tmp)
+			return fmt.Errorf("failed to remove %s: %w: %w", dest_tmp, err, rmErr)
+		}
 		return err
 	}
 
@@ -95,17 +101,19 @@ func dirShouldNotExist(path string) error {
 	return nil
 }
 
-func removeDirBeforeRenaming(path string) error {
+func removePath(path string) error {
 	found, err := util.FileExists(path)
 	if err != nil {
 		return fmt.Errorf("failed to check if %s exists: %w", path, err)
 	}
 	if found {
-		klog.Infof("Removing %s before renaming tmp directory", path)
+		klog.Infof("Removing %s", path)
 		if err := os.RemoveAll(path); err != nil {
 			return fmt.Errorf("failed to remove %s: %w", path, err)
 		}
+		klog.Infof("Removed %s", path)
 	}
+	klog.Infof("Path %s not found - not removed", path)
 	return nil
 }
 
@@ -123,15 +131,5 @@ func copyDataDir(dest string) error {
 
 func renameDir(from, to string) error {
 	klog.InfoS("Renaming directory", "from", from, "to", to)
-	err := os.Rename(from, to)
-	if err == nil {
-		return nil
-	}
-
-	klog.ErrorS(err, "Renaming directory failed - deleting 'from' dir", "from", from, "to", to)
-	if rmErr := os.RemoveAll(from); rmErr != nil {
-		klog.ErrorS(rmErr, "Failed to remove directory", "dir", from)
-		return fmt.Errorf("failed to remove %s: %w: %w", from, err, rmErr)
-	}
-	return nil
+	return os.Rename(from, to)
 }

From 3d45018febf3899eb2e32ef3a4e3e868387eaaa5 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 08:46:36 +0200
Subject: [PATCH 15/27] rename microshiftShouldNotRun

---
 pkg/admin/data/backup.go    | 2 +-
 pkg/admin/data/run_check.go | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index c23281f587..7b1ee6982a 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -31,7 +31,7 @@ func makeBackup(cfg BackupConfig) error {
 		return fmt.Errorf("backup name must not be empty")
 	}
 
-	if err := microshiftShouldNotRun(); err != nil {
+	if err := microshiftIsNotRunning(); err != nil {
 		return err
 	}
 
diff --git a/pkg/admin/data/run_check.go b/pkg/admin/data/run_check.go
index 164f829fcb..ee6f61f7c3 100644
--- a/pkg/admin/data/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -16,7 +16,7 @@ var (
 	services = []string{"microshift.service", "microshift-etcd.scope"}
 )
 
-func microshiftShouldNotRun() error {
+func microshiftIsNotRunning() error {
 	for _, service := range services {
 		cmd := exec.Command("systemctl", "show", "-p", "ActiveState", "--value", service)
 		out, err := cmd.CombinedOutput()

From b566524a5e9e5268cc7f789ec150dcfb92624d2e Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 09:10:49 +0200
Subject: [PATCH 16/27] rename existing backup instead of deleting it before
 having new one in place

---
 pkg/admin/data/backup.go | 79 +++++++++++++++++++++++-----------------
 pkg/util/util.go         |  2 +-
 2 files changed, 46 insertions(+), 35 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 7b1ee6982a..93b330eaa6 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -1,6 +1,7 @@
 package data
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
@@ -41,8 +42,9 @@ func makeBackup(cfg BackupConfig) error {
 
 	dest := filepath.Join(cfg.Storage, cfg.Name)
 	dest_tmp := dest + ".tmp"
+	dest_old := dest + ".old"
 
-	if err := dirShouldNotExist(dest_tmp); err != nil {
+	if err := removePath(dest_tmp); err != nil {
 		return err
 	}
 
@@ -50,18 +52,26 @@ func makeBackup(cfg BackupConfig) error {
 		return err
 	}
 
-	klog.Infof("Removing %s before renaming %s", dest, dest_tmp)
-	if err := removePath(dest); err != nil {
+	backupAlreadyExists, err := pathExists(dest)
+	if err != nil {
 		return err
+	} else if backupAlreadyExists {
+		if err := renamePath(dest, dest_old); err != nil {
+			return err
+		}
+	}
+
+	if err := renamePath(dest_tmp, dest); err != nil {
+		klog.Errorf("Renaming directory failed - renaming %s back and deleting %s: %v", dest_old, dest_tmp, err)
+		renameErr := renamePath(dest_old, dest)
+		rmErr := removePath(dest_tmp)
+		return errors.Join(err, renameErr, rmErr)
 	}
 
-	if err := renameDir(dest_tmp, dest); err != nil {
-		klog.Errorf("Renaming directory failed - deleting %s: %v", dest_tmp, err)
-		if rmErr := removePath(dest_tmp); rmErr != nil {
-			klog.ErrorS(rmErr, "Failed to remove directory", "dir", dest_tmp)
-			return fmt.Errorf("failed to remove %s: %w: %w", dest_tmp, err, rmErr)
+	if backupAlreadyExists {
+		if err := removePath(dest_old); err != nil {
+			return err
 		}
-		return err
 	}
 
 	klog.InfoS("Data backed up", "data", config.DataDir, "backup", dest)
@@ -69,14 +79,14 @@ func makeBackup(cfg BackupConfig) error {
 }
 
 func ensureDirExists(path string) error {
-	klog.Infof("Making sure %s exists", path)
+	klog.InfoS("Making sure directory exists", "path", path)
 
-	found, err := util.FileExists(path)
+	found, err := pathExists(path)
 	if err != nil {
-		return fmt.Errorf("failed checking if %s exists: %w", path, err)
+		return err
 	}
 	if found {
-		klog.Infof("Directory %s already exists", path)
+		klog.InfoS("Directory already exists", "path", path)
 		return nil
 	}
 
@@ -84,36 +94,24 @@ func ensureDirExists(path string) error {
 	if err != nil {
 		return fmt.Errorf("failed creating %s: %w", path, err)
 	}
-	klog.Infof("Directory %s created", path)
-	return nil
-}
-
-func dirShouldNotExist(path string) error {
-	klog.Infof("Making sure %s does not exist", path)
-
-	found, err := util.FileExists(path)
-	if err != nil {
-		return fmt.Errorf("failed to check if %s exists: %w", path, err)
-	}
-	if found {
-		return fmt.Errorf("directory %s already exists", path)
-	}
+	klog.InfoS("Directory created", "path", path)
 	return nil
 }
 
 func removePath(path string) error {
-	found, err := util.FileExists(path)
+	found, err := pathExists(path)
 	if err != nil {
-		return fmt.Errorf("failed to check if %s exists: %w", path, err)
+		return err
 	}
+
 	if found {
-		klog.Infof("Removing %s", path)
+		klog.InfoS("Removing path", "path", path)
 		if err := os.RemoveAll(path); err != nil {
 			return fmt.Errorf("failed to remove %s: %w", path, err)
 		}
-		klog.Infof("Removed %s", path)
+		klog.InfoS("Removed path", "path", path)
 	}
-	klog.Infof("Path %s not found - not removed", path)
+	klog.InfoS("Path not found - not removed", "path", path)
 	return nil
 }
 
@@ -129,7 +127,20 @@ func copyDataDir(dest string) error {
 	return nil
 }
 
-func renameDir(from, to string) error {
+func renamePath(from, to string) error {
 	klog.InfoS("Renaming directory", "from", from, "to", to)
-	return os.Rename(from, to)
+	if err := os.Rename(from, to); err != nil {
+		klog.ErrorS(err, "Failed to rename path", "from", from, "to", to)
+		return fmt.Errorf("renaming %s to %s failed: %w", from, to, err)
+	}
+	return nil
+}
+
+func pathExists(path string) (bool, error) {
+	exists, err := util.PathExists(path)
+	if err != nil {
+		klog.ErrorS(err, "Failed to check if path exists", "path", path)
+		return false, fmt.Errorf("checking if %s exists failed: %w", path, err)
+	}
+	return exists, nil
 }
diff --git a/pkg/util/util.go b/pkg/util/util.go
index a48bb0e101..61725b28d2 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -19,7 +19,7 @@ func Default(s string, defaultS string) string {
 	return s
 }
 
-func FileExists(path string) (bool, error) {
+func PathExists(path string) (bool, error) {
 	if _, err := os.Stat(path); err == nil {
 		return true, nil
 	} else if errors.Is(err, os.ErrNotExist) {

From 58f172bdf3e669c5bc3ca89875130b45974426f4 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 09:39:37 +0200
Subject: [PATCH 17/27] move BackupCfg validation to separate func

---
 pkg/admin/data/backup.go  |  7 ++-----
 pkg/admin/data/manager.go | 16 +++++++++++++++-
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 93b330eaa6..1c3de5c9ee 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -25,11 +25,8 @@ var (
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
 func makeBackup(cfg BackupConfig) error {
-	if cfg.Storage == "" {
-		return fmt.Errorf("backup storage must not be empty")
-	}
-	if cfg.Name == "" {
-		return fmt.Errorf("backup name must not be empty")
+	if err := cfg.Validate(); err != nil {
+		return fmt.Errorf("invalid BackupConfig: %w", err)
 	}
 
 	if err := microshiftIsNotRunning(); err != nil {
diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index 2deee0e008..c5a3054c48 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -1,6 +1,9 @@
 package data
 
-import "fmt"
+import (
+	"errors"
+	"fmt"
+)
 
 type BackupConfig struct {
 	// Storage is the base directory storing all backups
@@ -10,6 +13,17 @@ type BackupConfig struct {
 	Name string
 }
 
+func (bc BackupConfig) Validate() error {
+	var err error
+	if bc.Storage == "" {
+		err = fmt.Errorf("backup storage must not be empty")
+	}
+	if bc.Name == "" {
+		err = errors.Join(err, fmt.Errorf("backup name must not be empty"))
+	}
+	return err
+}
+
 type Manager interface {
 	Backup(BackupConfig) error
 	Restore(BackupConfig) error

From 28fb00717073ff10fd742a12232e0343253c60fb Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 10:18:56 +0200
Subject: [PATCH 18/27] tweak logs

---
 pkg/admin/data/backup.go    | 35 ++++++++++++++++++++++++-----------
 pkg/admin/data/run_check.go |  6 +++---
 2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 1c3de5c9ee..52b359ddad 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -25,6 +25,8 @@ var (
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
 func makeBackup(cfg BackupConfig) error {
+	klog.V(2).InfoS("Starting backup procedure", "cfg", cfg)
+
 	if err := cfg.Validate(); err != nil {
 		return fmt.Errorf("invalid BackupConfig: %w", err)
 	}
@@ -53,13 +55,14 @@ func makeBackup(cfg BackupConfig) error {
 	if err != nil {
 		return err
 	} else if backupAlreadyExists {
+		klog.V(2).InfoS("Backup already exists - renaming temporarily", "path", dest)
 		if err := renamePath(dest, dest_old); err != nil {
 			return err
 		}
 	}
 
 	if err := renamePath(dest_tmp, dest); err != nil {
-		klog.Errorf("Renaming directory failed - renaming %s back and deleting %s: %v", dest_old, dest_tmp, err)
+		klog.Errorf("Renaming path failed - renaming %s back and deleting %s: %v", dest_old, dest_tmp, err)
 		renameErr := renamePath(dest_old, dest)
 		rmErr := removePath(dest_tmp)
 		return errors.Join(err, renameErr, rmErr)
@@ -76,14 +79,14 @@ func makeBackup(cfg BackupConfig) error {
 }
 
 func ensureDirExists(path string) error {
-	klog.InfoS("Making sure directory exists", "path", path)
+	klog.V(2).InfoS("Making sure directory exists", "path", path)
 
 	found, err := pathExists(path)
 	if err != nil {
 		return err
 	}
 	if found {
-		klog.InfoS("Directory already exists", "path", path)
+		klog.V(2).InfoS("Directory already exists", "path", path)
 		return nil
 	}
 
@@ -96,40 +99,50 @@ func ensureDirExists(path string) error {
 }
 
 func removePath(path string) error {
-	found, err := pathExists(path)
+	klog.V(2).InfoS("Path removal attempt", "path", path)
+
+	exists, err := pathExists(path)
 	if err != nil {
 		return err
 	}
 
-	if found {
-		klog.InfoS("Removing path", "path", path)
+	if exists {
+		klog.V(2).InfoS("Path exists - removing", "path", path)
 		if err := os.RemoveAll(path); err != nil {
 			return fmt.Errorf("failed to remove %s: %w", path, err)
 		}
-		klog.InfoS("Removed path", "path", path)
+		klog.InfoS("Path removed", "path", path)
+	} else {
+		klog.V(2).InfoS("Path not found", "path", path)
 	}
-	klog.InfoS("Path not found - not removed", "path", path)
 	return nil
 }
 
 func copyDataDir(dest string) error {
 	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
-	klog.InfoS("Executing command", "cmd", cmd)
+	klog.V(2).InfoS("Executing command", "cmd", cmd)
+
 	out, err := cmd.CombinedOutput()
-	klog.InfoS("Command finished running", "cmd", cmd, "output", out)
+	klog.V(2).InfoS("Command finished running", "cmd", cmd, "output", out)
 
 	if err != nil {
+		klog.ErrorS(err, "Command failed", "cmd", cmd, "output", out)
 		return fmt.Errorf("failed to copy data: %w", err)
 	}
+
+	klog.InfoS("Command successful", "cmd", cmd)
 	return nil
 }
 
 func renamePath(from, to string) error {
-	klog.InfoS("Renaming directory", "from", from, "to", to)
+	klog.V(2).InfoS("Renaming path", "from", from, "to", to)
+
 	if err := os.Rename(from, to); err != nil {
 		klog.ErrorS(err, "Failed to rename path", "from", from, "to", to)
 		return fmt.Errorf("renaming %s to %s failed: %w", from, to, err)
 	}
+
+	klog.InfoS("Path renamed", "from", from, "to", to)
 	return nil
 }
 
diff --git a/pkg/admin/data/run_check.go b/pkg/admin/data/run_check.go
index ee6f61f7c3..5d3ff9ce6d 100644
--- a/pkg/admin/data/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -25,11 +25,11 @@ func microshiftIsNotRunning() error {
 			return fmt.Errorf("error when checking if %s is active: %w", service, err)
 		}
 
+		klog.V(2).InfoS("Service state", "service", service, "state", state)
+
 		if state != expectedState {
-			return fmt.Errorf("%s is %s - expected to be %s", service, state, expectedState)
+			return fmt.Errorf("service %s is %s - expected to be %s", service, state, expectedState)
 		}
-
-		klog.Infof("Service %s is %s", service, state)
 	}
 
 	return nil

From 41d41d2ba0edd156449bf295c3ae55a2a99aa2d1 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 10:20:30 +0200
Subject: [PATCH 19/27] tweak logic re errs in BackupConfig.Validate()

---
 pkg/admin/data/manager.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index c5a3054c48..6986adaafc 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -14,14 +14,14 @@ type BackupConfig struct {
 }
 
 func (bc BackupConfig) Validate() error {
-	var err error
+	var errs []error
 	if bc.Storage == "" {
-		err = fmt.Errorf("backup storage must not be empty")
+		errs = append(errs, fmt.Errorf("backup storage must not be empty"))
 	}
 	if bc.Name == "" {
-		err = errors.Join(err, fmt.Errorf("backup name must not be empty"))
+		errs = append(errs, fmt.Errorf("backup name must not be empty"))
 	}
-	return err
+	return errors.Join(errs...)
 }
 
 type Manager interface {

From e90d88a6df256f33ab49e22aaae752a129d4945d Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 10:24:33 +0200
Subject: [PATCH 20/27] change backup unit of work start log

---
 pkg/admin/data/backup.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 52b359ddad..726dd098f8 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -25,7 +25,7 @@ var (
 // MakeBackup backs up MicroShift data (/var/lib/microshift) to
 // target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
 func makeBackup(cfg BackupConfig) error {
-	klog.V(2).InfoS("Starting backup procedure", "cfg", cfg)
+	klog.InfoS("Backup started", "cfg", cfg)
 
 	if err := cfg.Validate(); err != nil {
 		return fmt.Errorf("invalid BackupConfig: %w", err)
@@ -74,7 +74,7 @@ func makeBackup(cfg BackupConfig) error {
 		}
 	}
 
-	klog.InfoS("Data backed up", "data", config.DataDir, "backup", dest)
+	klog.InfoS("Backup finished", "backup", dest, "data", config.DataDir)
 	return nil
 }
 

From aced3e682bf199cd1a4204bba01aaa890b7bdf71 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Fri, 26 May 2023 11:41:36 +0200
Subject: [PATCH 21/27] move microshift running check

---
 pkg/admin/data/backup.go    | 4 ----
 pkg/admin/data/run_check.go | 2 +-
 pkg/cmd/admin.go            | 6 ++++++
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
index 726dd098f8..0f14197877 100644
--- a/pkg/admin/data/backup.go
+++ b/pkg/admin/data/backup.go
@@ -31,10 +31,6 @@ func makeBackup(cfg BackupConfig) error {
 		return fmt.Errorf("invalid BackupConfig: %w", err)
 	}
 
-	if err := microshiftIsNotRunning(); err != nil {
-		return err
-	}
-
 	if err := ensureDirExists(cfg.Storage); err != nil {
 		return err
 	}
diff --git a/pkg/admin/data/run_check.go b/pkg/admin/data/run_check.go
index 5d3ff9ce6d..c7b67724ea 100644
--- a/pkg/admin/data/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -16,7 +16,7 @@ var (
 	services = []string{"microshift.service", "microshift-etcd.scope"}
 )
 
-func microshiftIsNotRunning() error {
+func MicroShiftIsNotRunning() error {
 	for _, service := range services {
 		cmd := exec.Command("systemctl", "show", "-p", "ActiveState", "--value", service)
 		out, err := cmd.CombinedOutput()
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index e62764bafb..559647d0c2 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -32,9 +32,15 @@ func newAdminDataCommand() *cobra.Command {
 			if cmd.Flag("storage").Value.String() == "" {
 				return fmt.Errorf("--storage must not be empty")
 			}
+
 			if cmd.Flag("name").Value.String() == "" {
 				return fmt.Errorf("--name must not be empty")
 			}
+
+			if err := data.MicroShiftIsNotRunning(); err != nil {
+				return fmt.Errorf("microshift must not be running: %w", err)
+			}
+
 			return nil
 		},
 	}

From 25dd11b3802d0f0996dcfd858f0853920e0af2b2 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 29 May 2023 12:38:39 +0200
Subject: [PATCH 22/27] --force to overwrite backup via cli

---
 pkg/admin/data/backup.go  | 152 --------------------------------------
 pkg/admin/data/manager.go | 149 +++++++++++++++++++++++++++++++------
 pkg/admin/data/types.go   |  24 ++++++
 pkg/cmd/admin.go          |  45 ++++++-----
 4 files changed, 179 insertions(+), 191 deletions(-)
 delete mode 100644 pkg/admin/data/backup.go
 create mode 100644 pkg/admin/data/types.go

diff --git a/pkg/admin/data/backup.go b/pkg/admin/data/backup.go
deleted file mode 100644
index 0f14197877..0000000000
--- a/pkg/admin/data/backup.go
+++ /dev/null
@@ -1,152 +0,0 @@
-package data
-
-import (
-	"errors"
-	"fmt"
-	"os"
-	"os/exec"
-	"path/filepath"
-
-	"github.com/openshift/microshift/pkg/config"
-	"github.com/openshift/microshift/pkg/util"
-
-	"k8s.io/klog/v2"
-)
-
-var (
-	cpArgs = []string{
-		"--verbose",
-		"--recursive",
-		"--preserve",
-		"--reflink=auto",
-	}
-)
-
-// MakeBackup backs up MicroShift data (/var/lib/microshift) to
-// target/name/ (e.g. /var/lib/microshift-backups/backup-00001).
-func makeBackup(cfg BackupConfig) error {
-	klog.InfoS("Backup started", "cfg", cfg)
-
-	if err := cfg.Validate(); err != nil {
-		return fmt.Errorf("invalid BackupConfig: %w", err)
-	}
-
-	if err := ensureDirExists(cfg.Storage); err != nil {
-		return err
-	}
-
-	dest := filepath.Join(cfg.Storage, cfg.Name)
-	dest_tmp := dest + ".tmp"
-	dest_old := dest + ".old"
-
-	if err := removePath(dest_tmp); err != nil {
-		return err
-	}
-
-	if err := copyDataDir(dest_tmp); err != nil {
-		return err
-	}
-
-	backupAlreadyExists, err := pathExists(dest)
-	if err != nil {
-		return err
-	} else if backupAlreadyExists {
-		klog.V(2).InfoS("Backup already exists - renaming temporarily", "path", dest)
-		if err := renamePath(dest, dest_old); err != nil {
-			return err
-		}
-	}
-
-	if err := renamePath(dest_tmp, dest); err != nil {
-		klog.Errorf("Renaming path failed - renaming %s back and deleting %s: %v", dest_old, dest_tmp, err)
-		renameErr := renamePath(dest_old, dest)
-		rmErr := removePath(dest_tmp)
-		return errors.Join(err, renameErr, rmErr)
-	}
-
-	if backupAlreadyExists {
-		if err := removePath(dest_old); err != nil {
-			return err
-		}
-	}
-
-	klog.InfoS("Backup finished", "backup", dest, "data", config.DataDir)
-	return nil
-}
-
-func ensureDirExists(path string) error {
-	klog.V(2).InfoS("Making sure directory exists", "path", path)
-
-	found, err := pathExists(path)
-	if err != nil {
-		return err
-	}
-	if found {
-		klog.V(2).InfoS("Directory already exists", "path", path)
-		return nil
-	}
-
-	err = util.MakeDir(path)
-	if err != nil {
-		return fmt.Errorf("failed creating %s: %w", path, err)
-	}
-	klog.InfoS("Directory created", "path", path)
-	return nil
-}
-
-func removePath(path string) error {
-	klog.V(2).InfoS("Path removal attempt", "path", path)
-
-	exists, err := pathExists(path)
-	if err != nil {
-		return err
-	}
-
-	if exists {
-		klog.V(2).InfoS("Path exists - removing", "path", path)
-		if err := os.RemoveAll(path); err != nil {
-			return fmt.Errorf("failed to remove %s: %w", path, err)
-		}
-		klog.InfoS("Path removed", "path", path)
-	} else {
-		klog.V(2).InfoS("Path not found", "path", path)
-	}
-	return nil
-}
-
-func copyDataDir(dest string) error {
-	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
-	klog.V(2).InfoS("Executing command", "cmd", cmd)
-
-	out, err := cmd.CombinedOutput()
-	klog.V(2).InfoS("Command finished running", "cmd", cmd, "output", out)
-
-	if err != nil {
-		klog.ErrorS(err, "Command failed", "cmd", cmd, "output", out)
-		return fmt.Errorf("failed to copy data: %w", err)
-	}
-
-	klog.InfoS("Command successful", "cmd", cmd)
-	return nil
-}
-
-func renamePath(from, to string) error {
-	klog.V(2).InfoS("Renaming path", "from", from, "to", to)
-
-	if err := os.Rename(from, to); err != nil {
-		klog.ErrorS(err, "Failed to rename path", "from", from, "to", to)
-		return fmt.Errorf("renaming %s to %s failed: %w", from, to, err)
-	}
-
-	klog.InfoS("Path renamed", "from", from, "to", to)
-	return nil
-}
-
-func pathExists(path string) (bool, error) {
-	exists, err := util.PathExists(path)
-	if err != nil {
-		klog.ErrorS(err, "Failed to check if path exists", "path", path)
-		return false, fmt.Errorf("checking if %s exists failed: %w", path, err)
-	}
-	return exists, nil
-}
diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/manager.go
index 6986adaafc..ffa640768a 100644
--- a/pkg/admin/data/manager.go
+++ b/pkg/admin/data/manager.go
@@ -3,42 +3,147 @@ package data
 import (
 	"errors"
 	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+
+	"github.com/openshift/microshift/pkg/config"
+	"github.com/openshift/microshift/pkg/util"
+	"k8s.io/klog/v2"
 )
 
-type BackupConfig struct {
-	// Storage is the base directory storing all backups
-	Storage string
+var (
+	cpArgs = []string{
+		"--verbose",
+		"--recursive",
+		"--preserve",
+		"--reflink=auto",
+	}
+)
+
+func NewManager(storage StoragePath) (*manager, error) {
+	if storage == "" {
+		return nil, &EmptyArgErr{argName: "storage"}
+	}
+	return &manager{storage: storage}, nil
+}
+
+type manager struct {
+	storage StoragePath
+}
+
+func (dm *manager) GetBackupPath(name BackupName) string {
+	return filepath.Join(string(dm.storage), string(name))
+}
 
-	// Name is backup's directory name
-	Name string
+func (dm *manager) BackupExists(name BackupName) (bool, error) {
+	return pathExists(dm.GetBackupPath(name))
 }
 
-func (bc BackupConfig) Validate() error {
-	var errs []error
-	if bc.Storage == "" {
-		errs = append(errs, fmt.Errorf("backup storage must not be empty"))
+func (dm *manager) Backup(name BackupName) error {
+	klog.InfoS("Backing up the data",
+		"storage", dm.storage, "name", name, "data", config.DataDir)
+
+	if name == "" {
+		return &EmptyArgErr{"name"}
+	}
+
+	if found, err := pathExists(string(dm.storage)); err != nil {
+		return err
+	} else if !found {
+		if makeDirErr := util.MakeDir(string(dm.storage)); makeDirErr != nil {
+			return fmt.Errorf("making %s directory failed: %w", dm.storage, makeDirErr)
+		}
+		klog.InfoS("Backup storage directory created", "path", dm.storage)
+	} else {
+		klog.InfoS("Backup storage directory already existed", "path", dm.storage)
+	}
+
+	dest := dm.GetBackupPath(name)
+	tmp := dest + ".tmp"
+	old := dest + ".old"
+
+	// Make sure /storage/backup.tmp does not exist, so data isn't copied into that directory
+	if err := os.RemoveAll(tmp); err != nil {
+		return fmt.Errorf("failed to remove %s: %w", tmp, err)
+	}
+
+	if err := copyDataDir(tmp); err != nil {
+		return err
 	}
-	if bc.Name == "" {
-		errs = append(errs, fmt.Errorf("backup name must not be empty"))
+
+	backupExists, err := dm.BackupExists(name)
+	if err != nil {
+		return err
+	} else if backupExists {
+		if err := renamePath(dest, old); err != nil {
+			return err
+		}
+		klog.InfoS("Temporarily renamed existing backup", "backup", dest, "renamed", old)
+	}
+
+	if err := renamePath(tmp, dest); err != nil {
+		klog.Errorf("Renaming path failed - renaming %s back and deleting %s: %v", old, tmp, err)
+		renameErr := renamePath(old, dest)
+		rmErr := removePath(tmp)
+		return errors.Join(err, renameErr, rmErr)
+	}
+
+	if backupExists {
+		if err := removePath(old); err != nil {
+			return err
+		}
 	}
-	return errors.Join(errs...)
+
+	klog.InfoS("Backup finished", "backup", dest, "data", config.DataDir)
+	return nil
 }
 
-type Manager interface {
-	Backup(BackupConfig) error
-	Restore(BackupConfig) error
+func (dm *manager) Restore(n BackupName) error {
+	return fmt.Errorf("not implemented")
 }
 
-func NewManager() *manager {
-	return &manager{}
+func removePath(path string) error {
+	exists, err := pathExists(path)
+	if err != nil {
+		return err
+	}
+
+	if exists {
+		if err := os.RemoveAll(path); err != nil {
+			return fmt.Errorf("failed to remove %s: %w", path, err)
+		}
+		klog.InfoS("Removed path", "path", path)
+	}
+	return nil
 }
 
-type manager struct{}
+func copyDataDir(dest string) error {
+	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
+	klog.InfoS("Executing command", "cmd", cmd)
+
+	out, err := cmd.CombinedOutput()
+	klog.InfoS("Command finished running", "cmd", cmd, "output", out)
 
-func (dm *manager) Backup(cfg BackupConfig) error {
-	return makeBackup(cfg)
+	if err != nil {
+		return fmt.Errorf("command %s failed: %w", cmd, err)
+	}
+
+	klog.InfoS("Command successful", "cmd", cmd)
+	return nil
 }
 
-func (dm *manager) Restore(cfg BackupConfig) error {
-	return fmt.Errorf("not implemented")
+func renamePath(from, to string) error {
+	if err := os.Rename(from, to); err != nil {
+		return fmt.Errorf("renaming %s to %s failed: %w", from, to, err)
+	}
+	return nil
+}
+
+func pathExists(path string) (bool, error) {
+	exists, err := util.PathExists(path)
+	if err != nil {
+		return false, fmt.Errorf("checking if %s exists failed: %w", path, err)
+	}
+	return exists, nil
 }
diff --git a/pkg/admin/data/types.go b/pkg/admin/data/types.go
new file mode 100644
index 0000000000..d11d59d104
--- /dev/null
+++ b/pkg/admin/data/types.go
@@ -0,0 +1,24 @@
+package data
+
+import (
+	"fmt"
+)
+
+type EmptyArgErr struct {
+	argName string
+}
+
+func (e *EmptyArgErr) Error() string {
+	return fmt.Sprintf("empty argument: %s", e.argName)
+}
+
+type StoragePath string
+type BackupName string
+
+type Manager interface {
+	Backup(BackupName) error
+	Restore(BackupName) error
+
+	BackupExists(BackupName) (bool, error)
+	GetBackupPath(BackupName) string
+}
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 559647d0c2..a252810a65 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -11,19 +11,36 @@ import (
 	"github.com/spf13/cobra"
 )
 
+func backup(cmd *cobra.Command, args []string) error {
+	storage := data.StoragePath(cmd.Flag("storage").Value.String())
+	name := data.BackupName(cmd.Flag("name").Value.String())
+
+	dataManager, err := data.NewManager(data.StoragePath(storage))
+	if err != nil {
+		return err
+	}
+
+	if exists, err := dataManager.BackupExists(name); err != nil {
+		return err
+	} else if exists {
+		if force, err := cmd.Flags().GetBool("force"); err != nil {
+			return err
+		} else if !force {
+			return fmt.Errorf("backup %s already exists, use --force to overwrite",
+				dataManager.GetBackupPath(name))
+		}
+	}
+
+	return dataManager.Backup(data.BackupName(name))
+}
+
 func newAdminDataCommand() *cobra.Command {
 	backup := &cobra.Command{
 		Use:   "backup",
 		Short: "Backup MicroShift data",
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return data.NewManager().Backup(
-				data.BackupConfig{
-					Storage: cmd.Flag("storage").Value.String(),
-					Name:    cmd.Flag("name").Value.String(),
-				},
-			)
-		},
+		RunE:  backup,
 	}
+	backup.PersistentFlags().Bool("force", false, "Overwrite existing backup")
 
 	data := &cobra.Command{
 		Use:   "data",
@@ -45,16 +62,10 @@ func newAdminDataCommand() *cobra.Command {
 		},
 	}
 	v := version.Get()
-	data.PersistentFlags().String(
-		"storage",
-		config.BackupsDir,
-		"Directory with backups",
-	)
-	data.PersistentFlags().String(
-		"name",
+	data.PersistentFlags().String("storage", config.BackupsDir, "Directory with backups")
+	data.PersistentFlags().String("name",
 		fmt.Sprintf("%s.%s__%s", v.Major, v.Minor, time.Now().UTC().Format("20060102_150405")),
-		"Backup name",
-	)
+		"Backup name")
 
 	data.AddCommand(backup)
 	return data

From 4bb597f0ebb896e5df848afdc93e12109dc7a4d4 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 29 May 2023 12:39:03 +0200
Subject: [PATCH 23/27] rename manager.go for better logs

klog includes filename in the logs, but if we add more manager.go
then it'll be harder to know which one logged the message
---
 pkg/admin/data/{manager.go => data_manager.go} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename pkg/admin/data/{manager.go => data_manager.go} (100%)

diff --git a/pkg/admin/data/manager.go b/pkg/admin/data/data_manager.go
similarity index 100%
rename from pkg/admin/data/manager.go
rename to pkg/admin/data/data_manager.go

From b5d84fed3e86a94bbc997193b1afe1dcb7d86776 Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Mon, 29 May 2023 12:43:11 +0200
Subject: [PATCH 24/27] interface impl assertion and better 'Restore not impl'
 log

---
 pkg/admin/data/data_manager.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go
index ffa640768a..84f3fc0ce3 100644
--- a/pkg/admin/data/data_manager.go
+++ b/pkg/admin/data/data_manager.go
@@ -28,6 +28,8 @@ func NewManager(storage StoragePath) (*manager, error) {
 	return &manager{storage: storage}, nil
 }
 
+var _ Manager = (*manager)(nil)
+
 type manager struct {
 	storage StoragePath
 }
@@ -100,7 +102,7 @@ func (dm *manager) Backup(name BackupName) error {
 }
 
 func (dm *manager) Restore(n BackupName) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("Restore not implemented")
 }
 
 func removePath(path string) error {

From aeb0b2efc4fc49f34f22edabc65cb5d74b302c9d Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Wed, 31 May 2023 10:05:48 +0200
Subject: [PATCH 25/27] fix linter

---
 pkg/cmd/admin.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index a252810a65..02008c0d6d 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -15,7 +15,7 @@ func backup(cmd *cobra.Command, args []string) error {
 	storage := data.StoragePath(cmd.Flag("storage").Value.String())
 	name := data.BackupName(cmd.Flag("name").Value.String())
 
-	dataManager, err := data.NewManager(data.StoragePath(storage))
+	dataManager, err := data.NewManager(storage)
 	if err != nil {
 		return err
 	}
@@ -31,7 +31,7 @@ func backup(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	return dataManager.Backup(data.BackupName(name))
+	return dataManager.Backup(name)
 }
 
 func newAdminDataCommand() *cobra.Command {

From 5816743cde68b80ce572a8c9a8a0c2942f7e877f Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Wed, 31 May 2023 10:18:05 +0200
Subject: [PATCH 26/27] separate std out and err

---
 pkg/admin/data/data_manager.go | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go
index 84f3fc0ce3..2b2419dc5c 100644
--- a/pkg/admin/data/data_manager.go
+++ b/pkg/admin/data/data_manager.go
@@ -1,11 +1,13 @@
 package data
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/openshift/microshift/pkg/config"
 	"github.com/openshift/microshift/pkg/util"
@@ -124,8 +126,14 @@ func copyDataDir(dest string) error {
 	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
 	klog.InfoS("Executing command", "cmd", cmd)
 
-	out, err := cmd.CombinedOutput()
-	klog.InfoS("Command finished running", "cmd", cmd, "output", out)
+	var outb, errb bytes.Buffer
+	cmd.Stdout = &outb
+	cmd.Stderr = &errb
+	err := cmd.Run()
+
+	klog.InfoS("Command finished running", "cmd", cmd,
+		"stdout", strings.ReplaceAll(outb.String(), "\n", `, `),
+		"stderr", errb.String())
 
 	if err != nil {
 		return fmt.Errorf("command %s failed: %w", cmd, err)

From a6e672c86a89cd3f66ac8ef48eb1c4c2d3c0c0af Mon Sep 17 00:00:00 2001
From: Patryk Matuszak <305846+pmtk@users.noreply.github.com>
Date: Wed, 31 May 2023 15:57:41 +0200
Subject: [PATCH 27/27] simplify by requiring non-existing backup name

---
 pkg/admin/data/data_manager.go | 56 +---------------------------------
 pkg/admin/data/run_check.go    |  2 +-
 pkg/cmd/admin.go               |  8 +----
 3 files changed, 3 insertions(+), 63 deletions(-)

diff --git a/pkg/admin/data/data_manager.go b/pkg/admin/data/data_manager.go
index 2b2419dc5c..64fcbf4605 100644
--- a/pkg/admin/data/data_manager.go
+++ b/pkg/admin/data/data_manager.go
@@ -2,9 +2,7 @@ package data
 
 import (
 	"bytes"
-	"errors"
 	"fmt"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"strings"
@@ -64,41 +62,11 @@ func (dm *manager) Backup(name BackupName) error {
 	}
 
 	dest := dm.GetBackupPath(name)
-	tmp := dest + ".tmp"
-	old := dest + ".old"
 
-	// Make sure /storage/backup.tmp does not exist, so data isn't copied into that directory
-	if err := os.RemoveAll(tmp); err != nil {
-		return fmt.Errorf("failed to remove %s: %w", tmp, err)
-	}
-
-	if err := copyDataDir(tmp); err != nil {
+	if err := copyDataDir(dest); err != nil {
 		return err
 	}
 
-	backupExists, err := dm.BackupExists(name)
-	if err != nil {
-		return err
-	} else if backupExists {
-		if err := renamePath(dest, old); err != nil {
-			return err
-		}
-		klog.InfoS("Temporarily renamed existing backup", "backup", dest, "renamed", old)
-	}
-
-	if err := renamePath(tmp, dest); err != nil {
-		klog.Errorf("Renaming path failed - renaming %s back and deleting %s: %v", old, tmp, err)
-		renameErr := renamePath(old, dest)
-		rmErr := removePath(tmp)
-		return errors.Join(err, renameErr, rmErr)
-	}
-
-	if backupExists {
-		if err := removePath(old); err != nil {
-			return err
-		}
-	}
-
 	klog.InfoS("Backup finished", "backup", dest, "data", config.DataDir)
 	return nil
 }
@@ -107,21 +75,6 @@ func (dm *manager) Restore(n BackupName) error {
 	return fmt.Errorf("Restore not implemented")
 }
 
-func removePath(path string) error {
-	exists, err := pathExists(path)
-	if err != nil {
-		return err
-	}
-
-	if exists {
-		if err := os.RemoveAll(path); err != nil {
-			return fmt.Errorf("failed to remove %s: %w", path, err)
-		}
-		klog.InfoS("Removed path", "path", path)
-	}
-	return nil
-}
-
 func copyDataDir(dest string) error {
 	cmd := exec.Command("cp", append(cpArgs, config.DataDir, dest)...) //nolint:gosec
 	klog.InfoS("Executing command", "cmd", cmd)
@@ -143,13 +96,6 @@ func copyDataDir(dest string) error {
 	return nil
 }
 
-func renamePath(from, to string) error {
-	if err := os.Rename(from, to); err != nil {
-		return fmt.Errorf("renaming %s to %s failed: %w", from, to, err)
-	}
-	return nil
-}
-
 func pathExists(path string) (bool, error) {
 	exists, err := util.PathExists(path)
 	if err != nil {
diff --git a/pkg/admin/data/run_check.go b/pkg/admin/data/run_check.go
index c7b67724ea..e3b353d3d3 100644
--- a/pkg/admin/data/run_check.go
+++ b/pkg/admin/data/run_check.go
@@ -25,7 +25,7 @@ func MicroShiftIsNotRunning() error {
 			return fmt.Errorf("error when checking if %s is active: %w", service, err)
 		}
 
-		klog.V(2).InfoS("Service state", "service", service, "state", state)
+		klog.InfoS("Service state", "service", service, "state", state)
 
 		if state != expectedState {
 			return fmt.Errorf("service %s is %s - expected to be %s", service, state, expectedState)
diff --git a/pkg/cmd/admin.go b/pkg/cmd/admin.go
index 02008c0d6d..117f62837f 100644
--- a/pkg/cmd/admin.go
+++ b/pkg/cmd/admin.go
@@ -23,12 +23,7 @@ func backup(cmd *cobra.Command, args []string) error {
 	if exists, err := dataManager.BackupExists(name); err != nil {
 		return err
 	} else if exists {
-		if force, err := cmd.Flags().GetBool("force"); err != nil {
-			return err
-		} else if !force {
-			return fmt.Errorf("backup %s already exists, use --force to overwrite",
-				dataManager.GetBackupPath(name))
-		}
+		return fmt.Errorf("backup %s already exists", dataManager.GetBackupPath(name))
 	}
 
 	return dataManager.Backup(name)
@@ -40,7 +35,6 @@ func newAdminDataCommand() *cobra.Command {
 		Short: "Backup MicroShift data",
 		RunE:  backup,
 	}
-	backup.PersistentFlags().Bool("force", false, "Overwrite existing backup")
 
 	data := &cobra.Command{
 		Use:   "data",