Skip to content

Commit

Permalink
Build docker container for all linux target architectures386, `amd6…
Browse files Browse the repository at this point in the history 
…4`, `arm/v6`, `arm/v7`, `arm64/v8`, `ppc64le`, `s390x` (#797)

* Fixes #768

* buildx build

* We need `setuptools_scm` to run `Makefile`

* Build `docker` container using `wheel`

* Download package distribution

* Adjust supported platforms

* Alpine only supports v6

* .

* May-be fix `lsb_release`

* Enable `multiarch` env for docker buildx

* Deprecate `DOCKER_IMAGE_TAG` which removes `write-scm-version.py` dependency.  Also pass targetplatform as an argument to `container-buildx` make target

* Remove `setuptools_scm` dep for docker workflow step

* woof

* Match all target platforms to match `python-alpine` docker

* yamllint

* Use `yamllint disable rule:line-length` for `Dockerfile`

* Tag the container using `dist-version`

* Replace + with . for dev version tag

* Add `PROXYPY_CONTAINER_VERSION` step
  • Loading branch information
@abhinavsingh
abhinavsingh authored Nov 25, 2021
1 parent 3ff7f21 commit e5e900f
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 66 deletions.
10 changes: 1 addition & 9 deletions .dockerignore
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
# Ignore everything
**

# Except proxy
!.git
!proxy
!requirements.txt
!pyproject.toml
!setup.cfg
!dist/*.whl
!README.md

# Ignore __pycache__ directory
proxy/__pycache__
4 changes: 4 additions & 0 deletions .github/buildkitd.toml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[worker.oci]
max-parallelism = 4
[registry."docker.io"]
mirrors = ["mirror.gcr.io"]

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 25, 2021

Contributor

Do you know you can also use GitHub Container Registry?
72 changes: 49 additions & 23 deletions .github/workflows/test-library.yml
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
---
# yamllint disable rule:line-length

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 25, 2021

Contributor

Why didn't you just fix overly long lines?

This comment has been minimized.

Sign in to view

Copy link
@abhinavsingh

abhinavsingh Nov 26, 2021

Author Owner

It was complaining about a shell command executed within GHA workflow, which practically made no sense to be broken down into smaller lines (as it would have broken the underlying shell command). I tried the recommendation and it broke the CI, so added this line at the top.

However, now I realize, that specific command is no longer used and this rule can again be enabled.

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 26, 2021

Contributor

You may want to learn about different ways YAML parsers can concatenate multiline content: https://yaml-multiline.info.
name: lib

on: # yamllint disable-line rule:truthy
Expand Down Expand Up @@ -609,7 +610,7 @@ jobs:
matrix:
os: [ubuntu, windows, macOS]
node: ['10.x', '11.x', '12.x']
max-parallel: 4
# max-parallel: 4
fail-fast: false
steps:
- uses: actions/checkout@v2
Expand All @@ -629,37 +630,62 @@ jobs:
cd ..
docker:
# To build our docker container, we must wait for check,
# TODO: To build our docker container, we must wait for check,
# so that we can use the same distribution available.
#
# TL;DR -- Docker must be packaged with package from
# test.pypi.org or from pypi.org.
# needs:
# - publish-pypi
runs-on: ${{ matrix.os }}-latest
name: 🐳 🐍${{ matrix.python }} @ ${{ matrix.os }}
needs:
- build
- pre-setup # transitive, for accessing settings
name: 🐳 🐍${{ matrix.python }} @ ${{ matrix.targetplatform }}
strategy:
matrix:
os: [ubuntu]
python: ['3.10']
max-parallel: 1
os:
- Ubuntu
python:
- '3.10'
targetplatform:
- 'linux/386'
- 'linux/amd64'
- 'linux/arm/v6'
- 'linux/arm/v7'
- 'linux/arm64/v8'
- 'linux/ppc64le'
- 'linux/s390x'
# max-parallel: 1
fail-fast: false
steps:
- uses: actions/checkout@v2
- name: Setup Python
uses: actions/setup-python@v2
- name: Checkout
uses: actions/checkout@v2
- name: Set up Docker Buildx
id: buildx
uses: docker/setup-buildx-action@v1
with:
python-version: ${{ matrix.python }}
- name: Install dependencies
buildkitd-flags: --debug
config: .github/buildkitd.toml
install: true
- name: Download all the dists
uses: actions/download-artifact@v2
with:
name: python-package-distributions
path: dist/
- name: Enable Multiarch # This slows down arm build by 4-5x
run: |
docker run --rm --privileged multiarch/qemu-user-static --reset -p yes
- name: Create builder
run: |
docker buildx create --name proxypybuilder
docker buildx use proxypybuilder
docker buildx inspect
docker buildx ls
- name: Set PROXYPY_CONTAINER_VERSION
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
pip install -r requirements-release.txt
pip install -r requirements-testing.txt
pip install -r requirements-tunnel.txt
- name: Build
echo "PROXYPY_CONTAINER_VERSION=$(echo '${{ needs.pre-setup.outputs.dist-version }}' | tr + .)" > $GITHUB_ENV

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 25, 2021

Contributor

Instead of declaring global env vars, you could just define a step output and reuse it in the next one.

This comment has been minimized.

Sign in to view

Copy link
@abhinavsingh

abhinavsingh Nov 26, 2021

Author Owner

Totally agreed, actually thought about it but didn't pursue. Let's make this change in follow up refactoring. We are not done with docker workflow yet.

Some of the steps from top of the mind:

  • Docker build after "check". If building before "check", at-least integration test for built container.
  • During the build step, we must also preserve built container image under artifacts. Or, if docker workflow step runs after "check", same step could also publish to hub.

I'll address the publish part soon. Want to see dev containers getting out there like our dev lib packages.

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 26, 2021

Contributor
  • During the build step, we must also preserve built container image under artifacts. Or, if docker workflow step runs after "check", same step could also publish to hub.

I don't think it'll fit in the artifact storage but I've successfully used the GitHub Container Registry for this. In my case, I was building image in one job, publishing it to GHCR, then the next job would need an env approval, when approved, it'd pick up the GHCR image, retag it and push to Quay.io.

The key here is to push the "shared" image with a unique workflow ID that is the same across the jobs.
- name: Build container
run: |
make container
make container-buildx \
-e PROXYPY_PKG_PATH='dist/${{ needs.pre-setup.outputs.wheel-artifact-name }}' \
-e BUILDX_TARGET_PLATFORM='${{ matrix.targetplatform }}' \
-e PROXYPY_CONTAINER_VERSION='${{ env.PROXYPY_CONTAINER_VERSION }}'
check: # This job does nothing and is only used for the branch protection
needs:
Expand Down
42 changes: 19 additions & 23 deletions Dockerfile
View file
Original file line number Diff line number Diff line change
@@ -1,30 +1,26 @@
FROM python:3.10-alpine as base
RUN apk add git

FROM base as builder

COPY .git /app/.git
COPY requirements.txt /app/
COPY pyproject.toml /app/
COPY setup.cfg /app/
COPY README.md /app/
COPY proxy/ /app/proxy/
WORKDIR /app
RUN pip install --upgrade pip && \
pip install --prefix=/deps .

FROM base

LABEL com.abhinavsingh.name="abhinavsingh/proxy.py" \
com.abhinavsingh.description="⚡⚡⚡ Fast, Lightweight, Pluggable, TLS interception capable proxy server focused on \
Network monitoring, controls & Application development, testing, debugging." \
com.abhinavsingh.url="https://github.com/abhinavsingh/proxy.py" \
com.abhinavsingh.vcs-url="https://github.com/abhinavsingh/proxy.py" \
com.abhinavsingh.docker.cmd="docker run -it --rm -p 8899:8899 abhinavsingh/proxy.py"
com.abhinavsingh.description="⚡ Fast • 🪶 Lightweight • 0️⃣ Dependency • 🔌 Pluggable • \
😈 TLS interception • 🔒 DNS-over-HTTPS • 🔥 Poor Man's VPN • ⏪ Reverse & ⏩ Forward • \
👮🏿 \"Proxy Server\" framework • 🌐 \"Web Server\" framework • ➵ ➶ ➷ ➠ \"PubSub\" framework • \
👷 \"Work\" acceptor & executor framework" \
com.abhinavsingh.url="https://github.com/abhinavsingh/proxy.py" \
com.abhinavsingh.vcs-url="https://github.com/abhinavsingh/proxy.py" \
com.abhinavsingh.docker.cmd="docker run -it --rm -p 8899:8899 abhinavsingh/proxy.py"
ENV PYTHONUNBUFFERED 1
ARG PROXYPY_PKG_PATH

COPY --from=builder /deps /usr/local
COPY README.md /
COPY $PROXYPY_PKG_PATH /
RUN pip install --upgrade pip && \
pip install \
--no-index \
--find-links file:/// \
proxy.py && \

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 25, 2021

Contributor

Why don't you just provide the exact filename via docker arg? Being specific is usually better because it may alert you about problems early, in case there's any mismatch.

This comment has been minimized.

Sign in to view

Copy link
@abhinavsingh

abhinavsingh Nov 26, 2021

Author Owner

Yep could have, but focus was primarily on getting other things right in this PR. Exact or generic doesn't make much of a difference here. pip will only find our whl (there ain't anything else in the container) and install it. Disabled pypi so it will never install from remote. Correct fix required more work, either passing more environment variable or ugly bash parsing. I skipped it as it would have added no additional benefit.

TL;DR

  • Installed proxy.py version is dictated by PROXYPY_PKG_PATH
  • PROXYPY_PKG_PATH is dictated by prior build steps either from GHA or locally

Being specific is usually better because it may alert you about problems early, in case there's any mismatch.

I don't see what mismatch can happen here. Anything that we have missed out here?

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 26, 2021

Contributor

Disabled pypi so it will never install from remote.

This is probably wrong but you got lucky. It'd fail under Python 3.7 because of the conditional dependency and isn't required under Python 3.10. So it works almost by accident.

I don't see what mismatch can happen here. Anything that we have missed out here?

That's the point: these mismatches may be sneaky and reveal themselves under a certain combination of conditions so you'd miss them. For example, in GHA such things may happen with the conditional jobs that only get triggered on release, and then you have an unpleasant surprise when everything seems ready. It could be even worth, you could attempt publishing other packages because of human error. My point is that some mistakes are non-recoverable and having measures to prevent them is better than causing a bad thing with undesired effect and then stressing about how to fix that when it's too late.

This comment has been minimized.

Sign in to view

Copy link
@abhinavsingh

abhinavsingh Nov 27, 2021

Author Owner

This is probably wrong but you got lucky

I hate such lucks :D. Agreed that explicit is better than implicit. And better to just do that here.

FWIW, I do intend to test the containers via workflows. We cannot just build and ship (even if check succeeded). Integration tests must also be run against containers. I planned for:

  1. Version assertion via e2e tests too
  2. Version assertion via container label inspection. Currently, we don't add git commit hash and version labels to the container.
rm *.whl

This comment has been minimized.

Sign in to view

Copy link
@webknjaz

webknjaz Nov 25, 2021

Contributor

That file already exists on the previous layer, removing it doesn't add much value as it won't reduce the resulting image size.

This comment has been minimized.

Sign in to view

Copy link
@abhinavsingh

abhinavsingh Nov 26, 2021

Author Owner

Yep, didn't remove it for size. I just didn't want a whl under the / directory when someone /bin/sh into the container :). Makes no sense for someone who just installed a binary to see a whl, since binary is already installed.

I could have continued the older builder from base strategy, but our wheel is ~200Kb and won't have made any difference on the size.

# Install openssl to enable TLS interception within container
# Install openssl to enable TLS interception & HTTPS proxy options within container
# NOTE: You can comment out this line if you don't intend to use those features.
RUN apk update && apk add openssl

EXPOSE 8899/tcp
Expand Down
39 changes: 29 additions & 10 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ SHELL := /bin/bash

NS ?= abhinavsingh
IMAGE_NAME ?= proxy.py
LATEST_TAG := $(NS)/$(IMAGE_NAME):latest
IMAGE_TAG := $(NS)/$(IMAGE_NAME):$(shell ./write-scm-version.sh)
# Override to target specific versions of proxy.py
PROXYPY_CONTAINER_VERSION := latest
# Used by container build and run targets
PROXYPY_CONTAINER_TAG := $(NS)/$(IMAGE_NAME):$(PROXYPY_CONTAINER_VERSION)

HTTPS_KEY_FILE_PATH := https-key.pem
HTTPS_CERT_FILE_PATH := https-cert.pem
Expand All @@ -14,6 +16,10 @@ CA_KEY_FILE_PATH := ca-key.pem
CA_CERT_FILE_PATH := ca-cert.pem
CA_SIGNING_KEY_FILE_PATH := ca-signing-key.pem

# Dummy invalid hardcoded value
PROXYPY_PKG_PATH := dist/proxy.py.whl
BUILDX_TARGET_PLATFORM := linux/amd64

OPEN=$(shell which open)
UNAME := $(shell uname)
ifeq ($(UNAME), Linux)
Expand All @@ -24,7 +30,7 @@ endif
.PHONY: lib-check lib-clean lib-test lib-package lib-coverage lib-lint lib-pytest
.PHONY: lib-release-test lib-release lib-profile lib-doc
.PHONY: lib-dep lib-flake8 lib-mypy
.PHONY: container container-run container-release
.PHONY: container container-run container-release container-build container-buildx
.PHONY: devtools dashboard dashboard-clean

all: lib-test
Expand Down Expand Up @@ -151,12 +157,25 @@ dashboard:
dashboard-clean:
if [[ -d dashboard/public ]]; then rm -rf dashboard/public; fi

container:
docker build -t $(LATEST_TAG) -t $(IMAGE_TAG) .

container-release:
docker push $(IMAGE_TAG)
docker push $(LATEST_TAG)
container: lib-package
$(MAKE) container-build -e PROXYPY_PKG_PATH=$$(ls dist/*.whl)

# Usage:
#
# make container-buildx \
# -e PROXYPY_PKG_PATH=$(ls dist/*.whl) \
# -e BUILDX_TARGET_PLATFORM=linux/arm64 \
# -e PROXYPY_CONTAINER_VERSION=latest
container-buildx:
docker buildx build \
--platform $(BUILDX_TARGET_PLATFORM) \
-t $(PROXYPY_CONTAINER_TAG) \
--build-arg PROXYPY_PKG_PATH=$(PROXYPY_PKG_PATH) .

container-build:
docker build \
-t $(PROXYPY_CONTAINER_TAG) \
--build-arg PROXYPY_PKG_PATH=$(PROXYPY_PKG_PATH) .

container-run:
docker run -it -p 8899:8899 --rm $(LATEST_TAG)
docker run -it -p 8899:8899 --rm $(PROXYPY_CONTAINER_TAG)
30 changes: 29 additions & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,30 @@ or from GitHub `master` branch

## Using Docker

Stable version container releases are available for following platforms:

- `linux/386`
- `linux/amd64`
- `linux/arm/v6`
- `linux/arm/v7`
- `linux/arm64/v8`
- `linux/ppc64le`
- `linux/s390x`

### Stable Version from Docker Hub

Run `proxy.py` latest container:

```console
❯ docker run -it -p 8899:8899 --rm abhinavsingh/proxy.py:latest
```

To run specific target platform container on multi-platform supported servers:

```console
❯ docker run -it -p 8899:8899 --rm --platform linux/arm64/v8 abhinavsingh/proxy.py:latest
```

### Build Development Version Locally

```console
Expand Down Expand Up @@ -394,6 +412,16 @@ To start `proxy.py` from source code follow these instructions:
❯ make lib-dep
```

- Generate `proxy/common/_scm_version.py`

NOTE: *Following step is not necessary for editable installs.*

This file writes SCM detected version to `proxy/common/_scm_version.py` file.

```console
❯ ./write-scm-version.sh
```

- Optionally, run tests

```console
Expand Down Expand Up @@ -2038,7 +2066,7 @@ usage: -m [-h] [--enable-events] [--enable-conn-pool] [--threadless]
[--filtered-url-regex-config FILTERED_URL_REGEX_CONFIG]
[--cloudflare-dns-mode CLOUDFLARE_DNS_MODE]

proxy.py v2.3.2.dev183+g808caa1.d20211124
proxy.py v2.3.2.dev190+ge60d80d.d20211124

options:
-h, --help show this help message and exit
Expand Down

0 comments on commit e5e900f

Please sign in to comment.