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

check error in TestDNSCycleRecursorCheckAllFail before asserting response to stop panic in CI. #20231

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

jmurret
Copy link
Member

@jmurret jmurret commented Jan 17, 2024

there is likely a flake in here as well, but this panic causes other tests not to run and stops gotestum from re-running tests with rerun aborted because previous run had a suspected panic and some test may not have run.. This PR is just meant to stabilize the panic and hopefully fixing the flake will be easier with an error message.

the original panic error is below when the test tries to check in.Rcode but has not checked for an error first:

2024-01-17T04:19:11.7723556Z === �[31mFAIL�[0m: agent TestDNSCycleRecursorCheckAllFail (6.18s)
2024-01-17T04:19:11.7724191Z panic: runtime error: invalid memory address or nil pointer dereference [recovered]
2024-01-17T04:19:11.7724772Z 	panic: runtime error: invalid memory address or nil pointer dereference
2024-01-17T04:19:11.7725299Z [signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x330d5bd]
2024-01-17T04:19:11.7725310Z 
2024-01-17T04:19:11.7725551Z goroutine 46254152 [running]:
2024-01-17T04:19:11.7725870Z testing.tRunner.func1.2({0x3977c20, 0x7b98970})
2024-01-17T04:19:11.7726726Z 	/home/runner/actions-runner/_work/_tool/go/1.21.6/x64/src/testing/testing.go:1545 +0x238
2024-01-17T04:19:11.7726952Z testing.tRunner.func1()
2024-01-17T04:19:11.7727805Z 	/home/runner/actions-runner/_work/_tool/go/1.21.6/x64/src/testing/testing.go:1548 +0x397
2024-01-17T04:19:11.7728030Z panic({0x3977c20?, 0x7b98970?})
2024-01-17T04:19:11.7728853Z 	/home/runner/actions-runner/_work/_tool/go/1.21.6/x64/src/runtime/panic.go:914 +0x21f
2024-01-17T04:19:11.7729561Z github.com/hashicorp/consul/agent.TestDNSCycleRecursorCheckAllFail.func1(0x21043ad?)
2024-01-17T04:19:11.7730559Z 	/home/runner/actions-runner/_work/consul-enterprise/consul-enterprise/agent/dns_test.go:324 +0x23d
2024-01-17T04:19:11.7730844Z testing.tRunner(0xc0b4e66b60, 0xc0bc9ac7e0)
2024-01-17T04:19:11.7731675Z 	/home/runner/actions-runner/_work/_tool/go/1.21.6/x64/src/testing/testing.go:1595 +0xff
2024-01-17T04:19:11.7732006Z created by testing.(*T).Run in goroutine 7722171
2024-01-17T04:19:11.7732861Z 	/home/runner/actions-runner/_work/_tool/go/1.21.6/x64/src/testing/testing.go:1648 +0x3ad

Description

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@jmurret jmurret added pr/no-changelog PR does not need a corresponding .changelog entry backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. pr/no-backport backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. labels Jan 17, 2024
@jmurret jmurret requested a review from hashi-derek January 17, 2024 14:04
@jmurret jmurret merged commit bc4da5f into main Jan 17, 2024
123 checks passed
@jmurret jmurret deleted the jm/fix-dns-test-panic branch January 17, 2024 14:25
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
jmurret added a commit that referenced this pull request Jan 17, 2024
…serting response to stop panic in CI. into release/1.16.x (#20234)

check error in TestDNSCycleRecursorCheckAllFail before asserting response to stop panic in CI. (#20231)

Co-authored-by: John Murret <john.murret@hashicorp.com>
jmurret added a commit that referenced this pull request Jan 17, 2024
…serting response to stop panic in CI. into release/1.15.x (#20233)

check error in TestDNSCycleRecursorCheckAllFail before asserting response to stop panic in CI. (#20231)

Co-authored-by: John Murret <john.murret@hashicorp.com>
jmurret added a commit that referenced this pull request Jan 17, 2024
…serting response to stop panic in CI. into release/1.17.x (#20235)

* no-op commit due to failed cherry-picking

* check error in TestDNSCycleRecursorCheckAllFail before asserting response to stop panic in CI. (#20231)

---------

Co-authored-by: temp <temp@hashicorp.com>
Co-authored-by: John Murret <john.murret@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This release series is no longer active on CE. Use backport/ent/1.15. backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants