From 7b7e1bfa97a62934e5a0227a9befbd4fa099b031 Mon Sep 17 00:00:00 2001 From: decentral1se Date: Tue, 25 Jun 2024 09:48:53 +0200 Subject: [PATCH] refactor!: server add/rm has better UI/UX Less confusing logging messages, clear "is created" / "already exists" output. Move the majority of logging to debug output to not confuse the situation. Some code cleanups also in there. --- cli/server/add.go | 101 +++++++++++++++++++++++++------------------ cli/server/remove.go | 2 +- pkg/client/client.go | 4 +- pkg/server/server.go | 4 +- 4 files changed, 63 insertions(+), 48 deletions(-) diff --git a/cli/server/add.go b/cli/server/add.go index 15d17830..598d91ca 100644 --- a/cli/server/add.go +++ b/cli/server/add.go @@ -23,29 +23,29 @@ var localFlag = &cli.BoolFlag{ Destination: &local, } -func cleanUp(domainName string) { - if domainName != "default" { - logrus.Infof("cleaning up context for %s", domainName) - if err := client.DeleteContext(domainName); err != nil { +// cleanUp cleans up the partially created context/client details for a failed +// "server add" attempt. +func cleanUp(name string) { + if name != "default" { + logrus.Debugf("serverAdd: cleanUp: cleaning up context for %s", name) + if err := client.DeleteContext(name); err != nil { logrus.Fatal(err) } } - logrus.Infof("attempting to clean up server directory for %s", domainName) - - serverDir := filepath.Join(config.SERVERS_DIR, domainName) + serverDir := filepath.Join(config.SERVERS_DIR, name) files, err := config.GetAllFilesInDirectory(serverDir) if err != nil { - logrus.Fatalf("unable to list files in %s: %s", serverDir, err) + logrus.Fatalf("serverAdd: cleanUp: 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) + logrus.Debugf("serverAdd: cleanUp: %s is not empty, aborting cleanup", serverDir) return } if err := os.RemoveAll(serverDir); err != nil { - logrus.Fatal(err) + logrus.Fatalf("serverAdd: cleanUp: failed to remove %s: %s", serverDir, err) } } @@ -53,39 +53,42 @@ func cleanUp(domainName string) { // Docker manages SSH connection details. These are stored to disk in // ~/.docker. Abra can manage this completely for the user, so it's an // implementation detail. -func newContext(c *cli.Context, domainName string) error { +func newContext(name string) (bool, error) { store := contextPkg.NewDefaultDockerContextStore() contexts, err := store.Store.List() if err != nil { - return err + return false, err } for _, context := range contexts { - if context.Name == domainName { - logrus.Debugf("context for %s already exists", domainName) - return nil + if context.Name == name { + logrus.Debugf("context for %s already exists", name) + return false, nil } } - logrus.Debugf("creating context with domain %s", domainName) + logrus.Debugf("creating context with domain %s", name) - if err := client.CreateContext(domainName); err != nil { - return err + if err := client.CreateContext(name); err != nil { + return false, nil } - return nil + return true, nil } // createServerDir creates the ~/.abra/servers/... directory for a new server. -func createServerDir(domainName string) error { - if err := server.CreateServerDir(domainName); err != nil { +func createServerDir(name string) (bool, error) { + if err := server.CreateServerDir(name); err != nil { if !os.IsExist(err) { - return err + return false, err } - logrus.Debugf("server dir for %s already created", domainName) + + logrus.Debugf("server dir for %s already created", name) + + return false, nil } - return nil + return true, nil } var serverAddCommand = cli.Command{ @@ -130,56 +133,68 @@ of your ~/.ssh/config. Checks for a valid online domain will be skipped: ArgsUsage: "", Action: func(c *cli.Context) error { if len(c.Args()) > 0 && local || !internal.ValidateSubCmdFlags(c) { - err := errors.New("cannot use and --local together") + err := errors.New("cannot use and --local together") internal.ShowSubcommandHelpAndError(c, err) } - var domainName string + var name string if local { - domainName = "default" + name = "default" } else { - domainName = internal.ValidateDomain(c) + name = internal.ValidateDomain(c) } if local { - if err := createServerDir(domainName); err != nil { + created, err := createServerDir(name) + if err != nil { logrus.Fatal(err) } - logrus.Infof("attempting to create client for %s", domainName) - if _, err := client.New(domainName); err != nil { - cleanUp(domainName) + logrus.Debugf("attempting to create client for %s", name) + + if _, err := client.New(name); err != nil { + cleanUp(name) logrus.Fatal(err) } - logrus.Info("local server added") + if created { + logrus.Info("local server successfully added") + } else { + logrus.Warn("local server already exists") + } return nil } if !internal.NoDomainChecks { - if _, err := dns.EnsureIPv4(domainName); err != nil { + if _, err := dns.EnsureIPv4(name); err != nil { logrus.Fatal(err) } } - if err := createServerDir(domainName); err != nil { + _, err := createServerDir(name) + if err != nil { logrus.Fatal(err) } - if err := newContext(c, domainName); err != nil { - cleanUp(domainName) + created, err := newContext(name) + if err != nil { + cleanUp(name) logrus.Fatal(err) } - logrus.Infof("attempting to create client for %s", domainName) - if _, err := client.New(domainName); err != nil { - cleanUp(domainName) - logrus.Debugf("failed to construct client for %s, saw %s", domainName, err.Error()) - logrus.Fatal(sshPkg.Fatal(domainName, err)) + logrus.Debugf("attempting to create client for %s", name) + if _, err := client.New(name); err != nil { + cleanUp(name) + logrus.Debugf("failed to construct client for %s, saw %s", name, err.Error()) + logrus.Fatal(sshPkg.Fatal(name, err)) } - logrus.Infof("%s added", domainName) + if created { + logrus.Infof("%s successfully added", name) + } else { + logrus.Warnf("%s already exists", name) + } return nil }, diff --git a/cli/server/remove.go b/cli/server/remove.go index 83817c5a..d8d2ca5d 100644 --- a/cli/server/remove.go +++ b/cli/server/remove.go @@ -41,7 +41,7 @@ rain. logrus.Fatal(err) } - logrus.Infof("server at %s has been lost in time, like tears in rain", serverName) + logrus.Infof("%s is now lost in time, like tears in rain", serverName) return nil }, diff --git a/pkg/client/client.go b/pkg/client/client.go index 76bdec81..f8f5e2d6 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -75,9 +75,9 @@ func New(serverName string) (*client.Client, error) { if info.Swarm.LocalNodeState == "inactive" { if serverName != "default" { return cl, fmt.Errorf("swarm mode not enabled on %s?", serverName) - } else { - return cl, errors.New("swarm mode not enabled on local server?") } + + return cl, errors.New("swarm mode not enabled on local server?") } return cl, nil diff --git a/pkg/server/server.go b/pkg/server/server.go index ec88a271..c8e4b982 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -17,11 +17,11 @@ func CreateServerDir(serverName string) error { return err } - logrus.Infof("%s already exists", serverPath) + logrus.Debugf("%s already exists", serverPath) return nil } - logrus.Infof("successfully created %s", serverPath) + logrus.Debugf("successfully created %s", serverPath) return nil }