feat(eventtemplates): declarative configuration of Event Template ConfigMaps #215
Conversation
| {{- range .Values.core.config.eventTemplates.configMapNames}} | ||
| - name: {{ . }} | ||
| configMap: | ||
| name: {{ . }} | ||
| {{- end }} |
There was a problem hiding this comment.
Any thoughts on using a projected volume here to combine all config-map contents into a single volume? One benefit is that we can define a single volumeMount on /opt/cryostat.d/templates.d, thus reducing the manifest/object def size.
Reference: https://kubernetes.io/docs/concepts/storage/projected-volumes/
There was a problem hiding this comment.
Sure, I could do this. It would be a single mount but the definition for the projected volume itself would still look about the same, so I'm not really sure I understand what benefit it brings... ?
There was a problem hiding this comment.
hm, I suppose reducing the number of volume mount definitions can help reduce the manifest size (thus, object size in etcd). And grouping them under a single volume seems to make it easier to recognize their purpose when reading the manifest (my opinion).
There was a problem hiding this comment.
I guess we can do the same for #219.
charts/cryostat/values.yaml
Outdated
| config: | ||
| eventTemplates: | ||
| ## @param core.config.eventTemplates.configMapNames [array] List of ConfigMap names. Each ConfigMap is expected to contain one or more files, which are .jfc (XML) JFR Event Templates, to be mounted to the Cryostat container. | ||
| configMapNames: [] |
There was a problem hiding this comment.
Wondering if we can allow specifying the key(s) within the configmaps? Something similar to Cryostat CR.
I guess one case could be that a user uses a Configmap to put all Cryostat configurations (i.e. Event template and Automated Rules). I might be overthinking :D
There was a problem hiding this comment.
I thought about that, but it seems like it'd be pretty messy trying to put all of the configurations of different types into one ConfigMap and use filenames to differentiate them. It would also much more quickly run into the ConfigMap size limit that way. I think using one or more ConfigMaps for each type of configuration file is a good medium option, where if there are many custom Event Templates that add up to too large a filesize then they can be split across multiple ConfigMaps, for example, but otherwise they can just be combined together as separate files and mounted that way.
There was a problem hiding this comment.
Sure, that makes sense! I think the current way is good then^^
| @@ -216,3 +222,8 @@ spec: | |||
| secret: | |||
| secretName: {{ .Release.Name }}-proxy-tls | |||
| {{- end }} | |||
| {{- range .Values.core.config.eventTemplates.configMapNames}} | |||
| - name: {{ . }} | |||
| configMap: | |||
There was a problem hiding this comment.
Also, should we enforce that these configmaps are available at the time of installation (i.e. optional: false)?
There was a problem hiding this comment.
That's a fair point.
There was a problem hiding this comment.
From what I can tell, I think optional: false is the default (which makes sense), but I can mark them as explicitly non-optional.
| @@ -216,3 +222,11 @@ spec: | |||
| secret: | |||
| secretName: {{ .Release.Name }}-proxy-tls | |||
| {{- end }} | |||
| - name: declarative-event-templates | |||
There was a problem hiding this comment.
Should we set the defaultMode to read-only (i.e. 0440)?
I think we should be okayy to set 0440 without running into #225 (comment), since Cryostat has default GID 0 in the image.
Fixes #214
Related to #225
Depends on cryostatio/cryostat#726
To test:
oc create configmap mytemplate --from-file=mytemplate.jfchelm install --set core.image.repository=quay.io/andrewazores/cryostat --set core.image.tag=event-templates-declarative-1 cryostat --set core.config.eventTemplates.configMapNames='{mytemplate}' ./charts/cryostatlocalhost:0custom target. Go to Events, then Event Templates, and ensure that themytemplatecustom Event Template is listed.