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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KevFan
Copy link
Contributor

@KevFan KevFan commented Sep 17, 2024

Description

Part of: Kuadrant/kuadrant-operator#769
Associated with: Kuadrant/kuadrant-operator#843

Currently, all built Quay images never expire and are consuming unnecessary quota on Quay.io. Quay provides a way to set tag expiration from a Dockerfile to prevent this issue for images that don't need to be kept indefinitely, such as development branches and nightly builds. This can help prevent the problem from escalating over time.

Verification

Since there is no auto trigger of building the images on dev branches, you at manually verify at least the makefile changes by:

  • Remove local image

    docker rmi quay.io/kuadrant/authorino-operator:latest
    docker rmi quay.io/kuadrant/authorino-operator-bundle:latest
    docker rmi quay.io/kuadrant/authorino-operator-catalog:latest
    
  • Build image

     # Delete the generated catalog Dockerfile as it won't regenerate again with the correct label if it's already present
     rm catalog/authorino-operator-catalog.Dockerfile 
     make docker-build bundle-build catalog-dockerfile catalog-build
    
  • Inspect image labels

    docker inspect quay.io/kuadrant/authorino-operator:latest --format='{{json .Config.Labels}}'
    docker inspect quay.io/kuadrant/authorino-operator-bundle:latest --format='{{json .Config.Labels}}'
    docker inspect quay.io/kuadrant/authorino-operator-catalog:latest --format='{{json .Config.Labels}}'
    
  • The quay.expires-after label should be set to never

    image

  • Remove local image again

    docker rmi quay.io/kuadrant/authorino-operator:latest
    docker rmi quay.io/kuadrant/authorino-operator-bundle:latest
    docker rmi quay.io/kuadrant/authorino-operator-catalog:latest
    
  • Build image with a set expiry

     # Delete the generated catalog Dockerfile as it won't regenerate again with the correct label if it's already present
     rm catalog/authorino-operator-catalog.Dockerfile 
     QUAY_IMAGE_EXPIRY=3d make docker-build bundle-build catalog-dockerfile catalog-build
    
  • Inspect image labels

    docker inspect quay.io/kuadrant/authorino-operator:latest --format='{{json .Config.Labels}}'
    docker inspect quay.io/kuadrant/authorino-operator-bundle:latest --format='{{json .Config.Labels}}'
    docker inspect quay.io/kuadrant/authorino-operator-catalog:latest --format='{{json .Config.Labels}}'
    
  • The quay.expires-after label should be set to 3d

    image

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.78%. Comparing base (82e7719) to head (078dc33).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #212   +/-   ##
=======================================
  Coverage   61.78%   61.78%           
=======================================
  Files           2        2           
  Lines         785      785           
=======================================
  Hits          485      485           
  Misses        249      249           
  Partials       51       51           
Flag Coverage Δ
unit 61.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KevFan KevFan force-pushed the quay-expire-image branch 2 times, most recently from bbbd5c3 to f86be1c Compare September 17, 2024 13:54
secrets: inherit
with:
authorinoVersion: ${{ github.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.

I'm making an assumption that SHA build are equivalent to nightly builds that we want to expire 🤔

If anyone has a better expiry value, I can update this

Copy link
Collaborator

Choose a reason for hiding this comment

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

Until now, SHA builds have not been equivalent to nightly builds. As of today, builds are tagged additionally with the commit SHA on top of the other, more human-friendly tag. This includes latest (for main branch), feature branches, and long-lived release builds.

I'm not sure how I feel about breaking this link between tags, effectively building two separate images now, if this is only for the purpose of cleaning up the registry.

The quay.expires-after label seems easy enough, but it only works for expiring an entire image, not an individual image tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I guess the question is then, does authorino-operator images need / want to expire images through this label? To me, it seems like it may not since the current workflow triggers on merge to main, which would build with an equivalent SHA tag. These SHA tags are kept indefinitely, but maybe that is something we want.

Feature branch and release builds looks to require manual trigger of the same workflow which do not happen that regularly

@KevFan KevFan self-assigned this Sep 18, 2024
@KevFan KevFan added kind/enhancement New feature or request size/small area/tooling Makefile and scripts for the dev workflow, testing, etc labels Sep 18, 2024
@KevFan KevFan marked this pull request as ready for review September 18, 2024 10:31
Copy link
Collaborator

@guicassolato guicassolato left a comment

Choose a reason for hiding this comment

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

I think we should think this through a little more. Breaking the link between images tags into the single unified manifests they are currently build today does not seem the correct approach to solve the problem of cleaning up the registry.

Comment on lines -35 to -44
- name: Add latest tag
if: ${{ github.ref_name == env.MAIN_BRANCH_NAME }}
id: add-latest-tag
run: |
echo "IMG_TAGS=latest ${{ env.IMG_TAGS }}" >> $GITHUB_ENV
- name: Add branch tag
if: ${{ github.ref_name != env.MAIN_BRANCH_NAME }}
id: add-branch-tag
run: |
echo "IMG_TAGS=${GITHUB_REF_NAME/\//-} ${{ env.IMG_TAGS }}" >> $GITHUB_ENV
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this really what we want? This will break with the single manifest link between builds.

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 don't see how we can add the expire label for different types of builds unless we do this since latest & SHA builds are usually the same on merge to main.

Branch + SHA build would only happen if the workflow is triggered manually currently.

secrets: inherit
with:
authorinoVersion: ${{ github.sha }}
quayImageExpiry: 2w
Copy link
Collaborator

Choose a reason for hiding this comment

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

Until now, SHA builds have not been equivalent to nightly builds. As of today, builds are tagged additionally with the commit SHA on top of the other, more human-friendly tag. This includes latest (for main branch), feature branches, and long-lived release builds.

I'm not sure how I feel about breaking this link between tags, effectively building two separate images now, if this is only for the purpose of cleaning up the registry.

The quay.expires-after label seems easy enough, but it only works for expiring an entire image, not an individual image tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tooling Makefile and scripts for the dev workflow, testing, etc kind/enhancement New feature or request size/small
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants