Parameterize gcr, quay, and docker image repo defines #5146

qingkunl · 2019-09-06T07:03:31Z

This allows to easily override the gcr, quay, and docker repos with the
mirror repos in countries like China, where the default accesses are
blocked or unstable.

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test

/kind feature

/kind flake

What this PR does / why we need it:

Allows to use ansible variables to define gcr, quay, and docker repos, so that people can easily override them (e.g. with the mirror repos) in the inventory file. This is very useful for Kubespray users in countries like China, where the access to gcr, quay, and docker repos are blocked or unstable.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

I've tested this change locally on the machines in China. Without this change, the gcr, quay, and docker images couldn't not be downloaded.

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2019-09-06T07:03:32Z

Welcome @qingkunl!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-09-06T07:03:33Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

qingkunl · 2019-09-06T07:17:04Z

/assign @woopstar

qingkunl · 2019-09-06T18:32:00Z

/retest

k8s-ci-robot · 2019-09-06T18:32:26Z

@qingkunl: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

mirwan

Unless I'm wrong images from docker hub could benefit from the same variable with "docker.io" as default.

qingkunl · 2019-09-07T17:41:05Z

Unless I'm wrong images from docker hub could benefit from the same variable with "docker.io" as default.

It's true that "docker.io" is blocked in China. But we've already got ways to specify the docker mirror by overriding 'docker_registry_mirrors' defined in 'roles/kubespray-defaults/defaults/main.yaml':

## A list of additional registry mirrors, for example China registry mirror. Empty by default.
# docker_registry_mirrors:
#   - https://registry.docker-cn.com
#   - https://mirror.aliyuncs.com
docker_registry_mirrors: []

mirwan · 2019-09-07T22:08:15Z

docker_registry_mirrors is only used if docker is used as container runtime, with containerd for example, some images for docker.io get pulled

mirwan · 2019-09-07T22:10:56Z

Have the same variable for docker.io as for gcr.io and qua.ioy would ease the overridings of all *_image_repo variables in offline installations as well

Timoses · 2019-09-08T07:28:54Z

Why not use something like the following:

roles/download/defaults/main.yml

downloads:
  cilium:
    enabled: "{{ kube_network_plugin == 'cilium' }}"
    container: true
    repo: docker.io/cilium/cilium
    tag: "{{ cilium_version }}"
    sha256: "{{ cilium_digest_checksum|default(None) }}"
    groups:
      - k8s-cluster

User then wants to use another registry:

downloads_overrides:
  cilium:
    repo: mycustomregistry.io/myproject/cilium

Then roles/download/tasks/main.yml would use

downloads | combine(downloads_overrides)

This would get rid of all the *_image_repo and *_image_tag variables while allowing the user to specify which download should be adjusted.

Additionally, it would be possible to use:

downloads:
  cilium:
    repo: "{{ downloads_image_registry | default('docker.io/cilium/cilium') }}"

in case the user wants to use a single registry.

Note: Be aware that the download role has a dynamic part:

kubespray/roles/download/tasks/main.yml

Line 30 in 7f74906

- name: download | Get kubeadm binary and list of required images

qingkunl · 2019-09-08T07:30:38Z

@mirwan That makes sense. I've updated the PR to add the variable for "docker.io" as well. Thanks.

qingkunl · 2019-09-08T07:50:01Z

Why not use something like the following:

roles/download/defaults/main.yml
downloads:
  cilium:
    enabled: "{{ kube_network_plugin == 'cilium' }}"
    container: true
    repo: docker.io/cilium/cilium
    tag: "{{ cilium_version }}"
    sha256: "{{ cilium_digest_checksum|default(None) }}"
    groups:
      - k8s-cluster
User then wants to use another registry:
downloads_overrides:
  cilium:
    repo: mycustomregistry.io/myproject/cilium
Then roles/download/tasks/main.yml would use
downloads | combine(downloads_overrides)
This would get rid of all the *_image_repo and *_image_tag variables while allowing the user to specify which download should be adjusted.

Additionally, it would be possible to use:
downloads:
  cilium:
    repo: "{{ downloads_image_registry | default('docker.io/cilium/cilium') }}"
in case the user wants to use a single registry.

Note: Be aware that the download role has a dynamic part:

kubespray/roles/download/tasks/main.yml

Line 30 in 7f74906

- name: download | Get kubeadm binary and list of required images

Thanks for the suggestion. But it doesn't solve the problem I'm trying to solve in this PR. I'd like to be able to just override the repo for all gcr/quay/docker images with a simple repo override (so that I can easily use mirror repos), instead of having to go through and override each image (etcd, cilium, dashboard, nodelocaldns, etc.).

mirwan

The images repos with no namespace should have the library namespace added like nginx_image_repo: "{{ docker_image_repo }}/library/nginx"

qingkunl · 2019-09-09T07:30:01Z

The images repos with no namespace should have the library namespace added like nginx_image_repo: "{{ docker_image_repo }}/library/nginx"

Updated the PR to add library namespace. Thanks

mirwan · 2019-09-09T18:23:15Z

/lgtm

qingkunl · 2019-09-16T21:31:00Z

It's been silent for a while. Anything else I should do here?

riverzhang · 2019-09-17T05:42:48Z

roles/download/defaults/main.yml

-kube_ovn_cni_image_repo: "docker.io/kubeovn/kube-ovn-cni"
+kube_ovn_db_image_repo: "{{ docker_image_repo }}/kubeovn/kube-ovn-db"
+kube_ovn_node_image_repo: "{{ docker_image_repo }}/kubeovn/kube-ovn-node"
+kube_ovn_cni_image_repo: "{{ docker_image_repo }}/kubeovn/kube-ovn-cni"
 kube_ovn_controller_image_repo: "kubeovn/kube-ovn-controller"


{{ docker_image_repo }}/kubeovn/kube-ovn-controller
I think this needs to be fixed.

Good catch!

Fixed in the latest commit

riverzhang · 2019-09-17T06:18:07Z

Have the same variable for docker.io as for gcr.io and qua.ioy would ease the overridings of all *_image_repo variables in offline installations as well

I agree with this approach, otherwise users will have a lot of configuration options.
@qingkunl in case the user wants to use a single registry. This is very useful for Chinese users, or harbor users. I think @Timoses 's suggestion can be considered，What do you think?

riverzhang · 2019-09-17T06:26:36Z

/hold

This allows to easily override the gcr, quay, and docker repos with the mirror repos in countries like China, where the default accesses are blocked or unstable.

qingkunl · 2019-09-18T05:27:42Z

Have the same variable for docker.io as for gcr.io and qua.ioy would ease the overridings of all *_image_repo variables in offline installations as well

I agree with this approach, otherwise users will have a lot of configuration options.

I think @mirwan meant to also define a variable for docker.io, as I originally only defined variables for gcr.io and quay.io, and I've already done that. Please let me know if I misunderstood.

@qingkunl in case the user wants to use a single registry. This is very useful for Chinese users, or harbor users. I think @Timoses 's suggestion can be considered，What do you think?

It may not help much in my case, as I need to override all images with the mirror repos and don't want to go through and override each of them (etcd, cilium, dashboard, nodelocaldns, etc.). In addition, since it requires a more involved code change and refactor, I suggest consider it in a separate PR. And in case we do want @Timoses 's suggestion, I still suggest keeping this PR to allow the mirror repo override.

riverzhang · 2019-09-18T09:30:54Z

@qingkunl Thanks,
/lgtm
/approve

k8s-ci-robot · 2019-09-18T09:31:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qingkunl, riverzhang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [riverzhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

riverzhang · 2019-09-18T09:48:40Z

/hold cancel

* 'master' of https://github.com/kubernetes-sigs/kubespray: Add support for k8s v1.14.6 (kubernetes-sigs#5182) Parameterize gcr, quay, and docker image repo defines (kubernetes-sigs#5146) use hyperkubeimage to run controlplane containers (kubernetes-sigs#5178) Update main.yml (kubernetes-sigs#5166) Fixes issue kubernetes-sigs#5160 (kubernetes-sigs#5171) Use more native way to update kubeconfigs using kubeadm (kubernetes-sigs#5165) Fix macro on local_volume_provisioner (kubernetes-sigs#5168) Move cri_socket var to kubespray-defaults (kubernetes-sigs#5149) Add support for k8s v1.16.0-beta.2 (kubernetes-sigs#5148) Fix ansible task titles (kubernetes-sigs#5154) Adjust endpoints for kube-proxy,controller,scheduler to proper ip (kubernetes-sigs#5150) Documenting Terraform variable `az_list` explicitly (kubernetes-sigs#5132) Make haproxy/nginx client timeout configurable (kubernetes-sigs#5140) print hostnames (kubernetes-sigs#5110) Cleanup: fix typo in doc (kubernetes-sigs#5105) Use python3-libselinux on RHEL8/Centos8 (kubernetes-sigs#5127)

…s#5146) This allows to easily override the gcr, quay, and docker repos with the mirror repos in countries like China, where the default accesses are blocked or unstable.

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 6, 2019

k8s-ci-robot requested review from holmsten and Miouge1 September 6, 2019 07:03

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 6, 2019

k8s-ci-robot assigned woopstar Sep 6, 2019

mirwan reviewed Sep 7, 2019

View reviewed changes

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 8, 2019

qingkunl changed the title ~~Parameterize gcr and quay image repo defines~~ Parameterize gcr, quay, and docker image repo defines Sep 8, 2019

mirwan suggested changes Sep 8, 2019

View reviewed changes

mirwan approved these changes Sep 9, 2019

View reviewed changes

k8s-ci-robot assigned mirwan Sep 9, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 9, 2019

riverzhang reviewed Sep 17, 2019

View reviewed changes

riverzhang closed this Sep 17, 2019

riverzhang reopened this Sep 17, 2019

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 17, 2019

Parameterize gcr, quay, and docker image repo defines

66bb98b

This allows to easily override the gcr, quay, and docker repos with the mirror repos in countries like China, where the default accesses are blocked or unstable.

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2019

k8s-ci-robot assigned riverzhang Sep 18, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 18, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 18, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 18, 2019

k8s-ci-robot merged commit 3c7f682 into kubernetes-sigs:master Sep 18, 2019

yankay mentioned this pull request Oct 17, 2022

[Docs] support the download mirror for China #9396

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameterize gcr, quay, and docker image repo defines #5146

Parameterize gcr, quay, and docker image repo defines #5146

qingkunl commented Sep 6, 2019 •

edited

Loading

k8s-ci-robot commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

qingkunl commented Sep 6, 2019

qingkunl commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

mirwan left a comment

qingkunl commented Sep 7, 2019 •

edited

Loading

mirwan commented Sep 7, 2019

mirwan commented Sep 7, 2019

Timoses commented Sep 8, 2019 •

edited

Loading

qingkunl commented Sep 8, 2019

qingkunl commented Sep 8, 2019

mirwan left a comment

qingkunl commented Sep 9, 2019

mirwan commented Sep 9, 2019

qingkunl commented Sep 16, 2019

riverzhang Sep 17, 2019

qingkunl Sep 18, 2019

qingkunl Sep 18, 2019

riverzhang commented Sep 17, 2019

riverzhang commented Sep 17, 2019

qingkunl commented Sep 18, 2019

riverzhang commented Sep 18, 2019

k8s-ci-robot commented Sep 18, 2019

riverzhang commented Sep 18, 2019

Parameterize gcr, quay, and docker image repo defines #5146

Parameterize gcr, quay, and docker image repo defines #5146

Conversation

qingkunl commented Sep 6, 2019 • edited Loading

k8s-ci-robot commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

qingkunl commented Sep 6, 2019

qingkunl commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

mirwan left a comment

Choose a reason for hiding this comment

qingkunl commented Sep 7, 2019 • edited Loading

mirwan commented Sep 7, 2019

mirwan commented Sep 7, 2019

Timoses commented Sep 8, 2019 • edited Loading

qingkunl commented Sep 8, 2019

qingkunl commented Sep 8, 2019

mirwan left a comment

Choose a reason for hiding this comment

qingkunl commented Sep 9, 2019

mirwan commented Sep 9, 2019

qingkunl commented Sep 16, 2019

riverzhang Sep 17, 2019

Choose a reason for hiding this comment

qingkunl Sep 18, 2019

Choose a reason for hiding this comment

qingkunl Sep 18, 2019

Choose a reason for hiding this comment

riverzhang commented Sep 17, 2019

riverzhang commented Sep 17, 2019

qingkunl commented Sep 18, 2019

riverzhang commented Sep 18, 2019

k8s-ci-robot commented Sep 18, 2019

riverzhang commented Sep 18, 2019

qingkunl commented Sep 6, 2019 •

edited

Loading

qingkunl commented Sep 7, 2019 •

edited

Loading

Timoses commented Sep 8, 2019 •

edited

Loading