flatten ghost package with better envs handling #3398
Conversation
yuwenma
requested review from
droot,
justinsb,
mengqiy,
mortent and
natasha41575
as code owners
yuwenma
changed the title
| @@ -5,7 +5,7 @@ metadata: | |||
| name: mariadb | |||
| namespace: example | |||
| data: | |||
| my.ini: |- | |||
| my.cnf: |- | |||
There was a problem hiding this comment.
I remember some discussion about keepting the extension to ini. Is it intended change ?
There was a problem hiding this comment.
yeah, I added the reason in the PR description. The my.cnf is MariaDB default config file name
| value: "true" | ||
| envFrom: | ||
| - configMapRef: | ||
| name: ghost-env |
There was a problem hiding this comment.
One disadvantage of using the same configmap here is that any config change to that configmap affects restarting ghost-app and mysql. ghost-app config is superset of mysql. Like changing the ghost-app's host is going to restart the mysql for now reason.
So an optimal solution could be, define two config-maps. one for db config and another for pure ghost-app config.
ghost-app's config will be provided by both the config-map and mysql one will be powered by just the db configmap.
There was a problem hiding this comment.
One disadvantage of using the same configmap here is that any config change to that configmap affects restarting ghost-app and mysql. ghost-app config is superset of mysql. Like changing the ghost-app's host is going to restart the mysql for now reason.
That's true. Restarting both to align the env change is expected behavior. One issue I had today is that I only updated the env config for MariaDB (so we saw the MariaDB pod works) but not Ghost host.
So an optimal solution could be, define two config-maps. one for db config and another for pure ghost-app config.
ghost-app's config will be provided by both the config-map and mysql one will be powered by just the db configmap.
Sure, I can use two ConfigMaps since the envFrom accept a list
There was a problem hiding this comment.
Actually thought some more and realized, this looks nice from config organization perspective but functionally is inferior to previous approach of inline config in the deployment itself. Because now I have to worry about updating the deployment or statefulset otherwise new change will not rollout and in the previous approach it will happen automatically :)
There was a problem hiding this comment.
Another thing I discovered is that the envFrom cannot be updated. This seems not to be the model we are looking for.
Related to #3343.
The nested structure for ghost package is unnecessary.
Nested package actually makes user configuration harder:
DeploymentandStatefulSetspec.template.spec.containers[].envhave shared env configurations likeBITNAMI_DEBUGALLOW_EMPTY_PASSWORD. Users today need to update the config twice. This progress can be much simplified (and better protected) if we can change thecontainers[].envto useconfigMapKeyRef.This PR also fixes the MariaDB config name
my.cnf, This file name is a MariaDB convention doc