chore: pull in cherry picks for v2.24 (coder#18674)

Co-authored-by: Danny Kopping <danny@coder.com>
Co-authored-by: Steven Masley <Emyrk@users.noreply.github.com>
Co-authored-by: Bruno Quaresma <bruno@coder.com>
Co-authored-by: Sas Swart <sas.swart.cdk@gmail.com>
Co-authored-by: Susana Ferreira <susana@coder.com>
Co-authored-by: Danielle Maywood <danielle@themaywoods.com>
Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Asher <ash@coder.com>
Co-authored-by: Hugo Dutka <hugo@coder.com>

Author: 11 people

agent/agent.go

@@ -91,6 +91,7 @@ type Options struct {
 	Execer                       agentexec.Execer
 	Devcontainers                bool
 	DevcontainerAPIOptions       []agentcontainers.Option // Enable Devcontainers for these to be effective.
+	Clock                        quartz.Clock
 }
 
 type Client interface {
@@ -144,6 +145,9 @@ func New(options Options) Agent {
 	if options.PortCacheDuration == 0 {
 		options.PortCacheDuration = 1 * time.Second
 	}
+	if options.Clock == nil {
+		options.Clock = quartz.NewReal()
+	}
 
 	prometheusRegistry := options.PrometheusRegistry
 	if prometheusRegistry == nil {
@@ -157,6 +161,7 @@ func New(options Options) Agent {
 	hardCtx, hardCancel := context.WithCancel(context.Background())
 	gracefulCtx, gracefulCancel := context.WithCancel(hardCtx)
 	a := &agent{
+		clock:                              options.Clock,
 		tailnetListenPort:                  options.TailnetListenPort,
 		reconnectingPTYTimeout:             options.ReconnectingPTYTimeout,
 		logger:                             options.Logger,
@@ -204,6 +209,7 @@ func New(options Options) Agent {
 }
 
 type agent struct {
+	clock             quartz.Clock
 	logger            slog.Logger
 	client            Client
 	exchangeToken     func(ctx context.Context) (string, error)
@@ -273,7 +279,7 @@ type agent struct {
 
 	devcontainers       bool
 	containerAPIOptions []agentcontainers.Option
-	containerAPI        atomic.Pointer[agentcontainers.API] // Set by apiHandler.
+	containerAPI        *agentcontainers.API
 }
 
 func (a *agent) TailnetConn() *tailnet.Conn {
@@ -330,6 +336,19 @@ func (a *agent) init() {
 	// will not report anywhere.
 	a.scriptRunner.RegisterMetrics(a.prometheusRegistry)
 
+	if a.devcontainers {
+		containerAPIOpts := []agentcontainers.Option{
+			agentcontainers.WithExecer(a.execer),
+			agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
+			agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
+				return a.logSender.GetScriptLogger(logSourceID)
+			}),
+		}
+		containerAPIOpts = append(containerAPIOpts, a.containerAPIOptions...)
+
+		a.containerAPI = agentcontainers.NewAPI(a.logger.Named("containers"), containerAPIOpts...)
+	}
+
 	a.reconnectingPTYServer = reconnectingpty.NewServer(
 		a.logger.Named("reconnecting-pty"),
 		a.sshServer,
@@ -1141,17 +1160,27 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 			}
 
 			var (
-				scripts          = manifest.Scripts
-				scriptRunnerOpts []agentscripts.InitOption
+				scripts             = manifest.Scripts
+				devcontainerScripts map[uuid.UUID]codersdk.WorkspaceAgentScript
 			)
-			if a.devcontainers {
-				var dcScripts []codersdk.WorkspaceAgentScript
-				scripts, dcScripts = agentcontainers.ExtractAndInitializeDevcontainerScripts(manifest.Devcontainers, scripts)
-				// See ExtractAndInitializeDevcontainerScripts for motivation
-				// behind running dcScripts as post start scripts.
-				scriptRunnerOpts = append(scriptRunnerOpts, agentscripts.WithPostStartScripts(dcScripts...))
+			if a.containerAPI != nil {
+				// Init the container API with the manifest and client so that
+				// we can start accepting requests. The final start of the API
+				// happens after the startup scripts have been executed to
+				// ensure the presence of required tools. This means we can
+				// return existing devcontainers but actual container detection
+				// and creation will be deferred.
+				a.containerAPI.Init(
+					agentcontainers.WithManifestInfo(manifest.OwnerName, manifest.WorkspaceName, manifest.AgentName),
+					agentcontainers.WithDevcontainers(manifest.Devcontainers, manifest.Scripts),
+					agentcontainers.WithSubAgentClient(agentcontainers.NewSubAgentClientFromAPI(a.logger, aAPI)),
+				)
+
+				// Since devcontainer are enabled, remove devcontainer scripts
+				// from the main scripts list to avoid showing an error.
+				scripts, devcontainerScripts = agentcontainers.ExtractDevcontainerScripts(manifest.Devcontainers, scripts)
 			}
-			err = a.scriptRunner.Init(scripts, aAPI.ScriptCompleted, scriptRunnerOpts...)
+			err = a.scriptRunner.Init(scripts, aAPI.ScriptCompleted)
 			if err != nil {
 				return xerrors.Errorf("init script runner: %w", err)
 			}
@@ -1168,7 +1197,18 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 				// finished (both start and post start). For instance, an
 				// autostarted devcontainer will be included in this time.
 				err := a.scriptRunner.Execute(a.gracefulCtx, agentscripts.ExecuteStartScripts)
-				err = errors.Join(err, a.scriptRunner.Execute(a.gracefulCtx, agentscripts.ExecutePostStartScripts))
+
+				if a.containerAPI != nil {
+					// Start the container API after the startup scripts have
+					// been executed to ensure that the required tools can be
+					// installed.
+					a.containerAPI.Start()
+					for _, dc := range manifest.Devcontainers {
+						cErr := a.createDevcontainer(ctx, aAPI, dc, devcontainerScripts[dc.ID])
+						err = errors.Join(err, cErr)
+					}
+				}
+
 				dur := time.Since(start).Seconds()
 				if err != nil {
 					a.logger.Warn(ctx, "startup script(s) failed", slog.Error(err))
@@ -1187,14 +1227,6 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 				}
 				a.metrics.startupScriptSeconds.WithLabelValues(label).Set(dur)
 				a.scriptRunner.StartCron()
-
-				// If the container API is enabled, trigger an immediate refresh
-				// for quick sub agent injection.
-				if cAPI := a.containerAPI.Load(); cAPI != nil {
-					if err := cAPI.RefreshContainers(ctx); err != nil {
-						a.logger.Error(ctx, "failed to refresh containers", slog.Error(err))
-					}
-				}
 			})
 			if err != nil {
 				return xerrors.Errorf("track conn goroutine: %w", err)
@@ -1204,6 +1236,38 @@ func (a *agent) handleManifest(manifestOK *checkpoint) func(ctx context.Context,
 	}
 }
 
+func (a *agent) createDevcontainer(
+	ctx context.Context,
+	aAPI proto.DRPCAgentClient26,
+	dc codersdk.WorkspaceAgentDevcontainer,
+	script codersdk.WorkspaceAgentScript,
+) (err error) {
+	var (
+		exitCode  = int32(0)
+		startTime = a.clock.Now()
+		status    = proto.Timing_OK
+	)
+	if err = a.containerAPI.CreateDevcontainer(dc.WorkspaceFolder, dc.ConfigPath); err != nil {
+		exitCode = 1
+		status = proto.Timing_EXIT_FAILURE
+	}
+	endTime := a.clock.Now()
+
+	if _, scriptErr := aAPI.ScriptCompleted(ctx, &proto.WorkspaceAgentScriptCompletedRequest{
+		Timing: &proto.Timing{
+			ScriptId: script.ID[:],
+			Start:    timestamppb.New(startTime),
+			End:      timestamppb.New(endTime),
+			ExitCode: exitCode,
+			Stage:    proto.Timing_START,
+			Status:   status,
+		},
+	}); scriptErr != nil {
+		a.logger.Warn(ctx, "reporting script completed failed", slog.Error(scriptErr))
+	}
+	return err
+}
+
 // createOrUpdateNetwork waits for the manifest to be set using manifestOK, then creates or updates
 // the tailnet using the information in the manifest
 func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(context.Context, proto.DRPCAgentClient26) error {
@@ -1227,7 +1291,6 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
 			// agent API.
 			network, err = a.createTailnet(
 				a.gracefulCtx,
-				aAPI,
 				manifest.AgentID,
 				manifest.DERPMap,
 				manifest.DERPForceWebSockets,
@@ -1262,9 +1325,9 @@ func (a *agent) createOrUpdateNetwork(manifestOK, networkOK *checkpoint) func(co
 			network.SetBlockEndpoints(manifest.DisableDirectConnections)
 
 			// Update the subagent client if the container API is available.
-			if cAPI := a.containerAPI.Load(); cAPI != nil {
+			if a.containerAPI != nil {
 				client := agentcontainers.NewSubAgentClientFromAPI(a.logger, aAPI)
-				cAPI.UpdateSubAgentClient(client)
+				a.containerAPI.UpdateSubAgentClient(client)
 			}
 		}
 		return nil
@@ -1382,7 +1445,6 @@ func (a *agent) trackGoroutine(fn func()) error {
 
 func (a *agent) createTailnet(
 	ctx context.Context,
-	aAPI proto.DRPCAgentClient26,
 	agentID uuid.UUID,
 	derpMap *tailcfg.DERPMap,
 	derpForceWebSockets, disableDirectConnections bool,
@@ -1515,10 +1577,7 @@ func (a *agent) createTailnet(
 	}()
 	if err = a.trackGoroutine(func() {
 		defer apiListener.Close()
-		apiHandler, closeAPIHAndler := a.apiHandler(aAPI)
-		defer func() {
-			_ = closeAPIHAndler()
-		}()
+		apiHandler := a.apiHandler()
 		server := &http.Server{
 			BaseContext:       func(net.Listener) context.Context { return ctx },
 			Handler:           apiHandler,
@@ -1532,7 +1591,6 @@ func (a *agent) createTailnet(
 			case <-ctx.Done():
 			case <-a.hardCtx.Done():
 			}
-			_ = closeAPIHAndler()
 			_ = server.Close()
 		}()
 
@@ -1871,6 +1929,12 @@ func (a *agent) Close() error {
 		a.logger.Error(a.hardCtx, "script runner close", slog.Error(err))
 	}
 
+	if a.containerAPI != nil {
+		if err := a.containerAPI.Close(); err != nil {
+			a.logger.Error(a.hardCtx, "container API close", slog.Error(err))
+		}
+	}
+
 	// Wait for the graceful shutdown to complete, but don't wait forever so
 	// that we don't break user expectations.
 	go func() {