GoogleContainerTools / skaffold

Easy and Repeatable Kubernetes Development
https://skaffold.dev/
Apache License 2.0
14.99k stars 1.62k forks source link

Skaffold & local docker daemon registry mirror #7368

Open iusetabs opened 2 years ago

iusetabs commented 2 years ago

Expected behavior

Actual behavior

Information

<paste your skaffold.yaml here>
# Copyright 2021 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#      http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: skaffold/v2beta26
kind: Config
build:
  local:
    concurrency: 8
    tryImportMissing: true

  artifacts:
    # image tags are relative; to specify an image repo (e.g. GCR), you
    # must provide a "default repo" using one of the methods described
    # here:
    # https://skaffold.dev/docs/concepts/#image-repository-handling
    - image: servicename
      context: src/servicename

  tagPolicy:
    gitCommit: {}
deploy:
  kubectl:
    manifests:
      - ./kubernetes-manifests/**.yaml

Steps to reproduce the behavior

tejal29 commented 2 years ago

@iusetabs Skaffold relies on docker client lib to pull images. We don't do anything special and hence this should work out of the box. That said, can you please provide us the docker daemon config filename and the contents?

iosifnicolae2 commented 2 years ago

We're using kaniko, and we've configured registryMirror option but it seems that skaffold somehow uses index.docker.io when parsing ONBUILD instructions.

oursland commented 2 years ago

It appears that a skaffold dependency module is at fault, but several refer to index.docker.io so I am not sure which one.

~I believe that prefixing the dockerhub images with docker.io/library/ will circumvent the automated insertion of index.docker.io/library/ that breaks the Docker's registry mirror configuration.~

The issue appears to be with go-containerregistry and the lack of mirror support. See issue #1200.

iusetabs commented 2 years ago

@oursland Thx for the link to the issue, I've been spending some time looking into this again today. I'm not familiar with the source code of Skaffold at all, but I was digging into the code and once I hit the ConfigFile function at pkg/skaffold/docker/image.go:273 things got interesting 😉 I agree that this issue seems to be with the go-containerregistry library. Seems we must take this fight over there!

Even though this isn't a skaffold issue, I will have to remove skaffold from the pipeline I work with unless this can be fixed soon. This pull rate error is blocking my team😢 Unless somebody can suggest some hacky approach to quickly work around this issue 😉

iusetabs commented 2 years ago

After building skaffold locally and figuring out how it actually interacts with the go-containerregistry module, I was able to modify a local version of the go-containerregistry module and point skaffold at it. I added my mirror to the local go-containerregistry module and now it pulls through the mirror without any rate limit error.

However in order to get any change of this nature into a skaffold release, one needs to tweak the skaffold dependencies. go-containerregistry is on version 0.11 and skaffold is using version 0.8 of the module. When I tried updating the dependencies I got stuck with other dependency errors. So this might have unforeseen consequences and require changes in other parts of the skaffold codebase.

Also skaffold was particularly hard to debug, I had to put print statements in multiple places to see what was going on. It would be super nice to connect the IntelliJ Ultimate debugger to a debug version of the skaffold binary, has anybody any experience with that, maybe with Visual Code?

I will leave this issue open, as work will need to be done with Skaffold in order to leverage the docker mirrors whenever that feature is developed with go-containerregistry .

ericzzzzzzz commented 1 year ago

Needs to upgrade go-containerregistr version

ericzzzzzzz commented 1 year ago

go-containerregistry has been upgraded on HEAD, closing this out.

Agalin commented 1 year ago

Does it really fix the problem? I don't see anything in go-containerregistry's changelog about adding mirrors support, GoogleContainerTools/go-containerregistry#1200 is still open.

iusetabs commented 1 year ago

Agreed @ericzzzzzzz this issue was about skaffold not supporting docker mirrors. Specifically one will get rate limited as the go container registry dependency is pinging index.docker.io ( due to a hardcoded value ) even when using a docker mirror. Please re-open this issue as there may be changes required to skaffold in order to support this ( hopefully not though! )

ericzzzzzzz commented 1 year ago

sorry I misunderstood the problem, re-opening this

iusetabs commented 1 year ago

No worries! Thanks very much 😄

renzodavid9 commented 1 year ago

Comment to make triage-party happy. Please ignore

ematvey commented 2 weeks ago

Any progress on this? This issue prevents us from deploying skaffold.

oursland commented 1 week ago

@ematvey The underlying issue is with go-containerregistry. Recently @sherine-k submitted PR https://github.com/google/go-containerregistry/pull/2010 which is a start towards enabling the necessary feature within that library.