Deprecation of `etcd-snapshot` command in v1.26 (#6575)

* Consolidate etcd snapshot commands
* Consolidate secrets encryption commands
* Move etcd-snapshot to fatal error stage.

Signed-off-by: Derek Nola <derek.nola@suse.com>

cmd/encrypt/main.go

@@ -15,14 +15,13 @@ import (
 func main() {
 	app := cmds.NewApp()
 	app.Commands = []cli.Command{
-		cmds.NewSecretsEncryptCommand(cli.ShowAppHelp,
-			cmds.NewSecretsEncryptSubcommands(
-				secretsencrypt.Status,
-				secretsencrypt.Enable,
-				secretsencrypt.Disable,
-				secretsencrypt.Prepare,
-				secretsencrypt.Rotate,
-				secretsencrypt.Reencrypt),
+		cmds.NewSecretsEncryptCommands(
+			secretsencrypt.Status,
+			secretsencrypt.Enable,
+			secretsencrypt.Disable,
+			secretsencrypt.Prepare,
+			secretsencrypt.Rotate,
+			secretsencrypt.Reencrypt,
 		),
 	}
 

cmd/etcdsnapshot/main.go

@@ -15,12 +15,12 @@ import (
 func main() {
 	app := cmds.NewApp()
 	app.Commands = []cli.Command{
-		cmds.NewEtcdSnapshotCommand(etcdsnapshot.Save,
-			cmds.NewEtcdSnapshotSubcommands(
-				etcdsnapshot.Delete,
-				etcdsnapshot.List,
-				etcdsnapshot.Prune,
-				etcdsnapshot.Save),
+		cmds.NewEtcdSnapshotCommands(
+			etcdsnapshot.Run,
+			etcdsnapshot.Delete,
+			etcdsnapshot.List,
+			etcdsnapshot.Prune,
+			etcdsnapshot.Save,
 		),
 	}
 

cmd/k3s/main.go

@@ -51,21 +51,20 @@ func main() {
 		cmds.NewCRICTL(externalCLIAction("crictl", dataDir)),
 		cmds.NewCtrCommand(externalCLIAction("ctr", dataDir)),
 		cmds.NewCheckConfigCommand(externalCLIAction("check-config", dataDir)),
-		cmds.NewEtcdSnapshotCommand(etcdsnapshotCommand,
-			cmds.NewEtcdSnapshotSubcommands(
-				etcdsnapshotCommand,
-				etcdsnapshotCommand,
-				etcdsnapshotCommand,
-				etcdsnapshotCommand),
+		cmds.NewEtcdSnapshotCommands(
+			etcdsnapshotCommand,
+			etcdsnapshotCommand,
+			etcdsnapshotCommand,
+			etcdsnapshotCommand,
+			etcdsnapshotCommand,
 		),
-		cmds.NewSecretsEncryptCommand(secretsencryptCommand,
-			cmds.NewSecretsEncryptSubcommands(
-				secretsencryptCommand,
-				secretsencryptCommand,
-				secretsencryptCommand,
-				secretsencryptCommand,
-				secretsencryptCommand,
-				secretsencryptCommand),
+		cmds.NewSecretsEncryptCommands(
+			secretsencryptCommand,
+			secretsencryptCommand,
+			secretsencryptCommand,
+			secretsencryptCommand,
+			secretsencryptCommand,
+			secretsencryptCommand,
 		),
 		cmds.NewCertCommand(
 			cmds.NewCertSubcommands(

cmd/server/main.go

@@ -48,21 +48,20 @@ func main() {
 		cmds.NewKubectlCommand(kubectl.Run),
 		cmds.NewCRICTL(crictl.Run),
 		cmds.NewCtrCommand(ctr.Run),
-		cmds.NewEtcdSnapshotCommand(etcdsnapshot.Save,
-			cmds.NewEtcdSnapshotSubcommands(
-				etcdsnapshot.Delete,
-				etcdsnapshot.List,
-				etcdsnapshot.Prune,
-				etcdsnapshot.Save),
+		cmds.NewEtcdSnapshotCommands(
+			etcdsnapshot.Run,
+			etcdsnapshot.Delete,
+			etcdsnapshot.List,
+			etcdsnapshot.Prune,
+			etcdsnapshot.Save,
 		),
-		cmds.NewSecretsEncryptCommand(cli.ShowAppHelp,
-			cmds.NewSecretsEncryptSubcommands(
-				secretsencrypt.Status,
-				secretsencrypt.Enable,
-				secretsencrypt.Disable,
-				secretsencrypt.Prepare,
-				secretsencrypt.Rotate,
-				secretsencrypt.Reencrypt),
+		cmds.NewSecretsEncryptCommands(
+			secretsencrypt.Status,
+			secretsencrypt.Enable,
+			secretsencrypt.Disable,
+			secretsencrypt.Prepare,
+			secretsencrypt.Rotate,
+			secretsencrypt.Reencrypt,
 		),
 		cmds.NewCertCommand(
 			cmds.NewCertSubcommands(

main.go

@@ -32,21 +32,20 @@ func main() {
 		cmds.NewAgentCommand(agent.Run),
 		cmds.NewKubectlCommand(kubectl.Run),
 		cmds.NewCRICTL(crictl.Run),
-		cmds.NewEtcdSnapshotCommand(etcdsnapshot.Save,
-			cmds.NewEtcdSnapshotSubcommands(
-				etcdsnapshot.Delete,
-				etcdsnapshot.List,
-				etcdsnapshot.Prune,
-				etcdsnapshot.Save),
+		cmds.NewEtcdSnapshotCommands(
+			etcdsnapshot.Run,
+			etcdsnapshot.Delete,
+			etcdsnapshot.List,
+			etcdsnapshot.Prune,
+			etcdsnapshot.Save,
 		),
-		cmds.NewSecretsEncryptCommand(cli.ShowAppHelp,
-			cmds.NewSecretsEncryptSubcommands(
-				secretsencrypt.Status,
-				secretsencrypt.Enable,
-				secretsencrypt.Disable,
-				secretsencrypt.Prepare,
-				secretsencrypt.Rotate,
-				secretsencrypt.Reencrypt),
+		cmds.NewSecretsEncryptCommands(
+			secretsencrypt.Status,
+			secretsencrypt.Enable,
+			secretsencrypt.Disable,
+			secretsencrypt.Prepare,
+			secretsencrypt.Rotate,
+			secretsencrypt.Reencrypt,
 		),
 		cmds.NewCertCommand(
 			cmds.NewCertSubcommands(

pkg/cli/cmds/etcd_snapshot.go

@@ -99,61 +99,57 @@ var EtcdSnapshotFlags = []cli.Flag{
 	},
 }
 
-func NewEtcdSnapshotCommand(action func(*cli.Context) error, subcommands []cli.Command) cli.Command {
+func NewEtcdSnapshotCommands(run, delete, list, prune, save func(ctx *cli.Context) error) cli.Command {
 	return cli.Command{
 		Name:            EtcdSnapshotCommand,
 		Usage:           "Trigger an immediate etcd snapshot",
 		SkipFlagParsing: false,
 		SkipArgReorder:  true,
-		Action:          action,
-		Subcommands:     subcommands,
-		Flags:           EtcdSnapshotFlags,
-	}
-}
-
-func NewEtcdSnapshotSubcommands(delete, list, prune, save func(ctx *cli.Context) error) []cli.Command {
-	return []cli.Command{
-		{
-			Name:            "delete",
-			Usage:           "Delete given snapshot(s)",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          delete,
-			Flags:           EtcdSnapshotFlags,
-		},
-		{
-			Name:            "ls",
-			Aliases:         []string{"list", "l"},
-			Usage:           "List snapshots",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          list,
-			Flags: append(EtcdSnapshotFlags, &cli.StringFlag{
-				Name:        "o,output",
-				Usage:       "(db) List format. Default: standard. Optional: json",
-				Destination: &ServerConfig.EtcdListFormat,
-			}),
-		},
-		{
-			Name:            "prune",
-			Usage:           "Remove snapshots that match the name prefix that exceed the configured retention count",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          prune,
-			Flags: append(EtcdSnapshotFlags, &cli.IntFlag{
-				Name:        "snapshot-retention",
-				Usage:       "(db) Number of snapshots to retain.",
-				Destination: &ServerConfig.EtcdSnapshotRetention,
-				Value:       defaultSnapshotRentention,
-			}),
-		},
-		{
-			Name:            "save",
-			Usage:           "Trigger an immediate etcd snapshot",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          save,
-			Flags:           EtcdSnapshotFlags,
+		Action:          run,
+		Subcommands: []cli.Command{
+			{
+				Name:            "delete",
+				Usage:           "Delete given snapshot(s)",
+				SkipFlagParsing: false,
+				SkipArgReorder:  true,
+				Action:          delete,
+				Flags:           EtcdSnapshotFlags,
+			},
+			{
+				Name:            "ls",
+				Aliases:         []string{"list", "l"},
+				Usage:           "List snapshots",
+				SkipFlagParsing: false,
+				SkipArgReorder:  true,
+				Action:          list,
+				Flags: append(EtcdSnapshotFlags, &cli.StringFlag{
+					Name:        "o,output",
+					Usage:       "(db) List format. Default: standard. Optional: json",
+					Destination: &ServerConfig.EtcdListFormat,
+				}),
+			},
+			{
+				Name:            "prune",
+				Usage:           "Remove snapshots that match the name prefix that exceed the configured retention count",
+				SkipFlagParsing: false,
+				SkipArgReorder:  true,
+				Action:          prune,
+				Flags: append(EtcdSnapshotFlags, &cli.IntFlag{
+					Name:        "snapshot-retention",
+					Usage:       "(db) Number of snapshots to retain.",
+					Destination: &ServerConfig.EtcdSnapshotRetention,
+					Value:       defaultSnapshotRentention,
+				}),
+			},
+			{
+				Name:            "save",
+				Usage:           "Trigger an immediate etcd snapshot",
+				SkipFlagParsing: false,
+				SkipArgReorder:  true,
+				Action:          save,
+				Flags:           EtcdSnapshotFlags,
+			},
 		},
+		Flags: EtcdSnapshotFlags,
 	}
 }

pkg/cli/cmds/secrets_encrypt.go

@@ -7,100 +7,83 @@ import (
 
 const SecretsEncryptCommand = "secrets-encrypt"
 
-var EncryptFlags = []cli.Flag{
-	DataDirFlag,
-	ServerToken,
-	cli.StringFlag{
-		Name:        "server, s",
-		Usage:       "(cluster) Server to connect to",
-		EnvVar:      version.ProgramUpper + "_URL",
-		Value:       "https://127.0.0.1:6443",
-		Destination: &ServerConfig.ServerURL,
-	},
-}
-
-func NewSecretsEncryptCommand(action func(*cli.Context) error, subcommands []cli.Command) cli.Command {
-	return cli.Command{
-		Name:            SecretsEncryptCommand,
-		Usage:           "Control secrets encryption and keys rotation",
-		SkipFlagParsing: false,
-		SkipArgReorder:  true,
-		Action:          action,
-		Subcommands:     subcommands,
+var (
+	forceFlag = cli.BoolFlag{
+		Name:        "f,force",
+		Usage:       "Force this stage.",
+		Destination: &ServerConfig.EncryptForce,
 	}
-}
-
-func NewSecretsEncryptSubcommands(status, enable, disable, prepare, rotate, reencrypt func(ctx *cli.Context) error) []cli.Command {
-	return []cli.Command{
-		{
-			Name:            "status",
-			Usage:           "Print current status of secrets encryption",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          status,
-			Flags: append(EncryptFlags, &cli.StringFlag{
-				Name:        "output,o",
-				Usage:       "Status format. Default: text. Optional: json",
-				Destination: &ServerConfig.EncryptOutput,
-			}),
-		},
-		{
-			Name:            "enable",
-			Usage:           "Enable secrets encryption",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          enable,
-			Flags:           EncryptFlags,
-		},
-		{
-			Name:            "disable",
-			Usage:           "Disable secrets encryption",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          disable,
-			Flags:           EncryptFlags,
+	EncryptFlags = []cli.Flag{
+		DataDirFlag,
+		ServerToken,
+		cli.StringFlag{
+			Name:        "server, s",
+			Usage:       "(cluster) Server to connect to",
+			EnvVar:      version.ProgramUpper + "_URL",
+			Value:       "https://127.0.0.1:6443",
+			Destination: &ServerConfig.ServerURL,
 		},
-		{
-			Name:            "prepare",
-			Usage:           "Prepare for encryption keys rotation",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          prepare,
-			Flags: append(EncryptFlags, &cli.BoolFlag{
-				Name:        "f,force",
-				Usage:       "Force preparation.",
-				Destination: &ServerConfig.EncryptForce,
-			}),
-		},
-		{
-			Name:            "rotate",
-			Usage:           "Rotate secrets encryption keys",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          rotate,
-			Flags: append(EncryptFlags, &cli.BoolFlag{
-				Name:        "f,force",
-				Usage:       "Force key rotation.",
-				Destination: &ServerConfig.EncryptForce,
-			}),
-		},
-		{
-			Name:            "reencrypt",
-			Usage:           "Reencrypt all data with new encryption key",
-			SkipFlagParsing: false,
-			SkipArgReorder:  true,
-			Action:          reencrypt,
-			Flags: append(EncryptFlags,
-				&cli.BoolFlag{
-					Name:        "f,force",
-					Usage:       "Force secrets reencryption.",
-					Destination: &ServerConfig.EncryptForce,
-				},
-				&cli.BoolFlag{
-					Name:        "skip",
-					Usage:       "Skip removing old key",
-					Destination: &ServerConfig.EncryptSkip,
+	}
+)
+
+func NewSecretsEncryptCommands(status, enable, disable, prepare, rotate, reencrypt func(ctx *cli.Context) error) cli.Command {
+	return cli.Command{
+		Name:           SecretsEncryptCommand,
+		Usage:          "Control secrets encryption and keys rotation",
+		SkipArgReorder: true,
+		Subcommands: []cli.Command{
+			{
+				Name:           "status",
+				Usage:          "Print current status of secrets encryption",
+				SkipArgReorder: true,
+				Action:         status,
+				Flags: append(EncryptFlags, &cli.StringFlag{
+					Name:        "output,o",
+					Usage:       "Status format. Default: text. Optional: json",
+					Destination: &ServerConfig.EncryptOutput,
 				}),
+			},
+			{
+				Name:           "enable",
+				Usage:          "Enable secrets encryption",
+				SkipArgReorder: true,
+				Action:         enable,
+				Flags:          EncryptFlags,
+			},
+			{
+				Name:           "disable",
+				Usage:          "Disable secrets encryption",
+				SkipArgReorder: true,
+				Action:         disable,
+				Flags:          EncryptFlags,
+			},
+			{
+				Name:           "prepare",
+				Usage:          "Prepare for encryption keys rotation",
+				SkipArgReorder: true,
+				Action:         prepare,
+				Flags:          append(EncryptFlags, &forceFlag),
+			},
+			{
+				Name:           "rotate",
+				Usage:          "Rotate secrets encryption keys",
+				SkipArgReorder: true,
+				Action:         rotate,
+				Flags:          append(EncryptFlags, &forceFlag),
+			},
+			{
+				Name:           "reencrypt",
+				Usage:          "Reencrypt all data with new encryption key",
+				SkipArgReorder: true,
+				Action:         reencrypt,
+				Flags: append(EncryptFlags,
+					&forceFlag,
+					&cli.BoolFlag{
+						Name:        "skip",
+						Usage:       "Skip removing old key",
+						Destination: &ServerConfig.EncryptSkip,
+					}),
+			},
 		},
 	}
 }

pkg/cli/etcdsnapshot/etcd_snapshot.go

@@ -69,9 +69,10 @@ func commandSetup(app *cli.Context, cfg *cmds.Server, sc *server.Config) error {
 	return nil
 }
 
-// Run is an alias for Save, retained for compatibility reasons.
+// Run was an alias for Save
 func Run(app *cli.Context) error {
-	return Save(app)
+	cli.ShowAppHelp(app)
+	return fmt.Errorf("saving with etcd-snapshot was deprecated in v1.26, use \"etcd-snapshot save\" instead")
 }
 
 // Save triggers an on-demand etcd snapshot operation

tests/e2e/snapshotrestore/snapshotrestore_test.go

@@ -101,11 +101,12 @@ var _ = Describe("Verify Create", Ordered, func() {
 
 		It("Verifies Snapshot is created", func() {
 			Eventually(func(g Gomega) {
-				cmd := "sudo k3s etcd-snapshot"
+				cmd := "sudo k3s etcd-snapshot save"
 				_, err := e2e.RunCmdOnNode(cmd, "server-0")
 				g.Expect(err).NotTo(HaveOccurred())
 				cmd = "sudo ls /var/lib/rancher/k3s/server/db/snapshots/"
 				snapshotname, err = e2e.RunCmdOnNode(cmd, "server-0")
+				g.Expect(err).NotTo(HaveOccurred())
 				fmt.Println("Snapshot Name", snapshotname)
 				g.Expect(snapshotname).Should(ContainSubstring("on-demand-server-0"))
 			}, "420s", "10s").Should(Succeed())