diff --git a/cli/server/add.go b/cli/server/add.go index cc275e6f..d3e94888 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{ @@ -95,23 +98,19 @@ var serverAddCommand = cli.Command{ Description: ` Add a new server to your configuration so that it can be managed by Abra. -Abra uses the SSH command-line to discover connection details for your server. -It is advised to configure an entry per-host in your ~/.ssh/config for each -server. For example: +Abra relies on the standard SSH command-line and ~/.ssh/config for client +connection details. You must configure an entry per-host in your ~/.ssh/config +for each server. For example: -Host example.com example - Hostname example.com - User exampleUser - Port 12345 - IdentityFile ~/.ssh/example@somewhere + Host example.com example + Hostname example.com + User exampleUser + Port 12345 + IdentityFile ~/.ssh/example@somewhere -Abra can then load SSH connection details from this configuratiion with: +You can then add a server like so: - abra server add example.com - -Or using the name "example" which is not resolvable on the public Internet: - - abra server add -D example + abra server add example.com If "--local" is passed, then Abra assumes that the current local server is intended as the target server. This is useful when you want to have your entire @@ -119,8 +118,10 @@ Co-op Cloud config located on the server itself, and not on your local developer machine. The domain is then set to "default". You can also pass "--no-domain-checks/-D" flag to use any arbitrary name -instead of a real domain. Host will be resolved with the "hostname" entry of -your SSH configuration. Checks for a valid online domain will be skipped. +instead of a real domain. The host will be resolved with the "Hostname" entry +of your ~/.ssh/config. Checks for a valid online domain will be skipped: + + abra server add -D example `, Flags: []cli.Flag{ internal.DebugFlag, @@ -129,59 +130,75 @@ your SSH configuration. Checks for a valid online domain will be skipped. localFlag, }, Before: internal.SubCommandBefore, - ArgsUsage: "", + 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) } + // NOTE(d1): reasonable 5 second timeout for connections which can't + // succeed. The connection is attempted twice, so this results in 10 + // seconds. + timeout := client.WithTimeout(5) + 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, timeout); 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, timeout); err != nil { + cleanUp(name) + 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..e11c4806 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -16,11 +16,26 @@ import ( "github.com/sirupsen/logrus" ) +// Conf is a Docker client configuration. +type Conf struct { + Timeout int +} + +// Opt is a Docker client option. +type Opt func(c *Conf) + +// WithTimeout specifies a timeout for a Docker client. +func WithTimeout(timeout int) Opt { + return func(c *Conf) { + c.Timeout = timeout + } +} + // New initiates a new Docker client. New client connections are validated so // that we ensure connections via SSH to the daemon can succeed. It takes into // account that you may only want the local client and not communicate via SSH. // For this use-case, please pass "default" as the contextName. -func New(serverName string) (*client.Client, error) { +func New(serverName string, opts ...Opt) (*client.Client, error) { var clientOpts []client.Opt if serverName != "default" { @@ -34,7 +49,12 @@ func New(serverName string) (*client.Client, error) { return nil, err } - helper, err := commandconnPkg.NewConnectionHelper(ctxEndpoint) + conf := &Conf{} + for _, opt := range opts { + opt(conf) + } + + helper, err := commandconnPkg.NewConnectionHelper(ctxEndpoint, conf.Timeout) if err != nil { return nil, err } @@ -75,9 +95,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 } diff --git a/pkg/ssh/ssh.go b/pkg/ssh/ssh.go index 9a3a9efd..97d627a0 100644 --- a/pkg/ssh/ssh.go +++ b/pkg/ssh/ssh.go @@ -19,7 +19,7 @@ func Fatal(hostname string, err error) error { } else if strings.Contains(out, "Permission denied") { return fmt.Errorf("ssh auth: permission denied for %s", hostname) } else if strings.Contains(out, "Network is unreachable") { - return fmt.Errorf("unable to connect to %s, network is unreachable?", hostname) + return fmt.Errorf("unable to connect to %s, please check your SSH config", hostname) } return err diff --git a/pkg/upstream/commandconn/connection.go b/pkg/upstream/commandconn/connection.go index 63886eb9..f48a77af 100644 --- a/pkg/upstream/commandconn/connection.go +++ b/pkg/upstream/commandconn/connection.go @@ -2,6 +2,7 @@ package commandconn import ( "context" + "fmt" "net" "net/url" @@ -14,14 +15,17 @@ import ( ) // GetConnectionHelper returns Docker-specific connection helper for the given URL. -// GetConnectionHelper returns nil without error when no helper is registered for the scheme. -// -// ssh:// URL requires Docker 18.09 or later on the remote host. -func GetConnectionHelper(daemonURL string) (*connhelper.ConnectionHelper, error) { - return getConnectionHelper(daemonURL) +func GetConnectionHelper(daemonURL string, timeout int) (*connhelper.ConnectionHelper, error) { + if timeout != 0 { + return getConnectionHelper(daemonURL, []string{fmt.Sprintf("-o ConnectTimeout=%v", timeout)}) + } + + return getConnectionHelper(daemonURL, []string{}) } -func getConnectionHelper(daemonURL string) (*connhelper.ConnectionHelper, error) { +// getConnectionHelper generates a connection helper from the underlying Docker +// libraries. +func getConnectionHelper(daemonURL string, sshFlags []string) (*connhelper.ConnectionHelper, error) { url, err := url.Parse(daemonURL) if err != nil { return nil, err @@ -35,7 +39,7 @@ func getConnectionHelper(daemonURL string) (*connhelper.ConnectionHelper, error) return &connhelper.ConnectionHelper{ Dialer: func(ctx context.Context, network, addr string) (net.Conn, error) { - return New(ctx, "ssh", ctxConnDetails.Args("docker", "system", "dial-stdio")...) + return New(ctx, "ssh", append(sshFlags, ctxConnDetails.Args("docker", "system", "dial-stdio")...)...) }, Host: "http://docker.example.com", }, nil @@ -46,8 +50,8 @@ func getConnectionHelper(daemonURL string) (*connhelper.ConnectionHelper, error) } // NewConnectionHelper creates new connection helper for a remote docker daemon. -func NewConnectionHelper(daemonURL string) (*connhelper.ConnectionHelper, error) { - helper, err := GetConnectionHelper(daemonURL) +func NewConnectionHelper(daemonURL string, timeout int) (*connhelper.ConnectionHelper, error) { + helper, err := GetConnectionHelper(daemonURL, timeout) if err != nil { return nil, err } diff --git a/tests/integration/server_add.bats b/tests/integration/server_add.bats index 72d9a1cc..0d0086d7 100644 --- a/tests/integration/server_add.bats +++ b/tests/integration/server_add.bats @@ -1,8 +1,25 @@ #!/usr/bin/env bash +setup_file(){ + load "$PWD/tests/integration/helpers/common" + _common_setup + + run docker swarm leave --force +} + setup(){ load "$PWD/tests/integration/helpers/common" _common_setup + + run docker swarm init + assert_success +} + +teardown(){ + run docker swarm leave --force + assert_success + + _rm_server } @test "add new server" { @@ -16,29 +33,24 @@ setup(){ assert bash -c "docker context ls | grep -q $TEST_SERVER" } -@test "error if using domain and --local together" { +@test "error if using name and --local together" { run $ABRA server add "$TEST_SERVER" --local assert_failure - assert_output --partial 'cannot use and --local together' + assert_output --partial 'cannot use and --local together' } @test "create local server" { - run docker swarm init - assert_success - run $ABRA server add --local assert_success assert_exists "$ABRA_DIR/servers/default" assert bash -c "docker context ls | grep -q default" - assert_output --partial 'local server added' - - docker swarm leave --force - assert_success - - _rm_default_server + assert_output --partial 'local server successfully added' } @test "create local server fails when no docker swarm" { + run docker swarm leave --force + assert_success + run $ABRA server add --local assert_failure assert_not_exists "$ABRA_DIR/servers/default"