kubernetes / kubernetes

Production-Grade Container Scheduling and Management
https://kubernetes.io
Apache License 2.0
110.13k stars 39.41k forks source link

CSI expanding persistent volume does not call csi.ControllerExpandVolume #79716

Closed Ntr0 closed 5 years ago

Ntr0 commented 5 years ago

What happened: Deployed a csi driver with volume expansion capability (online). Defined following storage class:

allowVolumeExpansion: true
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  annotations:
    storageclass.kubernetes.io/is-default-class: "true"
  name: ionos-enterprise-hdd
parameters:
  type: HDD
provisioner: cloud.ionos.com
reclaimPolicy: Delete
volumeBindingMode: WaitForFirstConsumer

Created a pvc:

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: nginx-data
spec:
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 2Gi
  storageClassName:  ionos-enterprise-hdd

Increasing the requested storage parameter in the pvc works as expected: First ControllerExpandVolume is called, then NodeExpandVolume. But increasing the storage size in the underlying pv has the effect, that only NodeExpandVolume is called, (which of course does not work, since ControllerExpandVolume is required beforehands). What you expected to happen: If it is actually allowed to change the persistent volume, than ControllerExpandVolume must be called.

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know?:

Environment:

Ntr0 commented 5 years ago

@kubernetes/sig-storage-bugs

k8s-ci-robot commented 5 years ago

@Ntr0: Reiterating the mentions to trigger a notification: @kubernetes/sig-storage-bugs

In response to [this](https://github.com/kubernetes/kubernetes/issues/79716#issuecomment-508075611): >@kubernetes/sig-storage-bugs Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
msau42 commented 5 years ago

I'm unsure increasing PV size is a supported sequence. cc @gnufied

Ntr0 commented 5 years ago

me neither. And I would be fine with the solution, that it is not supported. But then NodeExpandVolume should not be called, right?

gnufied commented 5 years ago

When you manually increase the PV size without expanding the underlying volume, you have basically broken the contract of a PV. PV should always store actual size of the volume since PV has no PV.Status.Size field.

Calling NodeExpandVolume is right thing to do because PV has bigger size than what is stored in PVC's status.

Ntr0 commented 5 years ago

Alright, thanks for the clarification.