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

Use local docker registry to push and pull app images #1355

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

npezza93
Copy link
Contributor

Allow applications to be deployed without needing to set up a repository in a remote Docker registry.

If the registry server starts with localhost, Kamal will start a local docker registry on that port and push the app image to it.

Then when pulling the image onto the servers, we use net-ssh to forward the that port from the app server to the deployment server.

This is branched off @djmb 's initial work. I have this working on a simple app but curious to open this up to try out to find issues and possibly optimize. Saw an overall speed up of around 10-15 seconds.

To get this to work update your deploy.yml files registry section to look something like:

registry:
  server: "localhost:5555"

@npezza93 npezza93 force-pushed the local-docker-registry branch from c217a8b to f24e74c Compare January 14, 2025 19:39
def setup
combine \
docker(:start, "kamal-docker-registry"),
docker(:run, "--detach", "-p", "127.0.0.1:#{local_port}:5000", "--name", "kamal-docker-registry", "registry:2"),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Publishing the port on your local machine will make it available in the same network segment (see moby/moby#45610). That's why I didn't go any further with this at the time.

I think we can mitigate this though by generating a password for the registry when we boot it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm looks like a pr was merged a few days ago to resolve that. Perhaps in the next release of buildkit it'll be fixed for us?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh awesome! This will be great 🎉

@npezza93 npezza93 force-pushed the local-docker-registry branch 2 times, most recently from 1bfc975 to 9685117 Compare January 17, 2025 16:26
djmb and others added 6 commits January 20, 2025 11:03
Allow applications to be deployed without needing to set up a repository
in a remote Docker registry.

If the registry server starts with `localhost`, Kamal will start a local
docker registry on that port and push the app image to it.

Then when pulling the image onto the servers, we use net-ssh to forward
the that port from the app server to the deployment server.
@npezza93 npezza93 force-pushed the local-docker-registry branch from f3dc9f3 to 3c21394 Compare January 20, 2025 16:03
Copy link
Contributor

@ShPakvel ShPakvel left a comment

Choose a reason for hiding this comment

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

Thanks again for working on it ❤️ 🚀

pull_on_hosts(KAMAL.hosts - first_hosts)
else
pull_on_hosts(KAMAL.hosts)
Kamal::Cli::PortForwarding.new(KAMAL.hosts, KAMAL.config.registry.local_port).forward do
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you guys for working on this feature! 🙏🏼 I was thinking to try it myself 😄

Minor 2 cents suggestion:

  • This line read like forward ports always. While there is condition inside of class implementation. Even with local registry by default it kind of confusing in case of remote registry usage.
  • And the condition in the class if KAMAL.config.registry.local? is the only part that coupled to registry port. Extracting that condition will make class more general purpose PortForwarding to use for any other service port forwarding.

So how about, instead of this line here (and condition in the class), to do something like following with direct condition?

    def pull_on_hosts(hosts, forward_registry_port: KAMAL.config.registry.local?)
      if forward_registry_port
        return Kamal::Cli::PortForwarding.new(hosts, KAMAL.config.registry.local_port).forward do
          pull_on_hosts(hosts, forward_registry_port: false)
        end
      end

      ...
    end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @ShPakvel, I like the suggestion around making the PortForwarding class usable elsewhere and not tying it to if the registry is local. Didnt really follow the code suggestion but made some edits to make the class usable elsewhere. Let me know what you think

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.

3 participants