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

Writable snapshot support #92

Closed
wants to merge 10 commits into from
Closed

Writable snapshot support #92

wants to merge 10 commits into from

Conversation

donatwork
Copy link
Contributor

@donatwork donatwork commented Feb 18, 2025

PR Submission checklist

PR adds support for the basic APIs for managing writable snapshots.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1763

Common PR Checklist:

  • Have you made sure that the code compiles?
  • Have you commented your code, particularly in hard-to-understand areas
  • Did you run tests in a real Kubernetes cluster?
  • Have you maintained backward compatibility - No Modified an existing method to the public Quota operation to include the snapshots.

Description of your changes:

Added the basic Get/List/Create/Delete functions for managing writable snapshots.

) (string, error) {
// PAPI call: POST https://1.2.3.4:8080/platform/1/quota/quotas
// { "enforced" : true,
// "include_snapshots" : false,
// "include_snapshots" : true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to retain the current behavior unless it is really required. Snapshot doesn't consume much space- maybe a few bytes for the metadata, and the size is not directly related to the source volume size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Writable snaphots could diverge. I do agree that we probably should not include snapshots but this is in goscaleio so the work is already done to expose this so let the applications decide on the business rules. Does not hurt to keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although by exposing this we break compatibility. Terraform uses this module as well.

@donatwork donatwork changed the title Writeable snapshot support Writable snapshot support Feb 20, 2025
@donatwork donatwork marked this pull request as ready for review February 20, 2025 00:17
@donatwork donatwork requested a review from bpjain2004 February 20, 2025 00:18
@donatwork
Copy link
Contributor Author

Closing PR for now as the solution for the GitHub issue is under discussion.

@donatwork donatwork closed this Feb 20, 2025
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.

2 participants