search

plexsystems / sinker

A tool to sync images from one container registry to another
MIT License
609 stars 53 forks source link

Validate all images can be pulled before attempting to pull #38

Closed ravens closed 4 years ago

ravens commented 4 years ago

For any reason, if the pull process is experiencing some network issues on downloading a specific image, the whole process is getting interrupted. This could be problematic for CI environment. I am proposing some PR to don't block but still emit an error during the execution.

jpreese commented 4 years ago

Thanks for opening this issue @ravens. The scenario definitely isn't ideal.

Rather than logging an error during the pull operation, what are your thoughts on first verifying that you can successfully pull all images in the manifest? We could just call ImageExistsAtRemote on all of the images, and only pull them if all of the images are there. This would cover the case of invalid auth as well as if the image just didn't exist.

While not as detrimental as a push operation if something fails during the process, putting the manifest out of sync, it keeps things consistent and doesn't waste time pulling images onto a machine that won't get all of the images you actually wanted it to get. Potentially forcing you to re-download everything anyway if the pull is done on another machine (or cache invalidated) after fixing the root issue.

ravens commented 4 years ago

I think this makes sense ! I am just curious how this will handle intermittent network errors like the one I got the other day (which made me propose the PR actually) - the first call to check the image might work, but the second one might fail, right ?

jpreese commented 4 years ago

That is a fair point, theres no way to fully guarantee success due to network errors. That is the hope with Retry, to mitigate those (and they would hopefully be rare!).

But this would prevent issues relating to authorization, which looks like was the problem from the error message in the PR

7.4.2-SNAPSHOT: unauthorized: authentication required
ravens commented 4 years ago

So checking image presence and access makes sense. Perhaps a flag to have some blocking or non blocking behavior is more useful to cover interactive and non-interactive usages

jpreese commented 4 years ago

@ravens in v0.14.0 sinker will now check that it can pull all of the images before executing the pull command.