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.

Sign up for GitHub

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

feat: set quay image expiry to prevent overflow of images #851

Merged
merged 2 commits into from
Sep 16, 2024
Merged

feat: set quay image expiry to prevent overflow of images #851

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9dced5e
feat: set quay image expiry to prevent overflow of images
KevFan Sep 5, 2024
4321d37
fixup: rename QUAY_DOCKERFILE_LABEL to QUAY_EXPIRY_TIME_LABEL
KevFan Sep 12, 2024
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
12 changes: 12 additions & 0 deletions .github/workflows/build-images-base.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ on:
description: Bundle and catalog channels, comma separated
default: preview
type: string
quayImageExpiry:
description: When to expire the built quay images. The time values could be something like 1h, 2d, 3w for hours, days, and weeks, respectively, from the time the image is built.
default: never
type: string
workflow_dispatch:
inputs:
kuadrantOperatorTag:
Expand Down Expand Up @@ -69,6 +73,10 @@ on:
description: Bundle and catalog channels, comma separated
default: preview
type: string
quayImageExpiry:
description: When to expire the built quay images. The time values could be something like 1h, 2d, 3w for hours, days, and weeks, respectively, from the time the image is built.
default: never
type: string

env:
IMG_TAGS: ${{ inputs.kuadrantOperatorTag }}
Expand All @@ -77,6 +85,7 @@ env:
IMG_REGISTRY_ORG: kuadrant
MAIN_BRANCH_NAME: main
OPERATOR_NAME: kuadrant-operator
QUAY_IMAGE_EXPIRY: ${{ inputs.quayImageExpiry }}

jobs:
build:
Expand Down Expand Up @@ -107,6 +116,7 @@ jobs:
provenance: false
tags: |
${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}:${{ env.IMG_TAGS }}
build-args: QUAY_IMAGE_EXPIRY=${{ inputs.quayImageExpiry }}
- name: Print Image URL
run: echo "Image pushed to ${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}:${{ env.IMG_TAGS }}"

Expand Down Expand Up @@ -155,6 +165,7 @@ jobs:
provenance: false
tags: |
${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}-bundle:${{ env.IMG_TAGS }}
build-args: QUAY_IMAGE_EXPIRY=${{ inputs.quayImageExpiry }}
- name: Print Bundle Image URL
run: echo "Bundle image pushed to ${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}-bundle:${{ env.IMG_TAGS }}"

Expand Down Expand Up @@ -202,5 +213,6 @@ jobs:
provenance: false
tags: |
${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}-catalog:${{ env.IMG_TAGS }}
# The Quay image expiry label for the generated catalog Dockerfile is set via opm, using the value set in the QUAY_IMAGE_EXPIRY environment variable
- name: Print Catalog Image URL
run: echo "Catalog image pushed to ${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}-catalog:${{ env.IMG_TAGS }}"
2 changes: 2 additions & 0 deletions .github/workflows/build-images-branches.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,5 @@ jobs:
with:
kuadrantOperatorVersion: ${{ github.ref_name }}
kuadrantOperatorTag: ${{ github.ref_name }}
# Conditionally set quayImageExpiry to expire quay images only for non-release branches
quayImageExpiry: ${{ startsWith(github.ref_name, 'release') && 'never' || '1w' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Abitary value of when to expire branch images. If anyone has a better suited value, I can update it to this value.

I'm assuming we only want to expire dev branch images and not release branches. Or do we care about release branch images? Seems the actual tagged v.X.Y.Z(-RC) images are run manually using the base images workflow where the image should be set to never expire 🤔

1 change: 1 addition & 0 deletions .github/workflows/build-images-nightly.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,4 @@ jobs:
limitadorOperatorVersion: ${{ vars.LIMITADOR_OPERATOR_SHA }}
dnsOperatorVersion: ${{ vars.DNS_OPERATOR_SHA }}
wasmShimVersion: ${{ vars.WASM_SHIM_SHA }}
quayImageExpiry: 2w
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Abitary value of when to expire nighty images. If anyone has a better suited value, I can update it to this value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe @jsmolar (QE) can recommend a value that would work well

Copy link
Contributor

Choose a reason for hiding this comment

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

1 or 2 weeks for nightlies is reasonable. I don't see any reason why we would need to keep them longer.

7 changes: 6 additions & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Build the manager binary
FROM --platform=$BUILDPLATFORM golang:1.22 as builder
FROM --platform=$BUILDPLATFORM golang:1.22 AS builder

WORKDIR /workspace
# Copy the Go Modules manifests
Expand Down Expand Up @@ -28,4 +28,9 @@ WORKDIR /
COPY --from=builder /workspace/manager .
USER 65532:65532

# Quay image expiry
ARG QUAY_IMAGE_EXPIRY
ENV QUAY_IMAGE_EXPIRY=${QUAY_IMAGE_EXPIRY:-never}
LABEL quay.expires-after=$QUAY_IMAGE_EXPIRY

ENTRYPOINT ["/manager"]
5 changes: 3 additions & 2 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ run: generate fmt vet ## Run a controller from your host.
go run ./main.go

docker-build: ## Build docker image with the manager.
$(CONTAINER_ENGINE) build -t $(IMG) .
$(CONTAINER_ENGINE) build --build-arg QUAY_IMAGE_EXPIRY=$(QUAY_IMAGE_EXPIRY) -t $(IMG) .

docker-push: ## Push docker image with the manager.
$(CONTAINER_ENGINE) push $(IMG)
Expand Down Expand Up @@ -390,6 +390,7 @@ bundle: $(OPM) $(YQ) manifests dependencies-manifests kustomize operator-sdk ##
$(call update-operator-dependencies,dns-operator,$(DNS_OPERATOR_BUNDLE_IMG))
$(OPERATOR_SDK) bundle validate ./bundle
$(MAKE) bundle-ignore-createdAt
echo "$$QUAY_EXPIRY_TIME_LABEL" >> bundle.Dockerfile

.PHONY: bundle-ignore-createdAt
bundle-ignore-createdAt:
Expand All @@ -404,7 +405,7 @@ bundle-ignore-createdAt:

.PHONY: bundle-build
bundle-build: ## Build the bundle image.
$(CONTAINER_ENGINE) build -f bundle.Dockerfile -t $(BUNDLE_IMG) .
$(CONTAINER_ENGINE) build --build-arg QUAY_IMAGE_EXPIRY=$(QUAY_IMAGE_EXPIRY) -f bundle.Dockerfile -t $(BUNDLE_IMG) .

.PHONY: bundle-push
bundle-push: ## Push the bundle image.
Expand Down
5 changes: 5 additions & 0 deletions bundle.Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,8 @@ LABEL operators.operatorframework.io.test.config.v1=tests/scorecard/
COPY bundle/manifests /manifests/
COPY bundle/metadata /metadata/
COPY bundle/tests/scorecard /tests/scorecard/

# Quay image expiry
ARG QUAY_IMAGE_EXPIRY
ENV QUAY_IMAGE_EXPIRY=${QUAY_IMAGE_EXPIRY:-never}
LABEL quay.expires-after=${QUAY_IMAGE_EXPIRY}
15 changes: 14 additions & 1 deletion make/catalog.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,22 @@ CATALOG_IMG ?= $(IMAGE_TAG_BASE)-catalog:$(IMAGE_TAG)
CATALOG_FILE = $(PROJECT_PATH)/catalog/kuadrant-operator-catalog/operator.yaml
CATALOG_DOCKERFILE = $(PROJECT_PATH)/catalog/kuadrant-operator-catalog.Dockerfile

# Quay image default expiry
QUAY_IMAGE_EXPIRY ?= never

# A LABEL that can be appended to a generated Dockerfile to set the Quay image expiration through Docker arguments.
define QUAY_EXPIRY_TIME_LABEL

# Quay image expiry
ARG QUAY_IMAGE_EXPIRY
ENV QUAY_IMAGE_EXPIRY=$${QUAY_IMAGE_EXPIRY:-never}
LABEL quay.expires-after=$${QUAY_IMAGE_EXPIRY}
endef
export QUAY_EXPIRY_TIME_LABEL

$(CATALOG_DOCKERFILE): $(OPM)
-mkdir -p $(PROJECT_PATH)/catalog/kuadrant-operator-catalog
cd $(PROJECT_PATH)/catalog && $(OPM) generate dockerfile kuadrant-operator-catalog
cd $(PROJECT_PATH)/catalog && $(OPM) generate dockerfile kuadrant-operator-catalog -l quay.expires-after=$(QUAY_IMAGE_EXPIRY)
catalog-dockerfile: $(CATALOG_DOCKERFILE) ## Generate catalog dockerfile.
Comment on lines +25 to 26
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can also allow setting the Quay image expiry by following the same pattern to how it was done for the bundle image. If we choose to implement this, the Docker label will be consistently set via build arguments. However, I'm not sure if it's worth the extra step just for the sake of consistency. If anyone has any thoughts on this, feel free to share, otherwise achieving this through opm labeling is simpler.

Suggested change
cd $(PROJECT_PATH)/catalog && $(OPM) generate dockerfile kuadrant-operator-catalog -l quay.expires-after=$(QUAY_IMAGE_EXPIRY)
catalog-dockerfile: $(CATALOG_DOCKERFILE) ## Generate catalog dockerfile.
cd $(PROJECT_PATH)/catalog && $(OPM) generate dockerfile kuadrant-operator-catalog
echo "$$QUAY_DOCKERFILE_LABEL" >> $(CATALOG_DOCKERFILE)
catalog-dockerfile: $(CATALOG_DOCKERFILE) ## Generate catalog dockerfile.
# Build a catalog image by adding bundle images to an empty catalog using the operator package manager tool, 'opm'.
# Ref https://olm.operatorframework.io/docs/tasks/creating-a-catalog/#catalog-creation-with-raw-file-based-catalogs
.PHONY: catalog-build
catalog-build: ## Build a catalog image.
# Build the Catalog
docker build --build-arg QUAY_IMAGE_EXPIRY=$(QUAY_IMAGE_EXPIRY) $(PROJECT_PATH)/catalog -f $(PROJECT_PATH)/catalog/kuadrant-operator-catalog.Dockerfile -t $(CATALOG_IMG)

Copy link
Contributor

Choose a reason for hiding this comment

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

no hard opinion on this matter. For me what it matters is that the label should be there.

I do have a "soft opinion", though. I would do the labeling using the opm command. Mainly because we do not have a committed version of the dockerfile, it is autogenerated. So, the expiry time is not going to be committed in the repo even though it is hardcoded in the (ephemeral) dockerfile.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also do not like

echo "$$QUAY_DOCKERFILE_LABEL" >> $(CATALOG_DOCKERFILE)

if we need to do that, we do that. But modifying dynamically the dockerfile is somewhat unwanted. Can we ensure the value of $QUAY_DOCKERFILE_LABEL is syntactically correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we need to do that, we do that. But modifying dynamically the dockerfile is somewhat unwanted.

@eguzki Yes, we don't need to do this for the catalog dockerfile 👍 But I think we need to do this for the bundle Dockerfile since this is also autogenerated, is overwritten every time and committed.

kuadrant-operator/Makefile

Line 393 in 107947e

echo "$$QUAY_DOCKERFILE_LABEL" >> bundle.Dockerfile

kuadrant-operator/bundle.Dockerfile

Lines 23 to 26 in 107947e

# Quay image expiry
ARG QUAY_IMAGE_EXPIRY
ENV QUAY_IMAGE_EXPIRY=${QUAY_IMAGE_EXPIRY:-never}
LABEL quay.expires-after=${QUAY_IMAGE_EXPIRY}

I'm not aware of a way to add extra labels to the generated bundle dockerfile via operator-sdk 🤔

Can we ensure the value of $QUAY_DOCKERFILE_LABEL is syntactically correct?

It should be correct, but not sure how we can ensure this? It's functioning at least since the label was set correctly for the bundle image built 🤔

Copy link
Contributor

@eguzki eguzki Sep 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

right, I ran into the same issue when wanted to add openshift specific labels to the bundle image.

I am afraid, I do not know how to do this cleanly, either.

We decided to have git commited the bundle.Dockerfile (and bundle dir) for a reason. Reason being having traceability. Updating the file on-the-fly sort of defeats that purpose.

I propose to do what you suggest. A sacrifice for the greater good. Do no git update bundle.Dockerfile and commit it, change it on the fly instead and revert changes when the image build is done.

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 propose to do what you suggest. A sacrifice for the greater good. Do no git update bundle.Dockerfile and commit it, change it on the fly instead and revert changes when the image build is done.

@eguzki I'm thinking of reverting the last commit for this. Reason is the CI build for the bundle image needs to be updated also to account for this (why it's not working on the CI build currently) since it does not use the makefile commands to build the image, and likely be a duplicated effort to also do this in the CI.

kuadrant-operator/.github/workflows/build-images-base.yaml

Lines 138 to 168 in e4f3ac3

- name: Run make bundle
id: make-bundle
run: |
make bundle REGISTRY=${{ env.IMG_REGISTRY_HOST }} ORG=${{ env.IMG_REGISTRY_ORG }} \
VERSION=${{ env.VERSION }} IMAGE_TAG=${{ env.IMG_TAGS }} \
AUTHORINO_OPERATOR_VERSION=${{ inputs.authorinoOperatorVersion }} \
LIMITADOR_OPERATOR_VERSION=${{ inputs.limitadorOperatorVersion }} \
DNS_OPERATOR_VERSION=${{ inputs.dnsOperatorVersion }} \
WASM_SHIM_VERSION=${{ inputs.wasmShimVersion }} \
REPLACES_VERSION=${{ inputs.replacesVersion }} \
CHANNELS=${{ inputs.channels }}
- name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
- name: Login to container registry
uses: docker/login-action@v2
with:
username: ${{ secrets.IMG_REGISTRY_USERNAME }}
password: ${{ secrets.IMG_REGISTRY_TOKEN }}
registry: ${{ env.IMG_REGISTRY_HOST }}
- name: Build and Push Bundle Image
id: build-bundle-image
uses: docker/build-push-action@v5
with:
context: .
file: ./bundle.Dockerfile
platforms: linux/amd64,linux/arm64
push: true
provenance: false
tags: |
${{ env.IMG_REGISTRY_HOST }}/${{ env.IMG_REGISTRY_ORG }}/${{ env.OPERATOR_NAME }}-bundle:${{ env.IMG_TAGS }}
build-args: QUAY_IMAGE_EXPIRY=${{ inputs.quayImageExpiry }}

This, to me, is not as clean as the previous approach with just always appending the neccessay QUAY_DOCKERFILE_LABEL in the make bundle command after bundle image dockerfile has been generated, which will be the actual dockerfile used when building the bundle image rather than hiding this detail 🤔
Since we commit this Dockerfile, it's apparent from the Dockerfile that it can accept a build argument to allow expirying quay images if passed. Otherwise this detail is hidden and harder to debug in general I feel if an issue occurs.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

So, when you run bundle makefile target, $(OPERATOR_SDK) generate bundle ... will overwrite the bundle.Dockerfile and then we add the label again, right? something like

bundle:
    .... 
    $(OPERATOR_SDK) generate bundle -q --overwrite
    .....
    @echo "$$QUAY_DOCKERFILE_LABEL" >> bundle.Dockerfile

If this is what you suggest, I am ok with it. And I propose:

s/QUAY_DOCKERFILE_LABEL/QUAY_EXPIRY_TIME_LABEL/

Or something similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, when you run bundle makefile target, $(OPERATOR_SDK) generate bundle ... will overwrite the bundle.Dockerfile and then we add the label again, right? If this is what you suggest, I am ok with it.

Yes, exactly 👍

. And I propose: s/QUAY_DOCKERFILE_LABEL/QUAY_EXPIRY_TIME_LABEL/. Or something similar.

Sounds good, updated to QUAY_EXPIRY_TIME_LABEL 👍


CHANNELS ?= preview
Expand Down
Loading