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

Operator Controller Release #133

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 0 additions & 4 deletions .dockerignore

This file was deleted.

63 changes: 63 additions & 0 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
name: release

on:
workflow_dispatch:
push:
branches:
- 'main'
tags:
- 'v*'
pull_request:
branches:
- main

jobs:
goreleaser:
name: goreleaser
runs-on: ubuntu-latest
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Install Go
uses: actions/setup-go@v3
with:
go-version-file: "go.mod"

- name: Docker Login
if: ${{ github.event_name != 'pull_request' }}
uses: docker/login-action@v1
with:
registry: quay.io
username: ${{ secrets.QUAY_USERNAME }}
password: ${{ secrets.QUAY_PASSWORD }}

- name: Set the release related variables
run: |
if [[ $GITHUB_REF == refs/tags/* ]]; then
# Release tags.
echo IMAGE_TAG="${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV
echo GORELEASER_ARGS="--rm-dist" >> $GITHUB_ENV
echo DISABLE_RELEASE_PIPELINE=false >> $GITHUB_ENV
elif [[ $GITHUB_REF == refs/heads/* ]]; then
# Branch build.
echo IMAGE_TAG="$(echo "${GITHUB_REF#refs/heads/}" | sed -r 's|/+|-|g')" >> $GITHUB_ENV
echo GORELEASER_ARGS="--rm-dist --skip-validate" >> $GITHUB_ENV
elif [[ $GITHUB_REF == refs/pull/* ]]; then
# PR build.
echo IMAGE_TAG="pr-$(echo "${GITHUB_REF}" | sed -E 's|refs/pull/([^/]+)/?.*|\1|')" >> $GITHUB_ENV
else
echo IMAGE_TAG="$(git describe --tags --always)" >> $GITHUB_ENV
fi

- name: Generate the operator-controller release manifests
if: ${{ startsWith(github.ref, 'refs/tags/v') }}
run: |
make quickstart VERSION=${GITHUB_REF#refs/tags/}

- name: Run goreleaser
run: make release
env:
GITHUB_TOKEN: ${{ github.token }}
10 changes: 9 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*.dll
*.so
*.dylib
bin
bin/*
testbin/*
Dockerfile.cross

Expand All @@ -15,6 +15,11 @@ Dockerfile.cross
# Output of the go coverage tool, specifically when used with LiteIDE
*.out

# Release output
dist/**
.goreleaser.yml
operator-controller.yaml

# Kubernetes Generated files - skip generated files, except for vendored files

!vendor/**/zz_generated.*
Expand All @@ -24,3 +29,6 @@ Dockerfile.cross
*.swp
*.swo
*~

# TODO dfranz remove this line and the bin folder when tools binaries are moved to their own folder
!bin/.dockerignore
50 changes: 50 additions & 0 deletions .goreleaser.template.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
env:
- GOPROXY=https://proxy.golang.org|direct
- GO111MODULE=on
- CGO_ENABLED=0
before:
hooks:
- go mod tidy
- go mod download
builds:
- id: operator-controller
main: ./
binary: bin/manager
tags: $GO_BUILD_TAGS
goos:
- linux
Comment on lines +14 to +15
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if GH ate my comment or if I managed not to submit it.

This seems to build only a single binary for linux/$GOARCH. But we need to build binaries for the following:

  • linux/amd64
  • linux/arm64
  • linux/ppc64le
  • linux/s390x

Copy link
Member

Choose a reason for hiding this comment

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

We then need to propagate these to separate individual docker images, and then build the manifest list that combines all of them.

Copy link
Member

Choose a reason for hiding this comment

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

All this is probably fine to do in a follow-up, but might be good to do that before we tag this repo the first time.

ldflags:
- -X main.Version={{ .Version }}
dockers:
- image_templates:
- "{{ .Env.IMAGE_REPO }}:{{ .Env.IMAGE_TAG }}"
dockerfile: Dockerfile
goos: linux
docker_manifests:
- name_template: "{{ .Env.IMAGE_REPO }}:{{ .Env.IMAGE_TAG }}"
image_templates:
- "{{ .Env.IMAGE_REPO }}:{{ .Env.IMAGE_TAG }}"
checksum:
name_template: 'checksums.txt'
snapshot:
name_template: "{{ incpatch .Version }}-next"
changelog:
use: github-native
skip: $DISABLE_RELEASE_PIPELINE
release:
disable: $DISABLE_RELEASE_PIPELINE
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to remember why we do this convoluted envsubst thing rather than use the --snapshot flag when invoking goreleaser.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I remember: we want to build/push images/manifests -- but NOT release -- for builds of the main branch.

It seems like this problem is half solved. It's possible to template release.disable, but still not possible to template changelog.skip.

I opened an issue in goreleaser to see if we can make changelog.skip templatable as well: goreleaser/goreleaser#3828

Copy link
Member

Choose a reason for hiding this comment

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

Sweet. The maintainer already merged a PR that fixes this. Once there's a release, we can directly reference {{ .Env.DISABLE_RELEASE_PIPELINE }} as a template value in the actual goreleaser config file. No need for the envsubst shenanigans after that.

extra_files:
- glob: 'operator-controller.yaml'
header: |
## Installation

```bash
kubectl apply -f https://github.com/cert-manager/cert-manager/releases/download/{{ .Env.CERT_MGR_VERSION }}/cert-manager.yaml
kubectl wait --for=condition=Available --namespace=cert-manager deployment/cert-manager-webhook --timeout=60s
kubectl apply -f https://github.com/operator-framework/rukpak/releases/latest/download/rukpak.yaml
kubectl wait --for=condition=Available --namespace=rukpak-system deployment/core --timeout=60s
kubectl wait --for=condition=Available --namespace=rukpak-system deployment/helm-provisioner --timeout=60s
kubectl wait --for=condition=Available --namespace=rukpak-system deployment/rukpak-webhooks --timeout=60s
kubectl apply -f https://github.com/operator-framework/operator-controller/releases/download/{{ .Tag }}/operator-controller.yaml
kubectl wait --for=condition=Available --namespace=operator-controller-system deployment/operator-controller-controller-manager --timeout=60s
```
36 changes: 8 additions & 28 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,34 +1,14 @@
# Build the manager binary
FROM golang:1.19 as builder
ARG TARGETOS
ARG TARGETARCH
# Note: This dockerfile does not build the binaries
# required and is intended to be built only with the
# 'make build' or 'make release' targets.
FROM gcr.io/distroless/static:nonroot
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment to this file that says it's intended to be used with make docker-build or make release?


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
WORKDIR /

# Copy the go source
COPY main.go main.go
COPY api/ api/
COPY controllers/ controllers/
COPY internal/ internal/
COPY manager manager

Copy link
Member

Choose a reason for hiding this comment

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

Do we need to explicitly specify the user? Honestly not sure. I think my suggestion would be to specify it explicitly rather than inherit it from the base image.

We should also specify the entrypoint since this is the only binary we'll be using in this image.

Suggested change
USER 65532:65532
ENTRYPOINT ["/manager"]

# Build
# the GOARCH has not a default value to allow the binary be built according to the host where the command
# was called. For example, if we call make docker-build in a local env which has the Apple Silicon M1 SO
# the docker BUILDPLATFORM arg will be linux/arm64 when for Apple x86 it will be linux/amd64. Therefore,
# by leaving it empty we can ensure that the container and binary shipped on it will have the same platform.
RUN CGO_ENABLED=0 GOOS=${TARGETOS:-linux} GOARCH=${TARGETARCH} go build -a -o manager main.go
EXPOSE 8080

# 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/manager .
USER 65532:65532

ENTRYPOINT ["/manager"]
ENTRYPOINT ["/manager"]
38 changes: 30 additions & 8 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export IMAGE_REPO ?= quay.io/operator-framework/operator-controller
export IMAGE_TAG ?= devel
export GO_BUILD_TAGS ?= upstream
export CERT_MGR_VERSION ?= v1.9.0
export GORELEASER_VERSION ?= v1.15.2
export WAIT_TIMEOUT ?= 60s
IMG?=$(IMAGE_REPO):$(IMAGE_TAG)

Expand Down Expand Up @@ -94,8 +95,8 @@ kind-cluster-cleanup: kind ## Delete the kind cluster
##@ Build

.PHONY: build
build: manifests generate fmt vet ## Build manager binary.
go build -o bin/manager main.go
build: manifests generate fmt vet substitute goreleaser ## Build manager binary using goreleaser for current GOOS and GOARCH.
${GORELEASER} build ${GORELEASER_ARGS} --single-target -o bin/manager

.PHONY: run
run: docker-build kind-cluster kind-load cert-mgr rukpak install deploy wait ## Build the operator-controller then deploy it into a new kind cluster.
Expand All @@ -108,12 +109,9 @@ wait:
# (i.e. docker build --platform linux/arm64 ). However, you must enable docker buildKit for it.
# More info: https://docs.docker.com/develop/develop-images/build_enhancements/
.PHONY: docker-build
docker-build: generate ## Build docker image with the operator-controller.
docker build -t ${IMG} .

.PHONY: docker-push
docker-push: ## Push docker image with the manager.
docker push ${IMG}
docker-build: export GOOS=linux
docker-build: build ## Build docker image with the operator-controller.
docker build -t ${IMG} -f Dockerfile ./bin/
Copy link
Member

Choose a reason for hiding this comment

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

This seems like it'll fail because the Dockerfile expects the binary to be pre-built by goreleaser. The previous Dockerfile included the binary build as a pre-step in a multi-stage build.

I think we need to add build to the list of pre-reqs?

Copy link
Member

Choose a reason for hiding this comment

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

Also, the binary built in this case needs to be GOOS=linux, but in the non-docker build case should just take the default settings of the environment. That way, mac and linux machines can both build local binaries and container image binaries correctly.

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've updated the build target to use goreleaser build, and I believe that's also resolved your concern here. By default, when running make build, goreleaser will set GOARCH and GOOS to the user's setup. This won't apply though when building for release.

Copy link
Member

Choose a reason for hiding this comment

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

For docker-build I think we need to explicitly set GOOS=linux, right? If we don't won't a macOS user end up with a docker container that contains a darwin binary?

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH, building within the container generally improves compatibility within the container and improves build reproducibility.

Copy link
Member

@joelanford joelanford Mar 3, 2023

Choose a reason for hiding this comment

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

GoReleaser doesn't support that, so if we want binary builds happening inside docker, we can't use goreleaser I don't think.

I don't love this restriction, but my main interests are:

  • we have exactly one way to build the the binaries snd containers
  • the binaries we build ARE the binaries that end up in the container.

GoReleaser can be made to ensure reproducibility (except maybe Go compiler version) so I think we end up at the same place. It just takes a bit more care to get there.


# PLATFORMS defines the target platforms for the manager image be build to provide support to multiple
# architectures. (i.e. make docker-buildx IMG=myregistry/mypoperator:0.0.1). To use this option you need to:
Expand All @@ -132,6 +130,24 @@ docker-buildx: test ## Build and push docker image for the manager for cross-pla
- docker buildx rm project-v3-builder
rm Dockerfile.cross

###########
Copy link
Contributor

Choose a reason for hiding this comment

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

can we get some make help docs around release - when/how it should be used?

# Release #
###########

##@ Release:
export DISABLE_RELEASE_PIPELINE ?= true
export GORELEASER_ARGS ?= --snapshot --rm-dist

substitute:
envsubst < .goreleaser.template.yml > .goreleaser.yml
Comment on lines +141 to +142
Copy link
Member

Choose a reason for hiding this comment

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

Is this used anywhere other than release? If not, I'd propose to consolidate this into the release recipe.

Having a top-level target called substitute seems like it could be both too vague and potentially confusing for contributors wondering if/when they should use it.


release: substitute goreleaser ## Runs goreleaser for the operator-controller. By default, this will run only as a snapshot and will not publish any artifacts unless it is run with different arguments. To override the arguments, run with "GORELEASER_ARGS=...". When run as a github action from a tag, this target will publish a full release.
$(GORELEASER) $(GORELEASER_ARGS)

quickstart: VERSION ?= $(shell git describe --abbrev=0 --tags)
quickstart: kustomize generate ## Generate the installation release manifests
kubectl kustomize config/default | sed "s/:devel/:$(VERSION)/g" > operator-controller.yaml

##@ Deployment

ifndef ignore-not-found
Expand Down Expand Up @@ -178,6 +194,7 @@ KUSTOMIZE ?= $(LOCALBIN)/kustomize
CONTROLLER_GEN ?= $(LOCALBIN)/controller-gen
KIND ?= $(LOCALBIN)/kind
GINKGO ?= $(LOCALBIN)/ginkgo
GORELEASER := $(LOCALBIN)/goreleaser
ENVTEST ?= $(LOCALBIN)/setup-envtest

## Tool Versions
Expand All @@ -194,6 +211,11 @@ ginkgo: $(GINKGO) ## Download ginkgo locally if necessary.
$(GINKGO): $(LOCALBIN)
test -s $(LOCALBIN)/ginkgo || GOBIN=$(LOCALBIN) go install github.com/onsi/ginkgo/v2/ginkgo@v2.1.4

.PHONY: goreleaser
goreleaser: $(GORELEASER) ## Builds a local copy of goreleaser
$(GORELEASER): $(LOCALBIN)
test -s $(LOCALBIN)/goreleaser || GOBIN=$(LOCALBIN) go install github.com/goreleaser/goreleaser@${GORELEASER_VERSION}

KUSTOMIZE_INSTALL_SCRIPT ?= "https://raw.githubusercontent.com/kubernetes-sigs/kustomize/master/hack/install_kustomize.sh"
.PHONY: kustomize
kustomize: $(KUSTOMIZE) ## Download kustomize locally if necessary. If wrong version is installed, it will be removed before downloading.
Expand Down
8 changes: 8 additions & 0 deletions bin/.dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
# Ignore tools binaries
# TODO dfranz: We don't need this file anymore once we move the tool binaries out.
controller-gen
ginkgo
goreleaser
kind
kustomize