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

GCE: Enable using gcr.io as a Docker registry mirror. #25841

Merged

Conversation

ojarjur
Copy link
Contributor

@ojarjur ojarjur commented May 18, 2016

Use gcr.io as a Docker registry mirror when setting up a cluster in GCE.

Analytics

This only affects clusters running under GCE.

@k8s-bot
Copy link

k8s-bot commented May 18, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

1 similar comment
@k8s-bot
Copy link

k8s-bot commented May 18, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@ojarjur
Copy link
Contributor Author

ojarjur commented May 18, 2016

@dchen1107 Can you please take a look at this?

@k8s-bot
Copy link

k8s-bot commented May 18, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels May 18, 2016
@roberthbailey
Copy link
Contributor

ok to test

@roberthbailey
Copy link
Contributor

This looks ok to me, but I'd like @zmerlynn to take a look as well since he was recently changing a bunch of our scripts to deal with regional gcr registries.

@andyzheng0831 - we should enable this for GCI nodes on GCE as well.

@andyzheng0831
Copy link

@roberthbailey No problem. If this PR will not be cherry picked to release-1.2, I will only make change in cluster/gce/gci but not for cluster/gce/trusty

@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/old-docs size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. kind/design Categorizes issue or PR as related to design. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 20, 2016

# Decide whether to enable the cache
if [[ "${ENABLE_DOCKER_REGISTRY_CACHE:-}" == "true" ]]; then
REGION=$(echo "${ZONE}" | cut -f 1 -d -)

Choose a reason for hiding this comment

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

Where this env variable "${ZONE}" from? And it is better to use "${ZONE:-}" in case it is unset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andyzheng0831 Added an explanation of the source of the variables (the kube-env metadata value), and added special handling for the ZONE variable being empty.

PTAL

if [[ "${REGION}" == "europe" ]]; then
REGION="eu"
fi
echo "Enable docker registry cache at region: " $REGION

Choose a reason for hiding this comment

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

$REGION --> "${REGION}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@roberthbailey roberthbailey added ok-to-merge release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed needs-ok-to-merge labels May 20, 2016
@roberthbailey roberthbailey changed the title Enable using gcr.io as a Docker registry mirror. GCE/GKE: Enable using gcr.io as a Docker registry mirror. May 20, 2016
@roberthbailey roberthbailey changed the title GCE/GKE: Enable using gcr.io as a Docker registry mirror. GCE: Enable using gcr.io as a Docker registry mirror. May 20, 2016
@ojarjur ojarjur force-pushed the ojarjur/registry-mirror branch from 02ccca0 to 60de7f7 Compare May 24, 2016 00:34
@zmerlynn zmerlynn added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
This only affects clusters running under GCE.
@ojarjur ojarjur force-pushed the ojarjur/registry-mirror branch from 60de7f7 to 338b33f Compare May 24, 2016 15:12
@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
@roberthbailey roberthbailey added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 24, 2016
@roberthbailey
Copy link
Contributor

/cc @andyzheng0831 @wonderfly

@dchen1107
Copy link
Member

@andyzheng0831 I don't think this one need to cherrypick to 1.2 release branch.

@roberthbailey
Copy link
Contributor

I agree, but we should set it on GCI masters on the master branch.

@ojarjur
Copy link
Contributor Author

ojarjur commented May 26, 2016

Is there something else I need to do for this PR?

@roberthbailey
Copy link
Contributor

No, just wait for it to get merged. You can track your PR's progress at http://submit-queue.k8s.io/#/queue

@wonderfly
Copy link
Contributor

I agree, but we should set it on GCI masters on the master branch.

Can we file an issue to GCI team and assign it to somebody so that it doesn't get lost in emails?

@andyzheng0831
Copy link

No need to log a separate issue for GCI. For cases like this PR, we can make GCI side change when the original PR is finalized (i.e., marked with lgtm). So, it is the time, I will make a PR for GCI

@roberthbailey roberthbailey added this to the v1.3 milestone Jun 2, 2016
@roberthbailey
Copy link
Contributor

@k8s-bot unit test this issue: #26730

@andyzheng0831
Copy link

I will make a PR for GCI config change.

As we already switched k8s master to GCI and we are testing it in various places, I think we may encourage k8s developers to make the GCI side change if necessary when they prepare their PRs. I understand people may need some time to know how to do GCI config change and how to test it. But it is better to gradually do it, and we have seen more and more developers are doing it which is pretty good. For GCI config change, please feel free to cc me for review.

k8s-github-robot pushed a commit that referenced this pull request Jun 3, 2016
Automatic merge from submit-queue

GCI/Trusty: support the Docker registry mirror

@roberthbailey @zmerlynn please review it.

cc/ @fabioy @dchen1107 @kubernetes/goog-image FYI.

cc/ @ojarjur it is very straightforward to add support for GCI, which is pretty much like the change to ContainerVM's configure-vm.sh in your original PR #25841.
@goltermann
Copy link
Contributor

@ojarjur @roberthbailey @zmerlynn @andyzheng0831 What's needed here? Just prod the test bot along?

@roberthbailey
Copy link
Contributor

Yes. Ironically, the PR that copied this for GCI has already been merged but this one is still sitting.

@roberthbailey
Copy link
Contributor

@k8s-bot test this issue: #IGNORE (the details links for the two failed tests show no output)

mtaufen pushed a commit to mtaufen/kubernetes that referenced this pull request Jun 6, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jun 6, 2016

GCE e2e build/test passed for commit 338b33f.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 98c432a into kubernetes:master Jun 6, 2016
@chrislovecnm
Copy link
Contributor

Can someone give me more details on this? Is gcr.io a full mirror of docker?

@mattmoor
Copy link
Contributor

mattmoor commented Jun 9, 2016

The short answer is that XX-mirror.gcr.io for XX in {eu,us,asia} is compatible with --registry-mirror=https://XX-mirror.gcr.io. It is not a full mirror, it is purely demand based.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.