docker: change how we model "image builds show up in the cluster imme…

…diately" (#4598) * docker: change how we model "image builds show up in the cluster immediately" We used to treat this as a property of the cluster type + the container runtime. But this made it impossible to support clusters that sometimes use your docker runtime, and sometimes do not. For examples, see: - #4587 - #3654 - #1729 - #4544 This changes the data model so that "image builds show up in the cluster" is a property of the Docker client, not of the cluster. This should be much more flexible and correct, and help us support multiple clusters. Fixes #4544 * Update internal/docker/env.go Co-authored-by: Maia McCormick <maia@windmill.engineering> Co-authored-by: Maia McCormick <maia@windmill.engineering>
tilt-dev · Jun 7, 2021 · bc91f22 · bc91f22
1 parent 1fc575b
commit bc91f22
Show file tree

Hide file tree

Showing 22 changed files with 321 additions and 193 deletions.
diff --git a/internal/build/docker_builder.go b/internal/build/docker_builder.go
@@ -17,6 +17,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/tilt-dev/tilt/internal/container"
+	"github.com/tilt-dev/tilt/internal/k8s"
 
 	"github.com/tilt-dev/tilt/internal/docker"
 	"github.com/tilt-dev/tilt/internal/dockerfile"
@@ -35,6 +36,9 @@ type dockerImageBuilder struct {
 }
 
 type DockerBuilder interface {
+	// Returns whether this docker builder is going to build to the given kubernetes context.
+	WillBuildToKubeContext(kctx k8s.KubeContext) bool
+
 	BuildImage(ctx context.Context, ps *PipelineState, refs container.RefSet, db model.DockerBuild, filter model.PathMatcher) (container.TaggedRefs, error)
 	DumpImageDeployRef(ctx context.Context, ref string) (reference.NamedTagged, error)
 	PushImage(ctx context.Context, name reference.NamedTagged) error
@@ -55,6 +59,10 @@ func NewDockerImageBuilder(dCli docker.Client, extraLabels dockerfile.Labels) *d
 	}
 }
 
+func (d *dockerImageBuilder) WillBuildToKubeContext(kctx k8s.KubeContext) bool {
+	return d.dCli.Env().WillBuildToKubeContext(kctx)
+}
+
 func (d *dockerImageBuilder) BuildImage(ctx context.Context, ps *PipelineState, refs container.RefSet, db model.DockerBuild, filter model.PathMatcher) (container.TaggedRefs, error) {
 	paths := []PathMapping{
 		{

diff --git a/internal/build/test_utils.go b/internal/build/test_utils.go
@@ -48,7 +48,7 @@ func newDockerBuildFixture(t testing.TB) *dockerBuildFixture {
 	ctx, _, _ := testutils.CtxAndAnalyticsForTest()
 	env := k8s.EnvGKE
 
-	dEnv := docker.ProvideClusterEnv(ctx, env, wmcontainer.RuntimeDocker, k8s.FakeMinikube{})
+	dEnv := docker.ProvideClusterEnv(ctx, "gke", env, wmcontainer.RuntimeDocker, k8s.FakeMinikube{})
 	dCli := docker.NewDockerClient(ctx, docker.Env(dEnv))
 	_, ok := dCli.(*docker.Cli)
 	// If it wasn't an actual Docker client, it's an exploding client

diff --git a/internal/cli/wire_gen.go b/internal/cli/wire_gen.go
diff --git a/internal/containerupdate/docker_container_updater.go b/internal/containerupdate/docker_container_updater.go
@@ -11,6 +11,7 @@ import (
 	"github.com/tilt-dev/tilt/internal/build"
 	"github.com/tilt-dev/tilt/internal/container"
 	"github.com/tilt-dev/tilt/internal/docker"
+	"github.com/tilt-dev/tilt/internal/k8s"
 	"github.com/tilt-dev/tilt/internal/store"
 	"github.com/tilt-dev/tilt/pkg/logger"
 	"github.com/tilt-dev/tilt/pkg/model"
@@ -26,6 +27,10 @@ func NewDockerUpdater(dCli docker.Client) *DockerUpdater {
 	return &DockerUpdater{dCli: dCli}
 }
 
+func (cu *DockerUpdater) WillBuildToKubeContext(kctx k8s.KubeContext) bool {
+	return cu.dCli.Env().WillBuildToKubeContext(kctx)
+}
+
 func (cu *DockerUpdater) UpdateContainer(ctx context.Context, cInfo store.ContainerInfo,
 	archiveToCopy io.Reader, filesToDelete []string, cmds []model.Cmd, hotReload bool) error {
 	l := logger.Get(ctx)

diff --git a/internal/docker/client_test.go b/internal/docker/client_test.go
@@ -132,10 +132,13 @@ func TestProvideEnv(t *testing.T) {
 			},
 		},
 		{
-			env:             k8s.EnvMicroK8s,
-			runtime:         container.RuntimeDocker,
-			expectedCluster: Env{Host: microK8sDockerHost},
-			expectedLocal:   Env{},
+			env:     k8s.EnvMicroK8s,
+			runtime: container.RuntimeDocker,
+			expectedCluster: Env{
+				Host:                microK8sDockerHost,
+				BuildToKubeContexts: []string{"microk8s-me"},
+			},
+			expectedLocal: Env{},
 		},
 		{
 			env:     k8s.EnvMicroK8s,
@@ -151,11 +154,12 @@ func TestProvideEnv(t *testing.T) {
 				"DOCKER_API_VERSION": "1.35",
 			},
 			expectedCluster: Env{
-				TLSVerify:     "1",
-				Host:          "tcp://192.168.99.100:2376",
-				CertPath:      "/home/nick/.minikube/certs",
-				APIVersion:    "1.35",
-				IsOldMinikube: true,
+				TLSVerify:           "1",
+				Host:                "tcp://192.168.99.100:2376",
+				CertPath:            "/home/nick/.minikube/certs",
+				APIVersion:          "1.35",
+				IsOldMinikube:       true,
+				BuildToKubeContexts: []string{"minikube-me"},
 			},
 		},
 		{
@@ -169,10 +173,11 @@ func TestProvideEnv(t *testing.T) {
 				"DOCKER_API_VERSION": "1.35",
 			},
 			expectedCluster: Env{
-				TLSVerify:  "1",
-				Host:       "tcp://192.168.99.100:2376",
-				CertPath:   "/home/nick/.minikube/certs",
-				APIVersion: "1.35",
+				TLSVerify:           "1",
+				Host:                "tcp://192.168.99.100:2376",
+				CertPath:            "/home/nick/.minikube/certs",
+				APIVersion:          "1.35",
+				BuildToKubeContexts: []string{"minikube-me"},
 			},
 		},
 		{
@@ -210,16 +215,18 @@ func TestProvideEnv(t *testing.T) {
 				"DOCKER_CERT_PATH":  "/home/nick/.minikube/certs",
 			},
 			expectedCluster: Env{
-				TLSVerify:     "1",
-				Host:          "tcp://192.168.99.100:2376",
-				CertPath:      "/home/nick/.minikube/certs",
-				IsOldMinikube: true,
+				TLSVerify:           "1",
+				Host:                "tcp://192.168.99.100:2376",
+				CertPath:            "/home/nick/.minikube/certs",
+				IsOldMinikube:       true,
+				BuildToKubeContexts: []string{"minikube-me"},
 			},
 			expectedLocal: Env{
-				TLSVerify:     "1",
-				Host:          "tcp://192.168.99.100:2376",
-				CertPath:      "/home/nick/.minikube/certs",
-				IsOldMinikube: true,
+				TLSVerify:           "1",
+				Host:                "tcp://192.168.99.100:2376",
+				CertPath:            "/home/nick/.minikube/certs",
+				IsOldMinikube:       true,
+				BuildToKubeContexts: []string{"minikube-me"},
 			},
 		},
 		{
@@ -275,10 +282,11 @@ func TestProvideEnv(t *testing.T) {
 			}
 
 			mkClient := k8s.FakeMinikube{DockerEnvMap: c.mkEnv, FakeVersion: minikubeV}
-			cluster := ProvideClusterEnv(context.Background(), c.env, c.runtime, mkClient)
+			kubeContext := k8s.KubeContext(fmt.Sprintf("%s-me", c.env))
+			cluster := ProvideClusterEnv(context.Background(), kubeContext, c.env, c.runtime, mkClient)
 			assert.Equal(t, c.expectedCluster, Env(cluster))
 
-			local := ProvideLocalEnv(context.Background(), cluster)
+			local := ProvideLocalEnv(context.Background(), kubeContext, c.env, cluster)
 			assert.Equal(t, c.expectedLocal, Env(local))
 		})
 	}

diff --git a/internal/docker/clients.go b/internal/docker/clients.go
@@ -2,6 +2,8 @@ package docker
 
 import (
 	"context"
+
+	"github.com/google/go-cmp/cmp"
 )
 
 type LocalClient Client
@@ -16,7 +18,7 @@ func ProvideClusterCli(ctx context.Context, lEnv LocalEnv, cEnv ClusterEnv, lCli
 	// If the Cluster Env and the LocalEnv are the same, we can re-use the cluster
 	// client as a local client.
 	var cClient ClusterClient
-	if Env(lEnv) == Env(cEnv) {
+	if cmp.Equal(Env(lEnv), Env(cEnv)) {
 		cClient = ClusterClient(lClient)
 	} else {
 		cClient = NewDockerClient(ctx, Env(cEnv))