From a93786c6be87c2845395f6e93a35743a4d2e124f Mon Sep 17 00:00:00 2001 From: decentral1se Date: Mon, 23 Jan 2023 13:29:46 +0100 Subject: [PATCH 1/3] fix!: make "app rm" more explicit & simpler We point users to "app volume/secret remove" for more specific deletion of other app data resources now. The idea is that if you lose the env file locally, then you can't clean up anything after. So it is handy to have a sort of WARNING barrier to deleting that file. This flow is the only way to get Abra to delete your local env file. It now feels more documented and sufficiently scary in the UI/UX to merit that. Hopefully addresses the ticket sufficiently. Closes https://git.coopcloud.tech/coop-cloud/organising/issues/335. --- cli/app/remove.go | 97 ++++++++++++++++------------------------------- 1 file changed, 33 insertions(+), 64 deletions(-) diff --git a/cli/app/remove.go b/cli/app/remove.go index 2810946c..6c0378e8 100644 --- a/cli/app/remove.go +++ b/cli/app/remove.go @@ -15,35 +15,43 @@ import ( "github.com/urfave/cli" ) -// Volumes stores the variable from VolumesFlag -var Volumes bool - -// VolumesFlag is used to specify if volumes should be deleted when deleting an app -var VolumesFlag = &cli.BoolFlag{ - Name: "volumes, V", - Destination: &Volumes, -} - var appRemoveCommand = cli.Command{ Name: "remove", Aliases: []string{"rm"}, ArgsUsage: "", - Usage: "Remove an already undeployed app", + Usage: "Remove all app data, locally and remotely", + Description: ` +This command removes everything related to an app which is already undeployed. + +By default, it will prompt for confirmation before proceeding. All secrets, +volumes and the local app env file will be deleted. + +Only run this command when you are sure you want to completely remove the app +and all associated app data. This is a destructive action, Be Careful! + +If you would like to delete specific volumes or secrets, please use removal +sub-commands under "app volume" and "app secret" instead. + +Please note, if you delete the local app env file without removing volumes and +secrets first, Abra will *not* be able to help you remove them afterwards. + +To delete everything without prompt, use the "--force/-f" or the "--no-input/n" +flag. +`, Flags: []cli.Flag{ - VolumesFlag, internal.ForceFlag, internal.DebugFlag, internal.NoInputFlag, }, - Before: internal.SubCommandBefore, + BashComplete: autocomplete.AppNameComplete, + Before: internal.SubCommandBefore, Action: func(c *cli.Context) error { app := internal.ValidateApp(c) if !internal.Force && !internal.NoInput { response := false - prompt := &survey.Confirm{ - Message: fmt.Sprintf("about to remove %s, are you sure?", app.Name), - } + msg := "ALERTA ALERTA: this will completely remove %s data and configurations locally and remotely, are you sure?" + prompt := &survey.Confirm{Message: fmt.Sprintf(msg, app.Name)} if err := survey.AskOne(prompt, &response); err != nil { logrus.Fatal(err) } @@ -84,26 +92,7 @@ var appRemoveCommand = cli.Command{ } if len(secrets) > 0 { - var secretNamesToRemove []string - - if !internal.Force && !internal.NoInput { - secretsPrompt := &survey.MultiSelect{ - Message: "which secrets do you want to remove?", - Help: "'x' indicates selected, enter / return to confirm, ctrl-c to exit, vim mode is enabled", - VimMode: true, - Options: secretNames, - Default: secretNames, - } - if err := survey.AskOne(secretsPrompt, &secretNamesToRemove); err != nil { - logrus.Fatal(err) - } - } - - if internal.Force || internal.NoInput { - secretNamesToRemove = secretNames - } - - for _, name := range secretNamesToRemove { + for _, name := range secretNames { err := cl.SecretRemove(context.Background(), secrets[name]) if err != nil { logrus.Fatal(err) @@ -131,44 +120,24 @@ var appRemoveCommand = cli.Command{ } if len(vols) > 0 { - if Volumes { - var removeVols []string - if !internal.Force && !internal.NoInput { - volumesPrompt := &survey.MultiSelect{ - Message: "which volumes do you want to remove?", - Help: "'x' indicates selected, enter / return to confirm, ctrl-c to exit, vim mode is enabled", - VimMode: true, - Options: vols, - Default: vols, - } - if err := survey.AskOne(volumesPrompt, &removeVols); err != nil { - logrus.Fatal(err) - } + var removeVols []string + for _, vol := range removeVols { + err := cl.VolumeRemove(context.Background(), vol, internal.Force) // last argument is for force removing + if err != nil { + logrus.Fatal(err) } - - for _, vol := range removeVols { - err := cl.VolumeRemove(context.Background(), vol, internal.Force) // last argument is for force removing - if err != nil { - logrus.Fatal(err) - } - logrus.Info(fmt.Sprintf("volume %s removed", vol)) - } - } else { - logrus.Info("no volumes were removed") + logrus.Info(fmt.Sprintf("volume %s removed", vol)) } } else { - if Volumes { - logrus.Info("no volumes to remove") - } + logrus.Info("no volumes to remove") } - err = os.Remove(app.Path) - if err != nil { + if err = os.Remove(app.Path); err != nil { logrus.Fatal(err) } + logrus.Info(fmt.Sprintf("file: %s removed", app.Path)) return nil }, - BashComplete: autocomplete.AppNameComplete, } -- 2.49.0 From 27e0708ac7661685e728ff9213104de50554f562 Mon Sep 17 00:00:00 2001 From: decentral1se Date: Mon, 23 Jan 2023 13:56:27 +0100 Subject: [PATCH 2/3] fix: don't delete server dir on cleanup if not empty Part of https://git.coopcloud.tech/coop-cloud/organising/issues/325. --- cli/server/add.go | 14 +++++++++++++- pkg/config/app.go | 2 +- pkg/config/env.go | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/cli/server/add.go b/cli/server/add.go index af0b5e3f..59be899c 100644 --- a/cli/server/add.go +++ b/cli/server/add.go @@ -80,7 +80,19 @@ func cleanUp(domainName string) { } logrus.Warnf("cleaning up server directory for %s", domainName) - if err := os.RemoveAll(filepath.Join(config.SERVERS_DIR, domainName)); err != nil { + + serverDir := filepath.Join(config.SERVERS_DIR, domainName) + files, err := config.GetAllFilesInDirectory(serverDir) + if err != nil { + logrus.Fatalf("unable to list files in %s: %s", serverDir, err) + } + + if len(files) > 0 { + logrus.Warnf("aborting clean up of %s because it is not empty", serverDir) + return + } + + if err := os.RemoveAll(serverDir); err != nil { logrus.Fatal(err) } } diff --git a/pkg/config/app.go b/pkg/config/app.go index 46717572..17af55c4 100644 --- a/pkg/config/app.go +++ b/pkg/config/app.go @@ -203,7 +203,7 @@ func LoadAppFiles(servers ...string) (AppFiles, error) { for _, server := range servers { serverDir := path.Join(SERVERS_DIR, server) - files, err := getAllFilesInDirectory(serverDir) + files, err := GetAllFilesInDirectory(serverDir) if err != nil { return appFiles, fmt.Errorf("server %s doesn't exist? Run \"abra server ls\" to check", server) } diff --git a/pkg/config/env.go b/pkg/config/env.go index 44e16efb..e799f58b 100644 --- a/pkg/config/env.go +++ b/pkg/config/env.go @@ -66,8 +66,8 @@ func ReadServerNames() ([]string, error) { return serverNames, nil } -// getAllFilesInDirectory returns filenames of all files in directory -func getAllFilesInDirectory(directory string) ([]fs.FileInfo, error) { +// GetAllFilesInDirectory returns filenames of all files in directory +func GetAllFilesInDirectory(directory string) ([]fs.FileInfo, error) { var realFiles []fs.FileInfo files, err := ioutil.ReadDir(directory) -- 2.49.0 From b089109c94173a549e6e90d3336285278e1cdf8a Mon Sep 17 00:00:00 2001 From: decentral1se Date: Mon, 23 Jan 2023 14:56:34 +0100 Subject: [PATCH 3/3] fix: more robust docker context problem handling See https://git.coopcloud.tech/coop-cloud/organising/issues/325 See https://git.coopcloud.tech/coop-cloud/organising/issues/340 --- cli/app/list.go | 5 ++++ cli/internal/new.go | 5 ++++ cli/internal/validate.go | 5 ++++ cli/server/list.go | 20 +++++++++++++++- pkg/context/context.go | 32 ++++++++++++++++++++++++++ pkg/upstream/commandconn/connection.go | 5 ++++ 6 files changed, 71 insertions(+), 1 deletion(-) diff --git a/cli/app/list.go b/cli/app/list.go index 4810e56f..d38626cb 100644 --- a/cli/app/list.go +++ b/cli/app/list.go @@ -8,6 +8,7 @@ import ( "coopcloud.tech/abra/cli/internal" "coopcloud.tech/abra/pkg/config" + "coopcloud.tech/abra/pkg/context" "coopcloud.tech/abra/pkg/formatter" "coopcloud.tech/abra/pkg/recipe" "coopcloud.tech/abra/pkg/ssh" @@ -97,6 +98,10 @@ can take some time. alreadySeen := make(map[string]bool) for _, app := range apps { if _, ok := alreadySeen[app.Server]; !ok { + if err := context.HasDockerContext(app.Server); err != nil { + logrus.Fatal(err) + } + if err := ssh.EnsureHostKey(app.Server); err != nil { logrus.Fatal(fmt.Sprintf(internal.SSHFailMsg, app.Server)) } diff --git a/cli/internal/new.go b/cli/internal/new.go index cdeb9f0b..56b4c681 100644 --- a/cli/internal/new.go +++ b/cli/internal/new.go @@ -6,6 +6,7 @@ import ( "coopcloud.tech/abra/pkg/app" "coopcloud.tech/abra/pkg/config" + "coopcloud.tech/abra/pkg/context" "coopcloud.tech/abra/pkg/formatter" "coopcloud.tech/abra/pkg/jsontable" "coopcloud.tech/abra/pkg/recipe" @@ -146,6 +147,10 @@ func NewAction(c *cli.Context) error { var secrets AppSecrets var secretTable *jsontable.JSONTable if Secrets { + if err := context.HasDockerContext(NewAppServer); err != nil { + logrus.Fatal(err) + } + if err := ssh.EnsureHostKey(NewAppServer); err != nil { logrus.Fatal(err) } diff --git a/cli/internal/validate.go b/cli/internal/validate.go index a60ccf89..02a024de 100644 --- a/cli/internal/validate.go +++ b/cli/internal/validate.go @@ -8,6 +8,7 @@ import ( "coopcloud.tech/abra/pkg/app" "coopcloud.tech/abra/pkg/config" + "coopcloud.tech/abra/pkg/context" "coopcloud.tech/abra/pkg/recipe" "coopcloud.tech/abra/pkg/ssh" "github.com/AlecAivazis/survey/v2" @@ -138,6 +139,10 @@ func ValidateApp(c *cli.Context) config.App { logrus.Fatal(err) } + if err := context.HasDockerContext(app.Server); err != nil { + logrus.Fatal(err) + } + if err := ssh.EnsureHostKey(app.Server); err != nil { logrus.Fatal(err) } diff --git a/cli/server/list.go b/cli/server/list.go index 32e53f8e..a9f03d20 100644 --- a/cli/server/list.go +++ b/cli/server/list.go @@ -12,11 +12,20 @@ import ( "github.com/urfave/cli" ) +var problemsFilter bool + +var problemsFilterFlag = &cli.BoolFlag{ + Name: "problems, p", + Usage: "Show only servers with potential connection problems", + Destination: &problemsFilter, +} + var serverListCommand = cli.Command{ Name: "list", Aliases: []string{"ls"}, Usage: "List managed servers", Flags: []cli.Flag{ + problemsFilterFlag, internal.DebugFlag, internal.MachineReadableFlag, }, @@ -48,6 +57,7 @@ var serverListCommand = cli.Command{ // No local context found, we can continue safely continue } + if ctx.Name == serverName { sp, err := ssh.ParseURL(endpoint) if err != nil { @@ -56,6 +66,7 @@ var serverListCommand = cli.Command{ row = []string{serverName, sp.Host, sp.User, sp.Port} } } + if len(row) == 0 { if serverName == "default" { row = []string{serverName, "local", "n/a", "n/a"} @@ -63,7 +74,14 @@ var serverListCommand = cli.Command{ row = []string{serverName, "unknown", "unknown", "unknown"} } } - table.Append(row) + + if problemsFilter { + if row[1] == "unknown" { + table.Append(row) + } + } else { + table.Append(row) + } } return nil diff --git a/pkg/context/context.go b/pkg/context/context.go index 75417aee..3412180b 100644 --- a/pkg/context/context.go +++ b/pkg/context/context.go @@ -2,6 +2,7 @@ package context import ( "errors" + "fmt" "github.com/docker/cli/cli/command" dConfig "github.com/docker/cli/cli/config" @@ -42,3 +43,34 @@ func GetContextEndpoint(ctx contextStore.Metadata) (string, error) { func newContextStore(dir string, config contextStore.Config) contextStore.Store { return contextStore.New(dir, config) } + +// MissingContextMsg helps end-uers debug missing docker context issues. +var missingContextMsg = `unable to find Docker context for %s? + +Please run "abra server ls" to confirm. If you see "unknown" in the table +output then you need to run the following command: + + abra server add %s + +See "abra server add --help" for more. +` + +// HasDockerContext figures out if a local setup has a working docker context +// configuration or not. This usually tells us if they'll be able to make a SSH +// connection to a server or not and can be a useful way to signal to end-users +// that they need to fix something up if missing. +func HasDockerContext(serverName string) error { + dockerContextStore := NewDefaultDockerContextStore() + contexts, err := dockerContextStore.Store.List() + if err != nil { + return err + } + + for _, ctx := range contexts { + if ctx.Name == serverName { + return nil + } + } + + return fmt.Errorf(missingContextMsg, serverName, serverName) +} diff --git a/pkg/upstream/commandconn/connection.go b/pkg/upstream/commandconn/connection.go index 6f632a6e..80b52a75 100644 --- a/pkg/upstream/commandconn/connection.go +++ b/pkg/upstream/commandconn/connection.go @@ -6,6 +6,7 @@ import ( "net" "net/url" + ctxPkg "coopcloud.tech/abra/pkg/context" sshPkg "coopcloud.tech/abra/pkg/ssh" "github.com/docker/cli/cli/connhelper" "github.com/docker/cli/cli/connhelper/ssh" @@ -36,6 +37,10 @@ func getConnectionHelper(daemonURL string, sshFlags []string) (*connhelper.Conne return nil, errors.Wrap(err, "ssh host connection is not valid") } + if err := ctxPkg.HasDockerContext(ctxConnDetails.Host); err != nil { + return nil, err + } + if err := sshPkg.EnsureHostKey(ctxConnDetails.Host); err != nil { return nil, err } -- 2.49.0