-
Couldn't load subscription status.
- Fork 30
Mark images for deletion on release deletion #2113
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
base: master
Are you sure you want to change the base?
Conversation
1aed661 to
4b9fde2
Compare
0300ed4 to
f3d59f1
Compare
f3d59f1 to
385c7a1
Compare
|
/rebase |
385c7a1 to
f453f27
Compare
| const token = generateToken('admin', REGISTRY2_HOST, [ | ||
| { | ||
| name: repo, | ||
| type: 'repository', | ||
| actions: ['delete'], | ||
| }, | ||
| ]); |
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.
We may instead want to generate a wildcard token with delete access to all repos
f453f27 to
2ebdd60
Compare
| const schema = { | ||
| type: 'object', | ||
| properties: { | ||
| images: { | ||
| type: 'array', | ||
| items: { | ||
| type: 'array', | ||
| items: { | ||
| type: 'string', | ||
| }, | ||
| maxItems: 2, | ||
| minItems: 2, | ||
| }, | ||
| }, | ||
| }, | ||
| required: ['images'], | ||
| }; |
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.
@joshbwlng just for my own information, is there any specific reason you chose this schema instead of
| const schema = { | |
| type: 'object', | |
| properties: { | |
| images: { | |
| type: 'array', | |
| items: { | |
| type: 'array', | |
| items: { | |
| type: 'string', | |
| }, | |
| maxItems: 2, | |
| minItems: 2, | |
| }, | |
| }, | |
| }, | |
| required: ['images'], | |
| }; | |
| const schema = { | |
| type: 'object', | |
| properties: { | |
| images: { | |
| type: 'array', | |
| items: { | |
| type: 'object', | |
| properties: { | |
| repo: { type: 'string' }, | |
| hash: { type: 'string' }, | |
| }, | |
| required: ['repo', 'hash'], | |
| additionalProperties: false, | |
| }, | |
| }, | |
| }, | |
| required: ['images'], | |
| }; |
where the properties have explicit naming? (slightly easier to read imo)
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.
Having the named keys does make it easier (I had them in my initial source), but decided to drop them in favor of reducing task DB record size - especially since cascade deletions can lead to a very large number of images being processed here.
| // Need to make requests one image at a time, no batch endpoint available | ||
| for (let retries = 0; retries < RATE_LIMIT_RETRIES; retries++) { | ||
| const [{ statusCode, statusMessage, headers }] = await requestAsync({ | ||
| url: `https://${REGISTRY2_HOST}/v2/${repo}/manifests/${hash}`, |
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.
Are all repo/hashes we have here guaranteed to belong to our own registry or could these be images on custom customer registry?
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.
Why would they belong on a custom customer registry? Even if it's balenaMachine the value should point to their respective registry service (REGISTRY2_HOST).
If you're asking if the domain is always guaranteed to match the current value of REGISTRY2_HOST, then that's a no. We have images in production/staging in which the domain is registry2.resin.io for example instead of our current registry2.balena-cloud.com value.
Regardless, the idea is to attempt the deletion using the one and only registry our API has access to. If the response from the registry is 404, well then there's nothing else we can do anyway - the manifest we wanted to get rid of is already gone.
2ebdd60 to
8bc3cf7
Compare
|
/rebase |
Change-type: minor
8bc3cf7 to
60a9b3e
Compare
Change-type: minor
See: https://balena.fibery.io/Work/Project/Mark-associated-resources-for-deletion-when-a-release-is-deleted-1779
My biggest concern is properly handling a potentially large number of images in the task - analogous to the service install task work.
Note: We can merge this before the delete images endpoint is enabled on the registry as this function is toggled via an env var. Once the delete endpoint is available we can turn this feature on with the env var.