Update flannel use install-cni-plugin to fit upstream #8714
Conversation
k8s-ci-robot
added
kind/feature
k8s-ci-robot
added
the
size/S
|
A couple of questions:
|
roles/download/defaults/main.yml
Outdated
| @@ -830,6 +830,8 @@ etcd_image_repo: "{{ quay_image_repo }}/coreos/etcd" | |||
| etcd_image_tag: "{{ etcd_version }}{%- if image_arch != 'amd64' -%}-{{ image_arch }}{%- endif -%}" | |||
| flannel_image_repo: "{{ quay_image_repo }}/coreos/flannel" | |||
| flannel_image_tag: "{{ flannel_version }}-{{ image_arch }}" | |||
| flannel_init_image_repo: "rancher/mirrored-flannelcni-flannel-cni-plugin" | |||
There was a problem hiding this comment.
The offline deployment feature cannot work if container images contain static repo names like rancher/ because the repo name parts({{ quay_image_repo }} for example) are replaced with local repos for offline deployment.
Related to @cristicalin's comment, can we use a common repo for that instead of the static rancher/ ?
There was a problem hiding this comment.
Ok, I've replaced it to docker.io/flannelcni/flannel-cni-plugin:v1.0.0
There was a problem hiding this comment.
I don't quite understand the comment in https://github.com/flannel-io/flannel/blob/349feae40ae4c8a57c5abbaf9c4fcfe43cadc264/Documentation/kube-flannel.yml#L169-L170, what docker hub limitations affect ppc64le? I'm asking this since we now support ppc64le arch and we should be aware (and maybe document) the limitation.
There was a problem hiding this comment.
Now Kubespray is using flannel-cni-plugin v1.0.0 and the flannel-cni-plugin image on the dockerhub supports
linux/amd64
linux/arm/v6
linux/arm64/v8
linux/ppc64le
linux/s390x
as https://hub.docker.com/r/flannelcni/flannel-cni-plugin/tags
It is better to investigate more to know the reason of the comment.
There was a problem hiding this comment.
I think I see the reason of the above comment.
flannel-io/flannel#1482 (comment) shows
- The dockerhub flannel-cni-plugin is the official image
- The rancher flannel-cni-plugin is for avoiding a rate limit issue due to the dockerhub policy
image: flannelcni/flannel-cni-plugin:v1.0.1 for ppc64le (dockerhub limitations may apply)means the rancher flannel-cni-plugin doesn't support ppc64le and we need to enable the dockerhub one instead for ppc64le if we are using.
To support ppc64le also, it is necessary to use the dockerhub image.
If using the dockerhub image, a rate limit issue could happen as the above comment.
However Kubespray already is using a lot of dockerhub images, I don't feel that can be a matter for us.
@cristicalin Thanks, that's because want to be consistent with upstream.
|
I think you should remove that part in this PR since having both methods would cause binaries to be replaced during the deployment procedure. You can do this in a separate commit in this PR just to make it clear that there are separate steps to the change. |
k8s-ci-robot
added
size/M
|
Thanks for doing this @zhengtianbao ! /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cristicalin, zhengtianbao 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
/lgtm |
k8s-ci-robot
added
the
lgtm
floryut
added
kind/network
…s#8714) * Update flannel use install-cni-plugin to fit upstream * Replace flannel cni repo * Remove download flannel binary
…s#8714) * Update flannel use install-cni-plugin to fit upstream * Replace flannel cni repo * Remove download flannel binary
What type of PR is this?
/kind feature
What this PR does / why we need it:
use rancher/mirrored-flannelcni-flannel-cni-plugin:v1.0.0 initContainer to install flannel binary to host like flannel-io/flannel did.
Which issue(s) this PR fixes:
Fixes #8333
Special notes for your reviewer:
The flannel-cni-plugin verison v1.0.0 is compatible with flannel v0.15.1
flannel-io/flannel@dda69bc
Does this PR introduce a user-facing change?: