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

adds ability to purge when images in container are broken #11

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

Conversation

orangewolf
Copy link

@orangewolf orangewolf commented Oct 23, 2018

we had a bunch of registries with 0 length or otherwise undeleteable tags in them. We hacked around this by pushing a small image to those tags and then re-running the delete.

if purge:
url = self.registry.replace("https://", '') + "/" + repo + ":" + tag
call(["docker", "tag", "busybox:latest", url])
call(["doicker", "push", url])
Copy link
Owner

Choose a reason for hiding this comment

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

Please fix typo

Suggested change
call(["doicker", "push", url])
call(["docker", "push", url])

@glensc
Copy link
Contributor

glensc commented Oct 24, 2018

@orangewolf do you have ideas why it happens? it has happened for me too in the past, when i excessively ran delete from gitlab ui. but i haven't noticed these 0 byte errors for several months. yet i have now cleanup automation in place and run registry-garbage-collect. or perhaps gitlab fixed something?

@@ -76,22 +76,31 @@ def get_digest(self, repo, tag):
response = requests.head(self.registry + path, headers=headers, verify=self.requests_verify)
return response.headers["Docker-Content-Digest"]

def delete_image(self, repo, tag):
def delete_image(self, repo, tag, purge):
Copy link
Contributor

Choose a reason for hiding this comment

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

please add default value for purge, making it api compatible and having the parameter optiona.

logging.error(response.text)

counter = 0
while counter < 2:
Copy link
Contributor

Choose a reason for hiding this comment

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

yuk. perhaps add another method to the class delete_with_purge, instead of making original delete method such ugliness?

Copy link
Contributor

Choose a reason for hiding this comment

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

i even think such logic should not be in this class... rather find problematic images/tags and fix them elsewise. some new script using same class?

logging.error(response.text)

counter = 0
while counter < 2:
Copy link
Contributor

Choose a reason for hiding this comment

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

i even think such logic should not be in this class... rather find problematic images/tags and fix them elsewise. some new script using same class?

@n0madic
Copy link
Owner

n0madic commented Oct 26, 2018

I agree with @glensc - you need to check the return value of the delete_image function and make the logic in __main__

except:
if purge:
url = self.registry.replace("https://", '') + "/" + repo + ":" + tag
call(["docker", "tag", "busybox:latest", url])
Copy link
Contributor

Choose a reason for hiding this comment

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

docker has builtin image named scratch, which can be used in Dockerfile. unfortunately, it's not pullable.

but at some point I created smallest scratch image that returns exit 0:

docker pull glen/scratch

so it's 222 bytes compressed

https://hub.docker.com/r/glen/scratch/

@orangewolf
Copy link
Author

thank you for the great feedback. I won't have cycles to digest and respond until Friday, but I will pick it back up then.

@lexfrei
Copy link

lexfrei commented Sep 30, 2020

@orangewolf which Friday? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants