Skip to content

Commit

Permalink
docker: remove ping from boot sequence (#6331)
Browse files Browse the repository at this point in the history
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 #5841

Signed-off-by: Nick Santos <[email protected]>
  • Loading branch information
nicks authored Mar 22, 2024
1 parent 998a78a commit 3359b56
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 81 deletions.
7 changes: 6 additions & 1 deletion internal/build/docker_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion internal/cli/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions internal/cli/doctor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
12 changes: 9 additions & 3 deletions internal/controllers/core/cluster/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
}
}

Expand Down
7 changes: 6 additions & 1 deletion internal/controllers/core/tiltfile/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
119 changes: 66 additions & 53 deletions internal/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path/filepath"
"strconv"
"sync"
"time"

"github.com/blang/semver"
"github.com/distribution/reference"
Expand Down Expand Up @@ -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:/ubuntu/microk8s/blob/master/docs/dockerd.md
const microK8sDockerHost = "unix:///var/snap/microk8s/current/docker.sock"
Expand All @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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:/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:/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 }
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/docker/exploding.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/docker/fake_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions internal/docker/switch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 3359b56

Please sign in to comment.