search

kubernetes-sigs / azurefile-csi-driver

Azure File CSI Driver
Apache License 2.0
157 stars 143 forks source link

Standard_ZRS will silently create a LRS share instead with NFS protocol #1480

Closed davidkarlsen closed 8 months ago

davidkarlsen commented 1 year ago

What happened:

Requesting a Standard_ZRS sku share will silently create a Premium_LRS instead

What you expected to happen: Fail if not supported, create a Standard_ZRS if supported.

How to reproduce it: See yamls below. Go into portal, observe:

Performance
:
Premium
Replication
:
Locally-redundant storage (LRS)
Account kind
:
FileStorage
Provisioning state
:
Succeeded
Created
:
09/08/2023, 12:19:25

Anything else we need to know?:

Environment: sc:

apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
  name: azurefile-csi-standard-zrs-nfs
provisioner: file.csi.azure.com
parameters:
  # skuName: Premium_ZRS - The sku Sku: Premium_ZRS, Kind: FileStorage is not available in zone.
  skuName: Standard_ZRS
  protocol: nfs
allowVolumeExpansion: true
reclaimPolicy: Delete
volumeBindingMode: Immediate
mountOptions:
  - nconnect=4

pvc:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: disktest
  namespace: disktest
spec:
  storageClassName: azurefile-csi-standard-zrs-nfs
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 1Gi
andyzhangx commented 1 year ago

the driver would search Standard_ZRS account first, from below info, it's got a Premium_LRS account, @davidkarlsen have you got a wrong account?

Performance
:
Premium
Replication
:
Locally-redundant storage (LRS)
Account kind
:
FileStorage
Provisioning state
:
Succeeded
Created
:
09/08/2023, 12:19:25
davidkarlsen commented 1 year ago

But the driver should search for existing accounts, use one if a match is there, else create a new one? https://docs.openshift.com/container-platform/4.12/storage/container_storage_interface/persistent-storage-csi-azure.html#persistent-storage-csi-azure-disk-sc-zrs_persistent-storage-csi-azure At least that is what happened for my other LRS disks.

andyzhangx commented 1 year ago

I see where the problem is, since NFS protocol only supports Premium storage, it would enforce to use Premium_LRS if you are using standard storage

davidkarlsen commented 1 year ago

exactly - wouldn't it make sens to raise an error for this - rather than silently provisioning another sku type?

andyzhangx commented 1 year ago

unfortunately that's the way now, if we raise error now, it may break existing users. if you set "skuName: Premium_ZRS", it should work.

frank-m commented 1 year ago

Just ran into this issue. I would say it is pretty much unacceptable to keep it this way as it may lead to unrecoverable data loss if people expect this to be zone redundant.

Also sounds like a huge logic error in the code..

k8s-triage-robot commented 9 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 8 months ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

andyzhangx commented 8 months ago

I will work out a PR to raise error with NFS + Standard account combination.