EIDA / mediatorws

EIDA NG Mediator/Federator web services
GNU General Public License v3.0
6 stars 6 forks source link

Update Dockerfile; config to use psql #76

Closed Jollyfant closed 5 years ago

Jollyfant commented 5 years ago

Currently we don't have a good way to pass environment variables to:

Maybe we should describe the manual postgres setup & federator configuration in a section in the Docker documentation?

damb commented 5 years ago

@Jollyfant,

first of all: :+1: (Though, I didn't test it, yet.)

Respective the environment variables: I assume you'd like to go for the approach of parsing environment vars by the programs themselves, right? If yes, would you implement this feature here (i.e. within this PR), or would you rather implement it in a new feature?

Jollyfant commented 5 years ago

Well I'm considering now that since the db parameters must be hardcoded in the Dockerfile for the cronjob there is not really a point for making them configurable during runtime. Thoughts?

Jollyfant commented 5 years ago

Or we can insert the cronjob during init like you linked before:

https://github.com/phusion/baseimage-docker#running_startup_scripts

damb commented 5 years ago

Well I'm considering now that since the db parameters must be hardcoded in the Dockerfile for the cronjob there is not really a point for making them configurable during runtime. Thoughts?

Within the Dockerfile env variables can be used quite easily when they are configured within the docker-compose.yml. Again, variables within the docker-compose.yml might be configured using the .env file approach (see: https://stackoverflow.com/questions/51642221/dockerfile-how-to-set-env-variable-from-file-contents). The .env file must be provided by individual users. IMO this should solve the problem.

Jollyfant commented 5 years ago

For the crontab I chose the approach in commit de65c0f08b5c38572e512283f89e4b571bf5abda. Environment variables can be set too in docker-compose.yml.

What's left is reading the db_url and db_engine from the environment variables. Or we could have a single one that defined the connection string fully. Perhaps you could help me out with the Python code since you're more familiar with it.

damb commented 5 years ago

What's left is reading the db_url and db_engine from the environment variables. Or we could have a single one that defined the connection string fully. Perhaps you could help me out with the Python code since you're more familiar with it.

I can care about the source code changes, necessary. Though, I wouldn't use a single connection string. IMO the variables should both work for the Postgres DB container and eida-federator specific configuration. This prevents us from ambiguities.

Jollyfant commented 5 years ago

Added shared environment file for the psql database and federator. When running docker-compose for the first time the psql is created using the same credentials. The operator has to call init_db and kickstart harvesting manually.

Jollyfant commented 5 years ago

I think the best approach would be to call in this order:

# No point in starting federator without stationlite
$ docker-compose start psql
$ init_db
$ harvesting

# Both federator and psql
$ docker-compose up
damb commented 5 years ago

@Jollyfant, do you agree that it would be already sufficient if services would allow the interpretation/interpolation of environment variables (e.g. POSTGRES_USER, ...) within the configuration file. IMO this would already suite our needs, right?

Jollyfant commented 5 years ago

@Jollyfant, do you agree that it would be already sufficient if services would allow the interpretation/interpolation of environment variables (e.g. POSTGRES_USER, ...) within the configuration file. IMO this would already suite our needs, right?

Yeah I guess so if that is possible during runtime.. with the confd approach?

Jollyfant commented 5 years ago

By the way is there a reason that the station-lite-harvest in cron needs to have the connection string as a parameter? Can it not read from the configuration as well?

damb commented 5 years ago

with the confd approach?

Rather with builtin ConfigParser facilities.

damb commented 5 years ago

Can it not read from the configuration as well?

Which configuration do you exactly mean?

Jollyfant commented 5 years ago

Well my crontab says:

 1 * * * /var/www/stationlite/venv3/bin/eida-stationlite-harvest postgresql://user:pass@stationlite-psql:5432/stationlite

and under stationlite-harvest in eidangws_config it says:

db_engine = postgresql://user:pass@stationlite-psql:5432/stationlite

Seems like the eida-stationlite-harvest executable could use the db_engine variable?

damb commented 5 years ago

Seems like the eida-stationlite-harvest executable could use the db_engine variable?

Correct. Hence,

 1 * * * /var/www/stationlite/venv3/bin/eida-stationlite-harvest --config /var/www/mediatorws/config/eidangws_config

should do the job. The value of the --config option is explicitly set. Even though, it is the default :rofl:.

Jollyfant commented 5 years ago

Okidoki, I reverted the crontab change.. not necessary anymore with this change.

Jollyfant commented 5 years ago

By the way, I have absolutely no idea why I copied all configuration files to the docker folder. Should I modify this pull request to take the configuration from the proper folders..?

damb commented 5 years ago

By the way, I have absolutely no idea why I copied all configuration files to the docker folder. Should I modify this pull request to take the configuration from the proper folders..?

IMO in this case copying is fine. The configuration files added within https://github.com/EIDA/mediatorws/tree/master/config are exemplary configs / templates. So, I think it is better to maintain a copy for docker related configuration purposes.

Jollyfant commented 5 years ago

So this pull request is pretty done. Do you want to merge it in to your feature branch and maybe have a look at the ConfigParser when you have time?

damb commented 5 years ago

Thanks, again @Jollyfant.