From fb733c9919dc7ffe947b52bafd5eed835940bdd4 Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Mon, 19 Feb 2024 00:06:32 -0500 Subject: [PATCH] docker: remove ping from boot sequence this ensures that if the Docker daemon is dead or not responding, it doesn't block tiltfile execution. now that we have reconcilers that are checking the cluster status, i think this is safer than it used to be. fixes https://github.com/tilt-dev/tilt/issues/5841 Signed-off-by: Nick Santos --- internal/build/docker_builder.go | 7 +- internal/cli/docker.go | 5 +- internal/cli/doctor.go | 20 +-- .../controllers/core/cluster/reconciler.go | 12 +- .../controllers/core/tiltfile/reconciler.go | 7 +- internal/docker/client.go | 119 ++++++++++-------- internal/docker/exploding.go | 8 +- internal/docker/fake_client.go | 8 +- internal/docker/switch.go | 8 +- 9 files changed, 113 insertions(+), 81 deletions(-) diff --git a/internal/build/docker_builder.go b/internal/build/docker_builder.go index a1aba9c524..c878e5c717 100644 --- a/internal/build/docker_builder.go +++ b/internal/build/docker_builder.go @@ -232,8 +232,13 @@ func (d *DockerBuilder) buildToDigest(ctx context.Context, spec v1alpha1.DockerI return "", nil, fmt.Errorf("reading build context: %v", err) } + builderVersion, err := d.dCli.BuilderVersion(ctx) + if err != nil { + return "", nil, err + } + // Buildkit allows us to use a fs sync server instead of uploading up-front. - useFSSync := allowBuildkit && d.dCli.BuilderVersion() == types.BuilderBuildKit + useFSSync := allowBuildkit && builderVersion == types.BuilderBuildKit if !useFSSync { pipeReader, pipeWriter := io.Pipe() w := NewProgressWriter(ctx, pipeWriter) diff --git a/internal/cli/docker.go b/internal/cli/docker.go index 0ed44d66fb..7d9387c3d6 100644 --- a/internal/cli/docker.go +++ b/internal/cli/docker.go @@ -46,7 +46,10 @@ func (c *dockerCmd) run(ctx context.Context, args []string) error { } dockerEnv := client.Env() - builder := client.BuilderVersion() + builder, err := client.BuilderVersion(ctx) + if err != nil { + return errors.Wrap(err, "Failed to get Docker builder") + } buildkitEnv := "DOCKER_BUILDKIT=0" if builder == types.BuilderBuildKit { diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index c9f94f5e7e..29a0a57bee 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -71,12 +71,12 @@ func (c *doctorCmd) run(ctx context.Context, args []string) error { } printField("Host", host, nil) - version := clusterDocker.ServerVersion() - printField("Server Version", version.Version, nil) - printField("API Version", version.APIVersion, nil) + version, err := clusterDocker.ServerVersion(ctx) + printField("Server Version", version.Version, err) + printField("API Version", version.APIVersion, err) - builderVersion := clusterDocker.BuilderVersion() - printField("Builder", builderVersion, nil) + builderVersion, err := clusterDocker.BuilderVersion(ctx) + printField("Builder", builderVersion, err) } if multipleClients { @@ -93,12 +93,12 @@ func (c *doctorCmd) run(ctx context.Context, args []string) error { } printField("Host", host, nil) - version := localDocker.ServerVersion() - printField("Server Version", version.Version, nil) - printField("Version", version.APIVersion, nil) + version, err := localDocker.ServerVersion(ctx) + printField("Server Version", version.Version, err) + printField("Version", version.APIVersion, err) - builderVersion := localDocker.BuilderVersion() - printField("Builder", builderVersion, nil) + builderVersion, err := localDocker.BuilderVersion(ctx) + printField("Builder", builderVersion, err) } } diff --git a/internal/controllers/core/cluster/reconciler.go b/internal/controllers/core/cluster/reconciler.go index 362f37fadf..84bf2c07cb 100644 --- a/internal/controllers/core/cluster/reconciler.go +++ b/internal/controllers/core/cluster/reconciler.go @@ -262,7 +262,11 @@ func (r *Reconciler) readKubernetesArch(ctx context.Context, client k8s.Client) // Reads the arch from a Docker cluster, or "unknown" if we can't // figure out the architecture. func (r *Reconciler) readDockerArch(ctx context.Context, client docker.Client) string { - arch := client.ServerVersion().Arch + serverVersion, err := client.ServerVersion(ctx) + if err != nil { + return ArchUnknown + } + arch := serverVersion.Arch if arch == "" { return ArchUnknown } @@ -432,8 +436,10 @@ func (r *Reconciler) populateDockerMetadata(ctx context.Context, conn *connectio } if conn.serverVersion == "" { - versionInfo := conn.dockerClient.ServerVersion() - conn.serverVersion = versionInfo.Version + versionInfo, err := conn.dockerClient.ServerVersion(ctx) + if err == nil { + conn.serverVersion = versionInfo.Version + } } } diff --git a/internal/controllers/core/tiltfile/reconciler.go b/internal/controllers/core/tiltfile/reconciler.go index 860acb90b1..515e38ec77 100644 --- a/internal/controllers/core/tiltfile/reconciler.go +++ b/internal/controllers/core/tiltfile/reconciler.go @@ -6,6 +6,7 @@ import ( "sync" "time" + dockertypes "github.com/docker/docker/api/types" "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -330,10 +331,14 @@ func (r *Reconciler) run(ctx context.Context, nn types.NamespacedName, tf *v1alp if requiresDocker(tlr) { dockerErr := r.dockerClient.CheckConnected() + var serverVersion dockertypes.Version + if dockerErr == nil { + serverVersion, dockerErr = r.dockerClient.ServerVersion(ctx) + } if tlr.Error == nil && dockerErr != nil { tlr.Error = errors.Wrap(dockerErr, "Failed to connect to Docker") } - r.reportDockerConnectionEvent(ctx, dockerErr == nil, r.dockerClient.ServerVersion()) + r.reportDockerConnectionEvent(ctx, dockerErr == nil, serverVersion) } if ctx.Err() == context.Canceled { diff --git a/internal/docker/client.go b/internal/docker/client.go index 57f9a91595..5d54adb21c 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strconv" "sync" + "time" "github.com/blang/semver" "github.com/distribution/reference" @@ -64,6 +65,8 @@ var minDockerVersion = semver.MustParse("1.23.0") var minDockerVersionStableBuildkit = semver.MustParse("1.39.0") var minDockerVersionExperimentalBuildkit = semver.MustParse("1.38.0") +var versionTimeout = 5 * time.Second + // microk8s exposes its own docker socket // https://github.com/ubuntu/microk8s/blob/master/docs/dockerd.md const microK8sDockerHost = "unix:///var/snap/microk8s/current/docker.sock" @@ -78,9 +81,9 @@ type Client interface { // If you'd like to call this Docker instance in a separate process, this // is the default builder version you want (buildkit or legacy) - BuilderVersion() types.BuilderVersion + BuilderVersion(ctx context.Context) (types.BuilderVersion, error) - ServerVersion() types.Version + ServerVersion(ctx context.Context) (types.Version, error) // Set the orchestrator we're talking to. This is only relevant to switchClient, // which can talk to either the Local or in-cluster docker daemon. @@ -141,12 +144,14 @@ var _ Client = &Cli{} type Cli struct { *client.Client - builderVersion types.BuilderVersion - serverVersion types.Version - authConfigs map[string]types.AuthConfig - authConfigsOnce sync.Once + authConfigsOnce func() map[string]types.AuthConfig env Env + + versionsOnce sync.Once + builderVersion types.BuilderVersion + serverVersion types.Version + versionError error } func NewDockerClient(ctx context.Context, env Env) Client { @@ -155,34 +160,12 @@ func NewDockerClient(ctx context.Context, env Env) Client { } d := env.Client.(*client.Client) - serverVersion, err := d.ServerVersion(ctx) - if err != nil { - return newExplodingClient(err) - } - - if !SupportedVersion(serverVersion) { - return newExplodingClient( - fmt.Errorf("Tilt requires a Docker server newer than %s. Current Docker server: %s", - minDockerVersion, serverVersion.APIVersion)) - } - builderVersion, err := getDockerBuilderVersion(serverVersion, env) - if err != nil { - return newExplodingClient(err) + return &Cli{ + Client: d, + env: env, + authConfigsOnce: sync.OnceValue(authConfigs), } - - cli := &Cli{ - Client: d, - env: env, - builderVersion: builderVersion, - serverVersion: serverVersion, - } - - if builderVersion == types.BuilderV1 { - go cli.initAuthConfigs(ctx) - } - - return cli } func SupportedVersion(v types.Version) bool { @@ -293,6 +276,34 @@ func CreateClientOpts(envMap map[string]string) ([]client.Opt, error) { return result, nil } +func (c *Cli) initVersion(ctx context.Context) { + c.versionsOnce.Do(func() { + ctx, cancel := context.WithTimeout(ctx, versionTimeout) + defer cancel() + + serverVersion, err := c.Client.ServerVersion(ctx) + if err != nil { + c.versionError = err + return + } + + if !SupportedVersion(serverVersion) { + c.versionError = fmt.Errorf("Tilt requires a Docker server newer than %s. Current Docker server: %s", + minDockerVersion, serverVersion.APIVersion) + return + } + + builderVersion, err := getDockerBuilderVersion(serverVersion, c.env) + if err != nil { + c.versionError = err + return + } + + c.builderVersion = builderVersion + c.serverVersion = serverVersion + }) +} + func (c *Cli) startBuildkitSession(ctx context.Context, g *errgroup.Group, key string, dirSource filesync.DirSource, sshSpecs []string, secretSpecs []string) (*session.Session, error) { session, err := session.NewSession(ctx, "tilt", key) if err != nil { @@ -352,20 +363,18 @@ func (c *Cli) startBuildkitSession(ctx context.Context, g *errgroup.Group, key s // Protocol (1) is very slow. If you're using the gcloud credential store, // fetching all the creds ahead of time can take ~3 seconds. // Protocol (2) is more efficient, but also more complex to manage. We manage it lazily. -func (c *Cli) initAuthConfigs(ctx context.Context) { - c.authConfigsOnce.Do(func() { - configFile := config.LoadDefaultConfigFile(io.Discard) - - // If we fail to get credentials for some reason, that's OK. - // even the docker CLI ignores this: - // https://github.com/docker/cli/blob/23446275646041f9b598d64c51be24d5d0e49376/cli/command/image/build.go#L386 - credentials, _ := configFile.GetAllCredentials() - authConfigs := make(map[string]types.AuthConfig, len(credentials)) - for k, auth := range credentials { - authConfigs[k] = types.AuthConfig(auth) - } - c.authConfigs = authConfigs - }) +func authConfigs() map[string]types.AuthConfig { + configFile := config.LoadDefaultConfigFile(io.Discard) + + // If we fail to get credentials for some reason, that's OK. + // even the docker CLI ignores this: + // https://github.com/docker/cli/blob/23446275646041f9b598d64c51be24d5d0e49376/cli/command/image/build.go#L386 + credentials, _ := configFile.GetAllCredentials() + authConfigs := make(map[string]types.AuthConfig, len(credentials)) + for k, auth := range credentials { + authConfigs[k] = types.AuthConfig(auth) + } + return authConfigs } func (c *Cli) CheckConnected() error { return nil } @@ -377,12 +386,14 @@ func (c *Cli) Env() Env { return c.env } -func (c *Cli) BuilderVersion() types.BuilderVersion { - return c.builderVersion +func (c *Cli) BuilderVersion(ctx context.Context) (types.BuilderVersion, error) { + c.initVersion(ctx) + return c.builderVersion, c.versionError } -func (c *Cli) ServerVersion() types.Version { - return c.serverVersion +func (c *Cli) ServerVersion(ctx context.Context) (types.Version, error) { + c.initVersion(ctx) + return c.serverVersion, c.versionError } type encodedAuth string @@ -495,7 +506,10 @@ func (c *Cli) ImageBuild(ctx context.Context, g *errgroup.Group, buildContext io sessionID := "" mustUseBuildkit := len(options.SSHSpecs) > 0 || len(options.SecretSpecs) > 0 || options.DirSource != nil - builderVersion := c.builderVersion + builderVersion, err := c.BuilderVersion(ctx) + if err != nil { + return types.ImageBuildResponse{}, err + } if options.ForceLegacyBuilder { builderVersion = types.BuilderV1 } @@ -519,8 +533,7 @@ func (c *Cli) ImageBuild(ctx context.Context, g *errgroup.Group, buildContext io if isUsingBuildkit { opts.SessionID = sessionID } else { - c.initAuthConfigs(ctx) - opts.AuthConfigs = c.authConfigs + opts.AuthConfigs = c.authConfigsOnce() } opts.Remove = options.Remove diff --git a/internal/docker/exploding.go b/internal/docker/exploding.go index 7bb3aa8f5b..a85a690008 100644 --- a/internal/docker/exploding.go +++ b/internal/docker/exploding.go @@ -36,11 +36,11 @@ func (c explodingClient) CheckConnected() error { func (c explodingClient) Env() Env { return Env{} } -func (c explodingClient) BuilderVersion() types.BuilderVersion { - return types.BuilderVersion("") +func (c explodingClient) BuilderVersion(ctx context.Context) (types.BuilderVersion, error) { + return types.BuilderV1, c.err } -func (c explodingClient) ServerVersion() types.Version { - return types.Version{} +func (c explodingClient) ServerVersion(ctx context.Context) (types.Version, error) { + return types.Version{}, c.err } func (c explodingClient) ContainerLogs(ctx context.Context, containerID string, options types.ContainerLogsOptions) (io.ReadCloser, error) { return nil, c.err diff --git a/internal/docker/fake_client.go b/internal/docker/fake_client.go index 41f9dcd1f9..f7d7653df0 100644 --- a/internal/docker/fake_client.go +++ b/internal/docker/fake_client.go @@ -167,14 +167,14 @@ func (c *FakeClient) CheckConnected() error { func (c *FakeClient) Env() Env { return c.FakeEnv } -func (c *FakeClient) BuilderVersion() types.BuilderVersion { - return types.BuilderV1 +func (c *FakeClient) BuilderVersion(ctx context.Context) (types.BuilderVersion, error) { + return types.BuilderV1, nil } -func (c *FakeClient) ServerVersion() types.Version { +func (c *FakeClient) ServerVersion(ctx context.Context) (types.Version, error) { return types.Version{ Arch: "amd64", Version: "20.10.11", - } + }, nil } func (c *FakeClient) SetExecError(err error) { diff --git a/internal/docker/switch.go b/internal/docker/switch.go index 31799c5000..c7da4ed3a0 100644 --- a/internal/docker/switch.go +++ b/internal/docker/switch.go @@ -71,11 +71,11 @@ func (c *switchCli) CheckConnected() error { func (c *switchCli) Env() Env { return c.client(context.Background()).Env() } -func (c *switchCli) BuilderVersion() types.BuilderVersion { - return c.client(context.Background()).BuilderVersion() +func (c *switchCli) BuilderVersion(ctx context.Context) (types.BuilderVersion, error) { + return c.client(ctx).BuilderVersion(ctx) } -func (c *switchCli) ServerVersion() types.Version { - return c.client(context.Background()).ServerVersion() +func (c *switchCli) ServerVersion(ctx context.Context) (types.Version, error) { + return c.client(ctx).ServerVersion(ctx) } func (c *switchCli) ContainerLogs(ctx context.Context, containerID string, options types.ContainerLogsOptions) (io.ReadCloser, error) { return c.client(ctx).ContainerLogs(ctx, containerID, options)