-
Notifications
You must be signed in to change notification settings - Fork 22
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
feat: async image pull and mount #71
Merged
mugdha-adhav
merged 1 commit into
warm-metal:master
from
vadasambar:feat/async-image-pull
Nov 30, 2023
Merged
feat: async image pull and mount #71
mugdha-adhav
merged 1 commit into
warm-metal:master
from
vadasambar:feat/async-image-pull
Nov 30, 2023
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
vadasambar
changed the title
[DON'T REVIEW] feat: wip add tests to reproduce
feat: async image pull and mount
Nov 8, 2023
context deadline exceeded
problem
Check if this error is unrelated to the current PR: #74 |
vadasambar
commented
Nov 14, 2023
dinesh
reviewed
Nov 17, 2023
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.
Overall, encouraging. Have few questions.
This was referenced Nov 30, 2023
mugdha-adhav
approved these changes
Nov 30, 2023
vadasambar
commented
Nov 30, 2023
vadasambar
commented
Nov 30, 2023
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
feat: wip add tests to reproduce `context deadline exceeded` problem test: add test to call `NodePublishVolume` constantly for 2 minutes until image is downloaded - this should fail for the current code feat: wip add logic for async pull - not working right now fix: async image pull timing out - mount is still a bottleneck feat: wip working async mount - fails with race conditions infrequently (<- looking at this) feat: fix concurrency errors - abstract mounting and pulling logic to separate packages - add readme for running tests refactor: remove redundant `uuid` related code fix: CI lint/fmt errors - use `defer cancel()` instead of discarding the fn - fix log fix: update mount status on finishing sync mount chore: remove extra whitespace in the log refactor: use localized error - to address PR comment and avoid confusion refactor: define magic numbers as constants refactor: define mount and pull ctx timeout as constants refactor: rename `--async-image-pulls` -> `--async-pull-mount` fix: ci failing because of image pull error fix: mounter is nil fix: image not mounting if the image already exists on the node fix: mounting the same volume more than once doesn't work refactor: add log if image hasn't been pulled or mount is still in progress refactor: make `NewNodeServer` call more compact refactor: defer unlocking mutex instead of explicitly calling unlock at multiple places refactor: remove leftover `ctx` in print statement chore: bump version in Makefile and Chart.yaml chore: revert bump to `v0.8.0` - causes CI to fail fix: remove conflict markers in Chart.yaml - was causing CI to fail chore: add a space after Chart.yaml - to remove newline diff in the PR
4 tasks
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.
Problem
warm metal driver throws
context deadline exceeded
errorsThis happens because
Solution
We pull and mount images asynchronously so that:
Tests
With asynchronous pull and mount:
With synchronous pull and mount:
Note that test passing here means, warm metal was not able to mount the image within 37 seconds