search

lucee / lucee-dockerfiles

Official Lucee Dockerfiles for Docker Hub build images
https://hub.docker.com/u/lucee/
MIT License
85 stars 50 forks source link

Travis Build Configuration #44

Closed rye closed 5 years ago

rye commented 6 years ago

As-is, this PR won't work unless: (i) Travis is enabled on this repository, (ii) the docker.io/lucee/lucee namespace exists, (or something like that; I will need to change the configuration either way once I know where this may get deployed) (iii) the environment variables DOCKER_USERNAME and DOCKER_PASSWORD are set to the username and password of an account that has access to the aforementioned namespace, and (iv) the values in matrix.yaml and other such documents are updated to match this configuration.

This PR:

Why Travis?

Travis lets us do things for free, and is happy to handle long-running builds, but also has support for environment matrices. I chose not to use something like CircleCI instead because without paying, we're limited in ways that Travis does not.

Why Python?

We generate the build matrix in Python primarily because we wanted to create a nice configuration file (matrix.yaml) which can be used for doing all upgrade work. One just needs to add lines and remove lines in this YAML file; as conflicts come up, one can add exclusions, etc. The build-images.py script takes this file as input as well, but the generate-matrix.py program generates the Travis configuration based on the initial configuration defined in matrix.yaml so that it matches.

Why build matrices?

We use a build matrix based on all of the different Tomcat version and platform combinations available to us, which means that each build worker downloads one specific Tomcat image and gets to reuse it for all of its builds. This build matrix also speeds up the overall build by introducing up to 8x (currently) parallelism, though practically this gets capped at 4x.

Some other notes:

This configuration generates the following tags:

Naturally, certain combinations can be excluded.

Please note that I'm very open to questions and feedback on this PR! This is a pretty huge change operationally, and I'm happy to make any fixes or refinements you would like. I'd call this highly-WIP as a result. I am also allowing edits from maintainers, so you can go ahead and edit anything, too, if you'd like.

To my understanding, this PR closes #40? Feel free to correct me if I'm wrong.

CC @hawkrives

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

justincarter commented 6 years ago

This is really awesome, thanks so much for the PR :) I'll put aside some time tomorrow to configure Travis and do some testing, and then I'll try to provide some feedback on any changes that I think we might need to make before we merge.

hawkrives commented 6 years ago

@rye I was looking at some other Docker Library images, and they tend to have separate folders per dockerfile.

Should we "pre-templatize" the images into folders, or just stay with the dynamic file we have here?

rye commented 6 years ago

@hawkrives, I've seen this pattern too.

I think that since—at least, from our testing—it works as it is, we don't really need to "pre-templatize" it. Then we're just copying the file around and introducing the possibility for divergent change, which may or may not be an intended feature. Since our Dockerfile downloads the .jar file based on what its version is, and that's pretty much the only part that is different, I'm inclined to think we can avoid that.

However, if such a possibility should remain open, we could just go the safer route and pre-templatize Dockerfiles as you suggest. This would also enable piecewise upgrades of each version of Lucee separately, (or the introduction of different behavior in future versions) and the build complexity would probably imaginably shrink.

hawkrives commented 6 years ago

It may be worth testing out? I also think it would shrink the complexity.

If we copy the supporting files for each minor version of Lucee, it should make it easier to support new versions if they need changes. And the existing versions would just need to download the JARs, so they shouldn't need to have folders for the patch releases.

Now I'm rambling. Anyway. I think we should prototype folders for the supporting files / Dockerfile.

justincarter commented 6 years ago

I've done a local merge of this PR to resolve conflicts and pushed a branch with the changes, configured Travis and temporarily set it up to push images to my own Docker Hub account as a test. The matrix has just been trimmed so that I could see it build a smaller number of image combinations, but I'll put them all back for the next tests.

I'm waiting on some JARs to be published on cdn.lucee.org so that the builds will work for the latest 5.2.x releases. Once they're up I'll do some new builds and test with some real apps. After that I think new releases should be ok to be built via Travis :)

As for having a Dockerfile per minor version, I think we can avoid it for now as the processes are all the same and I don't imagine that will change with Lucee 5.3.x either. If we need to move to separate Dockerfiles in the future then we can make those changes then.

@rye Did you have any other outstanding changes or things you might need to discuss? I'm aware this PR has been outstanding for quite a while, and I've been super busy/distracted with other things. Hopefully we can wrap this up in time for the next releases though. Please let me know what you think!

cheers, Justin

hawkrives commented 6 years ago

@justincarter As far as I'm concerned (seeing as I'm not @rye), I'm good with the current state of this PR! I've since dropped my pre-templatization idea.

Thanks for looking this over!

rye commented 6 years ago

@justincarter I'm very glad to hear that this PR is in the plans still!

I have no qualms. I can go through and do a check to see if there are any last-minute improvements I can offer, but besides that I think this is good to go.

One option that I can think of is to just start a new lucee-docker GitHub repository or something, so that existing history is preserved as it was and the lucee/lucee namespace on Docker Hub could then be the final push destination, which would prevent any kind of issues that people might have if they depend on the old scheme. But that is probably not necessary.

I will note that if future tooling is necessary so that custom Dockerfiles can be set, that's something we can revisit and might be able to help out with.

justincarter commented 6 years ago

Thanks for the feedback guys :)

I think I can create a branch for the old Docker Hub automated builds and continue to update those if necessary, and then we can merge the travis branch into master as the new default. The images will definitely be published into lucee/lucee on Docker Hub.

Next steps for me;

jamiejackson commented 6 years ago

Is there anything in this build pipeline to trigger new builds when updated upstream base images are pushed? It would be convenient if this PR also addressed #42.

hawkrives commented 6 years ago

@jamiejackson No, there's nothing to update automatically from Lucee's upstream. We'd need some sort of external service to update matrix.yaml and call generate-matrix.py and push back to the repository whenever a new version of Lucee was released.

rye commented 6 years ago

I will note that Travis has an API for triggering builds which could be called whenever a new .jar is uploaded or something. But to affect which versions of images are released, or to publish a new version, the files @hawkrives mentions will need to get updated.

hawkrives commented 6 years ago

Spitballing: I suppose we could do a "latest-5.2.x" build, and have that automatically fetch the latest 5.2.x image from Lucee's site, and keep "latest" up to date that way? That could be triggered by the Travis API more easily, perhaps.

justincarter commented 6 years ago

IMO they are both issues to consider after this branch is merged (triggering a build when a Lucee build is published, or triggering a build when a new Tomcat base image is published). I'm happy to maintain the version numbers manually for now, because the jump forward here is having more flexibility / choice of builds to use.

With upstream base image builds you also need to be careful about how that change in version of Tomcat might affect existing users of a specific tag. I prefer to advance the Tomcat version when the Lucee version also advances to avoid breaking end users apps. For older versions of Lucee we will come up with a strategy for that (noting that 4.5.x has since been updated, but it's also nearing EOL).

On Sat, 25 Aug. 2018, 3:29 am Hawken Rives, notifications@github.com wrote:

Spitballing: I suppose we could do a "latest-5.2.x" build, and have that automatically fetch the latest 5.2.x image from Lucee's site, and keep "latest" up to date that way? That could be triggered by the Travis API more easily, perhaps.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lucee/lucee-dockerfiles/pull/44#issuecomment-415827571, or mute the thread https://github.com/notifications/unsubscribe-auth/AAVYDflbquYGF7v0XEoifHXRXYm2TFBwks5uUDgSgaJpZM4UdmJ_ .

modius commented 5 years ago

Just a heads up.. @justincarter will be moving this into master post 5.3 release. And we'll be supporting the build matrix from that version onward. Hopefully the 5.3 release is ready to drop before Christmas 🤞 Otherwise we might have to rethink our plans.

justincarter commented 5 years ago

I've had a nice block of time to work on this before Lucee 5.3 drops and I've been able to add a few more features which should allow us to achieve automated builds. In a nutshell;

You can see the current images and tags here; https://hub.docker.com/r/lucee/lucee/tags/

After some more time for testing and writing up documentation I'll be happy to push this through as the set of officially supported Docker images. For now the latest code is still in the travis-build-matrix branch of this Git repo, and in the coming days I'll look to branch the old code in master for legacy purposes and merge this branch into master.

Finally, we should also be able to trigger the Docker image builds after a Lucee build finishes. This will be great for anyone who needs to take advantage of a fix in a Snapshot build and for getting the Docker images out immediately rather than waiting for a manual build to happen.

Please feel free to look at the changes, test the images, and offer any feedback that you have! 😃

hawkrives commented 5 years ago

@justincarter the changes look pretty good to me! (diff from @rye's branch: https://github.com/rye/lucee-dockerfiles/compare/travis-build-matrix...lucee:travis-build-matrix)