rojopolis / spellcheck-github-actions

Spell check action
MIT License
132 stars 38 forks source link

Investigate migration to Alpine over Debian #13

Closed jonasbn closed 3 years ago

jonasbn commented 4 years ago

This issue was spun of the below list.

This issue is aimed at addressing the migration to Alpine from Debian in the Docker image.

Hi @edumco

Thanks for the elaborate feedback. I have compiled your comments to this list:

Originally posted by @jonasbn in https://github.com/rojopolis/spellcheck-github-actions/pull/12#issuecomment-636716234

jonasbn commented 4 years ago

Comments lifted from #12

Just use the --no-cache and the apk will be found download and installed without touching the cache.

Ex: apk add --no-cache package

3) Temporary package installation

Sometimes we just gonna use a package during build or installation (gcc) so there are 2 approaches:

* Create a multistage build: Great but more complex

* Install, use and uninstall on the same `RUN`: good for a single package

Ex: RUN apt install unzip && unzip myfile.zip && apt uninstall unzip

4) Building wheel for lxml takes a huge amount of time.

This doeesn't feel right. i believe it could be some bad config in lxml setup.py.

5) Alpine is so good at making small images. Dont give up!

github-action-spellcheck:alpine 106MB github-action-spellcheck:latest 211MB

aSemy commented 3 years ago

Hi there, maybe a bit of a weird comment but I saw the discussion here about slimming down Docker images and I wanted to give it a go, as a learning experience. I'm not an expert at all, but here's how I think a multi-stage spellcheck Docker image could look.

I also used some of the new Docker features, like using COPY --chmod to combine setting permissions with the copying.

Massive disclaimer: I am not at ALL familiar with Github Actions, so I don't even know how to test it! Hopefully someone can complete my work, or use it as inspiration :)

Building

Target languages can be specified as a comma separated list a build-arg.

docker build -t spellcheck --build-arg SPELLCHECK_LANGS="en,fr,hr" .

Creating a new image with new languages will re-use the base image, hopefully meaning building is much faster.

docker build -t spellcheck2 --build-arg SPELLCHECK_LANGS="pa,ro,sk,uz" .

Dockerfile

# default are English and German
ARG SPELLCHECK_LANGS="en,de"

# Builder stage: configure env vars, labels, add plain files, install common packages
FROM python:3.9.2-slim as spellcheck-builder

LABEL "com.github.actions.name"="Spellcheck Action"
LABEL "com.github.actions.description"="Check spelling of files in repo"
LABEL "com.github.actions.icon"="clipboard"
LABEL "com.github.actions.color"="green"
LABEL "repository"="http://github.com/rojopolis/spellcheck-github-actions"
LABEL "homepage"="http://github.com/actions"
LABEL "maintainer"="rojopolis <rojo@deba.cl>"
# label as builder stage, for easy cleanup (e.g. `docker image prune --filter label=stage=builder`)
LABEL stage=builder 

COPY --chmod=755 entrypoint.sh /entrypoint.sh
COPY requirements.txt /requirements.txt
COPY spellcheck.yaml /spellcheck.yaml
RUN pip3 install -r /requirements.txt

# https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#apt-get
RUN apt-get update && apt-get install -y \
    aspell \
    && rm -rf /var/lib/apt/lists/*

######################################################################################################

# Custom built images with an arg
FROM spellcheck-builder as spellcheck

RUN apt-get update && apt-get install -y \
    # split arg SPELLCHECK_LANGS to space separated, and prepend each with 'aspell-'
    $(echo "$SPELLCHECK_LANGS" | sed 's/,/ /g' | xargs printf -- 'aspell-%s\n')     \
    && rm -rf /var/lib/apt/lists/*

WORKDIR /tmp
ENTRYPOINT ["/entrypoint.sh"]
edumco commented 3 years ago

Hi @aSemy this idea of creating a Build ARG to target the build on specific languages is great.

This thread has been closed but we could create a pull request with this suggestion so we could discuss this in a less polluted thread.

What do you think?

aSemy commented 3 years ago

Done. Thanks for the support!

jonasbn commented 3 years ago

What you came up with sounds like it is along the line of a good solution, will review the proposal (PR)

edumco commented 3 years ago

@jonasbn I think you can already close this issue.

jonasbn commented 3 years ago

@edumco done!

It will be included in release 0.15.0, which is currently shaping up