Retry downloading the boot image if it fails non-prod environment #6
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 boot image download can fail due to various reasons, such as network. We don't want it to cause failure in our E2E tests. This commit adds a retry logic to the boot image download in the E2E tests. I didn't add the retry logic for production case, because we might want to have a deeper look before blindly retrying.
If the image download fails, the temp file is left behind, which breaks the idempotency of the image download. This commit adds a cleanup step to remove the temp file if the download fails.
If two concurrent processes try to write to the same file, the second one would be able to open the temp file and would wait until the first one finishes, but the first one renames the temp file, which leaves the second process in a weird state. We fixed this problem in control plane's safe_write_to_file method, but we didn't apply the fix in the data plane version of it. This commit fixes the problem in the data plane version of the method by locking a lock file before opening the temp file.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Introduced retry logic for boot image downloads in non-production environments
- Ensured cleanup of temporary files if download fails
- Maintained idempotency by removing leftover temp files
- Added retry mechanism to improve E2E test reliability
- Changes do not affect production environments
4 file(s) reviewed, 1 comment(s)
| else | ||
| f.puts(content) | ||
| File.write(temp_filename, content) | ||
| end | ||
| File.rename(temp_filename, filename) |
There was a problem hiding this comment.
Consider adding error handling for File.rename to ensure the temporary file is cleaned up in case of failure.
|
this is the reference pr: ubicloud#1682 |
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.
Retry downloading the boot image if it fails non-prod environment
The boot image download can fail due to various reasons, such as network. We
don't want it to cause failure in our E2E tests. This commit adds a retry logic
to the boot image download in the E2E tests. I didn't add the retry logic for
production case, because we might want to have a deeper look before blindly
retrying.
Clean up the temp file if image download fails
If the image download fails, the temp file is left behind, which breaks
the idempotency of the image download. This commit adds a cleanup step
to remove the temp file if the download fails.