From 9f39f0cbebcbb3c694f7702b33f20e58c1073539 Mon Sep 17 00:00:00 2001 From: Leon Tappe Date: Fri, 17 Jul 2026 15:35:32 +0200 Subject: [PATCH 1/2] fix go vet findings: in-place job construction, buffered signal channel NewCommonJob and NewLazyJob copied baseJob/CommonJob by value even though baseJob contains sync.WaitGroup and atomic.Bool fields; construction now initializes the embedded baseJob in place via (*baseJob).init. The os.Signal channel handed to signal.Notify in cmd/up.go is now buffered so a signal arriving before the fan-out goroutine is scheduled is not lost. Fixes #112. Co-Authored-By: Claude Fable 5 --- cmd/up.go | 2 +- pkg/proc/basejob_test.go | 3 ++- pkg/proc/types.go | 42 +++++++++++++++++----------------------- 3 files changed, 21 insertions(+), 26 deletions(-) diff --git a/cmd/up.go b/cmd/up.go index 15f8772..4ecd1e0 100644 --- a/cmd/up.go +++ b/cmd/up.go @@ -74,7 +74,7 @@ var up = &cobra.Command{ return fmt.Errorf("failed while rendering files from ignition config, err: %w", err) } - signals := make(chan os.Signal) + signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGTERM, syscall.SIGINT, diff --git a/pkg/proc/basejob_test.go b/pkg/proc/basejob_test.go index 28d84cd..cf28654 100644 --- a/pkg/proc/basejob_test.go +++ b/pkg/proc/basejob_test.go @@ -14,7 +14,8 @@ import ( func startTestJob(t *testing.T) (*baseJob, chan error) { t.Helper() - job, err := newBaseJob(&config.BaseJobConfig{ + job := &baseJob{} + err := job.init(&config.BaseJobConfig{ Name: "test-job", Command: "sleep", Args: []string{"30"}, diff --git a/pkg/proc/types.go b/pkg/proc/types.go index 0acdc08..b275e07 100644 --- a/pkg/proc/types.go +++ b/pkg/proc/types.go @@ -131,21 +131,18 @@ type Job interface { GetName() string } -func newBaseJob(jobConfig *config.BaseJobConfig) (*baseJob, error) { - job := &baseJob{ - Config: jobConfig, - cmd: nil, - restart: false, - stop: false, - stdout: os.Stdout, - stderr: os.Stderr, - } +// init initializes the baseJob in place; baseJob must not be copied once +// initialized, since it contains sync.WaitGroup and atomic.Bool fields. +func (job *baseJob) init(jobConfig *config.BaseJobConfig) error { + job.Config = jobConfig + job.stdout = os.Stdout + job.stderr = os.Stderr job.phase.Set(JobPhaseReasonAwaitingReadiness) if len(jobConfig.Stdout) == 0 { - return job, nil + return nil } - return job, job.CreateAndOpenStdFile(jobConfig) + return job.CreateAndOpenStdFile(jobConfig) } func (job *baseJob) CreateAndOpenStdFile(jobConfig *config.BaseJobConfig) error { @@ -174,29 +171,26 @@ func (job *baseJob) CreateAndOpenStdFile(jobConfig *config.BaseJobConfig) error } func NewCommonJob(c *config.JobConfig) (*CommonJob, error) { - job, err := newBaseJob(&c.BaseJobConfig) - if err != nil { - return nil, err + j := CommonJob{ + Config: c, } - j := CommonJob{ - baseJob: *job, - Config: c, + if err := j.baseJob.init(&c.BaseJobConfig); err != nil { + return nil, err } return &j, nil } func NewLazyJob(c *config.JobConfig) (*LazyJob, error) { - commonJob, err := NewCommonJob(c) - if err != nil { - return nil, err + j := LazyJob{ + CommonJob: CommonJob{ + Config: c, + }, } - commonJob.phase.Set(JobPhaseReasonAwaitingReadiness) - - j := LazyJob{ - CommonJob: *commonJob, + if err := j.baseJob.init(&c.BaseJobConfig); err != nil { + return nil, err } if c.Laziness.SpinUpTimeout != "" { From ad4fefd40eaca87d3695669f848ae9b3cf027857 Mon Sep 17 00:00:00 2001 From: Leon Tappe Date: Fri, 24 Jul 2026 14:30:12 +0200 Subject: [PATCH 2/2] address review: open std files for stderr-only configs init returned early when stdout was unset, so a job configured with only stderr never had its stderr file created at init time (carried over from newBaseJob). CreateAndOpenStdFile no-ops for unset targets, so call it unconditionally. Co-Authored-By: Claude Fable 5 --- pkg/proc/types.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/proc/types.go b/pkg/proc/types.go index b275e07..bd6ccb1 100644 --- a/pkg/proc/types.go +++ b/pkg/proc/types.go @@ -138,10 +138,9 @@ func (job *baseJob) init(jobConfig *config.BaseJobConfig) error { job.stdout = os.Stdout job.stderr = os.Stderr job.phase.Set(JobPhaseReasonAwaitingReadiness) - if len(jobConfig.Stdout) == 0 { - return nil - } + // no-ops for unset stdout/stderr, so it is safe to call unconditionally; + // stderr may be configured without stdout return job.CreateAndOpenStdFile(jobConfig) }