diff --git a/cli/docker.go b/cli/docker.go index 9ac872c..1573612 100644 --- a/cli/docker.go +++ b/cli/docker.go @@ -7,11 +7,15 @@ import ( "io" "net/url" "os" + "os/exec" + "os/signal" "path" "path/filepath" "sort" "strconv" "strings" + "syscall" + "time" "github.com/docker/docker/api/types/container" "github.com/google/go-containerregistry/pkg/name" @@ -157,11 +161,24 @@ func dockerCmd() *cobra.Command { Short: "Create a docker-based CVM", RunE: func(cmd *cobra.Command, args []string) (err error) { var ( - ctx = cmd.Context() - log = slog.Make(slogjson.Sink(cmd.ErrOrStderr()), slogkubeterminate.Make()).Leveled(slog.LevelDebug) - blog buildlog.Logger = buildlog.JSONLogger{Encoder: json.NewEncoder(os.Stderr)} + ctx, cancel = context.WithCancel(cmd.Context()) //nolint + log = slog.Make(slogjson.Sink(cmd.ErrOrStderr()), slogkubeterminate.Make()).Leveled(slog.LevelDebug) + blog buildlog.Logger = buildlog.JSONLogger{Encoder: json.NewEncoder(os.Stderr)} ) + // We technically leak a context here, but it's impact is negligible. + signalCtx, signalCancel := context.WithCancel(ctx) + sigs := make(chan os.Signal, 1) + signal.Notify(sigs, syscall.SIGTERM, syscall.SIGINT, syscall.SIGWINCH) + + // Spawn a goroutine to wait for a signal. + go func() { + defer signalCancel() + log.Info(ctx, "waiting for signal") + <-sigs + log.Info(ctx, "got signal, canceling context") + }() + if flags.noStartupLogs { log = slog.Make(slogjson.Sink(io.Discard)) blog = buildlog.NopLogger{} @@ -169,6 +186,7 @@ func dockerCmd() *cobra.Command { httpClient, err := xhttp.Client(log, flags.extraCertsPath) if err != nil { + //nolint return xerrors.Errorf("http client: %w", err) } @@ -208,13 +226,19 @@ func dockerCmd() *cobra.Command { // Start sysbox-mgr and sysbox-fs in order to run // sysbox containers. case err := <-background.New(ctx, log, "sysbox-mgr", sysboxArgs...).Run(): - blog.Info(sysboxErrMsg) - //nolint - log.Fatal(ctx, "sysbox-mgr exited", slog.Error(err)) + if ctx.Err() == nil { + blog.Info(sysboxErrMsg) + //nolint + log.Critical(ctx, "sysbox-mgr exited", slog.Error(err)) + panic(err) + } case err := <-background.New(ctx, log, "sysbox-fs").Run(): - blog.Info(sysboxErrMsg) - //nolint - log.Fatal(ctx, "sysbox-fs exited", slog.Error(err)) + if ctx.Err() == nil { + blog.Info(sysboxErrMsg) + //nolint + log.Critical(ctx, "sysbox-fs exited", slog.Error(err)) + panic(err) + } } }() @@ -316,7 +340,7 @@ func dockerCmd() *cobra.Command { ) } - err = runDockerCVM(ctx, log, client, blog, flags) + bootstrapExecID, err := runDockerCVM(ctx, log, client, blog, flags) if err != nil { // It's possible we failed because we ran out of disk while // pulling the image. We should restart the daemon and use @@ -345,7 +369,7 @@ func dockerCmd() *cobra.Command { }() log.Debug(ctx, "reattempting container creation") - err = runDockerCVM(ctx, log, client, blog, flags) + bootstrapExecID, err = runDockerCVM(ctx, log, client, blog, flags) } if err != nil { blog.Errorf("Failed to run envbox: %v", err) @@ -353,6 +377,44 @@ func dockerCmd() *cobra.Command { } } + go func() { + defer cancel() + + <-signalCtx.Done() + log.Debug(ctx, "ctx canceled, forwarding signal to inner container") + + if bootstrapExecID == "" { + log.Debug(ctx, "no bootstrap exec id, skipping") + return + } + + shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), time.Second*90) + defer shutdownCancel() + + bootstrapPID, err := dockerutil.GetExecPID(shutdownCtx, client, bootstrapExecID) + if err != nil { + log.Error(shutdownCtx, "get exec pid", slog.Error(err)) + } + + log.Debug(shutdownCtx, "killing container", slog.F("bootstrap_pid", bootstrapPID)) + + // The PID returned is the PID _outside_ the container... + out, err := exec.CommandContext(shutdownCtx, "kill", "-TERM", strconv.Itoa(bootstrapPID)).CombinedOutput() //nolint:gosec + if err != nil { + log.Error(shutdownCtx, "kill bootstrap process", slog.Error(err), slog.F("output", string(out))) + return + } + + log.Debug(shutdownCtx, "sent kill signal waiting for process to exit") + err = dockerutil.WaitForExit(shutdownCtx, client, bootstrapExecID) + if err != nil { + log.Error(shutdownCtx, "wait for exit", slog.Error(err)) + return + } + + log.Debug(shutdownCtx, "bootstrap process successfully exited") + }() + return nil }, } @@ -390,25 +452,22 @@ func dockerCmd() *cobra.Command { return cmd } -func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client, blog buildlog.Logger, flags flags) error { +func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client, blog buildlog.Logger, flags flags) (string, error) { fs := xunix.GetFS(ctx) - - // Set our OOM score to something really unfavorable to avoid getting killed - // in memory-scarce scenarios. err := xunix.SetOOMScore(ctx, "self", "-1000") if err != nil { - return xerrors.Errorf("set oom score: %w", err) + return "", xerrors.Errorf("set oom score: %w", err) } ref, err := name.ParseReference(flags.innerImage) if err != nil { - return xerrors.Errorf("parse ref: %w", err) + return "", xerrors.Errorf("parse ref: %w", err) } var dockerAuth dockerutil.AuthConfig if flags.imagePullSecret != "" { dockerAuth, err = dockerutil.AuthConfigFromString(flags.imagePullSecret, ref.Context().RegistryStr()) if err != nil { - return xerrors.Errorf("parse auth config: %w", err) + return "", xerrors.Errorf("parse auth config: %w", err) } } @@ -417,7 +476,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client log.Info(ctx, "detected file", slog.F("image", flags.innerImage)) dockerAuth, err = dockerutil.AuthConfigFromPath(flags.dockerConfig, ref.Context().RegistryStr()) if err != nil && !xerrors.Is(err, os.ErrNotExist) { - return xerrors.Errorf("auth config from file: %w", err) + return "", xerrors.Errorf("auth config from file: %w", err) } } @@ -430,7 +489,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client // Add any user-specified mounts to our mounts list. extraMounts, err := parseMounts(flags.containerMounts) if err != nil { - return xerrors.Errorf("read mounts: %w", err) + return "", xerrors.Errorf("read mounts: %w", err) } mounts = append(mounts, extraMounts...) @@ -442,7 +501,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client blog.Info("Creating TUN device") dev, err := xunix.CreateTUNDevice(ctx, OuterTUNPath) if err != nil { - return xerrors.Errorf("creat tun device: %w", err) + return "", xerrors.Errorf("creat tun device: %w", err) } devices = append(devices, container.DeviceMapping{ @@ -457,7 +516,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client blog.Info("Creating FUSE device") dev, err := xunix.CreateFuseDevice(ctx, OuterFUSEPath) if err != nil { - return xerrors.Errorf("create fuse device: %w", err) + return "", xerrors.Errorf("create fuse device: %w", err) } devices = append(devices, container.DeviceMapping{ @@ -479,7 +538,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client ) err = fs.Chown(device.PathOnHost, UserNamespaceOffset, UserNamespaceOffset) if err != nil { - return xerrors.Errorf("chown device %q: %w", device.PathOnHost, err) + return "", xerrors.Errorf("chown device %q: %w", device.PathOnHost, err) } } @@ -492,7 +551,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client ProgressFn: dockerutil.DefaultLogImagePullFn(blog), }) if err != nil { - return xerrors.Errorf("pull image: %w", err) + return "", xerrors.Errorf("pull image: %w", err) } log.Debug(ctx, "remounting /sys") @@ -500,19 +559,19 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client // After image pull we remount /sys so sysbox can have appropriate perms to create a container. err = xunix.MountFS(ctx, "/sys", "/sys", "", "remount", "rw") if err != nil { - return xerrors.Errorf("remount /sys: %w", err) + return "", xerrors.Errorf("remount /sys: %w", err) } if flags.addGPU { if flags.hostUsrLibDir == "" { - return xerrors.Errorf("when using GPUs, %q must be specified", EnvUsrLibDir) + return "", xerrors.Errorf("when using GPUs, %q must be specified", EnvUsrLibDir) } // Unmount GPU drivers in /proc as it causes issues when creating any // container in some cases (even the image metadata container). _, err = xunix.TryUnmountProcGPUDrivers(ctx, log) if err != nil { - return xerrors.Errorf("unmount /proc GPU drivers: %w", err) + return "", xerrors.Errorf("unmount /proc GPU drivers: %w", err) } } @@ -528,7 +587,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client // with /sbin/init or something simple like 'sleep infinity'. imgMeta, err := dockerutil.GetImageMetadata(ctx, log, client, flags.innerImage, flags.innerUsername) if err != nil { - return xerrors.Errorf("get image metadata: %w", err) + return "", xerrors.Errorf("get image metadata: %w", err) } blog.Infof("Detected entrypoint user '%s:%s' with home directory %q", imgMeta.UID, imgMeta.UID, imgMeta.HomeDir) @@ -543,11 +602,11 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client uid, err := strconv.ParseInt(imgMeta.UID, 10, 32) if err != nil { - return xerrors.Errorf("parse image uid: %w", err) + return "", xerrors.Errorf("parse image uid: %w", err) } gid, err := strconv.ParseInt(imgMeta.GID, 10, 32) if err != nil { - return xerrors.Errorf("parse image gid: %w", err) + return "", xerrors.Errorf("parse image gid: %w", err) } for _, m := range mounts { @@ -568,13 +627,13 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client mounter := xunix.Mounter(ctx) err := mounter.Mount("", m.Source, "", []string{"remount,rw"}) if err != nil { - return xerrors.Errorf("remount: %w", err) + return "", xerrors.Errorf("remount: %w", err) } } err := fs.Chmod(m.Source, 0o2755) if err != nil { - return xerrors.Errorf("chmod mountpoint %q: %w", m.Source, err) + return "", xerrors.Errorf("chmod mountpoint %q: %w", m.Source, err) } var ( @@ -601,14 +660,14 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client // user. err = fs.Chown(m.Source, shiftedUID, shiftedGID) if err != nil { - return xerrors.Errorf("chown mountpoint %q: %w", m.Source, err) + return "", xerrors.Errorf("chown mountpoint %q: %w", m.Source, err) } } if flags.addGPU { devs, binds, err := xunix.GPUs(ctx, log, flags.hostUsrLibDir) if err != nil { - return xerrors.Errorf("find gpus: %w", err) + return "", xerrors.Errorf("find gpus: %w", err) } for _, dev := range devs { @@ -679,14 +738,14 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client MemoryLimit: int64(flags.memory), }) if err != nil { - return xerrors.Errorf("create container: %w", err) + return "", xerrors.Errorf("create container: %w", err) } blog.Info("Pruning images to free up disk...") // Prune images to avoid taking up any unnecessary disk from the user. _, err = dockerutil.PruneImages(ctx, client) if err != nil { - return xerrors.Errorf("prune images: %w", err) + return "", xerrors.Errorf("prune images: %w", err) } // TODO fix iptables when istio detected. @@ -694,7 +753,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client blog.Info("Starting up workspace...") err = client.ContainerStart(ctx, containerID, container.StartOptions{}) if err != nil { - return xerrors.Errorf("start container: %w", err) + return "", xerrors.Errorf("start container: %w", err) } log.Debug(ctx, "creating bootstrap directory", slog.F("directory", imgMeta.HomeDir)) @@ -714,7 +773,7 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client Args: []string{"-p", bootDir}, }) if err != nil { - return xerrors.Errorf("make bootstrap dir: %w", err) + return "", xerrors.Errorf("make bootstrap dir: %w", err) } cpuQuota, err := xunix.ReadCPUQuota(ctx, log) @@ -736,34 +795,54 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Client } blog.Info("Envbox startup complete!") - - // The bootstrap script doesn't return since it execs the agent - // meaning that it can get pretty noisy if we were to log by default. - // In order to allow users to discern issues getting the bootstrap script - // to complete successfully we pipe the output to stdout if - // CODER_DEBUG=true. - debugWriter := io.Discard - if flags.debug { - debugWriter = os.Stdout + if flags.boostrapScript == "" { + return "", nil } - // Bootstrap the container if a script has been provided. blog.Infof("Bootstrapping workspace...") - err = dockerutil.BootstrapContainer(ctx, client, dockerutil.BootstrapConfig{ - ContainerID: containerID, - User: imgMeta.UID, - Script: flags.boostrapScript, - // We set this because the default behavior is to download the agent - // to /tmp/coder.XXXX. This causes a race to happen where we finish - // downloading the binary but before we can execute systemd remounts - // /tmp. - Env: []string{fmt.Sprintf("BINARY_DIR=%s", bootDir)}, - StdOutErr: debugWriter, + + bootstrapExec, err := client.ContainerExecCreate(ctx, containerID, container.ExecOptions{ + User: imgMeta.UID, + Cmd: []string{"/bin/sh", "-s"}, + Env: []string{fmt.Sprintf("BINARY_DIR=%s", bootDir)}, + AttachStdin: true, + AttachStdout: true, + AttachStderr: true, + Detach: true, }) if err != nil { - return xerrors.Errorf("boostrap container: %w", err) + return "", xerrors.Errorf("create exec: %w", err) + } + + resp, err := client.ContainerExecAttach(ctx, bootstrapExec.ID, container.ExecStartOptions{}) + if err != nil { + return "", xerrors.Errorf("attach exec: %w", err) + } + + _, err = io.Copy(resp.Conn, strings.NewReader(flags.boostrapScript)) + if err != nil { + return "", xerrors.Errorf("copy stdin: %w", err) } + err = resp.CloseWrite() + if err != nil { + return "", xerrors.Errorf("close write: %w", err) + } + + go func() { + defer resp.Close() + go func() { + // Also close the response reader when the context is canceled. + defer resp.Close() + <-ctx.Done() + }() + rd := io.LimitReader(resp.Reader, 1<<10) + _, err := io.Copy(blog, rd) + if err != nil { + log.Error(ctx, "copy bootstrap output", slog.Error(err)) + } + log.Debug(ctx, "bootstrap output copied") + }() - return nil + return bootstrapExec.ID, nil } //nolint:revive diff --git a/cmd/envbox/main.go b/cmd/envbox/main.go index 2dfdb94..d7bb87e 100644 --- a/cmd/envbox/main.go +++ b/cmd/envbox/main.go @@ -14,7 +14,6 @@ func main() { _, _ = fmt.Fprintln(os.Stderr, err.Error()) os.Exit(1) } - // We exit the main thread while keepin all the other procs goin strong. runtime.Goexit() } diff --git a/dockerutil/container.go b/dockerutil/container.go index 2731cbc..ca27375 100644 --- a/dockerutil/container.go +++ b/dockerutil/container.go @@ -115,6 +115,7 @@ func BootstrapContainer(ctx context.Context, client Client, conf BootstrapConfig Stdin: strings.NewReader(conf.Script), Env: conf.Env, StdOutErr: conf.StdOutErr, + Detach: conf.Detach, }) if err != nil { err = xerrors.Errorf("boostrap container (%s): %w", out, err) diff --git a/dockerutil/dockerfake/client.go b/dockerutil/dockerfake/client.go index 7c7d012..8e348dc 100644 --- a/dockerutil/dockerfake/client.go +++ b/dockerutil/dockerfake/client.go @@ -163,7 +163,9 @@ func (m MockClient) ContainerExecCreate(ctx context.Context, name string, config func (m MockClient) ContainerExecInspect(ctx context.Context, id string) (dockertypes.ContainerExecInspect, error) { if m.ContainerExecInspectFn == nil { - return dockertypes.ContainerExecInspect{}, nil + return dockertypes.ContainerExecInspect{ + Pid: 123, + }, nil } return m.ContainerExecInspectFn(ctx, id) diff --git a/dockerutil/exec.go b/dockerutil/exec.go index 8f27f47..c3821c6 100644 --- a/dockerutil/exec.go +++ b/dockerutil/exec.go @@ -4,11 +4,14 @@ import ( "bytes" "context" "io" + "time" dockertypes "github.com/docker/docker/api/types" + "github.com/docker/docker/api/types/container" "golang.org/x/xerrors" "github.com/coder/envbox/xio" + "github.com/coder/retry" ) type ExecConfig struct { @@ -25,8 +28,8 @@ type ExecConfig struct { // ExecContainer runs a command in a container. It returns the output and any error. // If an error occurs during the execution of the command, the output is appended to the error. func ExecContainer(ctx context.Context, client Client, config ExecConfig) ([]byte, error) { - exec, err := client.ContainerExecCreate(ctx, config.ContainerID, dockertypes.ExecConfig{ - Detach: true, + exec, err := client.ContainerExecCreate(ctx, config.ContainerID, container.ExecOptions{ + Detach: config.Detach, Cmd: append([]string{config.Cmd}, config.Args...), User: config.User, AttachStderr: true, @@ -92,3 +95,39 @@ func ExecContainer(ctx context.Context, client Client, config ExecConfig) ([]byt return buf.Bytes(), nil } + +func GetExecPID(ctx context.Context, client Client, execID string) (int, error) { + for r := retry.New(time.Second, time.Second); r.Wait(ctx); { + inspect, err := client.ContainerExecInspect(ctx, execID) + if err != nil { + return 0, xerrors.Errorf("exec inspect: %w", err) + } + + if inspect.Pid == 0 { + continue + } + return inspect.Pid, nil + } + + return 0, ctx.Err() +} + +func WaitForExit(ctx context.Context, client Client, execID string) error { + for r := retry.New(time.Second, time.Second); r.Wait(ctx); { + inspect, err := client.ContainerExecInspect(ctx, execID) + if err != nil { + return xerrors.Errorf("exec inspect: %w", err) + } + + if inspect.Running { + continue + } + + if inspect.ExitCode > 0 { + return xerrors.Errorf("exit code %d", inspect.ExitCode) + } + + return nil + } + return ctx.Err() +} diff --git a/integration/docker_test.go b/integration/docker_test.go index 533d2ba..c7c1a6d 100644 --- a/integration/docker_test.go +++ b/integration/docker_test.go @@ -390,7 +390,7 @@ func TestDocker(t *testing.T) { // This indicates we've made it all the way to end // of the logs we attempt to push. - require.True(t, recorder.ContainsLog("Bootstrapping workspace...")) + require.True(t, recorder.ContainsLog("Envbox startup complete!")) }) // This tests the inverse of SelfSignedCerts. We assert that @@ -534,6 +534,59 @@ func TestDocker(t *testing.T) { require.NoError(t, err) require.Equal(t, "hello\n", string(output)) }) + t.Run("HandleSignals", func(t *testing.T) { + t.Parallel() + + pool, err := dockertest.NewPool("") + require.NoError(t, err) + + var ( + tmpdir = integrationtest.TmpDir(t) + binds = integrationtest.DefaultBinds(t, tmpdir) + ) + homeDir := filepath.Join(tmpdir, "home") + err = os.MkdirAll(homeDir, 0o777) + require.NoError(t, err) + + binds = append(binds, integrationtest.BindMount(homeDir, "/home/coder", false)) + + envs := []string{fmt.Sprintf("%s=%s:%s", cli.EnvMounts, "/home/coder", "/home/coder")} + // Run the envbox container. + resource := integrationtest.RunEnvbox(t, pool, &integrationtest.CreateDockerCVMConfig{ + Image: integrationtest.UbuntuImage, + Username: "root", + OuterMounts: binds, + Envs: envs, + BootstrapScript: sigtrapScript, + }) + + _, err = integrationtest.ExecInnerContainer(t, pool, integrationtest.ExecConfig{ + ContainerID: resource.Container.ID, + Cmd: []string{"/bin/sh", "-c", "stat /home/coder/foo"}, + }) + require.Error(t, err) + + // Simulate a shutdown. + integrationtest.StopContainer(t, pool, resource.Container.ID, 30*time.Second) + + err = resource.Close() + require.NoError(t, err) + + t.Logf("envbox %q started successfully, recreating...", resource.Container.ID) + // Run the envbox container. + resource = integrationtest.RunEnvbox(t, pool, &integrationtest.CreateDockerCVMConfig{ + Image: integrationtest.UbuntuImage, + Username: "root", + OuterMounts: binds, + Envs: envs, + BootstrapScript: sigtrapScript, + }) + _, err = integrationtest.ExecInnerContainer(t, pool, integrationtest.ExecConfig{ + ContainerID: resource.Container.ID, + Cmd: []string{"/bin/sh", "-c", "stat /home/coder/foo"}, + }) + require.NoError(t, err) + }) } func requireSliceNoContains(t *testing.T, ss []string, els ...string) { @@ -566,3 +619,17 @@ func tcpAddr(t testing.TB, l net.Listener) *net.TCPAddr { require.True(t, ok) return tcpAddr } + +const sigtrapScript = `#!/bin/bash +cleanup() { + echo "HANDLING A SIGNAL!" && touch /home/coder/foo && echo "touched file" + exit 0 +} + +trap 'cleanup' INT TERM + +while true; do + echo "Working..." + sleep 1 +done +` diff --git a/integration/integrationtest/docker.go b/integration/integrationtest/docker.go index 5b42bbe..0f820be 100644 --- a/integration/integrationtest/docker.go +++ b/integration/integrationtest/docker.go @@ -306,6 +306,31 @@ func ExecEnvbox(t *testing.T, pool *dockertest.Pool, conf ExecConfig) ([]byte, e return buf.Bytes(), nil } +func StopContainer(t *testing.T, pool *dockertest.Pool, id string, to time.Duration) { + t.Helper() + + err := pool.Client.KillContainer(docker.KillContainerOptions{ + ID: id, + Signal: docker.SIGTERM, + }) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(context.Background(), to) + defer cancel() + for r := retry.New(time.Second, time.Second); r.Wait(ctx); { + cnt, err := pool.Client.InspectContainer(id) + require.NoError(t, err) + + if cnt.State.Running { + continue + } + + return + } + + t.Fatalf("timed out waiting for container %s to stop", id) +} + // cmdLineEnvs returns args passed to the /envbox command // but using their env var alias. func cmdLineEnvs(c *CreateDockerCVMConfig) []string {