[itayherzberg]

Problem

The process never finishes a graceful shutdown. On SIGTERM it logs Stopping enhanced task manager... and then hangs until the kubelet kills it at the end of the 30-second grace period. Every rollout and every helm upgrade takes 30 s longer than it needs to, and nothing after the task manager in Server.Shutdown runs: the job scheduler, the caches, the health checker, and kubevirtClient.Close(), which stops the boot-once / import-pod / CDI-PVC watchers.

Cause

Worker.Start does w.wg.Add(1) and Worker.Stop does w.wg.Wait(), but the work loop started by Start never calls w.wg.Done() when it exits. TaskManager.Stop cancels the context (the loop does return) and then blocks forever on the first worker's Stop.

Unit test added by this PR, run against current main:

--- FAIL: TestWorkerStopReturns (5.00s)
    worker_test.go:42: TaskManager.Stop did not return within 5s: Worker.Stop waits on a WaitGroup that the work loop never releases

On a cluster (upstream b40c637, OpenShift 4.22), deleting the pod — its complete log after SIGTERM, then nothing for 30 s:

Shutting down server...
Stopping enhanced task manager...
Enhanced task manager cleanup routine stopped

pod terminated in 32.54s   (terminationGracePeriodSeconds: 30)

Fix

defer w.wg.Done() at the top of Worker.work.

Verification on the cluster

Same deletion with this branch:

Shutting down server...
Stopping enhanced task manager...
Enhanced task manager cleanup routine stopped
Enhanced task manager stopped, cleaned up 0 tasks
Stopping job scheduler...
Job scheduler stopped, cleared 3 jobs
Stopping memory manager...
…
Stopping health checker...
Health checker stopped
Stopping boot-once watcher
Boot-once watcher stopped
Stopping import pod watcher
Import pod watcher stopped
Stopping CDI PVC watcher
CDI PVC watcher stopped

pod terminated in 3.22s

The whole shutdown sequence now runs, including the watcher stops in kubevirtClient.Close(). Also verified together with #32 (whose informer stop path Close() could never reach before): terminated in 3.18 s, no panic.

Tests

• pkg/server/worker_test.go: TestWorkerStopReturns (fails on current main as shown above, passes with the fix).
• go build ./..., go vet ./..., go test ./..., gofmt, go mod tidy (no change) pass.

Related

One of four independent PRs from running many concurrent BMHs against kubevirt-redfish; each stands on its own and they can merge in any order: #30 (client-go rate limit), #31 (task worker pool), #32 (VM/VMI informer cache).

Found the same way but not fixed here: config hot reload never fires in a pod, because the kubelet updates a mounted Secret by swapping the ..data symlink and the cmd/main.go watcher only reacts to Write/Create events on config.yaml. Happy to open an issue for it.

[kubevirt-prow]

Hi @itayherzberg. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[MarSik]

/lgtm
/approve

[kubevirt-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MarSik

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [MarSik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment