Add mediawiki configmap into the api deployment #204
Conversation
rosalieper
changed the title
| {{- end }} | ||
| key: oauth-private.key | ||
| {{- if .Values.app.gce.serviceAccountSecret }} | ||
| volumeMounts: |
There was a problem hiding this comment.
is it ok that this is now always present even if .values.app.gce.serviceAccountSecret evaluates to false? I guess in this case it's just blank?
There was a problem hiding this comment.
I am not sure I understand
There was a problem hiding this comment.
by swapping this line (219) with the one below (220) now the volumeMounts: key line will always be present
| mountPath: "/var/run/secret/cloud.google.com" | ||
| {{- end }} | ||
|
|
||
| {{- if .Values.configMap.name }} |
There was a problem hiding this comment.
I think you now don't need to check for this value right? You hardcode the name now at https://github.com/wbstack/charts/pull/204/files#diff-3a215b8c5c0bd4abc39f477cd0bf136b50746e10fdad5d3f997f4af46df6fe6fR4 (which I'm fine with; I don't think the name needs to be variable)
There was a problem hiding this comment.
yeah, this is true
|
|
||
| {{- if .Values.configMap.name }} | ||
| - name: "mediawiki-configMap" | ||
| mountPath: {{ .Values.configMap.mountPath }} |
There was a problem hiding this comment.
I think probably this could become hardcoded. I can't imagine we'll want to change it regularly particularly since where we read it from will be specified in the api image
charts/api/values.yaml
Outdated
| ingress: | ||
| enabled: false | ||
|
|
||
| configMap: |
There was a problem hiding this comment.
I wonder if this should be more specific to make it clearer that this is for a specific config map
charts/api/values.yaml
Outdated
| configMap: | ||
| name: "mediawiki-host-map" | ||
| mountPath: "/config" | ||
| jsonMap: | |
There was a problem hiding this comment.
I guess you found this works? It's a little confusing to me having this json sent to toYaml but technically all json is valid yaml.
There was a problem hiding this comment.
I can change it to yaml. I thought having it as json would made things easier on the enpoint level.
| {{- end }} | ||
| key: oauth-private.key | ||
| {{- if .Values.app.gce.serviceAccountSecret }} | ||
| volumeMounts: |
There was a problem hiding this comment.
by swapping this line (219) with the one below (220) now the volumeMounts: key line will always be present
| enabled: false | ||
|
|
||
| mediawikiVersionConfigMap: | | ||
| mw1.39-wbs1: 139-app |
There was a problem hiding this comment.
I'm now wondering what this file should look like; all the laravel config files are actually .php. e.g. filesystems.php.
This is yaml right now but that or json seems a bit off. In general I struggled to find evidence with my googling of people loading whole config files at run (not build) time into laravel. Mostly they just seem to use environment variables.
I guess a php file in the config folder might not be the worst solution though.
| {{- end }} | ||
|
|
||
| - name: "mediawiki-configMap" | ||
| mountPath: "/config" |
There was a problem hiding this comment.
I think probably this wants to go somewhere specific but I'm stalling a little on exactly where.
Perhaps into the Laravel config directory? (/var/www/html/config).
And it should definitely have a more meaningful name than config (maybe mediawiki-backend-service-configmap.<extension>. See https://github.com/wbstack/charts/pull/204/files#r2523682791
There was a problem hiding this comment.
yes, I agree. this was a temporary location but I am also not yet sure where to place it.
Bug: T409530