-
Notifications
You must be signed in to change notification settings - Fork 8
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
hotfix: Iterate on all available tags to check if tag exist #54
Conversation
@toninis: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. I understand the commands that are listed here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Antonis! LGTM as a hotfix, although the tags number will only ever grow as of now (mattermostdevelopment/mm-ee-test
currently has ~1825 tags pushed over the course of ~3 months, so a ~600 tags/month increase), so perhaps it would still be more sustainable to eventually switch to a modern library, and directly retrieve the image manifest again.
Agreed . The api is really fast in any case . Should we establish a garbage collection policy ? |
/release-note-none |
No urgent need for this PR's purpose I think. But I would open a ticket in our backlog to have a discussion about this topic in general. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for fixing this
@@ -48,14 +48,16 @@ func (b *Builds) waitForImage(ctx context.Context, s *Server, reg *registry.Regi | |||
case <-ctx.Done(): | |||
return pr, errors.New("timed out waiting for image to publish") | |||
case <-time.After(30 * time.Second): | |||
_, err := reg.ManifestDigest(imageToCheck, desiredTag) | |||
availableTags, err := reg.Tags(imageToCheck) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if there is a test. if it's not it's ok.
Closing this as it is not needed . Once we introduce multi architecture images we will re-evaluate |
Summary
When images are built without
provenance=false
spinwick cannot validate the manifest.The library we are using does not support all OCI manifests so we are switching to simple tag checking which works every time.
Ticket Link
https://mattermost.atlassian.net/browse/CLD-5172
Related Issues
docker/buildx#1533
fraunhoferfokus/deckschrubber#43
Release Note