Artemis
Artemis copied to clipboard
`Development`: Use env-files instead of yaml configs for docker compose
Checklist
General
- [x] I tested all changes and their related features with all corresponding user types on a test server.
- [ ] This is a small issue that I tested locally and was confirmed by another developer on a test server.
- [x] Language: I followed the guidelines for inclusive, diversity-sensitive, and appreciative language.
- [x] I chose a title conforming to the naming conventions for pull requests.
Motivation and Context
The change tries to simplify the Artemis deployment through Docker Compose by including the variables that are currently spread in multiple yml files in the docker-compose.yml besides creating a central .env files for the environmental variables.
Description
Added the values of the application.yml, application-artemis.yml, and application-dev.yml to the .env and abstracted the environmental variables in the docker-compose.yml to the .env file.
Steps for Testing
First, we need to create a local version of Artemis image instead of using the remote Docker image. We also need to change the Dockerfile to use the env variables passed in the env files instead of yml ones. Then we change the docker compose file to use the local version and test it.
Run cd src/main/docker
, docker compose up artemis-mysql
then later docker-compose up artemis-app
.
Review Progress
Code Review
- [ ] Review 1
- [ ] Review 2
Manual Tests
- [ ] Test 1
- [ ] Test 2
Just for clarification: Is this PR meant for test deployments or production deployments? If the latter is the case, there are missing some properties for Jenkins, GitLab, SAML2 :)
Just for clarification: Is this PR meant for test deployments or production deployments? If the latter is the case, there are missing some properties for Jenkins, GitLab, SAML2 :)
Hi @dfuchss ! This is an early stage (almost a POC) for the docker file. We (me, Matthias, Simon and Ludwig) are testing if it is possible to centralize all variables needed in one single file. The next stage is to work with the .env file. As soon as we have a working version, we will add all the properties needed for both (prod and dev). You are more than invited to review and point to the properties we may have missed.
Just for clarification: Is this PR meant for test deployments or production deployments? If the latter is the case, there are missing some properties for Jenkins, GitLab, SAML2 :)
For test development deployments for now ;)
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.
The properties of the saml2 profile are missing, or am I wrong :) Just to be sure that I'm not missing anything.
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.
@dangcosta There are still some TODO comments where I am not sure how the syntax has to be for the env style. I wanted to double-check these but don't know how to do this.
Also, there are some values that seem to be replaced at some later stage (by spring and gradle) for instance the following in artemis.env
which still need some attention:
...
EUREKA_INSTANCE_METADATAMAP_VERSION=#project.version#
EUREKA_INSTANCE_METADATAMAP_GITVERSION=$${git.commit.id.describe:}
...
It seems like the 2nd key-value pair is using spring variables which get populated currently in both the environment and the config section. So I guess these are fine. Not sure if it works if you just use environment variables so.
The 1st key-value pair seems to be some custom solution for introducing the version with gradle which is a bit problematic for the env files as these are not packaged into the war file and are therefore not processed by gradle. You can see these problems in the following files: https://github.com/ls1intum/Artemis/blob/684eff1c8a2aede5e9b83559aae2d1fe6d49713f/gradle/profile_dev.gradle#L66 https://github.com/ls1intum/Artemis/blob/684eff1c8a2aede5e9b83559aae2d1fe6d49713f/gradle/profile_prod.gradle#L28
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.
Me and @dangcosta understood the requirement for this PR wrong. What should have happened and still should happen in the future is the following:
Defaults for configurations are located in the yaml files according to their profiles (application-dev.yml, ...) and the spring application.yml. The yaml file application-local.yml doesn't contain default values and is an override of these defaults for the local IDE. We might add a version-controlled copy of it with values you SHOULD override as they are secrets, etc. A .env file or environment variables should be an override for the docker-compose setups, NOT a replacement for all yaml config files! We should recheck the structure of the yaml files. Some profiles like lts are maybe not even used anymore!
I already got in contact with Daniel to discuss the future of this PR.
We assigned this task to me and I will reopen this PR when I have time to work on it.