Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a docker registry for cache #283

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

joelwurtz
Copy link
Member

@joelwurtz joelwurtz commented Feb 27, 2024

Fix #252

WHat it does :

  • remove USER_ID features from dockerfile: Without this change the USER_ID make the image dependant on the system that run this command wich creates differents images and make them not shareable
  • run frontend / builder / worker as a specific user id : we don't use a named user anymore to avoid headcache when mapping it to a different user id
  • local and shared home directory : This allow to debug home directory and files created in it (like composer cache), allow end users to put special scripts in it (like a custom bashrc), and also allow to cache this folder or specific subfolders of this directory within the CI (like the composer / yarn cache)
  • command to generate a bake file which allow to push images to the registry
  • sudo : it use gosu to fake it since we don't have a named user, it also disable by default as it may be a security risk if image is used in production

@joelwurtz joelwurtz force-pushed the feat/registry-docker branch from 211b26f to c0902e4 Compare April 25, 2024 14:06
@joelwurtz joelwurtz marked this pull request as ready for review April 25, 2024 14:32
@joelwurtz joelwurtz force-pushed the feat/registry-docker branch 5 times, most recently from 53e57e2 to 91c5947 Compare April 29, 2024 12:35
@lyrixx
Copy link
Member

lyrixx commented May 3, 2024

Is there a simple way to get sudo back? I need it quiet often

.castor/docker.php Show resolved Hide resolved
.castor/docker.php Outdated Show resolved Hide resolved
.castor/docker.php Outdated Show resolved Hide resolved
.castor/docker.php Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
infrastructure/docker/services/php/Dockerfile Show resolved Hide resolved
infrastructure/docker/services/php/Dockerfile Show resolved Hide resolved
infrastructure/docker/services/php/Dockerfile Show resolved Hide resolved
@joelwurtz joelwurtz force-pushed the feat/registry-docker branch from 23573a0 to 334ab81 Compare May 3, 2024 09:03
@joelwurtz
Copy link
Member Author

joelwurtz commented May 3, 2024

Is there a simple way to get sudo back? I need it quiet often

done

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

.castor/docker.php Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@joelwurtz joelwurtz force-pushed the feat/registry-docker branch 2 times, most recently from b408ca5 to 19efd5e Compare May 7, 2024 07:02
@lyrixx
Copy link
Member

lyrixx commented Jun 9, 2024

Hello,

Thanks for this PR!

I tested it on jolicode/qotd#62 and

  • I miss some doc about how to use cache (not how to push, but how to get it). For example, on GHA, you have to re-install docker to make it work
  • By default, on my computer, I had to change the driver to add support for registry cache storage. It could be nice to explain how to do that
    cat /etc/docker/daemon.json 
    {
      "features": {
        "containerd-snapshotter": true
      }
    }
    
  • However, cache is still missing on GHA
  • I also tried to create a VM, and the cache is missing there too
    Vagrant.configure("2") do |config|
      config.vm.box = "debian/bookworm64"
      config.vm.synced_folder "/home/gregoire/dev/github.com/jolicode/qotd2", "/qotd"
      config.vm.provider "virtualbox" do |vb|
        vb.memory = "4096"
      end
      config.vm.provision "shell", inline: <<-SHELL
        apt-get update
        sudo apt-get update
        sudo apt-get install -y ca-certificates curl
        sudo install -m 0755 -d /etc/apt/keyrings
        sudo curl -fsSL https://download.docker.com/linux/debian/gpg -o /etc/apt/keyrings/docker.asc
        sudo chmod a+r /etc/apt/keyrings/docker.asc
    
        # Add the repository to Apt sources:
        echo \
          "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.asc] https://download.docker.com/linux/debian \
          $(. /etc/os-release && echo "$VERSION_CODENAME") stable" | \
          sudo tee /etc/apt/sources.list.d/docker.list > /dev/null
        sudo apt-get update
    
        sudo apt-get -y install docker-ce docker-ce-cli containerd.io docker-buildx-plugin docker-compose-plugin
    
        sudo usermod -aG docker vagrant
    
        sudo wget https://github.com/jolicode/castor/releases/download/v0.17.1/castor.linux-amd64 -O /usr/local/bin/castor
        sudo chmod +x /usr/local/bin/castor
    
      SHELL
    end
    

@joelwurtz
Copy link
Member Author

joelwurtz commented Jun 9, 2024

Who push the cache ?

It is highly recommendend that cache is push by the CI, version of docker / config from where the cache is pushed need to be controled as special configuration or version may make him unshareable.

For the CI also i see this

image

Did you make sur that GHA has correct permissions to read this package ?

EDIT : In your case it miss the docker login step

@joelwurtz joelwurtz force-pushed the feat/registry-docker branch 2 times, most recently from 9755e6f to 323250c Compare June 10, 2024 08:35
@joelwurtz joelwurtz force-pushed the feat/registry-docker branch from 2964de5 to 064d0db Compare June 10, 2024 09:26

RUN phpenmod app-default \
&& phpenmod app-fpm

EXPOSE 80

CMD ["runsvdir", "-P", "/etc/service"]
CMD ["runsvdir", "-P", "/var/www/infrastructure/docker/services/php/frontend/etc/service"]
Copy link
Member

@lyrixx lyrixx Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this is not handy.

I have 2 symfony apps. But I want to use the vary same image php.

So i used:

    pro:
        build:
            context: services/php
            target: web
    back:
        build:
            context: services/php
            target: web

But since the volume is mounted on both applications, there is a conflict:

root@ab4e7cad923e:/var/www# runsvdir -P /var/www/infrastructure/docker/services/php/web/etc/service
runsv php-fpm: fatal: unable to lock supervise/lock: temporary failure
runsv nginx: fatal: unable to lock supervise/lock: temporary failure

IMHO, we should NOT share container state with the host.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed like that for now

--- a/infrastructure/docker/services/php/Dockerfile
+++ b/infrastructure/docker/services/php/Dockerfile
@@ -83,6 +83,9 @@ RUN useradd -s /bin/false nginx
 
 COPY web/php-configuration /etc/php/${PHP_VERSION}
 COPY web/etc/nginx/. /etc/nginx/
+RUN rm -rf /etc/service/
+COPY web/etc/service/. /etc/service/
+RUN chmod 777 /etc/service/*/supervise/
 
 RUN phpenmod app-default \
     && phpenmod app-fpm \
@@ -90,7 +93,7 @@ RUN phpenmod app-default \
 
 EXPOSE 80
 
-CMD ["runsvdir", "-P", "/var/www/infrastructure/docker/services/php/web/etc/service"]
+CMD ["runsvdir", "-P", "/etc/service"]
 
 FROM php-base AS worker

But I don't really like it

Comment on lines +1047 to +1068
* Use buildx in each GitHub action workflow step

```yaml
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
```

* Use the official Docker binary
```yaml
# Install official version of docker that correctly supports from-cache option in docker compose
- name: Set up Docker
uses: crazy-max/ghaction-setup-docker@v3
with:
set-host: true

# Docker socket path is different when using setup docker
- name: Set Docker Socket Host
run: echo "DOCKER_SOCKET_PATH=${DOCKER_HOST:5}" >> $GITHUB_ENV
```

The second option is faster (there is no need to transfer images between buildx and local docker), but it is not
officially supported by GitHub actions and may break in the future.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record, I tested both ways, and the second way was twice (2×) faster!
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a default Docker registry or layer cache by default
4 participants