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
2 changes: 1 addition & 1 deletion job/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion job/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package job

import (
"context"
"log/slog"
"sync"

"github.com/cilium/hive/cell"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
}
Expand Down
19 changes: 6 additions & 13 deletions job/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)

Expand All @@ -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 {
Expand Down Expand Up @@ -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()

Expand Down
Loading