Skip to content

Treat concurrent default-group creation as success - #6457

Open
lorenzozanee wants to merge 1 commit into
stacklok:mainfrom
lorenzozanee:fix/concurrent-default-group
Open

Treat concurrent default-group creation as success#6457
lorenzozanee wants to merge 1 commit into
stacklok:mainfrom
lorenzozanee:fix/concurrent-default-group

Conversation

@lorenzozanee

Copy link
Copy Markdown
Contributor

Summary

  • Starting several thv processes concurrently could fail with failed to ensure default group exists: group already exists: default when the processes raced the Exists-then-Create window in the default-group startup migration. This is reproducible in the core E2E shard, which runs Ginkgo with PROCS=4.
  • The migration now treats groups.ErrGroupAlreadyExists as success: when another process created the default group first, the desired end state already holds. Explicit group create calls (CLI and the API 409 response) still report the conflict.
  • --help, -h and help are now classified as informational commands, so printing help no longer triggers migrations or the container runtime check.
  • CheckTHVBinaryAvailable now includes stderr in its error so startup failures are diagnosable instead of being masked as thv binary not available.

Fixes #6359

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)
  • Linting (task lint-fix)
  • Manual testing (describe below)

New Test_EnsureDefaultGroupExistsConcurrent spawns 8 concurrent default-group migrations against a fresh shared store and requires all of them to succeed (it fails on main with group already exists: default). Manually verified: 10 rounds of 4 concurrent thv processes against a fresh shared XDG state directory all exit 0 (previously 6/10 rounds had at least one process fail), and thv --help on a fresh directory exits 0 without creating the default group or requiring a container runtime.

Does this introduce a user-facing change?

Yes. Concurrent thv startups that previously failed with group already exists: default now succeed, and thv --help no longer requires a container runtime or triggers migrations.

Concurrent thv startups racing the Exists-then-Create window in
EnsureDefaultGroupExists exited with "group already exists: default"
because the loser's ErrGroupAlreadyExists conflict was treated as a
fatal startup error. The ensure path now treats that conflict as
success: when another process created the default group first, the
desired end state already holds.

--help, -h and help are now informational commands, so printing help
no longer triggers migrations or the container runtime check.
CheckTHVBinaryAvailable now preserves stderr so startup failures are
diagnosable.

Fixes stacklok#6359

Signed-off-by: lorenzozanee <wyz0707@proton.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Handle concurrent default-group creation during startup

1 participant