cluster: improve local docker host detection (#142)

Fixes #132

pkg/cluster/admin_docker_desktop.go

@@ -6,18 +6,20 @@ import (
 	"runtime"
 
 	"github.com/tilt-dev/ctlptl/pkg/api"
+	"github.com/tilt-dev/ctlptl/pkg/docker"
 	"github.com/tilt-dev/localregistry-go"
 )
 
 // The DockerDesktop manages the Kubernetes cluster for DockerDesktop.
 // This is a bit different than the other admins, due to the overlap
 //
 type dockerDesktopAdmin struct {
-	os string
+	os   string
+	host string
 }
 
 func newDockerDesktopAdmin() *dockerDesktopAdmin {
-	return &dockerDesktopAdmin{os: runtime.GOOS}
+	return &dockerDesktopAdmin{os: runtime.GOOS, host: docker.GetHostEnv()}
 }
 
 func (a *dockerDesktopAdmin) EnsureInstalled(ctx context.Context) error { return nil }
@@ -26,6 +28,12 @@ func (a *dockerDesktopAdmin) Create(ctx context.Context, desired *api.Cluster, r
 		return fmt.Errorf("ctlptl currently does not support connecting a registry to docker-desktop")
 	}
 
+	isLocalHost := docker.IsLocalHost(a.host)
+	if !isLocalHost {
+		return fmt.Errorf("docker-desktop clusters are only available on a local Docker engine. Current DOCKER_HOST: %s",
+			a.host)
+	}
+
 	if a.os == "darwin" || a.os == "windows" {
 		return nil
 	}
@@ -37,6 +45,10 @@ func (a *dockerDesktopAdmin) LocalRegistryHosting(ctx context.Context, desired *
 }
 
 func (a *dockerDesktopAdmin) Delete(ctx context.Context, config *api.Cluster) error {
+	isLocalHost := docker.IsLocalHost(a.host)
+	if !isLocalHost {
+		return fmt.Errorf("docker-desktop cannot be deleted from a remote DOCKER_HOST: %s", a.host)
+	}
 	if a.os != "darwin" && a.os != "windows" {
 		return fmt.Errorf("docker-desktop delete not implemented on: %s", runtime.GOOS)
 	}

pkg/cluster/cluster.go

@@ -4,19 +4,18 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"os"
 	"sort"
 	"strconv"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/blang/semver/v4"
-	"github.com/docker/docker/client"
 	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	"github.com/tilt-dev/ctlptl/internal/socat"
 	"github.com/tilt-dev/ctlptl/pkg/api"
+	"github.com/tilt-dev/ctlptl/pkg/docker"
 	"github.com/tilt-dev/ctlptl/pkg/registry"
 	"github.com/tilt-dev/localregistry-go"
 	"golang.org/x/sync/errgroup"
@@ -160,12 +159,11 @@ func (c *Controller) getDockerClient(ctx context.Context) (dockerClient, error)
 		return c.dockerClient, nil
 	}
 
-	client, err := client.NewClientWithOpts(client.FromEnv)
+	client, err := newDockerWrapperFromEnv(ctx)
 	if err != nil {
 		return nil, err
 	}
 
-	client.NegotiateAPIVersion(ctx)
 	c.dockerClient = client
 	return client, nil
 }
@@ -238,6 +236,11 @@ func (c *Controller) admin(ctx context.Context, product Product) (Admin, error)
 
 	switch product {
 	case ProductDockerDesktop:
+		if !dockerClient.IsLocalHost() {
+			return nil, fmt.Errorf("Detected remote DOCKER_HOST. Remote Docker engines do not support Docker Desktop clusters: %s",
+				docker.GetHostEnv())
+		}
+
 		admin = newDockerDesktopAdmin()
 	case ProductKIND:
 		admin = newKindAdmin(c.iostreams)
@@ -929,26 +932,10 @@ func (c *Controller) List(ctx context.Context, options ListOptions) (*api.Cluste
 	}, nil
 }
 
-func isLocalDockerHost(dockerHost string) bool {
-	return dockerHost == "" ||
-
-		// Check all the "standard" docker localhosts.
-		// https://github.com/docker/cli/blob/a32cd16160f1b41c1c4ae7bee4dac929d1484e59/opts/hosts.go#L22
-		strings.HasPrefix(dockerHost, "tcp://localhost:") ||
-		strings.HasPrefix(dockerHost, "tcp://127.0.0.1:") ||
-
-		// https://github.com/moby/moby/blob/master/client/client_windows.go#L4
-		strings.HasPrefix(dockerHost, "npipe:") ||
-
-		// https://github.com/moby/moby/blob/master/client/client_unix.go#L6
-		strings.HasPrefix(dockerHost, "unix:")
-}
-
 // If the current cluster is on a remote docker instance,
 // we need a port-forwarder to connect it.
 func (c *Controller) maybeCreateForwarderForCurrentCluster(ctx context.Context) error {
-	dockerHost := os.Getenv("DOCKER_HOST")
-	if isLocalDockerHost(dockerHost) {
+	if docker.IsLocalHost(docker.GetHostEnv()) {
 		return nil
 	}
 

pkg/cluster/cluster_test.go

@@ -315,29 +315,6 @@ func TestClusterApplyKindConfig(t *testing.T) {
 	assert.Contains(t, f.errOut.String(), "desired Kind config does not match current")
 }
 
-type dockerHostTestCase struct {
-	host     string
-	expected bool
-}
-
-func TestIsLocalDockerHost(t *testing.T) {
-	cases := []dockerHostTestCase{
-		dockerHostTestCase{"", true},
-		dockerHostTestCase{"tcp://localhost:2375", true},
-		dockerHostTestCase{"tcp://127.0.0.1:2375", true},
-		dockerHostTestCase{"npipe:////./pipe/docker_engine", true},
-		dockerHostTestCase{"unix:///var/run/docker.sock", true},
-		dockerHostTestCase{"tcp://cluster:2375", false},
-		dockerHostTestCase{"http://cluster:2375", false},
-	}
-	for i, c := range cases {
-		c := c
-		t.Run(fmt.Sprintf("%s-%d", t.Name(), i), func(t *testing.T) {
-			assert.Equal(t, c.expected, isLocalDockerHost(c.host))
-		})
-	}
-}
-
 type fixture struct {
 	t            *testing.T
 	errOut       *bytes.Buffer
@@ -439,8 +416,13 @@ func newFakeController(t *testing.T) *Controller {
 }
 
 type fakeDockerClient struct {
-	started bool
-	ncpu    int
+	isRemoteHost bool
+	started      bool
+	ncpu         int
+}
+
+func (c *fakeDockerClient) IsLocalHost() bool {
+	return !c.isRemoteHost
 }
 
 func (c *fakeDockerClient) ServerVersion(ctx context.Context) (types.Version, error) {

pkg/cluster/docker.go

@@ -4,11 +4,35 @@ import (
 	"context"
 
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/client"
+	"github.com/tilt-dev/ctlptl/pkg/docker"
 )
 
 type dockerClient interface {
+	IsLocalHost() bool
 	ServerVersion(ctx context.Context) (types.Version, error)
 	Info(ctx context.Context) (types.Info, error)
 	ContainerInspect(ctx context.Context, containerID string) (types.ContainerJSON, error)
 	ContainerRemove(ctx context.Context, id string, options types.ContainerRemoveOptions) error
 }
+
+type dockerWrapper struct {
+	*client.Client
+	isLocalHost bool
+}
+
+func (w *dockerWrapper) IsLocalHost() bool { return w.isLocalHost }
+
+func newDockerWrapperFromEnv(ctx context.Context) (*dockerWrapper, error) {
+	c, err := client.NewClientWithOpts(client.FromEnv)
+	if err != nil {
+		return nil, err
+	}
+
+	c.NegotiateAPIVersion(ctx)
+	isLocalHost := docker.IsLocalHost(docker.GetHostEnv())
+	return &dockerWrapper{
+		Client:      c,
+		isLocalHost: isLocalHost,
+	}, nil
+}

pkg/cluster/machine.go

@@ -13,6 +13,7 @@ import (
 	"github.com/mitchellh/go-homedir"
 	"github.com/pkg/errors"
 	"github.com/tilt-dev/ctlptl/pkg/api"
+	"github.com/tilt-dev/ctlptl/pkg/docker"
 	"k8s.io/apimachinery/pkg/util/duration"
 	"k8s.io/apimachinery/pkg/util/wait"
 	klog "k8s.io/klog/v2"
@@ -88,6 +89,10 @@ func (m dockerMachine) EnsureExists(ctx context.Context) error {
 		return nil
 	}
 
+	if !m.dockerClient.IsLocalHost() {
+		return fmt.Errorf("Detected remote DOCKER_HOST, but no Docker running: %s", docker.GetHostEnv())
+	}
+
 	klog.V(2).Infoln("No Docker daemon running. Attempting to start Docker.")
 	if m.os == "darwin" || m.os == "windows" {
 		err := m.d4m.Open(ctx)
@@ -113,14 +118,21 @@ func (m dockerMachine) EnsureExists(ctx context.Context) error {
 }
 
 func (m dockerMachine) Restart(ctx context.Context, desired, existing *api.Cluster) error {
-	canChangeCPUs :=
-		m.os == "darwin" || m.os == "windows" || // DockerForMac and DockerForWindows can change the CPU on the VM
-			Product(desired.Product) == ProductMinikube // Minikube can change the CPU on the VM or on the container itself
+	canChangeCPUs := false
+	isLocalDockerDesktop := false
+	if m.dockerClient.IsLocalHost() && (m.os == "darwin" || m.os == "windows") {
+		canChangeCPUs = true // DockerForMac and DockerForWindows can change the CPU on the VM
+		isLocalDockerDesktop = true
+	} else if Product(desired.Product) == ProductMinikube {
+		// Minikube can change the CPU on the VM or on the container itself
+		canChangeCPUs = true
+	}
+
 	if existing.Status.CPUs < desired.MinCPUs && !canChangeCPUs {
 		return fmt.Errorf("Cannot automatically set minimum CPU to %d on this platform", desired.MinCPUs)
 	}
 
-	if m.os == "darwin" || m.os == "windows" {
+	if isLocalDockerDesktop {
 		settings, err := m.d4m.settings(ctx)
 		if err != nil {
 			return err

pkg/docker/docker.go

@@ -0,0 +1,25 @@
+package docker
+
+import (
+	"os"
+	"strings"
+)
+
+func GetHostEnv() string {
+	return os.Getenv("DOCKER_HOST")
+}
+
+func IsLocalHost(dockerHost string) bool {
+	return dockerHost == "" ||
+
+		// Check all the "standard" docker localhosts.
+		// https://github.com/docker/cli/blob/a32cd16160f1b41c1c4ae7bee4dac929d1484e59/opts/hosts.go#L22
+		strings.HasPrefix(dockerHost, "tcp://localhost:") ||
+		strings.HasPrefix(dockerHost, "tcp://127.0.0.1:") ||
+
+		// https://github.com/moby/moby/blob/master/client/client_windows.go#L4
+		strings.HasPrefix(dockerHost, "npipe:") ||
+
+		// https://github.com/moby/moby/blob/master/client/client_unix.go#L6
+		strings.HasPrefix(dockerHost, "unix:")
+}

pkg/docker/docker_test.go

@@ -0,0 +1,31 @@
+package docker
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+type dockerHostTestCase struct {
+	host     string
+	expected bool
+}
+
+func TestIsLocalDockerHost(t *testing.T) {
+	cases := []dockerHostTestCase{
+		dockerHostTestCase{"", true},
+		dockerHostTestCase{"tcp://localhost:2375", true},
+		dockerHostTestCase{"tcp://127.0.0.1:2375", true},
+		dockerHostTestCase{"npipe:////./pipe/docker_engine", true},
+		dockerHostTestCase{"unix:///var/run/docker.sock", true},
+		dockerHostTestCase{"tcp://cluster:2375", false},
+		dockerHostTestCase{"http://cluster:2375", false},
+	}
+	for i, c := range cases {
+		c := c
+		t.Run(fmt.Sprintf("%s-%d", t.Name(), i), func(t *testing.T) {
+			assert.Equal(t, c.expected, IsLocalHost(c.host))
+		})
+	}
+}

pkg/registry/registry.go

@@ -3,7 +3,6 @@ package registry
 import (
 	"context"
 	"fmt"
-	"os"
 	"sort"
 	"strings"
 	"time"
@@ -15,6 +14,7 @@ import (
 	"github.com/tilt-dev/ctlptl/internal/exec"
 	"github.com/tilt-dev/ctlptl/internal/socat"
 	"github.com/tilt-dev/ctlptl/pkg/api"
+	"github.com/tilt-dev/ctlptl/pkg/docker"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
@@ -238,8 +238,7 @@ func (c *Controller) Apply(ctx context.Context, desired *api.Registry) (*api.Reg
 }
 
 func (c *Controller) maybeCreateForwarder(ctx context.Context, port int) error {
-	dockerHost := os.Getenv("DOCKER_HOST")
-	if dockerHost == "" {
+	if docker.IsLocalHost(docker.GetHostEnv()) {
 		return nil
 	}