From a6c885ef044272f11203a4f8af4d71d85f8e1d71 Mon Sep 17 00:00:00 2001 From: Matej Vasek Date: Fri, 9 Sep 2022 15:34:29 +0200 Subject: [PATCH] feat: UX improvements for docker/podman usage (#1224) * Better error message I docker/podman not present. * Auto detect podman machine's socket on mac/win. Signed-off-by: Matej Vasek Signed-off-by: Matej Vasek --- cmd/func/main.go | 26 +++++++++++ docker/docker_client.go | 78 ++++++++++++++++++++++++++------ docker/docker_client_ssh_test.go | 6 ++- 3 files changed, 95 insertions(+), 15 deletions(-) diff --git a/cmd/func/main.go b/cmd/func/main.go index e40812b3a..8cc06254b 100644 --- a/cmd/func/main.go +++ b/cmd/func/main.go @@ -2,12 +2,15 @@ package main import ( "context" + "errors" "fmt" "os" + "os/exec" "os/signal" "syscall" "knative.dev/kn-plugin-func/cmd" + "knative.dev/kn-plugin-func/docker" ) // Statically-populated build metadata set by `make build`. @@ -39,6 +42,29 @@ func main() { if ctx.Err() != nil { os.Exit(130) } + + if errors.Is(err, docker.ErrNoDocker) { + if !dockerOrPodmanInstalled() { + fmt.Fprintln(os.Stderr, `Docker/Podman not installed. +Please consider installing one of these: + https://podman-desktop.io/ + https://www.docker.com/products/docker-desktop/`) + } else { + fmt.Fprintln(os.Stderr, `Possible causes: + The docker/podman daemon is not running. + The DOCKER_HOST environment variable is not set.`) + } + } + os.Exit(1) } } + +func dockerOrPodmanInstalled() bool { + _, err := exec.LookPath("podman") + if err == nil { + return true + } + _, err = exec.LookPath("docker") + return err == nil +} diff --git a/docker/docker_client.go b/docker/docker_client.go index 3ee9d36db..623a6ccae 100644 --- a/docker/docker_client.go +++ b/docker/docker_client.go @@ -3,6 +3,7 @@ package docker import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -15,11 +16,13 @@ import ( "syscall" "time" - "knative.dev/kn-plugin-func/ssh" - "github.com/docker/docker/client" + + "knative.dev/kn-plugin-func/ssh" ) +var ErrNoDocker = errors.New("docker/podman API not available") + // NewClient creates a new docker client. // reads the DOCKER_HOST envvar but it may or may not return it as dockerHost. // - For local connection (unix socket and windows named pipe) it returns the @@ -28,27 +31,45 @@ import ( // - For TCP connections it returns "" so it defaults in the remote (note that // one should not be use client.DefaultDockerHost in this situation). This is // needed beaus of TCP+tls connections. -func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerHost string, err error) { +func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerHostInRemote string, err error) { var _url *url.URL - dockerHost = os.Getenv("DOCKER_HOST") + dockerHost := os.Getenv("DOCKER_HOST") + dockerHostSSHIdentity := os.Getenv("DOCKER_HOST_SSH_IDENTITY") - if dockerHost == "" && runtime.GOOS == "linux" && podmanPresent() { + if dockerHost == "" { _url, err = url.Parse(defaultHost) if err != nil { return } _, err = os.Stat(_url.Path) switch { + case err == nil: + dockerHost = defaultHost case err != nil && !os.IsNotExist(err): return - case os.IsNotExist(err): - dockerClient, dockerHost, err = newClientWithPodmanService() - dockerClient = &closeGuardingClient{pimpl: dockerClient} - return + case os.IsNotExist(err) && podmanPresent(): + if runtime.GOOS == "linux" { + // on Linux: spawn temporary podman service + dockerClient, dockerHostInRemote, err = newClientWithPodmanService() + dockerClient = &closeGuardingClient{pimpl: dockerClient} + return + } else { + // on non-Linux: try to use connection to podman machine + dh, dhid := tryGetPodmanRemoteConn() + if dh != "" { + dockerHost, dockerHostSSHIdentity = dh, dhid + } + } } } + if dockerHost == "" { + return nil, "", ErrNoDocker + } + + dockerHostInRemote = dockerHost + _url, err = url.Parse(dockerHost) isSSH := err == nil && _url.Scheme == "ssh" isTCP := err == nil && _url.Scheme == "tcp" @@ -57,23 +78,23 @@ func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerH // With TCP, it's difficult to determine how to expose the daemon socket to lifecycle containers, // so we are defaulting to standard docker location by returning empty string. // This should work well most of the time. - dockerHost = "" + dockerHostInRemote = "" } if !isSSH { - dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) + dockerClient, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation(), client.WithHost(dockerHost)) dockerClient = &closeGuardingClient{pimpl: dockerClient} return } credentialsConfig := ssh.Config{ - Identity: os.Getenv("DOCKER_HOST_SSH_IDENTITY"), + Identity: dockerHostSSHIdentity, PassPhrase: os.Getenv("DOCKER_HOST_SSH_IDENTITY_PASSPHRASE"), PasswordCallback: ssh.NewPasswordCbk(), PassPhraseCallback: ssh.NewPassPhraseCbk(), HostKeyCallback: ssh.NewHostKeyCbk(), } - contextDialer, dockerHost, err := ssh.NewDialContext(_url, credentialsConfig) + contextDialer, dockerHostInRemote, err := ssh.NewDialContext(_url, credentialsConfig) if err != nil { return } @@ -101,7 +122,36 @@ func NewClient(defaultHost string) (dockerClient client.CommonAPIClient, dockerH } dockerClient = &closeGuardingClient{pimpl: dockerClient} - return dockerClient, dockerHost, err + return dockerClient, dockerHostInRemote, err +} + +// tries to get connection to default podman machine +func tryGetPodmanRemoteConn() (uri string, identity string) { + cmd := exec.Command("podman", "system", "connection", "list", "--format=json") + out, err := cmd.CombinedOutput() + if err != nil { + return "", "" + } + var connections []struct { + Name string + URI string + Identity string + Default bool + } + err = json.Unmarshal(out, &connections) + if err != nil { + return "", "" + } + + for _, c := range connections { + if c.Default { + uri = c.URI + identity = c.Identity + break + } + } + + return uri, identity } func podmanPresent() bool { diff --git a/docker/docker_client_ssh_test.go b/docker/docker_client_ssh_test.go index 7215dd918..0f214f369 100644 --- a/docker/docker_client_ssh_test.go +++ b/docker/docker_client_ssh_test.go @@ -39,12 +39,16 @@ func TestNewDockerClientWithSSH(t *testing.T) { defer WithEnvVar(t, "DOCKER_HOST", fmt.Sprintf("ssh://user:pwd@%s", sshConf.address))() - dockerClient, _, err := docker.NewClient(client.DefaultDockerHost) + dockerClient, dockerHostInRemote, err := docker.NewClient(client.DefaultDockerHost) if err != nil { t.Fatal(err) } defer dockerClient.Close() + if dockerHostInRemote != `unix://`+sshDockerSocket { + t.Errorf("bad remote DOCKER_HOST: expected %q but got %q", `unix://`+sshDockerSocket, dockerHostInRemote) + } + _, err = dockerClient.Ping(ctx) if err != nil { t.Error(err)