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

Multi-zone PVC's seem to break k8s-snapshotter #91

Closed
joshua-vandenhoek opened this issue Nov 28, 2019 · 5 comments
Closed

Multi-zone PVC's seem to break k8s-snapshotter #91

joshua-vandenhoek opened this issue Nov 28, 2019 · 5 comments

Comments

@joshua-vandenhoek
Copy link
Contributor

joshua-vandenhoek commented Nov 28, 2019

Hey there folks, so I am running this on a GKE cluster with dynamically provisioned PVs. The PVs are multi-zone out of necessity. I looked through the logs of the k8s-snapshotter pod and found (among the other surrounding errors) this line:

"Invalid value 'us-central1-c__us-central1-f'. Values must match the following regular expression: '[a-z](?:[-a-z0-9]{0,61}[a-z0-9])?'"

So I see that in the file k8s_snapshots/backends/google.py the zone field is being pulled from "failure-domain.beta.kubernetes.io/zone". I checked my PV and sure enough:

labels: failure-domain.beta.kubernetes.io/region: us-central1 failure-domain.beta.kubernetes.io/zone: us-central1-c__us-central1-f
All of our PVs are multi-zone, and we would really like to use k8s-snapshotter. If you can provide some insight as to how to resolve this issue, that would be great. Thanks all, let me know if you need more info!

EDIT:
Ok, so in doing some more research, it seems that disks().createSnapshot() only seems to work for zonal resources, and not with regional resources, like we are using. The key seems to be using the regionDisks().createSnapshot() is the key, but there is definitely some re-writing that needs to be done in order to incorporate regional PVCs into this project (I used the term multi-zone earlier, but really I should have said "regional")

@miracle2k
Copy link
Owner

Now that #92 is merged, do you think this would resolve your issue?

@joshua-vandenhoek
Copy link
Contributor Author

Hey there @miracle2k ! Yes, now that my PR has been merged, this issue can be closed. Thanks!

@miracle2k
Copy link
Owner

Didn't realize it was from you :) Perfect!

@joshua-vandenhoek
Copy link
Contributor Author

Hey there @miracle2k , so as per your comment above, you say you have merged our PR #92 to master, but when you go to the main page on GitHub, it does not seem like any of our changes are actually present. I'm not sure of how you control the flow of code up to master, perhaps I am missing something. At any rate, if you could have a look as to why our code hasn't made its way there yet I would be most appreciative. Thanks!

@miracle2k
Copy link
Owner

Not sure what actually happened there. It is included now.

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

No branches or pull requests

2 participants