Natalique commented on code in PR #32582:
URL: https://github.com/apache/airflow/pull/32582#discussion_r1266945426


##########
chart/templates/dags-persistent-volume-claim.yaml:
##########
@@ -46,7 +46,7 @@ spec:
   {{- if (eq "-" .Values.dags.persistence.storageClassName) }}
   storageClassName: ""
   {{- else }}
-  storageClassName: "{{ .Values.dags.persistence.storageClassName }}"
+  storageClassName: "{{ tpl .Values.dags.persistence.storageClassName . }}"

Review Comment:
   This won't work if SC name is a quoted string, have to have it quoted in PVC 
as well to be able to use it. 
   Tested locally to verify.
   
   sc.yaml - 
   ```
   kind: StorageClass
   apiVersion: storage.k8s.io/v1
   metadata:
     name: "123456"
   parameters:
     directoryPerms: "760"
     fileSystemId: fs-xxxxxxxxx
     provisioningMode: efs-ap
   provisioner: efs.csi.aws.com
   reclaimPolicy: Retain
   volumeBindingMode: Immediate
   ```
   
   pvc.yaml - 
   
   ```
   apiVersion: v1
   kind: PersistentVolumeClaim
   metadata:
     annotations:
     name: nat-search-logs
     namespace: natalie
   spec:
     accessModes:
     - ReadWriteMany
     resources:
       requests:
         storage: 10Gi
     storageClassName: 123456
     volumeMode: Filesystem
   ```
   
   Output - 
   `k apply -f pvc.yaml
   Error from server (BadRequest): error when creating "pvc.yaml": 
PersistentVolumeClaim in version "v1" cannot be handled as a 
PersistentVolumeClaim: json: cannot unmarshal number into Go struct field 
PersistentVolumeClaimSpec.spec.storageClassName of type string`
   
   When I set in PVC 
   `storageClassName: "123456"`
   I can apply it. 
   Admittedly it's not very common to name resources with quoted strings, but 
it can be done.
   So to avoid issues, IMO better to keep the quotes. 
   WDYT? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to