fix(pvc): split PVC configuration between Deployments #224
Conversation
| @@ -85,12 +85,12 @@ spec: | |||
| {{- toYaml . | nindent 8 }} | |||
| {{- end }} | |||
| volumes: | |||
| {{- if ((.Values.pvc).enabled) }} | |||
| {{- if ((.Values.db.resources.pvc).enabled) }} | |||
There was a problem hiding this comment.
| {{- if ((.Values.db.resources.pvc).enabled) }} | |
| {{- if (.Values.db.pvc).enabled }} |
I think .db.resources refers to container resource requirements :D
There was a problem hiding this comment.
Ah right, thanks. I initially nested it under resources, but then it would be annoying to set PVC settings and CPU/memory separately so I moved it back up a level. I'll change this when I'm back at my computer (probably Monday).
There was a problem hiding this comment.
I'll change this when I'm back at my computer (probably Monday).
Sounds good! Happy Black Friday weekends^^
| - name: {{ .Chart.Name }}-db | ||
| persistentVolumeClaim: | ||
| claimName: {{ .Release.Name }}-db | ||
| {{- end }} | ||
| {{- if not ((.Values.pvc).enabled) }} | ||
| {{- if not ((.Values.db.resources.pvc).enabled) }} |
| @@ -99,12 +99,12 @@ spec: | |||
| {{- toYaml . | nindent 8 }} | |||
| {{- end }} | |||
| volumes: | |||
| {{- if ((.Values.pvc).enabled) }} | |||
| {{- if ((.Values.storage.resources.pvc).enabled) }} | |||
There was a problem hiding this comment.
| {{- if ((.Values.storage.resources.pvc).enabled) }} | |
| {{- if (.Values.storage.pvc).enabled }} |
| - name: {{ .Chart.Name }}-storage | ||
| persistentVolumeClaim: | ||
| claimName: {{ .Release.Name }}-storage | ||
| {{- end }} | ||
| {{- if not ((.Values.pvc).enabled) }} | ||
| {{- if not ((.Values.storage.resources.pvc).enabled) }} |
charts/cryostat/values.yaml
Outdated
| - ReadWriteOnce | ||
| ## @param storage.pvc.selector [object] Selector for the persistentVolumeClaim. See: [Selector](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#persistentvolumeclaims) | ||
| selector: {} | ||
| ## @param storage.pvc.storageClassName [string, nullable] The name of the StorageClass for the persistentVolumeClaim. See: [Class](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#persistentvolumeclaims) |
There was a problem hiding this comment.
nit: I think we should the URL should include section point. For example:
https://kubernetes.io/docs/concepts/storage/persistent-volumes/#class
charts/cryostat/values.yaml
Outdated
| ## @param storage.pvc.selector [object] Selector for the persistentVolumeClaim. See: [Selector](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#persistentvolumeclaims) | ||
| selector: {} | ||
| ## @param storage.pvc.storageClassName [string, nullable] The name of the StorageClass for the persistentVolumeClaim. See: [Class](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#persistentvolumeclaims) | ||
| storageClassName: "" |
There was a problem hiding this comment.
I think these storageClassName are meant default to null or unset (i.e. to be commented out). This allows the default storageClass to to be set on the PVC.
https://kubernetes.io/docs/concepts/storage/persistent-volumes/#class-1
Related to #220
Splits PVC definition in two. Originally, storage only needed to be provided to the Cryostat container itself, for the archives. Later we added the h2:file database in 2.x, which was still part of the Cryostat container. Since Cryostat 3.0 we have a separate database and a separate S3 storage running as separate containers in the same Pod. In 4.0 we have separate Deployments for each of Cryostat, database, and storage. Therefore, it only makes sense that the database and storage have storage configuration parameters (and Cryostat does not), and both of these should also be configured separately since the size and speed requirements for each are likely to differ significantly.