-
Notifications
You must be signed in to change notification settings - Fork 721
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
[antithesis] Add tmpnet support to workloads to simplify development #3215
Conversation
24c80b1
to
0367e61
Compare
0367e61
to
f6d2bf4
Compare
f6d2bf4
to
8ec0f5d
Compare
8ec0f5d
to
4f9746b
Compare
4f9746b
to
7c095ec
Compare
7c095ec
to
5e26b9d
Compare
dbaf205
to
060ddee
Compare
060ddee
to
a7ef5ec
Compare
tests/other_test_context.go
Outdated
// TODO(marun) Consider a more descriptive name for this type | ||
type OtherTestContext struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional - seems like renaming this would be worthwhile in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DefaultTestContext
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be weird because it's not a default so much as, if nothing else (ie not ginkgo) fall back to this, but we typically use ginkgo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not a huge fan of Other
... Simple
? Fallback
? Default
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NonGinkgo
is a bit wordy, Default
isn't really true, so Simple
it is. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
860ad11
to
2a454b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - would also like to rename OtherTestContext
... But I don't love any of my ideas
tests/other_test_context.go
Outdated
// TODO(marun) Consider a more descriptive name for this type | ||
type OtherTestContext struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not a huge fan of Other
... Simple
? Fallback
? Default
?
2a454b2
to
d65bff9
Compare
d65bff9
to
b2ea47a
Compare
Why this should be merged
Iterating on the workload for the avalanchego test setup is simple - just launch a local node. Iterating on the tests for any VM is more complicated due to needing to deploy a subnet and chain. This PR adds tmpnet integration to make iterating on an antithesis workload as simple as iterating on an e2e test.
How this works
log.Fatalf
withrequire
and a panic-basedrequire.TestingT
to ensure deferred cleanup functions can execute to enable reliable teardown of tmpnet networkslog.Fatalf
callsos.Exit
which precludes the execution of deferred functionscontext.Background()
with a context created withsignal.NotifyContext
to support orderly shutdown whenSIGINT
(i.e. Ctrl-C) andSIGTERM
signals are receivedHow this was tested
Existing CI, plus added a new sanity check for workloads running with tmpnet
TODO