Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

internal/commands/server: prevent test panic on timeout #5249

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

johanbrandhorst
Copy link
Collaborator

@johanbrandhorst johanbrandhorst commented Nov 15, 2024

Previously, if one of these tests failed to start their servers before the timeout, there was a risk that the goroutine started to run the server would invoke t.Errorf after the test itself had concluded. Replace t.Errorf with fmt.Printf to avoid
the panic, and include the test name to help debugging.

These panics seem to have started happening recently - I recorded two instances in just the last two days. You can see the panic summary at the end of the test log, but also note the "timeout" message printed in the tests:

  1. https://github.com/hashicorp/boundary/actions/runs/11846826652/job/33015283733?pr=5242#step:9:29792
  2. https://github.com/hashicorp/boundary/actions/runs/11847490310/job/33017292501?pr=5245#step:9:12181

@johanbrandhorst johanbrandhorst added this to the 0.19.x milestone Nov 15, 2024
@github-actions github-actions bot added the core label Nov 15, 2024
@johanbrandhorst
Copy link
Collaborator Author

Looks like the test got stuck instead of panicing 😂. I'll see what I can do about it.

Previously, if one of these tests failed to start their servers before
the timeout, there was a risk that the goroutine started to run
the server would invoke t.Errorf after the test itself had concluded.
Replace t.Errorf with fmt.Printf to avoid the panic, and include
the test name for debugging.
@johanbrandhorst
Copy link
Collaborator Author

Yikes the cmd.Run function doesn't seem to return in some instances, and it doesn't seem to respect context cancelation, so there's no great way to ensure the function exits while the test is running, it seems.

@johanbrandhorst
Copy link
Collaborator Author

The new logging has gotten us a step closer to figuring out what's going on with these tests. It looks like most of them failed with a timeout this time. Errors are all looking like this:

TestReloadControllerDatabase: got a non-zero exit status: Error starting controller: error registering jobs: vault.RegisterJobs: token renewal job: scheduler.(Scheduler).RegisterJob: job.(Repository).UpsertJob: db.DoTx: unknown, unknown: error #0: dbw.Begin: failed to connect to `user=boundary database=boundary_test_qltqrubshvmfdnux`: 127.0.0.1:5432 (127.0.0.1): server error: FATAL: database "boundary_test_qltqrubshvmfdnux" does not exist (SQLSTATE 3D000)

1

TestReloadControllerRateLimits: got a non-zero exit status: Error initializing controller: error registering gcp host plugin: error looking up plugin by name: plugin.(Repository).LookupPluginByName: failed for: gcp: db.LookupWhere: unknown, unknown: error #0: dbw.LookupWhere: failed to connect to `user=boundary database=boundary_test_unncgapurajovkiw`: 127.0.0.1:5432 (127.0.0.1): server error: FATAL: database "boundary_test_unncgapurajovkiw" does not exist (SQLSTATE 3D000)

2

 TestReloadControllerRateLimitsDisable: got a non-zero exit status: Error initializing controller: error registering gcp host plugin: error looking up plugin by name: plugin.(Repository).LookupPluginByName: failed for: gcp: db.LookupWhere: unknown, unknown: error #0: dbw.LookupWhere: failed to connect to `user=boundary database=boundary_test_vmokqfhgfaokjgut`: 127.0.0.1:5432 (127.0.0.1): server error: FATAL: database "boundary_test_vmokqfhgfaokjgut" does not exist (SQLSTATE 3D000)

3

Seems like something went wrong with the database startup?

@johanbrandhorst johanbrandhorst marked this pull request as draft November 18, 2024 18:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants