-
Notifications
You must be signed in to change notification settings - Fork 928
feat(agent): send devcontainer CLI logs during recreate #17845
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import ( | |
"github.com/coder/coder/v2/agent/agentexec" | ||
"github.com/coder/coder/v2/coderd/httpapi" | ||
"github.com/coder/coder/v2/codersdk" | ||
"github.com/coder/coder/v2/codersdk/agentsdk" | ||
"github.com/coder/quartz" | ||
) | ||
|
||
|
@@ -43,6 +44,7 @@ type API struct { | |
cl Lister | ||
dccli DevcontainerCLI | ||
clock quartz.Clock | ||
scriptLogger func(logSourceID uuid.UUID) ScriptLogger | ||
|
||
// lockCh protects the below fields. We use a channel instead of a | ||
// mutex so we can handle cancellation properly. | ||
|
@@ -52,6 +54,8 @@ type API struct { | |
devcontainerNames map[string]struct{} // Track devcontainer names to avoid duplicates. | ||
knownDevcontainers []codersdk.WorkspaceAgentDevcontainer // Track predefined and runtime-detected devcontainers. | ||
configFileModifiedTimes map[string]time.Time // Track when config files were last modified. | ||
|
||
devcontainerLogSourceIDs map[string]uuid.UUID // Track devcontainer log source IDs. | ||
} | ||
|
||
// Option is a functional option for API. | ||
|
@@ -91,13 +95,30 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option { | |
// WithDevcontainers sets the known devcontainers for the API. This | ||
// allows the API to be aware of devcontainers defined in the workspace | ||
// agent manifest. | ||
func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer) Option { | ||
func WithDevcontainers(devcontainers []codersdk.WorkspaceAgentDevcontainer, scripts []codersdk.WorkspaceAgentScript) Option { | ||
return func(api *API) { | ||
if len(devcontainers) > 0 { | ||
api.knownDevcontainers = slices.Clone(devcontainers) | ||
api.devcontainerNames = make(map[string]struct{}, len(devcontainers)) | ||
for _, devcontainer := range devcontainers { | ||
api.devcontainerNames[devcontainer.Name] = struct{}{} | ||
if len(devcontainers) == 0 { | ||
return | ||
} | ||
api.knownDevcontainers = slices.Clone(devcontainers) | ||
api.devcontainerNames = make(map[string]struct{}, len(devcontainers)) | ||
api.devcontainerLogSourceIDs = make(map[string]uuid.UUID) | ||
for _, devcontainer := range devcontainers { | ||
api.devcontainerNames[devcontainer.Name] = struct{}{} | ||
for _, script := range scripts { | ||
// The devcontainer scripts match the devcontainer ID for | ||
// identification. | ||
if script.ID == devcontainer.ID { | ||
api.devcontainerLogSourceIDs[devcontainer.WorkspaceFolder] = script.LogSourceID | ||
break | ||
Comment on lines
+108
to
+113
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we warn if there are scripts that do not match up with any devcontainer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose it would be an error rather than warn, this is an expectation coming from the provisioner currently. Added. |
||
} | ||
} | ||
if api.devcontainerLogSourceIDs[devcontainer.WorkspaceFolder] == uuid.Nil { | ||
api.logger.Error(api.ctx, "devcontainer log source ID not found for devcontainer", | ||
slog.F("devcontainer", devcontainer.Name), | ||
slog.F("workspace_folder", devcontainer.WorkspaceFolder), | ||
slog.F("config_path", devcontainer.ConfigPath), | ||
) | ||
} | ||
} | ||
} | ||
|
@@ -112,6 +133,27 @@ func WithWatcher(w watcher.Watcher) Option { | |
} | ||
} | ||
|
||
// ScriptLogger is an interface for sending devcontainer logs to the | ||
// controlplane. | ||
type ScriptLogger interface { | ||
Send(ctx context.Context, log ...agentsdk.Log) error | ||
Flush(ctx context.Context) error | ||
} | ||
|
||
// noopScriptLogger is a no-op implementation of the ScriptLogger | ||
// interface. | ||
type noopScriptLogger struct{} | ||
|
||
func (noopScriptLogger) Send(context.Context, ...agentsdk.Log) error { return nil } | ||
func (noopScriptLogger) Flush(context.Context) error { return nil } | ||
|
||
// WithScriptLogger sets the script logger provider for devcontainer operations. | ||
func WithScriptLogger(scriptLogger func(logSourceID uuid.UUID) ScriptLogger) Option { | ||
return func(api *API) { | ||
api.scriptLogger = scriptLogger | ||
} | ||
} | ||
|
||
// NewAPI returns a new API with the given options applied. | ||
func NewAPI(logger slog.Logger, options ...Option) *API { | ||
ctx, cancel := context.WithCancel(context.Background()) | ||
|
@@ -127,7 +169,10 @@ func NewAPI(logger slog.Logger, options ...Option) *API { | |
devcontainerNames: make(map[string]struct{}), | ||
knownDevcontainers: []codersdk.WorkspaceAgentDevcontainer{}, | ||
configFileModifiedTimes: make(map[string]time.Time), | ||
scriptLogger: func(uuid.UUID) ScriptLogger { return noopScriptLogger{} }, | ||
} | ||
// The ctx and logger must be set before applying options to avoid | ||
// nil pointer dereference. | ||
for _, opt := range options { | ||
opt(api) | ||
} | ||
|
@@ -426,7 +471,26 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques | |
return | ||
} | ||
|
||
_, err = api.dccli.Up(ctx, workspaceFolder, configPath, WithRemoveExistingContainer()) | ||
// Send logs via agent logging facilities. | ||
logSourceID := api.devcontainerLogSourceIDs[workspaceFolder] | ||
if logSourceID == uuid.Nil { | ||
// Fallback to the external log source ID if not found. | ||
logSourceID = agentsdk.ExternalLogSourceID | ||
} | ||
scriptLogger := api.scriptLogger(logSourceID) | ||
defer func() { | ||
flushCtx, cancel := context.WithTimeout(api.ctx, 5*time.Second) | ||
defer cancel() | ||
if err := scriptLogger.Flush(flushCtx); err != nil { | ||
api.logger.Error(flushCtx, "flush devcontainer logs failed", slog.Error(err)) | ||
} | ||
}() | ||
infoW := agentsdk.LogsWriter(ctx, scriptLogger.Send, logSourceID, codersdk.LogLevelInfo) | ||
defer infoW.Close() | ||
errW := agentsdk.LogsWriter(ctx, scriptLogger.Send, logSourceID, codersdk.LogLevelError) | ||
defer errW.Close() | ||
|
||
_, err = api.dccli.Up(ctx, workspaceFolder, configPath, WithOutput(infoW, errW), WithRemoveExistingContainer()) | ||
if err != nil { | ||
httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ | ||
Message: "Could not recreate devcontainer", | ||
|
Uh oh!
There was an error while loading. Please reload this page.