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

when restoring a PV, don't remove its spec.storageClassName #1246

Merged
merged 2 commits into from
Feb 28, 2019

Conversation

skriss
Copy link
Member

@skriss skriss commented Feb 28, 2019

Signed-off-by: Steve Kriss krisss@vmware.com

Fixes #1097

I tested this on a couple public clouds and it worked fine.

Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss requested review from carlisia and nrb February 28, 2019 21:35
Signed-off-by: Steve Kriss <krisss@vmware.com>
@nrb
Copy link
Contributor

nrb commented Feb 28, 2019

Did you use this with a custom storage class across providers? For example, regional clusters on GCP require the user to create a custom storage class that represents the zones that the underlying volume is replicated against. This storage class isn't necessarily present in the destination cluster, so the PVs couldn't be scheduled.

@skriss
Copy link
Member Author

skriss commented Feb 28, 2019

StorageClasses themselves are restorable, and are properly prioritized ahead of PVs, so I think in your scenario the user could always restore those StorageClasses first or along with the workload. If a user needs to change the storage class a PV is tagged as, or to remove it entirely, I'd think they would be expected to write a restore plugin to do that.

Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@nrb
Copy link
Contributor

nrb commented Feb 28, 2019

@skriss Good points, thanks.

@nrb nrb merged commit fcf2181 into vmware-tanzu:master Feb 28, 2019
@skriss skriss deleted the preserve-storageclass branch March 11, 2019 16:37
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