Feature/updated image files #14
Conversation
|
As we should try to automate the builds later with docker hub: Will this also work there? I did not check if an option like |
|
It's on the "Roadmap" docker/hub-feedback#292 - since DockerHub automated build allows no testing I don't think we can use it. Docker Cloud is able to do this: https://docs.docker.com/docker-cloud/builds/automated-build/#set-the-build-context-and-dockerfile-location |
|
AFAIK Docker cloud is only an alternative UI. So if we use Docker Cloud, the images will still be published on docker hub. Can you verify this? |
|
@motin Can you help us out here? |
But we use travis for testing. I would not auto-trigger the builds on docker hub/cloud but always manually trigger builds. After an update we can wait until the travis reports success and then trigger an automated build when we see fit. |
Might also be possible with a web-hook, but why not simply push from Travis? |
|
That's too much automation for my taste. We don't want to trigger a build on each and every commit. |
While Docker hub allows for automatic builds which are pushed to a Docker hub repository, Docker cloud can be configured for:
I don't know how this is relevant to the testing workflow of yii2 since I am not using yii2. |
Sure we wanna trigger a build & test on every commit, but not a release & push. |
|
@motin Is there a (free) version of Docker Cloud, which can be used to build images with a custom build context, which is not possible on Docker Hub, at the moment. |
Yes, all accounts include 1 free managed node and building on Docker cloud's infrastructure is free as long as no private repositories are used.
Don't know, haven't tried using a custom build context. |
|
So before merging we should verify that custom build contexts are supported on docker cloud. |
That's what travis does. |
Check, works! See https://cloud.docker.com/swarm/schmunk42/repository/registry-1.docker.io/schmunk42/yiisoft-yii2-php/builds/34ff3a97-670c-46b0-8f90-0fda8e59de84 - you need an account I think. Could you check, if you can link this repo to an account, it does not show up in my list. |
|
There's already a yiisoft account on docker hub which probably also can be used for docker cloud. I've asked @samdark some time ago, to check if we can get access to this account. Actually we should make this an "Organization" - not sure if a similar entity exists at docker cloud. @schmunk42 Please wait before building any images with docker hub/cloud. It's still too early to go public with something. |
|
There is https://hub.docker.com/r/yiisoft/ @samdark @cebe You have control over this? We can use the one free private account initially - would also provide security scans. I don't wanna release it publicly yet also, but setting up the build pipelines is a major task of this project. |
I know, but please wait with this until we find an agreement of what we want to do, e.g. which flavours we want to build. It's still too early for this. |
|
It's all linked, this PR actually only makes sense after knowing that we can build on Docker Cloud and Travis with a build context. After merging this, I'd like to update #13 accordingly. No public releases involved. |
|
Yes, that's what I meant: A simple quick test with docker cloud is enough. Not sure what you mean by "build chain" - we don't need a complex setup there yet. Travis is a different story. The tests already pass, as is shown in the PR, so nothing else to check here. |
Ie. the combination of a GutHub repo, testing on Travis and potentially build & security scan on Docker Cloud.
Cool, so this is ready to merge. |
But shouldn't all this happen during the tests with travis? I see no connection to docker cloud as we should trigger builds manually there. |
I'd be fine with this.
Yes, one option. Or if there's a green build. I just wanted to make sure the setup works also on the Docker infrastructure. |
| @@ -50,5 +50,5 @@ RUN { \ | |||
| && a2enmod rewrite | |||
|
|
|||
| # Install composer | |||
| COPY install-composer /install-composer | |||
| COPY image-files / | |||
There was a problem hiding this comment.
Instead of adding the whole directory, shouldn't we be explicit, which files exactly we copy? Alternatively, copy the files to a directory /install-scripts or something in the container and below remove the complete directory again.
There was a problem hiding this comment.
@schmunk42 Have you seen my comment here?
There was a problem hiding this comment.
The structure within the image-files directory is the same as they'll land on the image.
So /usr/local/bin/docker-composer-install, according to https://github.com/docker-library/php/blob/master/7.1/Dockerfile#L129 - or opt or install-scripts
There was a problem hiding this comment.
@schmunk42 Actually I think, we should try to remove the install script completely and instead convert it to 2-3 lines in the Dockerfile. The instructions for how to install composer on getcomposer.com are confusing now to put it mildly. They show 2 ways, both of them using weird code. We can do better I think. For example this code from their website:
EXPECTED_SIGNATURE=$(wget -q -O - https://composer.github.io/installer.sig)
php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');"
They use wget in the first line and php in the second line for one and the same task: download a file. Why???
I can provide a simplified set of commands, if you want.
There was a problem hiding this comment.
Let's keep whatever way in a script, this is much easier to maintain than in the Dockerfiles, since there are already so many of them.
There was a problem hiding this comment.
You won't get less layers than one for all possible files we need to add - which we have with this method.
We hardly ever have to change it anymore.
I always doubt that, there are already 2 ones as you mentioned. Looks like this should be changed in composer ;)
There was a problem hiding this comment.
You won't get less layers than one for all possible files we need to add - which we have with this method.
We don't need the extra COPY and RUN statement, so it's 2 fewer layers.
The install script is so trivial that there can't be frequent changes. If it ever changes it makes no difference if we only have one file or if we have to go through a hadn full of Dockerfiles. The builds would break anyway so we'd notice for sure, if we forget something.
There was a problem hiding this comment.
We don't need the extra COPY and RUN statement, so it's 2 fewer layers.
It's just one fewer ;), because you need to RUN something. But can we optimize the layers a bit later on. On almost all other points we're following docker-library closely and this is a similar approach. It's really not necessary to copy and paste this into all files.
There was a problem hiding this comment.
It's just one fewer ;), because you need to RUN something.
We already have RUN statements above where we can add this. Official docker images do the same. We should not add files just for the sake of it. Otherwhise you could move almost the whole install process into an external "setup" script and only COPY and RUN that. But that's not what we do: We try to keep simple commands inside the Dockerfile. And installing composer is a simple command. As I said before: Their example is unneccessarily long and boils down to 2 commands.
There was a problem hiding this comment.
As additional reference see the official PHP image here. They don't use an external script for a way more complex install process: https://github.com/docker-library/php/blob/master/5.6/fpm/Dockerfile#L48.
Less files, mo' fun