Feature/add docker image

[ToshY]

#59

WIP

• Add initial Dockerfile + .dockerignore
• Add documentation for docker compose
• Updated docker-compose to V2 variant docker compose
• CI through docker compose (untested)
• CI push docker image to dockerhub on release (untested)

[bastien70]

Hi ! Thank you for this beautiful PR! I leave @jmsche the review, he will know better than me.

However, I think it will merge after my #62 which should change the way adapters are handled. Small changes will certainly be expected, at the level of the documentation that you have updated for example, since we will no longer need to use the environment variables of S3, for the reasons described below.

Thank you very much for the contribution! :)

[ToshY]

Hey @bastien70,

Thanks for the heads-up. I'll adapt the current documentation in the meantime. I currently have the PR mark as draft as I was still testing if the docker image works correctly in existing projects and with local development.

[bastien70]

Hi ! Small question, have you or are you going to write tests to check that the docker container you are setting up works and is correctly configured?

And if I understood correctly, you modified the CI so that the tests are launched thanks to Docker, right? In which case, your PR may be merged before mine.

[ToshY]

Could you clarify on what you mean with tests exactly?

If you mean to check if php-fpm is ready, I think there's several ways I guess.

1. First thing that comes to mind is adding it to the wait service with dbsaver:9000, which will check if it's able to connect to the service (if phpfpm is not running, it will not be able to connect to port 9000). Still, as it requires a "timeout" parameter to be set it isn't quite ideal. I could make seperate wait commands for mysql and the application itself, but still have to take in consideration that adding for example heavy migrations will increase execution time, and therefor the timeout has to be increased.

2. Also could add a healthcheck, like php-fpm-healthcheck to the docker image. To check if it's healthy it would be the following command:

docker compose exec dbsaver /bin/sh /usr/local/bin/php-fpm-healthcheck

3. Or check if container "running". So if docker compose ps --services --filter "status=running" | grep -Fxe dbsaver returns dbsaver, then it's running correctly. We'll just add restart: 'no' to docker-compose.test.yaml to make sure it doesn't restart.

The downside of option 2 is that you can not exec if the container is restarting or has exited. So you will get an error response indicating the status of the container. You could derive from that that it's failing, but I think I would still go with option 1 and/or 3.

Could you clarify what you mean with "correctly configured"?

What configuration should be exactly checked?

---

And you're correct that I modified the CI to up the services defined from the docker compose yaml files. While I've currently tested the majority of the commands locally, I'm assuming it will work in the CI, because I honestly have no idea how to properly dry-run/test workflows locally besides just manually executing the commands.

[bastien70]

Hello!

To be more specific on what I was asking:

Basically we have our tests (PHPUnit) which will test the entirety of dbsaver (verify that each feature works). Similarly, how are you going to ensure that the dbsaver docker image you created deploys correctly and without issues?

The goal is that when the CI will launch the PhpUnit tests, it can also check each time that your dbsaver docker image is still compliant, that it deploys correctly, that we have not forgotten anything.

How do you handle this? Maybe you have already answered it, but I must admit that Docker is not our specialty at jmsche and myself, so we will not know if what you have coded is good (but I have the feel like you know what you're doing), or if you've written any tests to make sure the dbsaver docker image works perfectly

[ToshY]

Basically we have our tests (PHPUnit) which will test the entirety of dbsaver (verify that each feature works).

So if I understand it correctly, one of the concerns is what I did with existing checks like the unit tests, phpcs and phpstan? Functionally I didn't change those, but the way it's tested in the CI is now inside the docker container.

If you check the continous-integration.yml, for the unit tests I've changed the following:

# from
run: vendor/bin/phpunit --coverage-clover=coverage.xml
# to
run: docker compose exec --env XDEBUG_MODE=coverage dbsaver vendor/bin/phpunit --coverage-clover=coverage.xml

The above just runs the same unit tests as before, but now inside the docker container. Same thing goes for phpcs and phpstan.

Similarly, how are you going to ensure that the dbsaver docker image you created deploys correctly and without issues?

If for some reason the docker container even fails to start (see Start containers step in CI), it would not even reach the point of being able to do the tests, and thus will fail relatively quickly in the CI.

When it comes to the actual creating and publishing of the image, that would only be done when a new release is created. Currently, the docker-release.yml is based on the example from Github docs. That workflow only does the creating and publishing of the image, nothing more at the moment. I could of course add the phpunit, phpcs and phpstan checks as well in there? That would make completely sure that the published image is covered by those checks (even if something magically slips through).

Writing all this, I realise I still have to some tweaks to do, but the point is that existing checks still will be run inside the running docker container in the CI.

---

Also, I would like to point out that I'm also not an "expert" on docker, and that this is actually the first time I try to create a docker image for a Symfony application. While I have some experience with docker compose both from hobby projects as well as work, this too is a learning experience. I try to use best practices for writing dockerfiles and hadolint to verify, and use existing sources like dunglas/symfony-docker in order to make a decent (starting) image.

[bastien70]

Hello @ToshY !

Indeed, now that the tests will be launched based on the docker image, you are right, if from the start it does not work, there is a direct problem with the docker image.

Regarding the docker-release, can you tell me more?

dbsaver docker image is created/updated on every merge? I'm not sure I understand this part correctly.

I could of course add the phpunit, phpcs and phpstan checks as well in there?

=> It would be safer yes, even if a priori we are already well thanks to the phpunit tests launched from docker

Also, I would like to point out that I'm also not an "expert" on docker,......

=> Although you're not an expert on it, it's still great PR! Good game :)

[ToshY]

Hey @bastien70

dbsaver docker image is created/updated on every merge? I'm not sure I understand this part correctly.

Maybe I was unclear about that, but I meant on a new release an image will be build and pushed. Currently in the docker-release.yml the following can be found:

on:
  release:
    types: [published]
...

So it would make sense that upon a new release the workflow gets triggered.

[ToshY]

I also have a question/request regarding the Taskfile. Using the docker compose commands in the CI makes it visually kind of crowded, so is it acceptable to install Taskfile in the CI? It would reduce the run for each step to just a single task command then. _{p.s. I normally use Makefiles so I don't have this dilemma :wink:}

[bastien70]

I also have a question/request regarding the Taskfile. Using the docker compose commands in the CI makes it visually kind of crowded, so is it acceptable to install Taskfile in the CI? It would reduce the run for each step to just a single task command then. p.s. I normally use Makefiles so I don't have this dilemma wink

Yeah no problem! :)

[ToshY]

Hey @bastien70

Hope you don't mind that this is taking some time, but I'll give you an update in the meantime:

• Simplified the commands in the CI by using Taskfile with the help of arduino/setup-task@v1 action.
• Simplified Dockerfile.
• Added a docker-compose.dev.yaml for local development. While it should be relatively easy to use, I will write documentation for it so future contributors (or you 😉) can also use it.

Todo:

• Further testing of dev image, and testing of production image.

One of the things that I think is important is that we should start building building a dev image, tag it as bastien70/dbsaver:dev and push it to Dockerhub. This way we can speed up the tests in the CI, because then it only needs to pull the image. If we don't create a dev image it needs to build the image for every run, which is significantly slower compared to just downloading it.

To do that I actually need to know if you already have a Dockerhub account? And if not, could you make one? As you stated that Docker is not your specialty, I'm not sure if you're familiar with building, tagging and pushing images? If not, the commands are relatively simply but I can guide you through it if needed (already added a command in the Taskfile for building the image).

If I'm confident enough in the current images, I'll let you know, so that we can start making the dev image before the actual review and merge of the PR.

[bastien70]

Hello !

Don't worry, it doesn't matter at all if it takes time ;)

I have a question, what is the caddy-server for?

For dockerhub, no I don't actually have an account. I can always create one.

On the other hand, I admit that what scares me in the future is that if there are updates in the future, which must force us to modify the docker files, if you are not there , it may be complicated because neither I nor jmsche will be able to do it. So the main question is: "Will you still be there to manage docker files?"

[ToshY]

Hey @bastien70

I have a question, what is the caddy-server for?

Well I realized that if I want to keep contributing to this application, it would be easier for me to add a relatively simple webserver, like caddy, to the docker-compose.dev.yaml. So I could just use task docker:up:dev to up the services, and after having add dbsaver.local / mail.dbsaver.local to my systems hosts file, I can easily access the application and mail client through those URLs.

So to answer your question: "For easier setup/access of the application using docker".

Even though you may not be very comfortable to docker yet, I hope you/jmsche will try it out before we actually merge the PR. I will of course update the documentation before that to try to make clear on how to use it 😉

"Will you still be there to manage docker files?"

Yeah I am still interested in contributing, that includes managing the Dockerfile. However, there shouldn't be a need to change the Dockerfile very often unless there's some (security) issues or need for additional (php) extensions or packages. I will probably still need to update the contribution guidelines to make sure that for new features that require certain extensions/packages that the Dockerfile is updated as well.

[bastien70]

Well I realized that if I want to keep contributing to this application, it would be easier for me to add a relatively simple webserver, like caddy, to the docker-compose.dev.yaml. So I could just use task docker:up:dev to up the services, and after having add dbsaver.local / mail.dbsaver.local to my systems hosts file, I can easily access the application and mail client through those URLs.

=> Ok for me ;)

And yes, when your PR will be ready, I'll try before :p

However, there shouldn't be a need to change the Dockerfile very often unless there's some (security) issues or need for additional (php) extensions or packages

=> OKay perfect :D

[ToshY]

Hey @bastien70

To give an update, the last couple of days I've been busy on fixing new CI workflows, making/publishing of the images, and testing the images. To ease the CI testing, I've done this in a temporary private repository, so I can test both the workflows as the images without making a mess here. I'm at the point that everything is green in the CI, so I'm almost ready to commit the last changes back here. I will also include Minio in it (as you mentioned it in #62 )

I hope to commit my last changes by the end of the week so it can get reviewed 👌

[bastien70]

Hello @ToshY

Greate news! :) Waiting for your last commit :)

[ToshY]

Hey @bastien70,

I think it's as good as done. I will review it myself once more this weekend before I'll mark it for review for @jmsche.

I'm missing French documentation regarding the docker compose contribution part, which I just cannot write. From my perspective, it also seems double the work to document in two languages, but maybe we should save that discussion for another time.

Currently the image repository is set to bastien70/dbsaver, which means that you should create a repository called dbsaver on Dockerhub. This is under the assumption that you've made an account with the username bastien70. If you've created your account under another username, please let me know so I can update this. After you created a Dockerhub account, could you update the repo's (action) secrets by adding DOCKER_USERNAME and DOCKER_PASSWORD? This is needed so the CI can push the new images to your dockerhub repository.

I also took the liberty to incorporate security-check and dependabot workflows. I could create a separate PR for this if you'd like, but I thought it would be just as easy to include it here as I was overhauling the CI anyway.

[bastien70]

Hello @ToshY ! Thanks for this PR :)

I will check for docker account on Monday. I have not yet looked in detail at the files you have modified, but suddenly you confirm that even with this PR it will be possible to launch the project locally and contribute without using docker for those who would like to do it "normally" ? And so you confirm once again that after this PR merged, it should be very rare that we have to edit it in the future?

[ToshY]

Hey @bastien70,

I have not yet looked in detail at the files you have modified, but suddenly you confirm that even with this PR it will be possible to launch the project locally and contribute without using docker for those who would like to do it "normally" ?

I have tried to keep the original way of working unaffected, and with that I mean, in case you and others still want to develop locally without docker, you should still be able to so (although I haven't tested this myself).

There are however some changes to existing files which could affect local development:

• The .env files are updated to use mysql:3306 instead of 127.0.0.1:3306 in the DATABASE_URL. You would need to create a .env.local and override the DATABASE_URL back to 127.0.0.1:3306 (in case you have a local MySQL instance).
• The contribution task has slightly changed, but because it already required docker compose before this change, it should not affect your current local development.
• The task commands for the current way of working are kept mostly the same, except for replacing symfony console by bin/console. This also should not effect your current local development.

And so you confirm once again that after this PR merged, it should be very rare that we have to edit it in the future?

Normally the Dockerfile itself shouldn't have the need for much update unless you decide to add some features that require additional php extensions, packages or patches for high/severe security updates. As an example, let's say you want to add the php redis extension for a new feature, this means the Dockerfile should simply be appended with the redis entry at RUN install-php-extensions redis. If you need additional packages, let's say for example FFmpeg, you can add that by just appending it to apk add ffmpeg part.

Regarding security, I made the workflow docker-base.yml, which will update both base and dev images weekly, so that you'll get patches and security updates (from PHP 8.1.X alpine) for those images. When you eventually publish a new release in Github, it will run the docker-release.yml workflow, which will build and push base, dev and prod images of the application to Dockerhub.

If you have any more questions let me know 😄

[bastien70]

Hello @ToshY !

I just created my docker account and added the secrets keys, you should be able to continue now :) https://hub.docker.com/repository/docker/bastien70/dbsaver

By the way, it could be good to add a readme to the docker repository page. If you have 5 minutes, It would be great if you send me what I should put as a readme to help people install the docker image :) Oh and you will be credited in the description of the docker image, which is normal

Tell me if you need anything else :D

And okay for your last message, so it's good for me :)

Waiting for @jmsche review

[ToshY]

Hey @bastien70

Keen eye, I totally forgot that 🤦‍♂️ I will start on that README.

[ToshY]

Hey @bastien70

Something like this?

[bastien70]

Hello @ToshY yes it seems good :)

[ToshY]

I haven't had much time to rework the last few weeks, so I'll try to this next week. Also realized that there still may be some small issues with the Dockerfile user permissions, so still have to investigate that further. Sorry for the delay in communication.

[ToshY]

Due to circumstances, I shall no longer continue my work on this. Sorry for any inconvenience this may have caused.