-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Port some 0.8.7 alloc runner tests #5349
Conversation
04dca99
to
80d2396
Compare
// TestAllocRuner_HandlesArtifactFailure ensures that if one task in a task group is | ||
// retrying fetching an artifact, other tasks in the group should be able | ||
// to proceed. | ||
func TestAllocRunner_HandlesArtifactFailure(t *testing.T) { |
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.
I changed the test name from TestAllocRunner_RetryArtifact
as we never tested that artifacts downloading get retried.
0c89ec8
to
138b30f
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.
Looks great! Thanks for porting so many. I know it's not exciting work. Core test logic all looks great.
require.NoErrorf(t, err, "%v not rendered", f2) | ||
} | ||
|
||
func TestTaskRunner_Template_NewVaultToken(t *testing.T) { |
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.
I've read the test twice and am still not positive what a good 1 line description would be 😬
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.
Your guess is as good as mine - I'm guessing that it's testing that when using consul templates, a new token is derived that gets revoked after alloc terminates?
I don't quite understand the EmbeddedTmpl
value; but I'm guessing it's an arbitrary value to trigger vault token creation.
Port TestAllocRunner_RetryArtifact from https://github.com/hashicorp/nomad/blob/v0.8.7/client/alloc_runner_test.go#L610-L672 I changed the test name because it doesn't actually test that artifact hooks is retried
When Vault token expires and task is restarted, emit `TaskRestartSignal` similar to v0.8.7
bf5e73f
to
e1e5053
Compare
@schmichael this is ready for another review now - I rebased and it's green. |
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
No description provided.