storage_class_name = "" is treated as nil, preventing Google Filestore use

[dpkirchner]

Terraform Version, Provider Version and Kubernetes Version

Terraform version: Terraform v1.0.5 on darwin_amd64
Kubernetes provider version: 2.4.1
Kubernetes version: v1.19.9-gke.1900

Affected Resource(s)

• kubernetes_persistent_volume
• kubernetes_persistent_volume_claim

Terraform Configuration Files

resource "kubernetes_persistent_volume" "tmp" {
  metadata {
    name = "tmp"
  }
  spec {
    capacity = {
      storage = "10Gi"
    }

    access_modes       = ["ReadWriteMany"]
    storage_class_name = ""

    persistent_volume_source {
      nfs {
        path   = "/whatever"
        server = var.filestore_nfs_server_ip
      }
    }
  }
}

resource "kubernetes_persistent_volume_claim" "tmp" {
  metadata {
    name      = "tmp"
    namespace = "default"
  }

  spec {
    access_modes       = ["ReadWriteMany"]
    storage_class_name = ""
    volume_name        = kubernetes_persistent_volume.tmp.metadata.0.name

    resources {
      requests = {
        storage = "10Gi"
      }
    }
  }
}

Debug Output

Can't paste the whole thing, contains proprietary information. Here's the relevant bits though: https://gist.github.com/dpkirchner/76447ded8e97ae7286a83faf034420df

Steps to Reproduce

1. terraform apply
2. kubectl get events -w

Expected Behavior

The PVC should have been created with an empty string for the storage class name.

Actual Behavior

The PVC ends up being created with the default ("standard") storage class name.

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
• If you are interested in working on this issue or have submitted a pull request, please leave a comment

[Akaame]

Hello,

I was able to reproduce your issue and even though I do not have any working knowledge of this library, here are the possible culprits

1. terraform-provider-kubernetes/kubernetes/structure_persistent_volume_claim.go

   Line 82 in b5770d4

   if v, ok := in["storage_class_name"].(string); ok && v != "" {

   
   2.

   terraform-provider-kubernetes/kubernetes/structure_persistent_volume_spec.go

   Line 388 in b5770d4

   if in.StorageClassName != "" {

This here is tricky.

terraform-provider-kubernetes/kubernetes/structure_persistent_volume_spec.go

Line 1230 in b5770d4

if d.HasChange(prefix + "storage_class_name") {

This checks if the old string is empty and emits addop or replaceop accordingly. However, the zero-state for storageClass is not empty string ( as it has a special meaning ) but nil which, I think, unearths another can of worms:

// PVC spec
// Name of the StorageClass required by the claim.
// More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#class-1
// +optional
StorageClassName *string `json:"storageClassName,omitempty" protobuf:"bytes,5,opt,name=storageClassName"`

// PV spec
// Name of StorageClass to which this persistent volume belongs. Empty value
// means that this volume does not belong to any StorageClass.
// +optional
StorageClassName string `json:"storageClassName,omitempty" protobuf:"bytes,6,opt,name=storageClassName"`

I have so several questions here:

1. Why is storageClassName a *string in one spec and string in another?
2. Why is there an omitempty in json encoding tag even though this field being empty has a special meaning?
3. Why is Dynamic Provision suppression bound to storageClassName being empty even though empty/nil strings are known to be a pain point in interoperability?

[steveizzle]

To emphasize the validity of this bug here is a reference to the official kubernetes docs , where you can see the comment

Empty string must be explicitly set otherwise default StorageClass will be set

[davidfischer-ch]

Using an empty storageClassName is something that is required for some of our on-premise use cases.

I had to patch the provider in the meantime with a quick fix.

Linked #590 and #872.

[Lillecarl]

This makes TF unable to handle static configured PV's for the EFS CSI controller too.

[sarg3nt]

This is an OLD problem that keeps getting kicked down the road.
#590
#567
and others.
I wouldn't hold out much hope.

[dinibu]

Yup, actually any class-less PVCs/PVs can't be described as resources currently.

I raised this as a core terraform bug, but they redirected to the provider.

Here's the relevant part:

I'm trying to specify a kubernetes_persistent_volume_claim to a manually defined (i.e. not automatically provisioned) Kubernetes persistent volume. Part of this requires that the storage_class_name attribute should be to be an empty string, as per the K8S docs:

A PVC with its storageClassName set equal to "" is always interpreted to be requesting a PV with no class, so it can only be bound to PVs with no class (no annotation or one set equal to ""). A PVC with no storageClassName is not quite the same and is treated differently by the cluster, depending on whether the DefaultStorageClass admission plugin is turned on.

However, it looks like Terraform K8S provider interprets that as "use the default", which in this case, means that it looks up and uses the default storage class as the field value, and so it does not match with a class-less PV as expected.

A literal empty string is required to tell Kubernetes that it should match against class-less persistent volumes.

It'd be awesome to have a way to set the storage_class_name to be an explicit empty string and disable the defaulting behavior from the provider.

[Lillecarl]

@dinibu https://registry.terraform.io/providers/hashicorp/kubernetes/latest/docs/resources/manifest Requires kubernetes access during planning, other than that it'll work just fine. You can convert existing specs with k2tf.

[github-actions]

Marking this issue as stale due to inactivity. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. This helps our maintainers find and focus on the active issues. Maintainers may also remove the stale label at their discretion. Thank you!

[Lillecarl]

still broken

[mike-sol]

Still broken. Trying to work around it by providing a kubernetes_manifest directly...

[mike-sol]

Yes, something like this works to get around this for now:

resource "kubernetes_manifest" "persistentvolumeclaim" {

  manifest = yamldecode(<<-EOF
    apiVersion: v1
    kind: PersistentVolumeClaim
    metadata:
      name: whatever-pvc-name
      namespace: whatever
    spec:
      accessModes:
      - ReadWriteMany
      resources:
        requests:
          storage: 1Mi
      volumeName: existing-pv-name-goes-here
      storageClassName: ""
  EOF
  )
}

[VardyNg]

+1 to this issue.

I am configuring a AWS S3 volume mount for EKS, and found the PVC's storage_class_name = "" being treated as storage_class_name = nil, the PVC storage class became gp2 (the default class). And it failed to attach to the PV because their storage_class_name does not align.

resource "kubernetes_persistent_volume" "default" {
  metadata {
    name = "${local.name}-pv"
  }
  spec {
    capacity = {
      storage = "5Gi"
    }
    access_modes = ["ReadWriteMany"]
    storage_class_name = "" # it is treated as empty string correctly
    persistent_volume_source {
      csi {
        driver = "s3.csi.aws.com"
        volume_handle = "s3-csi-driver-volume"
        volume_attributes = {
          bucketName: aws_s3_bucket.default.id
        }
      }
    }
  }
}

resource "kubernetes_persistent_volume_claim" "default" {
  metadata {
    name = "${local.name}-pvc"
    namespace = kubernetes_namespace_v1.ns.metadata.0.name
  }
  spec {
    access_modes = ["ReadWriteMany"]
    storage_class_name = "" # <- being treated as nil
    resources {
      requests = {
        storage = "3Gi"
      }
    }
    volume_name = kubernetes_persistent_volume.default.metadata.0.name
  }
  wait_until_bound = false
}

[Lillecarl]

@VardyNg use kubernetes_manifest instead, it just works ™

[VardyNg]

@VardyNg use kubernetes_manifest instead, it just works ™

It works! Hopefully there will be a fix for this parameter as well

[marek-knappe]

Still not fixed :/ will switch to manifest

[Lillecarl]

@marek-knappe https://registry.terraform.io/providers/alekc/kubectl/latest/docs/resources/kubectl_manifest