From fc853061215d5654a2e0ccdd8d47717e7d5261f7 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 27 Sep 2024 20:53:24 +0100 Subject: [PATCH] Revert "fix: add preExec to Run (#368)" (#371) (cherry picked from commit 5c150b3c1e35b7b331caa464393156eac4561159) --- cmd/envbuilder/main.go | 12 +----- envbuilder.go | 7 +--- integration/integration_test.go | 67 --------------------------------- 3 files changed, 2 insertions(+), 84 deletions(-) diff --git a/cmd/envbuilder/main.go b/cmd/envbuilder/main.go index 720c0c85..410e0897 100644 --- a/cmd/envbuilder/main.go +++ b/cmd/envbuilder/main.go @@ -37,12 +37,6 @@ func envbuilderCmd() serpent.Command { Options: o.CLI(), Handler: func(inv *serpent.Invocation) error { o.SetDefaults() - var preExec []func() - defer func() { // Ensure cleanup in case of error. - for _, fn := range preExec { - fn() - } - }() o.Logger = log.New(os.Stderr, o.Verbose) if o.CoderAgentURL != "" { if o.CoderAgentToken == "" { @@ -56,10 +50,6 @@ func envbuilderCmd() serpent.Command { if err == nil { o.Logger = log.Wrap(o.Logger, coderLog) defer closeLogs() - preExec = append(preExec, func() { - o.Logger(log.LevelInfo, "Closing logs") - closeLogs() - }) // This adds the envbuilder subsystem. // If telemetry is enabled in a Coder deployment, // this will be reported and help us understand @@ -88,7 +78,7 @@ func envbuilderCmd() serpent.Command { return nil } - err := envbuilder.Run(inv.Context(), o, preExec...) + err := envbuilder.Run(inv.Context(), o) if err != nil { o.Logger(log.LevelError, "error: %s", err) } diff --git a/envbuilder.go b/envbuilder.go index 94998165..683f6a54 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -84,9 +84,7 @@ type execArgsInfo struct { // Logger is the logf to use for all operations. // Filesystem is the filesystem to use for all operations. // Defaults to the host filesystem. -// preExec are any functions that should be called before exec'ing the init -// command. This is useful for ensuring that defers get run. -func Run(ctx context.Context, opts options.Options, preExec ...func()) error { +func Run(ctx context.Context, opts options.Options) error { var args execArgsInfo // Run in a separate function to ensure all defers run before we // setuid or exec. @@ -105,9 +103,6 @@ func Run(ctx context.Context, opts options.Options, preExec ...func()) error { } opts.Logger(log.LevelInfo, "=== Running the init command %s %+v as the %q user...", opts.InitCommand, args.InitArgs, args.UserInfo.user.Username) - for _, fn := range preExec { - fn() - } err = syscall.Exec(args.InitCommand, append([]string{args.InitCommand}, args.InitArgs...), args.Environ) if err != nil { diff --git a/integration/integration_test.go b/integration/integration_test.go index 79b678d5..66dfe846 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -23,8 +23,6 @@ import ( "testing" "time" - "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/envbuilder" "github.com/coder/envbuilder/devcontainer/features" "github.com/coder/envbuilder/internal/magicdir" @@ -60,71 +58,6 @@ const ( testImageUbuntu = "localhost:5000/envbuilder-test-ubuntu:latest" ) -func TestLogs(t *testing.T) { - t.Parallel() - - token := uuid.NewString() - logsDone := make(chan struct{}) - - logHandler := func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/api/v2/buildinfo": - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"version": "v2.8.9"}`)) - return - case "/api/v2/workspaceagents/me/logs": - w.WriteHeader(http.StatusOK) - tokHdr := r.Header.Get(codersdk.SessionTokenHeader) - assert.Equal(t, token, tokHdr) - var req agentsdk.PatchLogs - err := json.NewDecoder(r.Body).Decode(&req) - if err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return - } - for _, log := range req.Logs { - t.Logf("got log: %+v", log) - if strings.Contains(log.Output, "Closing logs") { - close(logsDone) - return - } - } - return - default: - t.Errorf("unexpected request to %s", r.URL.Path) - w.WriteHeader(http.StatusNotFound) - return - } - } - logSrv := httptest.NewServer(http.HandlerFunc(logHandler)) - defer logSrv.Close() - - // Ensures that a Git repository with a devcontainer.json is cloned and built. - srv := gittest.CreateGitServer(t, gittest.Options{ - Files: map[string]string{ - "devcontainer.json": `{ - "build": { - "dockerfile": "Dockerfile" - }, - }`, - "Dockerfile": fmt.Sprintf(`FROM %s`, testImageUbuntu), - }, - }) - _, err := runEnvbuilder(t, runOpts{env: []string{ - envbuilderEnv("GIT_URL", srv.URL), - "CODER_AGENT_URL=" + logSrv.URL, - "CODER_AGENT_TOKEN=" + token, - }}) - require.NoError(t, err) - ctx, cancel := context.WithTimeout(context.Background(), time.Minute) - defer cancel() - select { - case <-ctx.Done(): - t.Fatal("timed out waiting for logs") - case <-logsDone: - } -} - func TestInitScriptInitCommand(t *testing.T) { t.Parallel()