Enable multi-CPU architecture in Flannel CNI #6166

electrocucaracha · 2020-05-19T16:50:59Z

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:
Flannel CNI provides different Docker images to support multi-CPU architectures, this change provides the daemonsets required to enable them.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:
kube_network_plugin: flannel

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2020-05-19T16:51:07Z

Hi @electrocucaracha. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

EppO · 2020-05-20T21:07:44Z

CI is broken right now, need to retest this once it's fixed

electrocucaracha · 2020-05-21T14:51:24Z

/retest

k8s-ci-robot · 2020-05-21T14:51:37Z

@electrocucaracha: 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.

MrFreezeex · 2020-05-25T14:49:35Z

roles/network_plugin/flannel/templates/cni-flannel.yml.j2

@@ -42,7 +42,7 @@ data:
 apiVersion: apps/v1
 kind: DaemonSet
 metadata:
-  name: kube-flannel
+  name: kube-flannel-ds-amd64


To simplify multi arch support, could you use templating with a for loop in jinja ? AFAIK you just replace the arch name in 2/3 strings so It should be a good improvement...

That only applies for those case where you have an homogeneous architecture (all nodes of the cluster only using the amd64 or arm). Unfortunately the official flannel image is offered using different tags for different architectures.

Yes sure, I do know that but instead of copying+pasting the deamonset x times, you can use something like {% for arch in ['amd64', 'arm64', ...] %} or something similar to generate those...

This will enable you to have a smaller set of changes and improve general maintainability.

EppO · 2020-05-28T14:56:46Z

/lgtm

MrFreezeex · 2020-05-28T17:44:06Z

Just thinking about It, but maybe It would be nice to remove the old daemonset ?

EppO · 2020-05-28T18:21:49Z

Just thinking about It, but maybe It would be nice to remove the old daemonset ?

actually yes, for the upgrade use case we should test for its existence and remove it if it does because the new daemonset will have a different name.

EppO · 2020-05-28T18:26:11Z

/hold

Miouge1 · 2020-07-13T14:11:44Z

@EppO and @electrocucaracha any ideas forward?

Not sure if quay supports it, but multiarch docker registry is pretty useful for multi arch support.

electrocucaracha · 2020-07-13T17:24:26Z

@EppO and @electrocucaracha any ideas forward?

Not sure if quay supports it, but multiarch docker registry is pretty useful for multi arch support.

I think it's supported using different tags. It should be nice if the Flannel image could be offered using the BuildX plugin but I'm not sure if its source has restrictions during the build process.

EppO · 2020-07-16T22:16:22Z

I'm ok with the multi-arch approach but my concern is about the upgrade path, we may end up with multiple daemonsets of flannel because this code creates new ones with different names.

SpaaarK · 2020-07-31T11:54:52Z

Hi! I used this in a baremetal cluster with some amd64 and arm systems and I would like to make a suggestion:

Change the init container image used to be architecture specific adding also the -{{ arch }}
All the best and thank you!

SpaaarK

Change image name for the init container also

in line 108 image: {{ flannel_image_repo }}:{{ flannel_image_tag }}-{{ arch }}

Signed-off-by: Victor Morales <v.morales@samsung.com>

electrocucaracha · 2020-08-10T15:09:30Z

@Miouge1 @EppO @MrFreezeex I did some changes to preserve the amd64 daemonset and I fixed the name of image for the init container, I think this version will satisfy the upgrade scenario.

EppO · 2020-09-02T20:20:33Z

/lgtm

floryut · 2020-09-21T07:13:35Z

/unhold
/ok-to-test

floryut · 2020-09-22T07:41:32Z

/approve

k8s-ci-robot · 2020-09-22T07:41:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: electrocucaracha, floryut

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~~ [floryut]

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

* 'master' of https://github.com/kubernetes-sigs/kubespray: remove variable 'etcd_ionice', because ionice removed from container image etcd:v3.4.x (kubernetes-sigs#6735) calico: default to using kdd datastore (kubernetes-sigs#6693) Update docker packages to 19.03.13 + add docker f32 (kubernetes-sigs#6712) Fix snapshot.storage apiVersion (kubernetes-sigs#6711) properly generate extravolumes in kubeadmconfig for centos (kubernetes-sigs#6708) Fix reserved memory unit in kubelet configuration (kubernetes-sigs#6725) Fix unintended SIGPIPE (kubernetes-sigs#6721) Expose offline install overrides in inventory (kubernetes-sigs#6728) Added ability to set calico vxlan vni and port. defaults to calico's … (kubernetes-sigs#6678) Change health check from TCP to HTTPS (kubernetes-sigs#6487) Add multi architeture support to flannel (kubernetes-sigs#6166) Remove pypi repo and pip extra flags (kubernetes-sigs#6729) Fails if kubeadm_version do not matches kubernetes version (kubernetes-sigs#6302) Add external_openstack_lbaas_provider setting for occm (kubernetes-sigs#6566) add new variable allowing additionnal audit webhook server options (kubernetes-sigs#6726) Fix example value for etcd_quota_backend_bytes (kubernetes-sigs#6724) Added support for setting tiller_service_account and tiller_replicas (kubernetes-sigs#6696)

Signed-off-by: Victor Morales <v.morales@samsung.com>

k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 19, 2020

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 19, 2020

k8s-ci-robot requested review from bozzo and EppO May 19, 2020 16:51

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 19, 2020

MrFreezeex reviewed May 25, 2020

View reviewed changes

electrocucaracha force-pushed the improve_flannel branch from cb34fcc to 6724916 Compare May 25, 2020 23:38

k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 25, 2020

k8s-ci-robot assigned EppO May 28, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 28, 2020

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

SpaaarK suggested changes Jul 31, 2020

View reviewed changes

Add multi architeture support to flannel

0d87565

Signed-off-by: Victor Morales <v.morales@samsung.com>

electrocucaracha force-pushed the improve_flannel branch from 6724916 to 0d87565 Compare August 10, 2020 15:03

k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 10, 2020

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Aug 10, 2020

electrocucaracha mentioned this pull request Aug 20, 2020

Use "docker manifest" to publish multi-arch Docker images flannel-io/flannel#1327

Merged

3 tasks

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

floryut mentioned this pull request Sep 22, 2020

Add Flannel support for multi-arch deployments #6730

Closed

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

k8s-ci-robot merged commit 0d0cc8c into kubernetes-sigs:master Sep 22, 2020

electrocucaracha deleted the improve_flannel branch September 29, 2020 14:29

floryut mentioned this pull request Dec 19, 2020

Release Proposal v2.15 #7069

Closed

LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Jan 16, 2021

Add multi architeture support to flannel (kubernetes-sigs#6166)

d9cfa2f

Signed-off-by: Victor Morales <v.morales@samsung.com>

muzi502 mentioned this pull request Apr 27, 2021

Add image_arch in flannel image tag #7560

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable multi-CPU architecture in Flannel CNI #6166

Enable multi-CPU architecture in Flannel CNI #6166

electrocucaracha commented May 19, 2020

k8s-ci-robot commented May 19, 2020

EppO commented May 20, 2020 •

edited

Loading

electrocucaracha commented May 21, 2020

k8s-ci-robot commented May 21, 2020

MrFreezeex May 25, 2020

electrocucaracha May 25, 2020

MrFreezeex May 25, 2020 •

edited

Loading

EppO commented May 28, 2020

MrFreezeex commented May 28, 2020

EppO commented May 28, 2020 •

edited

Loading

EppO commented May 28, 2020

Miouge1 commented Jul 13, 2020

electrocucaracha commented Jul 13, 2020

EppO commented Jul 16, 2020

SpaaarK commented Jul 31, 2020 •

edited

Loading

SpaaarK left a comment •

edited

Loading

electrocucaracha commented Aug 10, 2020

EppO commented Sep 2, 2020

floryut commented Sep 21, 2020

floryut commented Sep 22, 2020

k8s-ci-robot commented Sep 22, 2020

Enable multi-CPU architecture in Flannel CNI #6166

Enable multi-CPU architecture in Flannel CNI #6166

Conversation

electrocucaracha commented May 19, 2020

k8s-ci-robot commented May 19, 2020

EppO commented May 20, 2020 • edited Loading

electrocucaracha commented May 21, 2020

k8s-ci-robot commented May 21, 2020

MrFreezeex May 25, 2020

Choose a reason for hiding this comment

electrocucaracha May 25, 2020

Choose a reason for hiding this comment

MrFreezeex May 25, 2020 • edited Loading

Choose a reason for hiding this comment

EppO commented May 28, 2020

MrFreezeex commented May 28, 2020

EppO commented May 28, 2020 • edited Loading

EppO commented May 28, 2020

Miouge1 commented Jul 13, 2020

electrocucaracha commented Jul 13, 2020

EppO commented Jul 16, 2020

SpaaarK commented Jul 31, 2020 • edited Loading

SpaaarK left a comment • edited Loading

Choose a reason for hiding this comment

electrocucaracha commented Aug 10, 2020

EppO commented Sep 2, 2020

floryut commented Sep 21, 2020

floryut commented Sep 22, 2020

k8s-ci-robot commented Sep 22, 2020

EppO commented May 20, 2020 •

edited

Loading

MrFreezeex May 25, 2020 •

edited

Loading

EppO commented May 28, 2020 •

edited

Loading

SpaaarK commented Jul 31, 2020 •

edited

Loading

SpaaarK left a comment •

edited

Loading