diff --git a/cli/daemon.go b/cli/daemon.go index 36b39ac560..e6ed479de3 100644 --- a/cli/daemon.go +++ b/cli/daemon.go @@ -14,6 +14,7 @@ import ( "sync" "time" + "github.com/creasty/defaults" "github.com/gobs/args" cfgvalidator "github.com/netresearch/ofelia/config" @@ -609,6 +610,8 @@ func (c *DaemonCommand) buildPersistedRunJob(name string, j *persist.Job, provid return nil, fmt.Errorf("docker provider unavailable for run job") } rj := core.NewRunJob(provider) + // struct-tag defaults are only applied by the config decoder — apply them here too. + _ = defaults.Set(rj) rj.Name = name rj.Schedule = j.Schedule rj.Command = j.Command diff --git a/cli/daemon_persisted_run_job_delete_default_test.go b/cli/daemon_persisted_run_job_delete_default_test.go new file mode 100644 index 0000000000..760818a26a --- /dev/null +++ b/cli/daemon_persisted_run_job_delete_default_test.go @@ -0,0 +1,40 @@ +// Copyright (c) 2025-2026 Netresearch DTT GmbH +// SPDX-License-Identifier: MIT + +package cli + +import ( + "testing" + + "github.com/netresearch/ofelia/core" + "github.com/netresearch/ofelia/core/persist" +) + +// TestBuildPersistedRunJob_DefaultsDelete pins the restore-path half of +// the container-leak fix. buildPersistedRunJob rebuilds a persisted +// job-run on every daemon restart (initPersistStore -> newDaemonForPersistTest +// et al.) through a path entirely separate from newRunJobFromRequest +// (web/server.go) — a job created before this fix, or reloaded across +// any restart, would keep leaking containers even after the API path +// was patched if this path is not fixed too. +func TestBuildPersistedRunJob_DefaultsDelete(t *testing.T) { + t.Parallel() + + c := newDaemonForPersistTest(t) + job, err := c.buildPersistedRunJob("persisted-run-job", &persist.Job{ + Type: persist.JobTypeRun, + Schedule: "@hourly", + Image: "busybox", + }, &mockDockerProvider{}) + if err != nil { + t.Fatalf("buildPersistedRunJob: %v", err) + } + + rj, ok := job.(*core.RunJob) + if !ok { + t.Fatalf("expected *core.RunJob, got %T", job) + } + if rj.Delete != "true" { + t.Fatalf("Delete = %q, want \"true\" — persisted run jobs must clean up their container on every restart", rj.Delete) + } +} diff --git a/web/server.go b/web/server.go index 0aea7df1ae..643e62c4c5 100644 --- a/web/server.go +++ b/web/server.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "github.com/creasty/defaults" "github.com/gobs/args" cron "github.com/netresearch/go-cron" @@ -754,6 +755,8 @@ func (s *Server) newRunJobFromRequest(req *jobRequest) (core.Job, error) { return nil, fmt.Errorf("docker provider unavailable for run job") } j := core.NewRunJob(s.provider) + // struct-tag defaults are only applied by the config decoder — apply them here too. + _ = defaults.Set(j) j.Name = req.Name j.Schedule = req.Schedule j.Command = req.Command diff --git a/web/server_run_job_delete_default_test.go b/web/server_run_job_delete_default_test.go new file mode 100644 index 0000000000..9ea53c1aed --- /dev/null +++ b/web/server_run_job_delete_default_test.go @@ -0,0 +1,46 @@ +// Copyright (c) 2025-2026 Netresearch DTT GmbH +// SPDX-License-Identifier: MIT + +package web + +import ( + "io" + "log/slog" + "testing" + + "github.com/netresearch/ofelia/core" +) + +// TestNewRunJobFromRequest_DefaultsDelete pins the fix for the +// container-leak bug: jobRequest has no "delete" field, and unlike +// config.ini-sourced jobs (whose RunJob.Delete defaults to "true" via +// the decoder's default-tag handling), a job built here from the API +// request used to leave Delete at its zero value (""). deleteContainer +// treats that as false, so every API-created run job's container was +// left behind and collided with the next run's `docker create` on the +// same name ("resource conflict"). Delete must default to "true" here +// to match the config.ini behavior. +func TestNewRunJobFromRequest_DefaultsDelete(t *testing.T) { + t.Parallel() + + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + s := NewServer("", core.NewScheduler(logger), nil, newHangingDockerProvider()) + + job, err := s.newRunJobFromRequest(&jobRequest{ + Name: "api-run-job", + Type: "run", + Schedule: "@hourly", + Image: "busybox", + }) + if err != nil { + t.Fatalf("newRunJobFromRequest: %v", err) + } + + rj, ok := job.(*core.RunJob) + if !ok { + t.Fatalf("expected *core.RunJob, got %T", job) + } + if rj.Delete != "true" { + t.Fatalf("Delete = %q, want \"true\" — API-created run jobs must clean up their container like config.ini ones do", rj.Delete) + } +}