Allow MIQ PG config overrides to be included #2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Check for the existance of MIQ PG overrides, if available, include them in postgresql.conf - Defaults taken from https://github.com/ManageIQ/manageiq-appliance/blob/master/COPY/etc/manageiq/postgresql.conf.d/01_miq_overrides.conf - We use include_dir , so more configs can be stacked, PG will process *.conf in a given dir - POSTGRESQL_CONFIG_DIR is set via environment variable and can be customized at users convenience - Can be backported to euwe/4.5 builds to allow functionality to existing users
fbladilo
added a commit
to fbladilo/manageiq-pods
that referenced
this pull request
Jul 21, 2017
- Configmap object created with MIQ default overrides settings - Configmap is injected as a volume into /etc/manageiq/postgresql.conf.d by default - POSTGRESQL_CONFIG_DIR supplied to on template to allow flexibility on location - Related to PR : ManageIQ/container-postgresql#2 - PG image will pickup and do an include_dir if the directory is found The most important piece is that the configuration is externalized and parametized.
carbonin
reviewed
Jul 24, 2017
bdunne
reviewed
Jul 24, 2017
docker-assets/run-postgresql
Outdated
| if [ -d "${POSTGRESQL_CONFIG_DIR}" ]; then | ||
| cat >> "$PGDATA/postgresql.conf" <<EOF | ||
| # Begin ManageIQ configuration overrides: | ||
| include_dir '${POSTGRESQL_CONFIG_DIR}' |
There was a problem hiding this comment.
Do we need to check if this is already here to avoid having multiple entries in the config?
There was a problem hiding this comment.
@bdunne Yep, working on another commit with slightly different logic for safety.
- Added a check to avoid creating multiple entries of the same config during reschedules - Create default directory location for PG_CONFIG_DIR on image not on scripts - Change default location for PG_CONFIG_DIR to a known writable location (avoid /etc)
|
@bdunne @carbonin A few changes :
|
bdunne
reviewed
Jul 24, 2017
docker-assets/run-postgresql
Outdated
|
|
||
| # Check for POSTGRESQL_CONFIG_DIR presence in postgresql.conf, if already there skip. | ||
| [[ $(grep ${POSTGRESQL_CONFIG_DIR} ${PGDATA}/postgresql.conf) ]] && RET=$? || RET=$? | ||
| if [[ $RET == "0" ]]; then |
There was a problem hiding this comment.
I think if [[ $? -eq 0 ]] would be simpler
There was a problem hiding this comment.
I tried to avoid checking straight on $? , too many times things get in the middle by accident. That's why I chose to save status on a VAR. Script is run with set -eu and and very picky, will probably quit immediately on a false test.
carbonin
approved these changes
Jul 25, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@carbonin @bdunne Please review, I have another PR coming with the corresponding configmaps and corresponding template modifications going to pods.
We stack after the default customs from SCL, this is how it looks on config :