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

kubernetes_persistent_volume resource should support storage_class_name #72

Closed
wants to merge 1 commit into from

Conversation

coderfi
Copy link

@coderfi coderfi commented Sep 19, 2017

This parameter is optional.
Allows for the fix described in https://stackoverflow.com/a/44891419

@sam-at-luther
Copy link

I believe adding this feature will fix the issues I mentioned in #74.

@ccope
Copy link

ccope commented Nov 20, 2017

I'm generally 👍 on this, it's less broken and more in-line with the recommendations in https://kubernetes.io/docs/concepts/storage/persistent-volumes/#writing-portable-configuration

I have one concern about backwards-compatibility: What would happen if a user has dynamically created a PV with a PVC claim using the current default storage_class_name value? Does Terraform maintain enough state to realize that the PV and PVC don't need to be created/updated?

The behavior of the default value of storageclassname in a PVC can be complicated, and may need some follow-up work: https://kubernetes.io/docs/concepts/storage/persistent-volumes/#class-1

@@ -29,6 +29,7 @@ resource "kubernetes_persistent_volume" "example" {
volume_path = "/absolute/path"
}
}
storage_class_name = "standard"
Copy link

Choose a reason for hiding this comment

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

This should probably be left out of the example resource, because it should be left empty by default.

@ccope
Copy link

ccope commented Nov 20, 2017

I'd also mark this as a bugfix, not just an enhancement. You can't currently create a PVC in Terraform for PV.

@moritzheiber
Copy link

Could we please get this PR merged? Deploying apps with state currently is pretty much impossible with this provider

@moritzheiber
Copy link

@coderfi Would you mind rebasing?

@fromz
Copy link

fromz commented Feb 26, 2018

Is anyone motivated to merge this? or has there been an alternative solution merged into master? This is a huge issue for people attempting to adopt terraform for kubernetes

@coderfi
Copy link
Author

coderfi commented Feb 27, 2018

Glad this was addressed in #111. Sorry for not keeping up with this PR.

@ghost ghost locked and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants