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

deploymentRegistry isn't used with buildMode of cluster-docker or kaniko #1034

Closed
jmeickle opened this issue Jul 26, 2019 · 6 comments · Fixed by #1597
Closed

deploymentRegistry isn't used with buildMode of cluster-docker or kaniko #1034

jmeickle opened this issue Jul 26, 2019 · 6 comments · Fixed by #1597
Assignees
Labels
priority:medium Medium priority issue or feature

Comments

@jmeickle
Copy link

Bug

Current Behavior

With a garden.yml Project like this:

environments:
  - name: staging
    providers:
      - name: kubernetes
        context: stg-admin@staging
        namespace: staging
        buildMode: cluster-docker
        deploymentRegistry:
          hostname: 997938224961.dkr.ecr.us-east-1.amazonaws.com

Remote builds will occur in the cluster, but the deploymentRegistry does not appear to get used (unlike a local buildMode). Instead, the push is to 127.0.0.1:5000/staging/qf. This prevents easily pulling the built image locally.

Expected behavior

I can docker pull from the listed deploymentRegistry to test the remote-built image locally.

Suggested solution(s)

The remote builder pushes to the in-cluster cache, but the in-cluster cache immediately starts a job to synchronize the push to the configured deploymentRegistry, so it will be available in both places.

Alternately, make deploymentRegistry a property of the buildMode, and don't allow setting it for cluster-docker and kaniko buildModes (as it's confusing that it does nothing). In that case, allow configuring the deploymentRegistry so that it's not 127.0.0.1:5000, so that local users can download images directly from the remote cache.

Additional context

In my case, the container is crash looping on Kubernetes and I want to rule out a code issue (or a sync issue, or something else specific to Garden) by running the same container with local, working config.

Your environment

[eronarn@ip-192-168-10-243 qf]$ garden version
kubectl 0.10.2
[eronarn@ip-192-168-10-243 qf]$ kubectl version
Client Version: version.Info{Major:"1", Minor:"14", GitVersion:"v1.14.3", GitCommit:"5e53fd6bc17c0dec8434817e69b04a25d8ae0ff0", GitTreeState:"clean", BuildDate:"2019-06-06T01:44:30Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"13", GitVersion:"v1.13.7", GitCommit:"4683545293d792934a7a7e12f2cc47d20b2dd01b", GitTreeState:"clean", BuildDate:"2019-06-06T01:39:30Z", GoVersion:"go1.11.5", Compiler:"gc", Platform:"linux/amd64"}
[eronarn@ip-192-168-10-243 qf]$ docker version
Client: Docker Engine - Community
 Version:           19.03.0-rc2
 API version:       1.40
 Go version:        go1.12.5
 Git commit:        f97efcc
 Built:             Wed Jun  5 01:37:53 2019
 OS/Arch:           darwin/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.0-rc2
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.12.5
  Git commit:       f97efcc
  Built:            Wed Jun  5 01:42:10 2019
  OS/Arch:          linux/amd64
  Experimental:     true
 containerd:
  Version:          v1.2.6
  GitCommit:        894b81a4b802e4eb2a91d1ce216b8817763c29fb
 runc:
  Version:          1.0.0-rc8
  GitCommit:        425e105d5a03fabd737a126ad93d62a9eeede87f
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
@jmeickle
Copy link
Author

(Sorry for the quantity of tickets - we're evaluating Garden on some internal projects so I'm going on a deep dive these past few days.)

@eysi09
Copy link
Collaborator

eysi09 commented Jul 29, 2019

Hi @Eronarn. Not at all, they are most welcome!

This one suggests that we need to improve our docs on the remote building functionality :)

When you use buildMode: docker | kaniko, Garden builds the image in-cluster, using its own deployment registry. So the deploymentRegistry field is only applicable if you build the images locally. The in-cluster registry wasn't really meant to be accessed from outside the cluster and currently Garden connects to it via a tunnel rather than using Docker.

As a workaround, you can use the publish command. When using in-cluster building, running garden publish my-module will pull the image from the in-cluster registry (via the tunnel), and push it to the registry specified with the image field at the module level. You can then pull it as usual with docker pull. Here's the docs for publishing images.

So the steps would be:

  1. Add image: my-org/my-module:tag to the garden.yml for the appropriate module, say my-module.
  2. Run garden publish my-module
  3. Run docker pull my-org/my-module:tag

Would that be sufficient in your case? Since we already have the mechanism for pulling the image from the in-cluster registry, we could also expose this as a specific command on the Kubernetes provider, if it's a common enough use-case.

@jmeickle
Copy link
Author

In our case, we're sending all traffic through a VPN. If we garden publish and pull/push the image locally in order to push it remotely, that'll be a fairly slow process (and also unnecessary, in that I only need the local copy, so I only actually need the first half of what the command does).

Also, this approach doesn't work well with the garden dev workflow, since it would involve running a command in a second terminal (and some risk of non-determinism depending on when you ran the command vs. what module builds were currently happening).

So we can use that workaround for now, but I think the ideal mode would involve one of:

  • An option to expose the in-cluster registry as a service, without tunneling, so non-Garden Docker consumers can use it
  • Background syncing builds to a "real" registry, via cluster->registry rather than cluster->local->registry
  • Background syncing builds locally (seems more complicated and bandwidth-intensive)

That being said, I saw you also have an issue open to investigate Squash. Having better debugging options in k8s reduces the need to run containers locally (though I think that'll always be an occasional need).

@edvald
Copy link
Collaborator

edvald commented Jul 30, 2019

I think the option we'll likely use is to allow overriding deploymentRegistry and using that instead of deploying our own registry in-cluster. The drawback will be slowing the deployment flow a bit, but the main reason we haven't yet implemented it is that we'll need to work out how to easily manage authentication to the specified registry.

As for your particular use-case, we're definitely looking towards ways to more easily debug stuff inside the cluster. That may include integrating Squash, and also #967.

@Tails
Copy link

Tails commented Nov 8, 2019

I also expected it to use a defined deploymentRegistry. My garden plugins kubernetes cluster-init hangs at deploying the registry-proxy (and build-sync...) so I thought I could remove that step by using a defined registry.

@stale
Copy link

stale bot commented Jan 15, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale Label that's automatically set by stalebot. Stale issues get closed after 14 days of inactivity. label Jan 15, 2020
@edvald edvald self-assigned this Jan 15, 2020
@stale stale bot removed the stale Label that's automatically set by stalebot. Stale issues get closed after 14 days of inactivity. label Jan 15, 2020
@edvald edvald added the priority:medium Medium priority issue or feature label Jan 15, 2020
edvald added a commit that referenced this issue Feb 13, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 13, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 14, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 14, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 14, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 14, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
edvald added a commit that referenced this issue Feb 14, 2020
Previously we would always deploy and use the in-cluster registry
when building in-cluster. Now we allow using the configured
`deploymentRegistry`, which is often preferable (and more scalable)
than using the simpler in-cluster registry.

Closes #1034
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:medium Medium priority issue or feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants