geopaparazzi / GSS

2 stars 5 forks source link

Add support for Docker #5

Closed frafra closed 3 years ago

frafra commented 3 years ago

Related: #3

I created a new Dockerfile and a docker-compose.yml file too. Postgres runs in a different container now.

There are various issue I encountered:

  1. Postgres username and Postgres password are read from stdin, which cannot be used with Docker easily: it would be better to read environment variables or support complete connection strings, like postgres://username:password@host/database. As workaround, I use test as database name, as it skips manual configuration via stdin.
  2. The software looks for a public directory to serve. It could be better to unbundle such functionality and use a dedicated container to serve the static files.
  3. I do not know what testPwd is meant for
  4. I used flutter create . because index.html is missing; should it be included in the repository instead of being created each time?
moovida commented 3 years ago

Hi @frafra , thanks for working on this. Your docker knowledge sure helps. Some comments:

  1. I have no real idea. It would be good to allow to configure completely. As such, you will need to bring into variable also the testPwd, since that is the one that the devices will use to connect. That one should be changable easily.

  2. I am not sure I understand. The public folder contains the flutter build folder, right. That one never gets written. As such I have the feeling it is best bundled with the backbone. But I might be wrong. I will paste at the end my bash script to show you how I right now generare the webapp.

  3. ah, ok, see 1. :-)

  4. that is strange, it is there, also because it has a message for the user, while the app is loading. Maybe that again depends on how I bundle thing. This is the script I run from the build folder locally:

PWD=`pwd`
NAME=GSS_$1
BUILDFOLDER=$PWD/$NAME
rm -rd $BUILDFOLDER
rm -rd $BUILDFOLDER.tar.gz
mkdir $BUILDFOLDER

cd $GSSSOURCEPATH

cd flutter_server_backbone/

mvn clean
mvn install 

cp -rv target/lib target/gss-backbone-$1.jar $BUILDFOLDER

cd ../flutter_server
rm -rf build/

flutter build web --release

mv build/web $BUILDFOLDER/public
cp web/favicon.png $BUILDFOLDER/public/

It is a bit messy, but you should get the idea of the steps.

frafra commented 3 years ago

@moovida I am glad to help :)

  1. Ok; I think that for a first working Dockerfile there is no need to change the Java code, but it would be a nice improvement.
  2. It is my understanding that you serve some static files from flutter_server which then interact with the flutter_server_backbone, and that your Java application is taking care of serving such static files. It is fine, but it could be worth allowing the administrator to run a web server detached from the Java application.
  3. Ok! I will test the connection between my smartphone and GSS soon.
  4. Thanks for the script. I see you use --release, which I just added to the proposed Dockerfile. I still need to use flutter create .; I wonder if that is because you have an uncommitted file or if this is just a compatibility issue I hit with Flutter 2.0.6.
moovida commented 3 years ago
  1. Ok; I think that for a first working Dockerfile there is no need to change the Java code, but it would be a nice improvement.

Not sure I am getting what you would like to have at a code level change. You mean it would be nice to have some java -Denvvar="blah" options?

  1. It is my understanding that you serve some static files from flutter_server which then interact with the flutter_server_backbone, and that your Java application is taking care of serving such static files. It is fine, but it could be worth allowing the administrator to run a web server detached from the Java application.

I would agree if we were talking about a webserver detached from the app. In this versione we are making use of an embedded jetty server, so I kind of see everything very mixed up and am keen keeping it as one single bundle.

  1. Ok! I will test the connection between my smartphone and GSS soon.
  2. Thanks for the script. I see you use --release, which I just added to the proposed Dockerfile. I still need to use flutter create .; I wonder if that is because you have an uncommitted file or if this is just a compatibility issue I hit with Flutter 2.0.6.

hah, you were absolutely right. It was ignored. Thanks for pointing out. I just pushed it up.

PS: I am right now solving some bugs that some highschool classes found while using the server intensively. I will be happy to assist you and am appreciating your work, but I might not be of much more help right now.

frafra commented 3 years ago

Not sure I am getting what you would like to have at a code level change. You mean it would be nice to have some java -Denvvar="blah" options?

Docker containers are usually configured by means of environmental variables. The proposed run.sh relies on some environmental variables, which are then passed as argument to your program. We could use the same approach for the Postgres username and password, but these are sensible information, so it could be better to have your program reading from environmental variables (or from a file), check if such variables exist and use them instead of asking information to the user interactively. System.getenv could be used I think. It could be a good idea to look for POSTGRES_USER and POSTGRES_PASSWORD as they are variable names already being used by the postgis/postgis container. I also think that the check could just be on the username, as password could be optional in some setups (even if they are uncommon).

I would agree if we were talking about a webserver detached from the app. In this versione we are making use of an embedded jetty server, so I kind of see everything very mixed up and am keen keeping it as one single bundle.

Nothing against that actually; these are more design choices :)

hah, you were absolutely right. It was ignored. Thanks for pointing out. I just pushed it up.

Great! I just rebased my branch against your latest master and the Flutter workaround is not needed anymore :)

PS: I am right now solving some bugs that some highschool classes found while using the server intensively. I will be happy to assist you and am appreciating your work, but I might not be of much more help right now.

Thank you for stating that. I do not expect to have support or help for free of course, so I was hoping to start contributing on things I am familiar with, while reporting issues on some other aspect which would require more time to fix, as I never modified the SMASH codebase.

moovida commented 3 years ago

[...]

System.getenv could be used I think. It could be a good idea to look for POSTGRES_USER and POSTGRES_PASSWORD as they are variable names already being used by the postgis/postgis container. I also think that the check could just be on the username, as password could be optional in some setups (even if they are uncommon).

Ok, that makes sense to me. One doesn't exclude the other. Actually, if you make me a list of environmental variables you would need, I can add them to the server right now, since I am on it. It would be a good thing to make the postgresql url a single variable, so it would follow teh actual course (jdbc:postgresql://$POSTGRES_HOST:$POSTGRES_PORT/$POSTGRES_DB).

So we would have:

What about the workspace path? Should we add that also?

PS: I am right now solving some bugs that some highschool classes found while using the server intensively. I will be happy to assist you and am appreciating your work, but I might not be of much more help right now.

Thank you for stating that. I do not expect to have support or help for free of course, so I was hoping to start to contributing on things I am familiar with, while reporting issues on some other aspect which would require more time to fix, as I never modified the SMASH codebase.

That is great, thanks.

frafra commented 3 years ago

Actually, if you make me a list of environmental variables you would need, I can add them to the server right now, since I am on it. It would be a good thing to make the postgresql url a single variable, so it would follow teh actual course (jdbc:postgresql://$POSTGRES_HOST:$POSTGRES_PORT/$POSTGRES_DB).

We could use a single variable for the connection string of course :)

I would reuse the environmental variables that containers like postgres or postgis/postgis use for better consistency (so in the compose file you can set the same variable for both postgis and gss at the same time - see &default-common-env-gss in my code changes).

So we would have:

* POSTGISURL -> expecting: jdbc:postgresql://host:port/database

I would suggest POSTGRES_URL just for consistency with the variables below.

* POSTGISUSER

I would suggest POSTGRES_USER for consistency with Postgres/PostGIS containers

* POSTGISPWD

I would suggest POSTGRES_PASSWORD (see above).

* MOBILEPWD

That is fine, but if you really like to have consistency something like MOBILE_PASSWORD would look more similar to the other variables (I used CLIENT_PASSWORD already in my code).

What about the workspace path? Should we add that also?

That is not really relevant in containers, so I would not change it.

moovida commented 3 years ago

Ok, you got me. I just pushed an implementation reading the environment with the names you propose.

I made it in the middle of some other task, so a functionality check would not hurt :-)

frafra commented 3 years ago

I think the changes made by commit a878cee570117b514b1506efe22bcfd802d2c2cc are not enough, as there is the "keystore" argument in the command line argument lists that is between the MOBILE_PASSWORD and the postgres credentials. That means that using the environmental variable MOBILE_PASSWORD would not be possible when specifying a keystore argument, so it could be needed to change the order of the arguments or just set the keystore to a default path if missing.

I had some issues because I was using the same connection string I used as command line argument as POSTGRES_URL, while they are different: instead of jdbc:postgresql/postgres:5432/databasename I had to use postgres:5432/databasename as the prefix is not stripped out, which is fine if you are aware of that :D

moovida commented 3 years ago

Outch, sorry for that. I had to leave. I will do this later or first thing tomorrow. I will keep you posted.

Actually this arguments are very weak and it might be better to make them a bit more robust in handling.

frafra commented 3 years ago

I rebased my branch on the latest master, further improved run.sh, Dockerfile and the compose file. I am satisfied with the final result. Thank you :)

frafra commented 3 years ago

I think that the branch can be merged. I would suggest squashing all the changes, as many commits make no sense with the current master.

moovida commented 3 years ago

I rebased my branch on the latest master, further improved run.sh, Dockerfile and the compose file. I am satisfied with the final result. Thank you :)

Ok, great. Just to make sure, where are we? :-)

I am fixing a last thing on distorted images and should then be able to make a little release, if nothing new comes up.

Does this docker file create a complete working image? Or two (one app and one postgis?) How is that built? Would you plane to push one to docker hub?

Last but not least, would you mind to create a docker folder and put relevant files in there? I would rather not haveing them lying around in the root folder.


@frafra, did you read this? It seems to be after your message, even if the timestamp is before.

frafra commented 3 years ago

Just to make sure, where are we? :-)

I am fixing a last thing on distorted images and should then be able to make a little release, if nothing new comes up.

I did some basic functionality testing and I didn't hit any major issue.

Does this docker file create a complete working image? Or two (one app and one postgis?)

This Dockerfile create a single image, but it is a multistage image. That means which it uses Flutter SDK to build the webapp, openjdk with maven to build the backbone, but the final image is based on openjdk only. I would like to create a way smaller image, by using jlink, but I have a problem with a dependency: https://github.com/TheHortonMachine/hortonmachine/issues/64

The compose file refers to two images: GSS, which is built, and a standard postgis image, which is fetched from DockerHub.

How is that built?

All the steps needed to build the image are included in Dockerfile, so it is just a matter of cloning the repository and running a docker-compose up --build for having the whole stack up and running. Let me know if you have specific questions about the Dockerfile.

Would you plane to push one to docker hub?

Dockerhub can be linked to a GitHub repository, so that it builds images automatically (master and tags for example). I was hoping the proposed Dockerfile could become official and replace the existing Docker image, After publishing the image, the compose file can be updated, adding an image attribute, so people can just fetch the image instead of building one if they like.

Last but not least, would you mind to create a docker folder and put relevant files in there? I would rather not haveing them lying around in the root folder.

Uhm, that could be done, but the context should be set when building the image, because Docker takes the current directory as context when looking for files and folders. It is something to take into account when configuring DockerHub autobuild or a CI system.

moovida commented 3 years ago

This Dockerfile create a single image, but it is a multistage image. That means which it uses Flutter SDK to build the webapp, openjdk with maven to build the backbone, but the final image is based on openjdk only. I would like to create a way smaller image, by using jlink, but I have a problem with a dependency: TheHortonMachine/hortonmachine#64

I saw that issue, but am not sure where the problem resides. If it pulls the maven dependencies, it should work, the release is on maven central. Do you have more info?

Dockerhub can be linked to a GitHub repository, so that it builds images automatically (master and tags for example). I was hoping the proposed Dockerfile could become official and replace the existing Docker image, After publishing the image, the compose file can be updated, adding an image attribute, so people can just fetch the image instead of building one if they like.

If you could set something like that up, that would be awesome. It would be great of we could create one on tag creation. The name is taken from the name you added to the pom, right?

Uhm, that could be done, but the context should be set when building the image, because Docker takes the current directory as context when looking for files and folders. It is something to take into account when configuring DockerHub autobuild or a CI system.

Ok, forget my previous request, I had not figured that yo wanted to go for an automated release. And I agree that would be good to have.

frafra commented 3 years ago

This Dockerfile create a single image, but it is a multistage image. That means which it uses Flutter SDK to build the webapp, openjdk with maven to build the backbone, but the final image is based on openjdk only. I would like to create a way smaller image, by using jlink, but I have a problem with a dependency: TheHortonMachine/hortonmachine#64

I saw that issue, but am not sure where the problem resides. If it pulls the maven dependencies, it should work, the release is on maven central. Do you have more info?

I have no idea why it fails. I tried to look on the internet, but I am not familiar with JLink or with Java in general. I also tried to exclude it, but there are two modules exporting the same package, which should not happen, and that blocks jlink ( Error: Modules xpp3 and java.xml export package javax.xml.namespace to module c3p0). It looks like that for the moment the final image would be a bit bigger than needed.

Dockerhub can be linked to a GitHub repository, so that it builds images automatically (master and tags for example). I was hoping the proposed Dockerfile could become official and replace the existing Docker image, After publishing the image, the compose file can be updated, adding an image attribute, so people can just fetch the image instead of building one if they like.

If you could set something like that up, that would be awesome. It would be great of we could create one on tag creation. The name is taken from the name you added to the pom, right?

I could, but it would be under my comany name. You could give permission to my account on dockerhub to manage GSS Docker containers, even temporarly, so I can set it up for you if you want.

Uhm, that could be done, but the context should be set when building the image, because Docker takes the current directory as context when looking for files and folders. It is something to take into account when configuring DockerHub autobuild or a CI system.

Ok, forget my previous request, I had not figured that yo wanted to go for an automated release. And I agree that would be good to have.

I just added a commit for that. Docker compose can take care of that now, and invoking docker build is pretty easy too: docker build -t gss -f docker/Dockerfile . (you just need to use the -f option). Different CI system can have slighly different way to specify a context which is different from where the Dockerfile is located.

Feel free to ignore my last commit if you wish. I can revert it and/or squash everything if needed.

frafra commented 3 years ago

I squashed all the commit except the last one. I am building an image on Dockerhub, for testing, because I do not think you can give me control over your dockerhub page, as you are using your personal account and not an organization. After that I can help you set up your dockerhub page and connect it to github if you with.

https://hub.docker.com/r/frafra/gss

moovida commented 3 years ago

I have no idea why it fails. I tried to look on the internet, but I am not familiar with JLink or with Java in general. I also tried to exclude it, but there are two modules exporting the same package, which should not happen, and that blocks jlink ( Error: Modules xpp3 and java.xml export package javax.xml.namespace to module c3p0). It looks like that for the moment the final image would be a bit bigger than needed.

I was testing this locally and on the HM build... I can't reproduce yours (if there are two packages with the same name it will indeed not build), but I have another error, which I am not sure where it comes from:

Could not resolve dependencies for project org.hortonmachine:hm-apps:jar:0.10.3-SNAPSHOT: Failed to collect dependencies at org.mapsforge:mapsforge-map-awt:jar:0.15.0 -> com.github.blackears:svgSalamander:jar:v1.1.1:

Seems a missing dependency, all of a sudden. :-/

I could, but it would be under my comany name. You could give permission to my account on dockerhub to manage GSS Docker containers, even temporarly, so I can set it up for you if you want.

Yes, would be happy to. What is your username?

I just added a commit for that. Docker compose can take care of that now, and invoking docker build is pretty easy too: docker build -t gss -f docker/Dockerfile . (you just need to use the -f option). Different CI system can have slighly different way to specify a context which is different from where the Dockerfile is located.

Feel free to ignore my last commit if you wish. I can revert it and/or squash everything if needed.

Ok, I will merge in with your last commit in it if that doesn't cause troubles.

moovida commented 3 years ago

I squashed all the commit except the last one. I am building an image on Dockerhub, for testing, because I do not think you can give me control over your dockerhub page, as you are using your personal account and not an organization. After that I can help you set up your dockerhub page and connect it to github if you with.

https://hub.docker.com/r/frafra/gss

Listen, I know I would not be able to keep up with this, so I would also be happy with you publishing this on your company's page, if that is of interest to you. For the project it means external community endorsement, which is always good, for me less work on something I am not good at. But sure, you will have to decide over that :-)

Tagging you just because I am not sure if you saw it @frafra

frafra commented 3 years ago

I have no idea why it fails. I tried to look on the internet, but I am not familiar with JLink or with Java in general. I also tried to exclude it, but there are two modules exporting the same package, which should not happen, and that blocks jlink ( Error: Modules xpp3 and java.xml export package javax.xml.namespace to module c3p0). It looks like that for the moment the final image would be a bit bigger than needed.

I was testing this locally and on the HM build... I can't reproduce yours (if there are two packages with the same name it will indeed not build), but I have another error, which I am not sure where it comes from:

Could not resolve dependencies for project org.hortonmachine:hm-apps:jar:0.10.3-SNAPSHOT: Failed to collect dependencies at org.mapsforge:mapsforge-map-awt:jar:0.15.0 -> com.github.blackears:svgSalamander:jar:v1.1.1:

Seems a missing dependency, all of a sudden. :-/

You are using a different version of hortonmachine (0.10.3-SNAPSHOT instead of 0.10.1, which is used in this repository).

I would also like to help you in setting up a CI system for the pull requests here, so that when someone proposes a change, such change can be tested a bit (like trying to build it and put it in a Docker image). This could avoid regressions etc.

I could, but it would be under my comany name. You could give permission to my account on dockerhub to manage GSS Docker containers, even temporarly, so I can set it up for you if you want.

Yes, would be happy to. What is your username?

frafra, both here and on docker hub.

I just added a commit for that. Docker compose can take care of that now, and invoking docker build is pretty easy too: docker build -t gss -f docker/Dockerfile . (you just need to use the -f option). Different CI system can have slighly different way to specify a context which is different from where the Dockerfile is located. Feel free to ignore my last commit if you wish. I can revert it and/or squash everything if needed.

Ok, I will merge in with your last commit in it if that doesn't cause troubles.

Ok! :)

moovida commented 3 years ago

Ok, merged, thanks. We can move on in parallel from here :-)

moovida commented 3 years ago

You are using a different version of hortonmachine (0.10.3-SNAPSHOT instead of 0.10.1, which is used in this repository).

Ah, right.

I would also like to help you in setting up a CI system for the pull requests here, so that when someone proposes a change, such change can be tested a bit (like trying to build it and put it in a Docker image). This could avoid regressions etc.

Always open for good suggestions that help the quality of the system.

We might better move discussion into a new issue for that?

frafra commented 3 years ago

Always open for good suggestions that help the quality of the system.

We might better move discussion into a new issue for that?

Sure!