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

Add snapshot validation webhook #407

Merged
merged 1 commit into from
Jan 5, 2022

Conversation

timoreimann
Copy link
Collaborator

@timoreimann timoreimann commented Dec 17, 2021

Add the snapshot validation webhook responsible for labeling invalid snapshots. This is the recommended approach in preparation to installing snapshots v1.

See the upstream docs for details.

@timoreimann timoreimann marked this pull request as ready for review December 19, 2021 21:50
@timoreimann timoreimann force-pushed the add-snapshot-validation-webhook branch 2 times, most recently from d8d1c0b to 8af0a31 Compare December 22, 2021 10:07
Add the snapshot validation webhook responsible for labeling invalid
snapshots. This is the recommended approach in preparation to installing
snapshots v1.
@timoreimann timoreimann force-pushed the add-snapshot-validation-webhook branch from 8af0a31 to 13772e8 Compare December 30, 2021 22:58
@varshavaradarajan
Copy link
Collaborator

@timoreimann - would this cause problems in legacy master droplet not being ready after an upgrade?

@timoreimann
Copy link
Collaborator Author

@varshavaradarajan if the webhook wasn't ready or working, then snapshot creates and updates would fail. Other operations should not be affected. In particular, it should not affect the general provisioning procedure.

Not 100% sure if it answers your question. If not, please let me know.

@varshavaradarajan
Copy link
Collaborator

There have been some support issues where user configured webhooks with FailurePolicy Fail has caused problems right after an upgrade making the master droplet unready. Do we need some sort of check in the bootstrap-master script where initially, the FailurePolicy of this webhook will be Ignore and then after the master is ready, we can set it to Fail?

@timoreimann
Copy link
Collaborator Author

@varshavaradarajan I think the kind of problems you are referring to are about failing web hooks that prevent critical components (e.g., Cilium) from coming up after an upgrade. In contrast, the snapshot validation webhook only applies to volume snapshot create and update requests, so it should not be a problem. At least, I cannot think of a scenario where a failing snapshot request could block provisioning, especially since the CSI driver should retry and eventually succeed once the web hook is up and running.

In the end though, it's up to the concrete user of our FOSS CSI driver to configure the web hook to their needs. As the changes to the README explain, the setup of the webhook in this PR is mostly to provide a (non-production-ready) example (and to enable easy end-to-end testing integration).

@timoreimann timoreimann merged commit a5f847e into master Jan 5, 2022
@timoreimann timoreimann deleted the add-snapshot-validation-webhook branch January 5, 2022 09:44
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.

3 participants