Create folder hierarchy

[krufab]

Step 2 of issue #100 .
Moved Dockerfiles in hierarchical subfolders divided by:

• jdk version (8 or 11)
• base image used (alpine, stretch, windows servercore/nano)
  Tests have slightly been adapted to print out the jdk version and base image and to build the Dockerfiles from the subfolders.
  In this way will be pretty easy and clean to add new jdk versions or base images, without polluting the root folder.

[krufab]

Regarding the naming conventions, I took inspirations from node js and openjdk.
They use the debian release name (i.e. stretch, buster) instead of the distro name, and they use alpine + release version for alpine (i.e. alpine3:10, alpine:3.11).
This makes immediate to know the base image and to choose which version to pull. Of course they tag their releases with i.e. node:10-alpine:3.11, but we also could do something similar.

If we add more jdk versions, we might need to support older base images as well as the new ones as they come out, so this folder division seems pretty flexible.

[slide]

and they use alpine + release version for alpine (i.e. alpine3:10, alpine:3.11).

We are not currently doing that though, we are just using alpine, so putting the release number in doesn't make sense unless we specify a certain version of alpine.

[krufab]

From my point of view, it makes sense for all the developers who need to use the docker-slave image as their base image and need to know which version of alpine they are running, so that they can check if which alpine packages and versions are available.
When it is not specified in the docker image tag, I check the github repository and dig down to the Dockerfile to find it out, so that I know that I can use that image or I should build my own, i.e. to support the latest node js or php-fpm (as examples, probably not with docker slave).

So for me, this information is very useful. Adding a new tag, i.e. docker-slave:alpine & docker-slave:alpine3.6, is essentially free, and would just give more information to any potential user.
Moreover, in the future, this project might support alpine3.8 and alpine3.10 for i.e. jdk 13 and you will need to tag it anyways.

Not to mention that this is a wide accepted practice with docker images.

Nevertheless, if this is absolutely not acceptable, i will rename the folder.

[slide]

I'm not saying having the version in the directory is bad, what I am saying is that the Dockerfile itself is not referencing a specific version of the alpine image, so having a version in the directory is not correct because the version of alpine could change if the upstream alpine image changes version. We need to make sure the directory and the Dockerfile match.

[krufab]

I see. In fact the Dockerfile has FROM openjdk:8-jdk-alpine. Now, checking better, that image is actually alpine3.9 and not even 3.6.
How about I replace the tag to FROM openjdk:8-jdk-alpine3.9? Using (an almost) fixed version tag is also one of the best practices for Dockerfiles.

[slide]

You could include that in the PR and it can be reviewed.

[krufab]

I checked the docker images ids for the actual images vs the ones I replaced in the PR and they are the same.

docker inspect openjdk:8-jdk-alpine  --format 'Id: {{.Id}}' 
Id: sha256:a3562aa0b991a80cfe8172847c8be6dbf6e46340b759c2b782f8b8be45342717
docker inspect openjdk:8-jdk-alpine3.9  --format 'Id: {{.Id}}'
Id: sha256:a3562aa0b991a80cfe8172847c8be6dbf6e46340b759c2b782f8b8be45342717

docker inspect openjdk:8-jdk  --format 'Id: {{.Id}}' 
Id: sha256:8c6851b1fc095a57b5bbf874261db7c7471730257128121efc60942aa44571ea
docker inspect openjdk:8-jdk-stretch  --format 'Id: {{.Id}}'
Id: sha256:8c6851b1fc095a57b5bbf874261db7c7471730257128121efc60942aa44571ea

docker inspect openjdk:11-jdk  --format 'Id: {{.Id}}'
Id: sha256:612d4d483eeea77ddc53efefaf85d4c82748768afd1ad576443b622a919614be
docker inspect openjdk:11-jdk-stretch  --format 'Id: {{.Id}}'
Id: sha256:612d4d483eeea77ddc53efefaf85d4c82748768afd1ad576443b622a919614be

[slide]

Is there possibly some way that we can find the items via script rather than hard coding the tags and so forth in multiple places? If that is beyond the scope for this PR, I understand. It's just going to be painful to move from alpine3.9 to alpine4.x (hypothetically speaking). We don't necessarily always want to publish an alpine3.9 image for instance, we may want to move to the next version, but don't want to change it all over the place. Just some thoughts.

[krufab]

It's just going to be painful to move from alpine3.9 to alpine4.x (hypothetically speaking). We don't necessarily always want to publish an alpine3.9 image for instance, we may want to move to the next version, but don't want to change it all over the place.

As first point, I disagree that it would be painful, as tags are cheap and can easily generated automatically. There is no maintenance needed as once a tag (i.e. 4.0-1) is pushed to Docker Hub, it stays there virtually forever without any intervention from the jenkinsci team.

As second point, this is exactly why many versioned tags are necessary. I would like to avoid this situation:
As developer, I need docker-slave as base image to run some scripts. At the moment of writing, it's only available latest and 4.0-1 (considering only debian stretch and jdk8 based, but same goes for alpine or jdk11).

• If I choose 4.0-1, then I have to update my base image name all the times a new one comes out, which is pretty frequent, therefore a some pointless manual work to do
• If I choose latest, then I will get the updated image without changing any name and this is pretty smooth.

However, if I'm using docker-slave:latest and by chance, jenkinsci teams decides to update it and use debian buster or decides that the new default is jdk11 or jdk15, then there is a high chance that my scripts will stop working after a image pull (which can happen for whichever reason...).
Of course jenkinsci team can inform that jdk8 or stretch will be deprecated on XX.XX.2020 and users will be able to switch to i.e. docker-slave:4.4 as the last stretch/jdk8 image. But this requires A LOT of manual work if I need to change docker-slave:latest to docker-slave:4.4 everywhere, especially on less used applications.

This whole situation could be avoided if I docker-slave comes out with some more tags, which will give me the choice to what to stick to:

• latest: jenkinsci team can update the remoting agent or the baseimage or the jdk version and I don't care
• stretch: jenkinsci team can update the remoting agent or the jdk version but not the base image as my scripts work only with stretch
• jdk8-stretch: jenkinsci team can update the remoting agent as my scripts work only with stretch and jdk8
• 4.0-1: basically my scripts work only with this version

Even in this way, the jenkinsci team is pretty much free to do any major update in every single variable (base OS, jdk or remoting) without any disruption for the users (clearly, only if they don't use latest).

The changes in this PR allow the repository to move forward in this direction. I agree that there might be better ways to automatically generate tags (i.e. like with stackbrew for official images), but this can come later on.

[slide]

The difficulty is not in the tagging, it is in making sure all the places that 3.9 is specified are modified to the new number.

[krufab]

Ok. I fail to understand your concerns, so I guess we can only agree to disagree on this point.

From my side, this PR is complete; I pushed some more changes in the Makefile so that it would build correctly (the previous build timed out) and to give examples of which tags I'd like to have for this image. I understand that the real docker-slave images are not build via that Makefile anyways.

If the jenkinsci team don't agree with my changes, then closing this PR is also an option.

[slide]

I think the changes are a step in a good direction, but I think we can improve upon it. I'll try and put some additional thoughts together.

[james-crowley]

Hey @krufab and @slide, I am trying to help out here with the PRs you, @krufab, raised but there is a lot of reading to do! :)

@krufab I like the idea of the docker-slave having more tags. More tags give users, like you, more choice. More choice is always a good thing, in my mind.

While adding more tags we need to be careful of not going crazy with tag bloat. I am all for the verbose tags and having less verbose tags as you get to the more “general” tags like alpine or debian-slim, etc.

From my understanding as docker-slave currently stands we are missing some of those additional tags. This would be a good PR/issue to raise and we can definitely come up with a solid list of the tags we want to support, but this PR is about folder hierarchy so let’s get the conversation back to the core of this PR.

I like the idea of having folders for different Java versions we support, I think that makes things more clear. I think the openjdk repo you listed is a good example of this. Another repo to look at is AdopOpenJDKs repo too, https://github.com/AdoptOpenJDK/openjdk-docker .

I think having the folder structure go something like this would be a good solution:

“Java Version(8,11,12,etc)”/”Base Linux Distribution(Alpine, Debian, ubuntu, etc)”/Dockerfile.XXX 

For example here are a couple possible folders we might see:

11/alpine/Dockerfile
8/debian/Dockerfile

I think the point @slide is getting at, is having a folder for just alpine-3.10 or alpine4.X is unneeded. I agree with this point, as it would be manually work to go in an clean up the folders/make new ones anytime a new release of alpine or any other distribution comes out.

There is a discussion to be had if we would want to have multiple Dockerfiles for those different releases of alpine or one general alpine Dockefile with a build arg for version. You can see that build arg here, https://github.com/jenkinsci/docker-slave/blob/b8a9a94ac4ec84af8f87a44f5c9242788fd9f1c9/Dockerfile-alpine#L26

I could see something like Adopts approach working out for us if we need more than one Dockerfile under a distros folder. For example check out, https://github.com/AdoptOpenJDK/openjdk-docker/tree/master/11/jdk/debian.

[krufab]

@james-crowley Thanks for your support.
My idea regarding versioned folders (i.e. alpine-3.10) comes from the fact that at one point there might be alpine 4.0 coming out and then this repo might need to build and support both versions, so then we can start having folders named accordingly anyways.
Other repositories follow this approach, i.e. node js https://github.com/nodejs/docker-node/tree/master/13.

[james-crowley]

@krufab Thanks for pointing out the Node JS repo, I saw it mentioned in your previous posts. If you take a look at the folders and the files inside, you should notice the only difference is the FROM statement. Hence, why right now I do not see the version folders being useful.

There is a discussion to be had if we would want to have multiple Dockerfiles for those different releases of alpine or one general alpine Dockerfile with a build arg for version. You can see that build arg here,

If you take a second a look at the bottom on my last comment, I think there might be a better approach to the folder version.

All these changes you are suggesting are good and are stuff that needs to be discussed. What works in the Node JS repo might work well for them, but might not work here.

[oleg-nenashev]

Merge conflict

[krufab]

@oleg-nenashev, Resolved the conflicts.
It seems to me that with the introduction of buster, this folder hierarchy shows its advantages and the verbose tagging introduces more clarity on the versions

[slide]

Please use windowsservercore instead of just servercore, this follows the usage in the adoptopenjdk docker repository (and others). They use windowsservercore because that is the root name of the image from MS. nanoserver is the root name for the nanoserver image from MS.

[slide]

Some initial review requests.

[slide]

This PR is looking really nice. I think there are just a couple of small things to discuss and fix up and then I will approve it. Thanks for your work on this!

[slide]

Anyone else have any changes to get in before we merge? I think this looks good. There are some small changes that I might make to the Windows stuff, but that could be done after this is merged.

[oleg-nenashev]

I will fix the DockerHub builds

[slide]

Is there a way we can get checks on the dockerhub builds somehow?