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

PROJQUAY-381 - failed mirror tag cleanup #516

Merged
merged 1 commit into from Aug 19, 2020

Conversation

thomasmckay
Copy link
Contributor

@thomasmckay thomasmckay commented Aug 11, 2020

Issue: https://issues.redhat.com/browse/PROJQUAY-381

Changelog:
Fixed: Mirror of repos with a large number of tags may hang due to buffering skopeo output.
Fixed: When a mirror job is pre-empted, original tags may be deleted. Mirror jobs that are killed are less likely now to remove existing tags.

Docs:

Testing:

  • Sync a repo with a lot of tags with debug logging enabled (eg. https://quay.io/repository/bitnami/sealed-secrets-controller ).
  • Confirm job succeeds.
  • Sync same repo again, cancelling after seeing the first skopeo inspect in the logs.
  • Confirm job cancelled and any tags that were being synced are reverted in the tag history.

Details:
A hung mirror job that was either pre-empted or killed would not always rollback the changes. These changes fix the hanging but also move the deletion of old tags to after all images are synced.

@thomasmckay thomasmckay changed the title PROJQUAY-381 - failed mirror tg cleanup PROJQUAY-381 - failed mirror tag cleanup Aug 18, 2020
kurtismullins
kurtismullins previously approved these changes Aug 19, 2020
Copy link
Contributor

@kurtismullins kurtismullins left a comment

Choose a reason for hiding this comment

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

I have left suggestions that I believe should be taken into consideration. I won't block the PR on them, though. If you decide to change things and want another approval, just ping me.

Otherwise, it LGTM

util/repomirror/skopeomirror.py Outdated Show resolved Hide resolved
workers/repomirrorworker/__init__.py Outdated Show resolved Hide resolved
workers/repomirrorworker/__init__.py Outdated Show resolved Hide resolved
workers/repomirrorworker/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@kurtismullins kurtismullins left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasmckay thomasmckay merged commit e81b9ee into quay:master Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants