diff --git a/job/job.go b/job/job.go index bbc81e4..5709616 100644 --- a/job/job.go +++ b/job/job.go @@ -95,7 +95,7 @@ func (c *registry) Stop(ctx cell.HookContext) error { return nil } c.started = false - return c.runtimeLifecycle.stop(ctx) + return c.runtimeLifecycle.stop(ctx, c.logger) } // PreStopHookMarker tells [cell.DefaultLifecycle] that this diff --git a/job/lifecycle.go b/job/lifecycle.go index b89af2b..ed63d65 100644 --- a/job/lifecycle.go +++ b/job/lifecycle.go @@ -5,6 +5,7 @@ package job import ( "context" + "log/slog" "sync" "github.com/cilium/hive/cell" @@ -56,7 +57,7 @@ func (r *jobLifecycle) remove(qj *queuedJob) { qj.next = nil } -func (r *jobLifecycle) stop(ctx cell.HookContext) error { +func (r *jobLifecycle) stop(ctx cell.HookContext, log *slog.Logger) error { // Collect jobs to stop and unlink them. We must stop them without holding the // lock as [queuedJob.Stop] will try to call [jobLifecycle.remove]. var jobsToStop []*queuedJob @@ -73,6 +74,9 @@ func (r *jobLifecycle) stop(ctx cell.HookContext) error { for _, job := range jobsToStop { job.Stop(ctx) if ctx.Err() != nil { + log.Error("Stop cancelled while waiting for job to stop", + "job", + job.job.info()) break } } diff --git a/job/lifecycle_test.go b/job/lifecycle_test.go index 0dea481..a7ff841 100644 --- a/job/lifecycle_test.go +++ b/job/lifecycle_test.go @@ -14,6 +14,7 @@ import ( "github.com/stretchr/testify/require" "github.com/cilium/hive/cell" + "github.com/cilium/hive/hivetest" ) func TestJobLifecycleInsertAndRemove(t *testing.T) { @@ -30,15 +31,15 @@ func TestJobLifecycleInsertAndRemove(t *testing.T) { requireRuntimeLifecycleList(t, r, third, second, first) - removeRuntimeLifecycleJob(r, second) + r.runtimeLifecycle.remove(second) requireRuntimeLifecycleList(t, r, third, first) requireRuntimeLifecycleJobUnlinked(t, r, second) - removeRuntimeLifecycleJob(r, third) + r.runtimeLifecycle.remove(third) requireRuntimeLifecycleList(t, r, first) requireRuntimeLifecycleJobUnlinked(t, r, third) - removeRuntimeLifecycleJob(r, first) + r.runtimeLifecycle.remove(first) requireRuntimeLifecycleList(t, r) requireRuntimeLifecycleJobUnlinked(t, r, first) } @@ -86,7 +87,7 @@ func TestJobLifecycleStopStopsJobsInReverseStartOrder(t *testing.T) { insertRuntimeLifecycleJobs(r, first, second, third) waitForLifecycleJobs(t, first, second, third) - require.NoError(t, stopRuntimeLifecycle(r, context.Background())) + require.NoError(t, r.runtimeLifecycle.stop(context.Background(), hivetest.Logger(t))) requireRuntimeLifecycleList(t, r) @@ -102,7 +103,7 @@ func TestJobLifecycleStopReturnsContextError(t *testing.T) { cancel() var lifecycle jobLifecycle - assert.ErrorIs(t, lifecycle.stop(ctx), context.Canceled) + assert.ErrorIs(t, lifecycle.stop(ctx, hivetest.Logger(t)), context.Canceled) } type blockingLifecycleJob struct { @@ -180,14 +181,6 @@ func insertRuntimeLifecycleJobs(r *registry, jobs ...*queuedJob) { } } -func removeRuntimeLifecycleJob(r *registry, qj *queuedJob) { - r.runtimeLifecycle.remove(qj) -} - -func stopRuntimeLifecycle(r *registry, ctx context.Context) error { - return r.runtimeLifecycle.stop(ctx) -} - func requireRuntimeLifecycleList(t *testing.T, r *registry, want ...*queuedJob) { t.Helper()