search

eXist-db / docker-existdb

Docker image builder for eXist-db
GNU Affero General Public License v3.0
11 stars 6 forks source link

Enhance 10 build stages #20

Closed grantmacken closed 6 years ago

grantmacken commented 6 years ago

This is a WIP rework of the Dockerfile to build from openjdk-slim It also trys to collapse the process logic in build.sh into the Dockerfile The only thing I had trouble with was copying over the webapp files I've commented out what I tried, and just copied over the whole webapp dir instead. I think I got it all, but there might be stuff I've missed

I created a config dir, moved known config files into it, and symlinked to back to origin. This allows the user to have a persistent docker volume consisting of eXist configuration files. See the docker-compose file for an example run-time declaration.

The startup logs look OK. Dashboard works. In logs seem to be getting errors

ERROR (EXistServlet.java [doGet]:278) - GET Connection has been interrupted
...
java.io.IOException: Broken pipe

Dockerfile adjustment tasks

docker-compose.yml adjustment tasks

duncdrum commented 6 years ago

very cool thank you. So we could basically delete build.sh ?

adamretter commented 6 years ago

Hmm so I don't know if I understand this, it's far out of my Docker comfort zone (which is fine), but I have a couple of questions:

  1. When you say:

    its quicker to build if we don't use git but grab the release tar The issue here is that I/we need to build any version of eXist-db into a Docker Image, not just releases. I should at any point on my own computer be able to use the docker-existdb to generate a new Docker repo of any git commit I want.

  2. How are you making the changes to conf.xml so that it points at the correct data-dir and journal-dir. Are you still using Augeas? or some other mechanism?

grantmacken commented 6 years ago

How are you making the changes to conf.xml so that it points at the correct data-dir and journal-dir. Are you still using Augeas? or some other mechanism

I'm not, the build output from the eXist default build process (i.e. like when you clone and run build.sh ) is not changed. So the default conf.xml is used, as well as the default data dir. The docker container difference is that conf.xml is now a symlink to config/conf.xml and the data and config dir can be docker volumes.

The only changed item that does not come from the fresh build (from eXist release sources) is the Customised Config Files section where the altered log4j2.xml is copied over

COPY ./src/log4j2.xml $EXIST_HOME/config
grantmacken commented 6 years ago

Are you still using Augeas

I tried to follow the process logic in build.sh as close as possible. When I tried to use apt to get Augeas it was not available in the debian repo. Its still possible to build from source, however I am not sure what will be gained by using Augeas, if the only alteration is log4j2.xml

grantmacken commented 6 years ago

So we could basically delete build.sh

Yep. Apart from

the cmd call sequence should be the same.

grantmacken commented 6 years ago

can you add more info to the readme about the conf dir

@duncdrum

adamretter commented 6 years ago

@grantmacken I think my query about the tar release vs git clone above in point (1) was missed - https://github.com/eXist-db/docker-existdb/pull/20#issuecomment-411072455

grantmacken commented 6 years ago

I think my query about the tar release vs git clone above in point (1) was missed - #20

The issue here is that I/we need to build any version of eXist-db into a Docker Image, not just releases. I should at any point on my own computer be able to use the docker-existdb to generate a new Docker repo of any git commit I want.

I was working from an understanding that there was a --build-arg hook that @duncdrum is working on, This hook, was in the first iteration going to be based on a release tag commit. The hook would autogenerate a build on dockerhub and provide a docker image available at the dockerhub registry. In this way we get understandable version releases available at the dockerhub registry, that users can run as containers 

docker run -it -d -p 8080:8080  existdb/existdb:4.3.1

The requirement, to build a docker image on each commit on the eXist repo,

This of course is going to be a different --build-arg most likely the commit sha unless you just want to default to latest commit to develop and master

grantmacken commented 6 years ago

@duncdrum Ok, will do. If I've got this right we are going to maintain 2 separate branches with 2 distinct Dockefiles

  1. the master Dockerfile will be used for generating images to be pushed to the dockerhub registry
  2. the develop Dockerfile will be used for generating images based on commits to eXists develop (and master?) branch

If this is the case, I will

  1. as suggested - create a new pr against master. The easiest way I think, is to create a new feature branch off master
  2. keep this branch PR open and adjust my task list to suit the purposes of 2 as it is already branched off develop
adamretter commented 6 years ago

You don't need to open a new PR. If you click edit on the PR you can change the branch you are merging to.

adamretter commented 6 years ago

I am still not clear. Whilst Docker hub is important, it's not the only use case.

Can I use this on my machine to build a Docker Image from any commit of eXist db? If not, then this is not too good, as we are loosing existing functionality that we already have, need, and use!

grantmacken commented 6 years ago

Can I use this on my machine to build a Docker Image from any commit of eXist db?

This was never the case. The build process we had was, given a BRANCH build-arg (default to develop ) passed to the docker build command , the build would clone the eXist repo, checkout a branch then compile from the checked out branch. Therefore the image created would be based on latest pushed commit to the specified remote branch.

With the latest commit, to #20 I have reverted, back to this given a branch build-arg scenario.

However we also have this requirement #14 as mentioned by @adamretter

As we only use Git tags for releases, I think we should have a hook which builds Git tags, generating an official point release Docker image .

This generated autotagging code by @duncdrum #21 ref: 62e81de079fae7b02cb87e141bcdd4c4a0733e25 Which has a VERSION build-arg so this is now coded into #22

So as it stands now

I don't think this is ideal, and should be seen as a temporary solution. We could end up with 2 or more different Dockerfiles or attempt a branching build based on whether the build-arg is VERSION or BRANCH

If you really want to build images based on previous commits ( not tagged commits ), then the scenario would go something like Given a eXist commit SHA as a SHA build-arg ... This is doable but not the basis of this PR

adamretter commented 6 years ago

the build would clone the eXist repo, checkout a branch then compile from the checked out branch.

So I see that the git command that is now used is:

git clone --progress --depth=1 --single-branch -b ${BRANCH} https://github.com/exist-db/exist.git

In the past we used to use:

git clone https://github.com/exist-db/exist.git "${EXIST_CLONE}"
cd "${EXIST_CLONE}"
git checkout "${BRANCH_NAME}"

@grantmacken With that command we could actually provide a git commit id instead as the BRANCH_NAME and it would checkout the exact commit. Is it possible to reinstate such behaviour?

grantmacken commented 6 years ago

With that command we could actually provide a git commit id instead as the BRANCH_NAME and it would checkout the exact commit. Is it possible to reinstate such behaviour?

Ok! Will do. It just means extra bandwith to pull in the remote refs, which I was trying to avoid.

adamretter commented 6 years ago

@grantmacken Thanks I appreciate the compromise. Unfortunately it does mean extra bandwidth, but it does allow us to do what we need.

Unless we add some sort of argument, that controls this, so by default it uses the bandwidth saving version?

adamretter commented 6 years ago

@duncdrum @grantmacken Okay, so I think this is likely ready to merge now. Do you agree? We can do further improvements in a new PR, but this one is getting quite long...