fix: improved "server add" UI/UX #421

Merged
decentral1se merged 4 commits from improve-ssh-timeout-server-add into main 2024-06-25 14:27:30 +00:00
7 changed files with 141 additions and 88 deletions

View File

@ -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: "<domain>",
ArgsUsage: "<name>",
Action: func(c *cli.Context) error {
if len(c.Args()) > 0 && local || !internal.ValidateSubCmdFlags(c) {
err := errors.New("cannot use <domain> and --local together")
err := errors.New("cannot use <name> 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
},

View File

@ -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
},

View File

@ -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

View File

@ -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
}

View File

@ -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

View File

@ -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://<host> 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
}

View File

@ -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 <domain> and --local together'
assert_output --partial 'cannot use <name> 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"