Add more diags on fast-launch enable step #446
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The logic for watiting for fast-launch to enable checks that the state transitions to either a success or failure state. While this is not a problem on its own (we don't want to wait indefinitely for a success if we already errored), we shouldn't treat the error states as a success, which is what the plugin is doing now. This commit therefores adds some extra logic to get the status of the fast-launch for the image, and error if it's not in a 'enabled' state.
The AWS tests with Windows/FastBoot failed because the AMI referenced was not available anymore (likely deprecated). To avoid this being a problem in the future, we use the AMI datasource to get the latest windows server 2016 core English for the test, so we don't risk the AMI ID being obsolete.
Since the same name was used for multiple tests, only the last one would produce an output, not each individual test. So to avoid this problem, we forward the test name we defined for each subtest as the acceptance test's name, so the logs and configuration are not overwritten.
When the fast launch step runs, it can do nothing if no AMI is found in state, which may be confusing to users if it succeeds silently and they didn't expect it. To avoid this, we add an explicit warning when that happens, as it could be either due to user input, or a plugin bug, but at least they will have an idea of what's going on regarding the feature.
JenGoldstrich
approved these changes
Dec 20, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following up on issue #445, this PR adds more diagnostics to the step that enables fast-launch.
Since the wait step only waited for the state to change to either "enabled", "enabled-failed" or "enabling-failed", but later on the code didn't check the state was indeed "enabled", we could end-up in a situation where the fast-launch step failed on AWS, but the plugin would count it as a success.
Note: I couldn't replicate such a scenario in my tests, but this remains a possibility, so I hope the community will someday share a template that we can adapt to ensure the plugin does take this into account.
Also, added some checks to signal the step is being skipped if no AMI's present in the plugin's state bag, as otherwise the step would silently succeed without doing anything.
Finally, we also fix the acceptance tests, as the AMI we picked for them did not exist anymore, and therefore would always fail.