-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Allow multiple destinations #184
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
I signed it! |
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 contributing, I left a couple comments!
cmd/executor/cmd/root.go
Outdated
logrus.Error(err) | ||
os.Exit(1) | ||
|
||
for _, destination := range destinations { |
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.
Could we instead pass the destinations into DoPush, and have the for loop there?
cmd/executor/cmd/root.go
Outdated
for _, destination := range destinations { | ||
if err := executor.DoPush(ref, image, destination, tarPath); err != nil { | ||
logrus.Error(err) | ||
} |
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 probably want to add the os.Exit(1)
here!
CLAs look good, thanks! |
Tested this, should be fine |
Great, thanks! |
Hello everyone,
I had the requirement to push an image to multiple container registries.
Since the image is already built it would make sense to push it to more than one registry (instead of building the image again with a different destination).
I recycled and existing type for multiple args and added a simple for-loop.
The missing
os.Exit
inside the loop made sense to me (why stop when only one of multiple pushes fail)Tested this in my fork.
You can now specify multiple
--destination
flags.If the
tarpath
is set, multiple tar-files (one for each destination) will be generated.Please note that I didn't touch the
README.md
in this pull request.