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

Docker Push Github Action #83

Merged
merged 11 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 33 additions & 13 deletions .github/workflows/build_and_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,51 @@ on:
- "**/*.png"
env:
GO_VERSION: 1.18.2
ENVOY_GATEWAY_DEV_IMAGE: envoyproxy/gateway-dev
ENVOY_GATEWAY_DEV_TAG: latest
jobs:
build:
build-and-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- name: build
run: make build
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3
with:
go-version: ${{ env.GO_VERSION }}
cache: true
- name: test and report coverage
- name: Test and report coverage
run: go test ./... -race -coverprofile=coverage.xml -covermode=atomic
- name: Upload coverage to Codecov
uses: codecov/codecov-action@v2
with:
fail_ci_if_error: true
files: ./coverage.xml
name: codecov-envoy-gateway
verbose: true
verbose: true
- name: Set up QEMU
uses: docker/setup-qemu-action@v2
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can drop this since we're only building a single arch now and when we add support for multi-arch images we can use Go's native cross-compilation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

based on the comments, added linux/arm64 to enable envoy-gateway images to run on arm64 as well (e.g. M1 Macbooks)

- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v2
- name: Build
uses: docker/build-push-action@v3
with:
context: .
platforms: linux/amd64
tags: ${{ env.ENVOY_GATEWAY_DEV_IMAGE }}:${{ github.sha }}
skriss marked this conversation as resolved.
Show resolved Hide resolved
cache-from: type=gha
cache-to: type=gha,mode=max
- name: Login to DockerHub
if: github.event_name == 'push'
uses: docker/login-action@v2
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_PASSWORD }}
- name: Push to envoyproxy/gateway-dev
if: github.event_name == 'push' && github.ref == 'refs/heads/main'
uses: docker/build-push-action@v3
with:
context: .
platforms: linux/amd64,linux/arm64
skriss marked this conversation as resolved.
Show resolved Hide resolved
push: true
tags: ${{ env.ENVOY_GATEWAY_DEV_IMAGE }}:${{ env.ENVOY_GATEWAY_DEV_TAG }}
arkodg marked this conversation as resolved.
Show resolved Hide resolved
cache-from: type=gha
cache-to: type=gha,mode=max
24 changes: 24 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
FROM golang:1.18.2 as builder

WORKDIR /workspace
# Copy the Go Modules manifests
COPY go.mod go.mod
COPY go.sum go.sum
# cache deps before building and copying source so that we don't need to re-download as much
# and so that source changes don't invalidate our downloaded layer
RUN go mod download

# Copy the go source
COPY . /workspace

# Build
RUN make build
Copy link
Member

Choose a reason for hiding this comment

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

Why build here again but not use artifact from build and test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

keeping the Dockerfile mutually exclusive from the CI flow so a

  • dev can run make docker-build or docker build . to build images and test them quickly
  • above flow eliminates volume mounts to share artifacts etc

Copy link
Member

Choose a reason for hiding this comment

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

I feel this is a bad practice: we are releasing a build which is not tested in the same environment.

buildx doesn't need volume mounts anyway, it is just ADD file?

Copy link
Contributor Author

@arkodg arkodg Jun 2, 2022

Choose a reason for hiding this comment

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

I disagree here, the workflow for pushing docker images would also run some form of e2e (which doesn't exist today) using the same image before pushing the image

Copy link
Member

Choose a reason for hiding this comment

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

The other disadvantage of this is we are going to use QEMU for future arm64 (and other) build which is way slower and hard to workaround. It is better to split the build and docker packaging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

prefer if we first tried the simpler way of building linux/arm64 using QEMU, quantified the slowness in CI, and then deleted the QEMU step in favor of a self hosted ARM runner

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have moved over the job as a step into the main workflow, we could further optimize this logic by retagging instead of relying on the build cache layers

Copy link
Member

Choose a reason for hiding this comment

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

You don't need even a self hosted ARM runner, Go cross compile works great without hassle, just need to copy the binary from local machine (or downloaded artifacts), instead of force docker build run Go build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cool, lets consider adding that as a follow up PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

raised #93 so this conversation can continue there and can unblock this PR


# Use distroless as minimal base image to package the manager binary
# Refer to https://github.com/GoogleContainerTools/distroless for more details
FROM gcr.io/distroless/static:nonroot
WORKDIR /
COPY --from=builder /workspace/bin/envoy-gateway .
USER 65532:65532

ENTRYPOINT ["/envoy-gateway"]
19 changes: 18 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
# REGISTRY is the image registry to use for build and push image targets.
REGISTRY ?= docker.io/envoyproxy
# IMAGE is the image URL for build and push image targets.
IMAGE ?= ${REGISTRY}/gateway-dev
# REV is the short git sha of latest commit.
REV=$(shell git rev-parse --short HEAD)
# Tag is the tag to use for build and push image targets.
TAG ?= $(REV)

.PHONY: build
build:
@go build -o ./bin/ github.com/envoyproxy/gateway/cmd/envoy-gateway
@CGO_ENABLED=0 go build -a -o ./bin/ github.com/envoyproxy/gateway/cmd/envoy-gateway

.PHONY: test
test:
@go test ./...

.PHONY: docker-build
docker-build: test ## Build the envoy-gateway docker image.
docker build -t $(IMAGE):$(TAG) -f Dockerfile .

.PHONY: docker-push
docker-push: ## Push the docker image for envoy-gateway.
docker push $(IMAGE):$(TAG)