Catch signal while running test runner#277
Conversation
|
Hm.. I found another issue: @ruflin Should it be HTTP 500 or HTTP 4x? |
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪 |
| if err := kib.AssignPolicyToAgent(agent, *policy); err != nil { | ||
| return result.WithError(errors.Wrap(err, "could not assign policy to agent")) | ||
| } | ||
|
|
There was a problem hiding this comment.
Just curious, why did this block need to be moved?
There was a problem hiding this comment.
Assigning a policy is a long operation (due to condition "wait for the policy to get assigned"). If you stop here, but in the mean time an agent gets assigned, you won't be able to delete policy. Just in case it's safer to reassign the original policy.
| } | ||
| os.Exit(1) | ||
| }() | ||
|
|
There was a problem hiding this comment.
This definitely works so I don't want to block this PR.
But in case some day elastic-package gets more long-running sub-commands, we might want to consider moving this logic to a higher level (not sure where exactly yet) so that a SIGTERM or SIGINT could invoke tear down, if implemented, on that sub command.
BTW, we already have another long-running sub command: elastic-package stack up (without the -d flag). But it looks like SIGINT is already being handled by the underlying lib.
There was a problem hiding this comment.
I can improve on this in follow-up issues, but this particular one I wanted to focus on improving developer experience of a person writing system tests (and rerunning tests multiple times).
There was a problem hiding this comment.
++ to follow up PRs, as needed.
Fixes: #253
This PR modifies the behavior of test runner to catch the signal, so we can cleanup resources while
ctrl+cis pressed.