Skip to content
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

Allow manually remove invalid snapshots on restore #345

Merged
merged 1 commit into from
Feb 13, 2023

Conversation

hanyuel
Copy link
Contributor

@hanyuel hanyuel commented Jan 24, 2023

Issue #, if available: closes #93
The problem that #93 tries to fix is that if the snapshotter crashes, it won't be able to restart because the snapshotter won't be able to restore the remote snapshots (this will error out), so it will crash again and again.

Description of changes:
cherry-pick containerd/stargz-snapshotter#901

Testing performed:
I was not able to reproduce the problem (details above) by killing (SIGKILL-ing) the soci-snapshotter-grpc process when the container is running. I might have repro'd once... so I was not able to verify this PR fixes the problem. But I think this PR is still worth merging considering 1) it's defensive programming 2) it's cherrypicked from stargz repo.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hanyuel hanyuel marked this pull request as ready for review January 24, 2023 19:00
@hanyuel hanyuel requested a review from a team as a code owner January 24, 2023 19:00
@Kern--
Copy link
Contributor

Kern-- commented Jan 24, 2023

Can you provide details on how you tested this?

@hanyuel
Copy link
Contributor Author

hanyuel commented Jan 26, 2023

Can you provide details on how you tested this?

Check the updated Testing section in PR description

sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 1, 2023
sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 1, 2023
sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 2, 2023
sparr added a commit that referenced this pull request Feb 2, 2023
sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 3, 2023
sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 6, 2023
sparr added a commit to sparr/soci-snapshotter that referenced this pull request Feb 6, 2023
Kern--
Kern-- previously approved these changes Feb 8, 2023
sbuckfelder
sbuckfelder previously approved these changes Feb 13, 2023
Copy link
Contributor

@sbuckfelder sbuckfelder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

turan18
turan18 previously approved these changes Feb 13, 2023
@hanyuel hanyuel dismissed stale reviews from turan18, sbuckfelder, and Kern-- via a12bd81 February 13, 2023 15:56
Copy link
Contributor

@sbuckfelder sbuckfelder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hanyuel hanyuel merged commit 9148a9a into awslabs:main Feb 13, 2023
@hanyuel hanyuel deleted the snapshot-restore branch February 13, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow manually remove invalid snapshots on restore
6 participants