Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions cmd/agent/container/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ func NewDaemonCmd() *cobra.Command {
}
daemonCmd.Flags().
StringVar(&cmd.Config.Timeout, "timeout", "", "The timeout to stop the container after")
daemonCmd.Flags().
StringVar(&cmd.Config.ShutdownAction, "shutdown-action", "", "The shutdown action (none or stopContainer)")
return daemonCmd
}

Expand Down Expand Up @@ -96,8 +98,8 @@ func (cmd *DaemonCmd) Run(c *cobra.Command, args []string) error {
})
}

// Start timeout monitor.
if timeoutDuration > 0 {
// Start timeout monitor unless shutdownAction is "none".
if timeoutDuration > 0 && cmd.Config.ShutdownAction != "none" {
tasksStarted = true
g.Go(func() error {
return runTimeoutMonitor(ctx, timeoutDuration)
Expand Down Expand Up @@ -158,6 +160,9 @@ func (cmd *DaemonCmd) loadConfig() error {
if cmd.Config.Timeout != "" {
cfg.Timeout = cmd.Config.Timeout
}
if cmd.Config.ShutdownAction != "" {
cfg.ShutdownAction = cmd.Config.ShutdownAction
}
cmd.Config = &cfg
}

Expand Down
26 changes: 16 additions & 10 deletions cmd/agent/container/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ func (cmd *SetupContainerCmd) setupPostAttach(
return err
}

if err := cmd.startContainerDaemon(sctx.workspaceInfo); err != nil {
shutdownAction := sctx.setupInfo.MergedConfig.ShutdownAction
if err := cmd.startContainerDaemon(sctx.workspaceInfo, shutdownAction); err != nil {
return err
}

Expand Down Expand Up @@ -400,6 +401,7 @@ func (cmd *SetupContainerCmd) cloneRepositoryIfNeeded(

func (cmd *SetupContainerCmd) startContainerDaemon(
workspaceInfo *provider2.ContainerWorkspaceInfo,
shutdownAction string,
) error {
if workspaceInfo.CLIOptions.Platform.Enabled ||
workspaceInfo.CLIOptions.DisableDaemon ||
Expand All @@ -418,15 +420,19 @@ func (cmd *SetupContainerCmd) startContainerDaemon(
return nil, err
}

//nolint:gosec // binaryPath is from os.Executable(), not user input
return exec.Command(
binaryPath,
"agent",
"container",
"daemon",
"--timeout",
workspaceInfo.ContainerTimeout,
), nil
args := []string{
"agent", "container", "daemon",
"--timeout", workspaceInfo.ContainerTimeout,
}
if shutdownAction != "" {
args = append(args, "--shutdown-action", shutdownAction)
}

daemonCmd := &exec.Cmd{
Path: binaryPath,
Args: append([]string{binaryPath}, args...),
}
return daemonCmd, nil
})
}

Expand Down
9 changes: 8 additions & 1 deletion cmd/agent/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import (
type DaemonCmd struct {
*flags.GlobalFlags

Interval string
Interval string
ShutdownAction string
}

// NewDaemonCmd creates a new command.
Expand All @@ -39,6 +40,8 @@ func NewDaemonCmd(flags *flags.GlobalFlags) *cobra.Command {
}
daemonCmd.Flags().
StringVar(&cmd.Interval, "interval", "", "The interval how to poll workspaces")
daemonCmd.Flags().
StringVar(&cmd.ShutdownAction, "shutdown-action", "", "The shutdown action (none or stopContainer)")
return daemonCmd
}

Expand Down Expand Up @@ -126,6 +129,10 @@ func (cmd *DaemonCmd) checkAndShutdown(
latestActivity *time.Time,
workspace *provider2.AgentWorkspaceInfo,
) {
if cmd.ShutdownAction == "none" {
return
}

// check timeout
timeout := agent.DefaultInactivityTimeout
if workspace.Agent.Timeout != "" {
Expand Down
7 changes: 7 additions & 0 deletions cmd/agent/workspace/up.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,17 @@ func installDaemon(workspaceInfo *provider.AgentWorkspaceInfo) error {
return nil
}

var shutdownAction string
if workspaceInfo.LastDevContainerConfig != nil &&
workspaceInfo.LastDevContainerConfig.Config != nil {
shutdownAction = workspaceInfo.LastDevContainerConfig.Config.ShutdownAction
}

log.Debugf("installing Devsy daemon into server")
return agentdaemon.InstallDaemon(
workspaceInfo.Agent.DataPath,
workspaceInfo.CLIOptions.DaemonInterval,
shutdownAction,
)
}

Expand Down
56 changes: 56 additions & 0 deletions e2e/tests/machineprovider/machineprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,5 +169,61 @@ var _ = ginkgo.Describe(
"machine did not shutdown in time",
)
})

ginkgo.It("test shutdownAction none suppresses inactivity timeout",
ginkgo.SpecTimeout(framework.GetTimeout()*5),
func(ctx context.Context) {
f := framework.NewDefaultFramework(initialDir + "/bin")

// copy test dir — uses devcontainer.json with shutdownAction: "none"
tempDir, err := framework.CopyToTempDirWithoutChdir(
initialDir + "/tests/machineprovider/testdata/machineprovider3",
)
framework.ExpectNoError(err)
ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir)

// create provider (same 5s inactivity timeout as machineprovider2)
_ = f.DevsyProviderDelete(ctx, "docker123")
err = f.DevsyProviderAdd(ctx, filepath.Join(tempDir, "provider.yaml"))
framework.ExpectNoError(err)
ginkgo.DeferCleanup(func(cleanupCtx context.Context) {
err = f.DevsyWorkspaceDelete(cleanupCtx, tempDir)
framework.ExpectNoError(err)
err = f.DevsyProviderDelete(cleanupCtx, "docker123")
framework.ExpectNoError(err)
})

// wait for devsy workspace to come online
err = f.DevsyUp(ctx, tempDir, "--debug", "--daemon-interval=3s")
framework.ExpectNoError(err)

// check initial status
status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false")
framework.ExpectNoError(err)
framework.ExpectEqual(
strings.ToUpper(status.State),
"RUNNING",
"workspace status did not match",
)

// stop and restart to trigger timeout monitor path
err = f.DevsyStop(ctx, tempDir)
framework.ExpectNoError(err)

err = f.DevsyUp(ctx, tempDir, "--daemon-interval=3s")
framework.ExpectNoError(err)

// verify workspace stays running well past the 5s timeout.
// The timeout would fire within ~15s (5s timeout + 10s ticker).
// We assert RUNNING for 30s to give ample margin.
gomega.Consistently(func() string {
status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false")
framework.ExpectNoError(err)
return strings.ToUpper(status.State)
}, 30*time.Second, 2*time.Second).Should(
gomega.Equal("RUNNING"),
"workspace should stay running when shutdownAction is none",
)
})
Comment on lines +173 to +227

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

LGTM — e2e coverage correctly exercises the shutdownAction: "none" path.

The flow (up → stop → up → Consistently RUNNING for 30s) exercises the code path that decodes the persisted DaemonConfig (including ShutdownAction) on restart, which is exactly where the new gating in cmd/agent/container/daemon.go runs. The 30s window comfortably exceeds the worst-case fire time (5s timeout + 10s ticker period in runTimeoutMonitor).

One minor consideration: because shutdownAction: "none" disables auto-stop, this test relies entirely on DevsyWorkspaceDelete in DeferCleanup to tear the container down. If the test aborts before DeferCleanup runs (e.g., SIGKILL on CI), a stray devsy-${MACHINE_ID} container could linger and interfere with later runs. Not a blocker — existing tests have the same pattern — but worth keeping in mind for CI flakiness triage.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@e2e/tests/machineprovider/machineprovider.go` around lines 173 - 227, Test
exercises shutdownAction:"none" path but may leave stray containers if the test
process is killed before DeferCleanup runs; to mitigate, add a defensive cleanup
step that forcibly deletes the workspace/container at start and/or end of the
test to ensure no stray devsy-${MACHINE_ID} containers remain: call
f.DevsyWorkspaceDelete (or f.DevsyProviderDelete) and ignore not-found errors
before creating the provider and again in the ginkgo.DeferCleanup closure (or
add an explicit defer immediately after tempDir creation) so the test always
attempts to remove the workspace/container even if the test is aborted;
reference DevsyWorkspaceDelete, DevsyProviderDelete, and the ginkgo.DeferCleanup
closure in machineprovider.go to locate where to add these calls.

},
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "Go",
"image": "ubuntu",
"shutdownAction": "none"
}
44 changes: 44 additions & 0 deletions e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: docker123
version: 0.0.1
description: |-
Devsy on Kubernetes
options:
NAMESPACE:
description: The namespace to use
default: devsy-e2e
INACTIVITY_TIMEOUT:
description: The timeout until the pod will be stopped
default: 5s
agent:
path: /usr/local/bin/devsy
inactivityTimeout: ${INACTIVITY_TIMEOUT}
exec:
shutdown: |-
kill 1
exec:
command: |-
docker exec -i devsy-${MACHINE_ID} sh -c "${COMMAND}"
stop: |-
docker stop devsy-${MACHINE_ID}
start: |-
docker start devsy-${MACHINE_ID}
sleep 5
status: |-
STATUS=$(docker inspect devsy-${MACHINE_ID} 2>/dev/null | ${DEVSY} helper json get "[0].State.Status" || true)
if [ -z $STATUS ]; then
echo "NOTFOUND"
else
if [ "$STATUS" = "exited" ]; then
echo "STOPPED"
elif [ "$STATUS" = "running" ]; then
echo "RUNNING"
else
echo "BUSY"
fi
fi
create: |-
docker run -d --privileged --name devsy-${MACHINE_ID} docker
docker exec devsy-${MACHINE_ID} mkdir /etc/init.d
sleep 5
delete: |-
docker stop devsy-${MACHINE_ID} && docker rm devsy-${MACHINE_ID}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Test123
22 changes: 16 additions & 6 deletions pkg/daemon/agent/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ type SshConfig struct {
}

type DaemonConfig struct {
Platform devsy.PlatformOptions `json:"platform"`
Ssh SshConfig `json:"ssh"`
Timeout string `json:"timeout"`
Platform devsy.PlatformOptions `json:"platform"`
Ssh SshConfig `json:"ssh"`
Timeout string `json:"timeout"`
ShutdownAction string `json:"shutdownAction,omitempty"`
}

func BuildWorkspaceDaemonConfig(
Expand Down Expand Up @@ -60,12 +61,18 @@ func BuildWorkspaceDaemonConfig(
// build info isn't required in the workspace and can be omitted
platformOptions.Build = nil

shutdownAction := "stopContainer"
if mergedConfig.ShutdownAction != "" {
shutdownAction = mergedConfig.ShutdownAction
}

daemonConfig := &DaemonConfig{
Platform: platformOptions,
Ssh: SshConfig{
Workdir: workdir,
User: user,
},
ShutdownAction: shutdownAction,
}

return daemonConfig, nil
Expand Down Expand Up @@ -155,7 +162,7 @@ func quoteSystemdArg(arg string) string {
return arg
}

func InstallDaemon(agentDir string, interval string) error {
func InstallDaemon(agentDir, interval, shutdownAction string) error {
if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
return fmt.Errorf("unsupported daemon os")
}
Expand All @@ -165,7 +172,7 @@ func InstallDaemon(agentDir string, interval string) error {
return fmt.Errorf("get executable path: %w", err)
}

args := buildDaemonArgs(executable, agentDir, interval)
args := buildDaemonArgs(executable, agentDir, interval, shutdownAction)

if !isSystemdAvailable() {
log.Warnf("systemd not available, falling back to background process")
Expand All @@ -186,14 +193,17 @@ func InstallDaemon(agentDir string, interval string) error {
return ensureServiceRunning(needsReload, executable, args)
}

func buildDaemonArgs(executable, agentDir, interval string) []string {
func buildDaemonArgs(executable, agentDir, interval, shutdownAction string) []string {
args := []string{executable, "agent", "daemon"}
if agentDir != "" {
args = append(args, "--agent-dir", agentDir)
}
if interval != "" {
args = append(args, "--interval", interval)
}
if shutdownAction != "" {
args = append(args, "--shutdown-action", shutdownAction)
}
return args
}

Expand Down
53 changes: 53 additions & 0 deletions pkg/daemon/agent/daemon_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package agent

import (
"testing"

"github.com/devsy-org/api/pkg/devsy"
"github.com/devsy-org/devsy/pkg/devcontainer/config"
provider2 "github.com/devsy-org/devsy/pkg/provider"
)

func TestBuildWorkspaceDaemonConfig_ShutdownAction(t *testing.T) {
tests := []struct {
name string
shutdownAction string
want string
}{
{
name: "defaults to stopContainer when empty",
shutdownAction: "",
want: "stopContainer",
},
{
name: "preserves none",
shutdownAction: "none",
want: "none",
},
{
name: "preserves stopContainer",
shutdownAction: "stopContainer",
want: "stopContainer",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
merged := &config.MergedDevContainerConfig{}
merged.ShutdownAction = tt.shutdownAction

cfg, err := BuildWorkspaceDaemonConfig(
devsy.PlatformOptions{},
&provider2.Workspace{},
&config.SubstitutionContext{},
merged,
)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if cfg.ShutdownAction != tt.want {
t.Errorf("ShutdownAction = %q, want %q", cfg.ShutdownAction, tt.want)
}
})
}
}
Loading