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

kamal deploy with --skip-push tries a local docker registry login #814

Closed
DazWorrall opened this issue May 21, 2024 · 2 comments · Fixed by #832
Closed

kamal deploy with --skip-push tries a local docker registry login #814

DazWorrall opened this issue May 21, 2024 · 2 comments · Fixed by #832

Comments

@DazWorrall
Copy link

Hey 👋 We're experimenting with Kamal at Shopify and it's allowed us to go from zero to one pretty quickly for a particular project, so thanks for that. For the most part we've been able to adopt Kamal pretty cleanly but there's one issue in particular I wanted to flag.

We have an existing build infra for container images so we can skip the build and push at kamal deploy time, and do so with the --skip-push flag. Because we build separately our deploy system does have access to a docker daemon, however Kamal will try to log into the registry locally even if no build and push is required:

def login
run_locally { execute *KAMAL.registry.login }
on(KAMAL.hosts) { execute *KAMAL.registry.login }

(#login is called here before the --skip-push flag is checked.)

We've worked around this by stubbing out the docker binary on the deploy system and with that we're able to deploy successfully, but it would be nice to do better than hack around the requirement like this. If --skip-push skipped the local login that would be fine - and if the intention of the local login is to check the image could be pushed before trying to build it, then that aligns with the intent of the flag - alternatively a separate flag could be added if you think that's warranted. Or something else I haven't thought of.

I'm happy to spend some time on a PR if you have a preferred fix in mind.

@djmb
Copy link
Collaborator

djmb commented Jun 5, 2024

Thanks for raising this @DazWorrall. There's no need for the local login when using skip push, so we can remove it 👍

@djmb djmb closed this as completed in #832 Jun 6, 2024
@DazWorrall
Copy link
Author

Thanks @djmb!

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 a pull request may close this issue.

2 participants