Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DB] Fix builtin DB init script. Document the DB existingSecret params #51

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

jeanpommier
Copy link
Member

Init Script:

  • Fix [DB] init configmap's name does not match the name in Values.yaml #48:
    name of init script should match the one provided in values.yaml

  • In case of Builtin DBs:
    Init script was giving ownership of all DBs to georchestra user, no using the config params for geodata and datafeeder DBs. Which were nevertheless used by the geodata and datafeeder connection secrets. So for instance datafeeder was trying to connect with datafeeder user, but it had not been created.
    This way, the init script actually creates the users and gives them ownership on their respective DB

Documentation:
clarify the usage of the existingSecret params

In case of Builtin DBs:
Init script was giving ownership of all DBs to georchestra user, no using
the config params for geodata and datafeeder DBs. Which were nevertheless
used by the geodata and datafeeder connection secrets. So for instance
datafeeder was trying to connect with `datafeeder` user, but it had not
been created.
This way, the init script actually creates the users and gives them
ownership on their respective DB

Documentation: clarify the usage of the existingSecret params
@jeanpommier
Copy link
Member Author

@edevosc2c @jeanmi151 would you have a minute to review this one please ?

Copy link
Collaborator

@jeanmi151 jeanmi151 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me, did you test it with other password than default one ?

@jeanpommier
Copy link
Member Author

did you test it with other password than default one ?

It was 2 weeks ago, so I don't remember (but suppose so). Checked it right now, seems fine
If it's OK for you, I'll merge this one. I'd like to avoid having to refresh it it the chart changes in between

Copy link
Member

@edevosc2c edevosc2c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish there was a better way to do it like this, as .Release.Name is not a recommended value for accessing an external resource created by the parent chart.

But I'm ok with the change because for now there is no other choice that comes to my mind and this database provisioning is not targeted towards advanced setups so it will work fine on simple setups.

@jeanpommier jeanpommier merged commit 9b4ef9a into main Sep 20, 2023
1 check passed
@jeanpommier jeanpommier deleted the builtin-db-fix branch September 20, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DB] init configmap's name does not match the name in Values.yaml
3 participants