-
Notifications
You must be signed in to change notification settings - Fork 301
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
Development
: Restructure docker compose setups
#5915
Conversation
…s/jobs/JenkinsJobPermissionsUtils.java Co-authored-by: Dominik Fuchß <dominik.fuchss@kit.edu>
@Mtze I participated in a testing session as discussed and got some more feedback which I already incorporated, but in general it ran if the students were on a @dfuchss and @b-fein Thx that you already took a look at this PR in the past! I hope I was able to address all of your and the other people's feedback. Would be cool if you could retry at least the @Hialus Thx for already integrating and testing this with the test servers. Would be cool if you could test it there and then, depending on how it goes, also give this PR an approval. If you need help ping me on Slack! |
Tried creating the mysql container using |
Tried the mysql & production setup - both work as expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested the production setup (Artemis, MySQL, Nginx) extensively over the last couple of weeks and continuously gave Ludwig feedback. This stack will be used as a basis for the test servers soon, so I tested them specifically in that context. The current docker compose files work as expected in that regard now.
The setup isn't really ideal, but thanks to docker compose 2.16 this is sadly required.
I also reviewed the code for the related files and they look good to me now. The changes to the build-deploy GitHub workflow also look good to me, though I hope ARM support comes soon 😉
Overall I approve of this PR, as it greatly cleans up the docker compose files and makes dev and prod deployment way easier. 🚀
Good job 👍
Now for those interested:
The test server setup now looks like this in the working directory:
- .env file for the secrets
- docker-compose.yml to override and set some additional values for the test server (e.g. which database to use)
- This also includes a way to check out a specific PR. More on that soon
- The Artemis repository to have these files available
- Some other folders that are passed to Artemis
- Due to docker compose 2.16 the command for interacting with this is rather lengthy and has the form
docker compose --project-directory $PROJECT_DIR -f $COMPOSE_FILE --env-file $ENV_FILE $ARGS
. The project dir points to the docker folder in the Artemis repository. $ARGS would be something likeup -d
All of these files and folders can be automatically created with ansible using the artemis-ansible-collection after the related PR there gets merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retested mysql prod setup again 👍
One small code comment, looks good otherwise.
src/main/java/de/tum/in/www1/artemis/service/connectors/bamboo/BambooBuildPlanService.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dockerfile works for us. I've added two minor comments. But overall, this PR looks good to me :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docker image build and run without any obvious issues for me. Also the E2E tests still seem to pass, so LGTM 👍
Development
: Restructure Docker Compose setupsDevelopment
: Restructure docker compose setups
Checklist
General
Motivation and Context
So far docker compose files were mainly written as little helpers for development but not for the usage in production, development, and the combination of multiple docker compose services.
This PR structures the docker compose files and documents the setups and services.
The Artemis Dockerfile was also refactored with the goal to build the whole application without the need of a .war file beforehand and therefore documenting the build process.
Description
This PR lead to the idea of restructuring the configuration files in the following PR, but is NOT depended on it:
General
: Clean up Spring configurations #5944Migration Path
Bamboo Tests
I re-added and updated the E2E bamboo scripts which @TheZoker and I debugged manually last week to get the E2E tests stable again. So these scripts have to be activated again (load from version control instead of inline) after merging this PR.
Links to the bamboo plans:
https://bamboo.ase.in.tum.de/build/admin/edit/editBuildTasks.action?buildKey=ARTEMIS-AETG-DA
https://bamboo.ase.in.tum.de/build/admin/edit/editBuildTasks.action?buildKey=ARTEMIS-AECF-DA
The tasks are 2x
CLEANUP
(has to point to the corresponding./.bamboo/.../cleanup.sh
) andDeploy Artemis and run Cypress tests
(has to point to the corresponding./.bamboo/.../execute.sh
) for each plan. After switching the script location fromInline
toFile
the corresponding paths are probably already set.Docker volumes
As some docker volumes were renamed, you might have to rename these if you want to keep your state of your docker containers.
Do so by looking up the current volume names:
The old volume names might have prefixes like
docker_
.This shows a table with the
old_volume
name and thenew_volume
name:[PREFIX]artemis-bamboo
artemis-bamboo-data
[PREFIX]artemis-bitbucket
artemis-bitbucket-data
[PREFIX]artemis-jira
artemis-jira-data
[PREFIX]artemis-gitlab-data
artemis-gitlab-data
[PREFIX]artemis-gitlab-logs
artemis-gitlab-logs
[PREFIX]artemis-gitlab-config
artemis-gitlab-config
[PREFIX]artemis-jenkins-data
artemis-jenkins-data
[PREFIX]artemis-mysql-data
artemis-mysql-data
[PREFIX]artemis-gitlabci-runner-config
artemis-gitlabci-runner-config
Then the following procedure should be done for each volume as there is no other way to rename a volume apparently in docker:
Hard-coded Docker IPs
Remove hard-coded docker IP addresses from your
application-local.yml
.Steps for Testing
Note: If you do not have time to build the Docker Image by yourself, you can also pull it from:
ghcr.io/ls1intum/artemis:pr-5915
Review Progress
Code Review
Manual Tests of these setups:
Exam Mode Test