diff --git a/README.md b/README.md index 8c44f62..8fb02eb 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,6 @@ The environment variables can be used to configure various aspects of the inner | `CODER_CPUS` | Dictates the number of CPUs to allocate the inner container. It is recommended to set this using the Kubernetes [Downward API](https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/#use-container-fields-as-values-for-environment-variables). | false | | `CODER_MEMORY` | Dictates the max memory (in bytes) to allocate the inner container. It is recommended to set this using the Kubernetes [Downward API](https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/#use-container-fields-as-values-for-environment-variables). | false | | `CODER_DISABLE_IDMAPPED_MOUNT` | Disables idmapped mounts in sysbox. For more information, see the [Sysbox Documentation](https://github.com/nestybox/sysbox/blob/master/docs/user-guide/configuration.md#disabling-id-mapped-mounts-on-sysbox). | false | -| `CODER_SHUTDOWN_TIMEOUT` | Configure a custom shutdown timeout to wait for the boostrap command to exit. Defaults to 1 minute. | false | ## Coder Template diff --git a/cli/docker.go b/cli/docker.go index 7ba0e04..65c54b2 100644 --- a/cli/docker.go +++ b/cli/docker.go @@ -13,7 +13,6 @@ import ( "sort" "strconv" "strings" - "time" dockertypes "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" @@ -40,10 +39,6 @@ const ( // EnvBoxContainerName is the name of the inner user container. EnvBoxPullImageSecretEnvVar = "CODER_IMAGE_PULL_SECRET" //nolint:gosec EnvBoxContainerName = "CODER_CVM_CONTAINER_NAME" - // We define a custom exit code to distinguish from the generic '1' when envbox exits due to a shutdown timeout. - // Docker claims exit codes 125-127 so we start at 150 to - // ensure we don't collide. - ExitCodeShutdownTimeout = 150 ) const ( @@ -82,9 +77,8 @@ const ( // with UID/GID 1000 will be mapped to `UserNamespaceOffset` + 1000 // on the host. Changing this value will result in improper mappings // on existing containers. - UserNamespaceOffset = 100000 - devDir = "/dev" - defaultShutdownTimeout = time.Minute + UserNamespaceOffset = 100000 + devDir = "/dev" ) var ( @@ -108,7 +102,6 @@ var ( EnvDockerConfig = "CODER_DOCKER_CONFIG" EnvDebug = "CODER_DEBUG" EnvDisableIDMappedMount = "CODER_DISABLE_IDMAPPED_MOUNT" - EnvShutdownTimeout = "CODER_SHUTDOWN_TIMEOUT" ) var envboxPrivateMounts = map[string]struct{}{ @@ -146,7 +139,6 @@ type flags struct { cpus int memory int disableIDMappedMount bool - shutdownTimeout time.Duration // Test flags. noStartupLogs bool @@ -358,7 +350,6 @@ func dockerCmd(ch chan func() error) *cobra.Command { cliflag.IntVarP(cmd.Flags(), &flags.cpus, "cpus", "", EnvCPUs, 0, "Number of CPUs to allocate inner container. e.g. 2") cliflag.IntVarP(cmd.Flags(), &flags.memory, "memory", "", EnvMemory, 0, "Max memory to allocate to the inner container in bytes.") cliflag.BoolVarP(cmd.Flags(), &flags.disableIDMappedMount, "disable-idmapped-mount", "", EnvDisableIDMappedMount, false, "Disable idmapped mounts in sysbox. Note that you may need an alternative (e.g. shiftfs).") - cliflag.DurationVarP(cmd.Flags(), &flags.shutdownTimeout, "shutdown-timeout", "", EnvShutdownTimeout, defaultShutdownTimeout, "Duration after which envbox will be forcefully terminated.") // Test flags. cliflag.BoolVarP(cmd.Flags(), &flags.noStartupLogs, "no-startup-log", "", "", false, "Do not log startup logs. Useful for testing.") @@ -735,31 +726,18 @@ func runDockerCVM(ctx context.Context, log slog.Logger, client dockerutil.Docker return xerrors.Errorf("exec inspect: %w", err) } - shutdownCh <- killBootstrapCmd(ctx, log, bootstrapPID, bootstrapExec.ID, client, flags.shutdownTimeout) + shutdownCh <- func() error { + log.Debug(ctx, "killing container", slog.F("bootstrap_pid", bootstrapPID)) - return nil -} - -// KillBootstrapCmd is the command we run when we receive a signal -// to kill the envbox container. -func killBootstrapCmd(ctx context.Context, log slog.Logger, pid int, execID string, client dockerutil.DockerClient, timeout time.Duration) func() error { - return func() error { - log.Debug(ctx, "killing container", - slog.F("bootstrap_pid", pid), - slog.F("timeout", timeout.String()), - ) - - ctx, cancel := context.WithTimeout(ctx, timeout) - defer cancel() // The PID returned is the PID _outside_ the container... //nolint:gosec - out, err := exec.CommandContext(ctx, "kill", "-TERM", strconv.Itoa(pid)).CombinedOutput() + out, err := exec.Command("kill", "-TERM", strconv.Itoa(bootstrapPID)).CombinedOutput() if err != nil { return xerrors.Errorf("kill bootstrap process (%s): %w", out, err) } log.Debug(ctx, "sent kill signal waiting for process to exit") - err = dockerutil.WaitForExit(ctx, client, execID) + err = dockerutil.WaitForExit(ctx, client, bootstrapExec.ID) if err != nil { return xerrors.Errorf("wait for exit: %w", err) } @@ -767,6 +745,8 @@ func killBootstrapCmd(ctx context.Context, log slog.Logger, pid int, execID stri log.Debug(ctx, "bootstrap process successfully exited") return nil } + + return nil } //nolint:revive diff --git a/cmd/envbox/main.go b/cmd/envbox/main.go index 995565d..ebb77f9 100644 --- a/cmd/envbox/main.go +++ b/cmd/envbox/main.go @@ -8,8 +8,6 @@ import ( "runtime" "syscall" - "golang.org/x/xerrors" - "cdr.dev/slog" "cdr.dev/slog/sloggers/slogjson" "github.com/coder/envbox/cli" @@ -31,9 +29,6 @@ func main() { err := fn() if err != nil { log.Error(ctx, "shutdown function failed", slog.Error(err)) - if xerrors.Is(err, context.DeadlineExceeded) { - os.Exit(cli.ExitCodeShutdownTimeout) - } os.Exit(1) } default: diff --git a/integration/docker_test.go b/integration/docker_test.go index 5639c91..d043e76 100644 --- a/integration/docker_test.go +++ b/integration/docker_test.go @@ -306,8 +306,10 @@ func TestDocker(t *testing.T) { require.Error(t, err) // Simulate a shutdown. - exitCode := integrationtest.StopContainer(t, pool, resource.Container.ID, 30*time.Second) - require.Equal(t, 0, exitCode) + 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. @@ -324,34 +326,6 @@ func TestDocker(t *testing.T) { }) require.NoError(t, err) }) - - t.Run("ShutdownTimeout", func(t *testing.T) { - t.Parallel() - - pool, err := dockertest.NewPool("") - require.NoError(t, err) - - var ( - tmpdir = integrationtest.TmpDir(t) - binds = integrationtest.DefaultBinds(t, tmpdir) - ) - - envs := []string{fmt.Sprintf("%s=%s", cli.EnvShutdownTimeout, "1s")} - - // Run the envbox container. - resource := integrationtest.RunEnvbox(t, pool, &integrationtest.CreateDockerCVMConfig{ - Image: integrationtest.UbuntuImage, - Username: "root", - Envs: envs, - Binds: binds, - BootstrapScript: sigtrapForeverScript, - }) - - // Simulate a shutdown. - exitCode := integrationtest.StopContainer(t, pool, resource.Container.ID, 30*time.Second) - // We expect it to timeout which should result in a special exit code. - require.Equal(t, cli.ExitCodeShutdownTimeout, exitCode) - }) } func requireSliceNoContains(t *testing.T, ss []string, els ...string) { @@ -384,20 +358,6 @@ func bindMount(src, dest string, ro bool) string { return fmt.Sprintf("%s:%s", src, dest) } -const sigtrapForeverScript = `#!/bin/bash -cleanup() { - echo "Got a signal, going to sleep!" && sleep infinity - exit 0 -} - -trap 'cleanup' INT TERM - -while true; do - echo "Working..." - sleep 1 -done -` - const sigtrapScript = `#!/bin/bash cleanup() { echo "HANDLING A SIGNAL!" && touch /home/coder/foo && echo "touched file" diff --git a/integration/integrationtest/docker.go b/integration/integrationtest/docker.go index 0b78369..088952c 100644 --- a/integration/integrationtest/docker.go +++ b/integration/integrationtest/docker.go @@ -91,12 +91,7 @@ func RunEnvbox(t *testing.T, pool *dockertest.Pool, conf *CreateDockerCVMConfig) host.CPUQuota = int64(conf.CPUs) * int64(dockerutil.DefaultCPUPeriod) }) require.NoError(t, err) - t.Cleanup(func() { - // Only delete the container if the test passes. - if !t.Failed() { - resource.Close() - } - }) + // t.Cleanup(func() { _ = pool.Purge(resource) }) waitForCVM(t, pool, resource) @@ -269,7 +264,7 @@ 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) int { +func StopContainer(t *testing.T, pool *dockertest.Pool, id string, to time.Duration) { t.Helper() err := pool.Client.KillContainer(docker.KillContainerOptions{ @@ -288,11 +283,10 @@ func StopContainer(t *testing.T, pool *dockertest.Pool, id string, to time.Durat continue } - return cnt.State.ExitCode + return } t.Fatalf("timed out waiting for container %s to stop", id) - return 1 } // cmdLineEnvs returns args passed to the /envbox command