Starefossen / docker-aspell

:whale: Docker image for running GNU Aspell spell checking
https://registry.hub.docker.com/u/starefossen/aspell/
MIT License
3 stars 4 forks source link

GitLab CI shared-runner compatibility #3

Open commonquail opened 6 years ago

commonquail commented 6 years ago

e6426a1b7f26db19e9b5c0da6d0fc8121e288315 ("Add a aspell as a default entrypoint") seems to break usage on GitLab CI (shared-runners). All past tags have been overwritten so I can't confirm with certainty that this is the cause.

Could you be persuaded to provide an alternative image with the default entrypoint?

The story of what works in GitLab CI is long and confused, mostly covered in gitlab-org/gitlab-runner#2692, and well summarised in this particular comment (emphasis mine):

  1. Solution with entrypoint: ["/bin/sh"] (either it's sh or bash) doesn't work at all.

  2. Solution with entrypoint: ["/bin/sh", "-c"] (either it's sh or bash) works and depends on shell available inside of the image. We can see that packer supports both, terraform and curl support sh.

  3. For some reason none of the shells is working for kcov image. I don't know why this is happening, because kcov image has both shells available. From what I found /bin/sh is a symlink to /bin/dash and that's what raises the sh: 1: set: Illegal option -o pipefail error. But why sh is used when bash is available and should be detected first? I don't know.

  4. Solution with entrypoint: [""] works for all images, but not with Docker 1.12.X.

This image exhibits the problem in case 3, making it unusable in GitLab's shared runners for now. Historically, the solution to this problem was to not override ENTRYPOINT. Naturally you have no interest in removing the custom entrypoint in the primary image, which works entirely as intended and far more ergonomically than without the custom entrypoint.

Whether it were a separate image or just a tag wouldn't matter to me. It seems like the non-overriding Dockerfile could be the work-around and the overriding one could inherit from the work-around to preserve current functionality.

commonquail commented 6 years ago

Immediately after spending 2 hours researching this problem and writing the ticket, I happened across rumours of a functioning work-around that delegates to /usr/bin/env and modifies $PATH. I've managed to confirm the following working as intended:

.gitlab-ci.yml:

spellcheck en-US:
  stage: test
  image:
    name: starefossen/aspell:0.60
    entrypoint:
    - "/usr/bin/env"
    - "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
  script:
  - ./spellcheck list

where spellcheck is basically

#!/bin/bash

aspell \
    --encoding 'UTF-8' \
    --mode html \
    --personal './.aspell.en.pws' \
    --lang en-US < README.md

exit 1

with quite a bit more sophistication (actual implementation).

Image id sha256:1d2a35ff9aa9af497e3a424b99b75a156a7234c1dc2a8db8e8d612e8c6941edd.

Given that, and the expectation that eventually GitLab's shared runners will adequately support entrypoint overriding, this ticket probably is not worth pursuing.

jamesmstone commented 6 years ago

It sounds like it would still be quite helpful for you to have a sans ENTRYPOINT version. I'd be happy to make one for you. Although I guess it's up to @Starefossen wants it tied to this repo

Starefossen commented 6 years ago

I am very sorry for breaking people’s CI pipelines. That was not my intention when merging the change. I think we should revert the entrypoint and make a separate tag with the entrypoint set.

I am away from my workstation, if anyone can propose a PR to revert the entrypoint I can approve it from here.

commonquail commented 6 years ago

For what it's worth, our CI didn't break, because we haven't taken this into use yet. You should not feel guilty on my account, and since nobody else has complained I think there was no damage done. We also usually mirror community images we take in precisely to avoid accidents.

I wouldn't mind submitting a change to revert the custom entrypoint but I'm actually not sure that's the best idea on its own: that change is a month old so you'd just risk breaking other people's work flows.

I don't do enough Docker work to know what kind of solution works best long-term but I think if it were up to me I would make 2 Dockerfiles, the one with the custom ENTRYPOINT inheriting from the other and retaining the current tag name. The official Debian images use a variant of this that distinguishes via directories but they also need more scalability. I've also seen a solution that generates a temporary Dockerfile by deleting the ENTRYPOINT at build time.