enable cilium CNI option #72

carpenike · 2020-10-05T15:36:12Z

Description

This is opinionated a bit, feel free to modify as you see fit or ignore altogether. :)

This enables the use of CNI within cluster along with an option to disable kube-proxy. Kube-proxy is enabled by default and CIlium is not the default CNI.

(Opinionated part): This also installs kube-router in the event that Cilium is leveraged and requires BGP with external service.

Type Of Change

To help us figure out who should review this PR, please put an X in all the areas that this PR affects.

Issue Ref (Optional)

Notes

Partially addresses #68

Signed-off-by: Ryan Holt <[email protected]>

carpenike · 2020-10-05T15:47:22Z

ansible/roles/cni/defaults/main.yml

+bgp_peer_asn: 64512
+cilium_helm_version: 1.8.3
+cilium_image_version: v1.8.3
+k8s_service_host: "{{ hostvars[groups['masters'][0]]['ansible_host'] }}"


@RobReus -- This really should be VIP IP if it's presented, I'm at the edge of my Ansible skills.

when using keepalived this is associated with keepalived_vip, but i don't know how running cilium affects running keepalived?

theoretically it should be outside of Cilium as it's a host level config, right?

However, this would need to be an either or I think; if VIP present use that, otherwise use the master IP?

yeah i think you're right. it should be an either/or situation. i'm not sure if something could be achieved with jinja logic, but my assumption is that it would be possible by using some jinja operators on the template, in addition to some sanity checking for each var.

Signed-off-by: Ryan Holt <[email protected]>

carpenike · 2020-10-05T17:04:46Z

@xunholy not sure why these are failing on files I didn't touch.

xunholy

We can ignore the errors that didn't originate from your changes but if we can address the ones we have that would be good!!

xunholy · 2020-10-06T01:36:02Z

ansible/roles/cluster/defaults/main.yml

@@ -13,3 +13,4 @@ cluster_pod_subnet: ""
 # Default etcd values, change these if you experience "leader changed" issues when running on a SD card
 cluster_etcd_heartbeat_interval: 100
 cluster_etcd_election_timeout: 1000
+kube_proxy: enabled


would be good to have a comment in here for users to roughly know why you'd have this disabled.

i'm unfamiliar with the requirements for cilium, so i am under the assumption that this is a requirement for it. if that is the case, then this var should be scoped as such. explicitly setting this var for later use could be misleading to some users. having this var set based on what CNI you're hoping to deploy would be the ideal situation, likely from a conditional import vars based on CNI selection from the user.

It's not a requirement, but it would modify the installation options for cilium if you're not using kube-proxy.

I would still prefer that the variable be scoped, or at the very least some comments be added to detail what you're enabling/disabling.

ansible/roles/cluster/tasks/initialize.yml

xunholy · 2020-10-06T01:40:12Z

ansible/roles/cni/tasks/cilium.yml

+    --set global.hubble.enabled="true" \
+    --set global.hubble.ui.enabled="true" \
+    --set global.hubble.relay.enabled="true" \


I wonder if there is an easier way to store these values to allow them to be easier to configure for users, for example, a user using arm might not be able to enable hubble right now. Would it make sense if we stored these in a values.yaml under files/ and we copy that and just --values=values.yaml on the host, this is just a thought?

yeah we could template out a cilium-values.yaml file.

Yep, if we can template out a file that users can use to edit helm values that would be perfect!

crutonjohn

this is looking great. glad to be able to support more use cases and implementations! thanks @carpenike

ansible/roles/cni/tasks/cilium.yml

crutonjohn · 2020-10-11T07:29:58Z

ansible/roles/cluster/defaults/main.yml

@@ -13,3 +13,4 @@ cluster_pod_subnet: ""
 # Default etcd values, change these if you experience "leader changed" issues when running on a SD card
 cluster_etcd_heartbeat_interval: 100
 cluster_etcd_election_timeout: 1000
+kube_proxy: enabled


i'm unfamiliar with the requirements for cilium, so i am under the assumption that this is a requirement for it. if that is the case, then this var should be scoped as such. explicitly setting this var for later use could be misleading to some users. having this var set based on what CNI you're hoping to deploy would be the ideal situation, likely from a conditional import vars based on CNI selection from the user.

Signed-off-by: Ryan Holt <[email protected]>

…b.com/carpenike/k8s-cluster-installation into add-cilium-cni-with-kube-proxy-option

Signed-off-by: Ryan Holt <[email protected]>

Signed-off-by: Michael Fornaro <[email protected]>

crutonjohn

Looks mostly good. Just need to resolve the one issue with the k8s_service_host var for cilium.

Signed-off-by: Michael Fornaro <[email protected]>

anthr76 · 2020-12-18T12:25:11Z

We should probably use the generic package module to see if helm is already present.

Considering for kubeadm manifest generation we use cluster_controlplane_endpoint shouldn't we use that over the IP of the first master?

k8s-cluster-installation/ansible/roles/cluster/templates/kubeadm-init.yml.j2

Line 19 in 80c7e12

controlPlaneEndpoint: {{ cluster_control_plane_endpoint }}

For:

k8s_service_host

https://github.com/carpenike/k8s-cluster-installation/blob/c3c2e1af8004bb91630b687dc0fae76c54f36e89/ansible/roles/cni/templates/values.yaml.j2#L101

Otherwise this looks pretty good. Going to do some testing later

My modifcations to pending PR are yet to be pushed (raspbernetes/k8s-cluster-installation#72). * Ansible (like always) needs to be cleaned up * Cilium + Kuberouter * Etcd/Controlplane is now kubeadm managed * Add new certs/creds Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

Opt to use newer offical image rather then 3rd party source. Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

Also update inventory source and delegate to localhost.. also for now. Change galaxy collection to new name. https://github.com/ansible-collections/community.kubernetes/issues/221 Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

This is required to leverage Kubernetes modules from ansible. Hopefully this changes in the future. According to git issues a .deb is planned. A RPM is available. There is a package in the AUR though opted to install with PIP instead because I would trust it better without testing. Signed-off-by: anthr76 <[email protected]>

…-kube-proxy-option Signed-off-by: anthr76 <[email protected]>

Set in a more elegant multi-os manner later. Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

https://docs.cilium.io/en/v1.6/kubernetes/requirements/#enable-automatic-node-cidr-allocation-recommended ansible/ansible#71824 Signed-off-by: anthr76 <[email protected]>

https://docs.cilium.io/en/v1.6/kubernetes/requirements/#enable-automatic-node-cidr-allocation-recommended Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

carpenike · 2021-01-25T21:45:42Z

instead of using helm on the node itself I'm opting for helm to be a requirement on the host itself. I would enjoy some input from others on this topic.

I think this would also require you to pull out the kubeconfig off of the master and put it on your local host. Part of doing it on the master node was to follow a pattern in place for other one-off kube API related changes / settings.

anthr76 · 2021-01-25T21:49:02Z

Yeah considering that the rest of the roles are configured to interact with the kubectl API on the main master that's what I ended up doing. It's now making the helm changes with the first master node.

Though you can just grab the output of the kubeconfig from cluster creation I think what you suggested is the best bet. It's really a matter of where you want to add the dependencies. I appreciate the input!

This was dropped due to the latest merge and made it into the working tree raspbernetes#84 Signed-off-by: anthr76 <[email protected]>

Disable kube-proxy by default for cilium. Revise var handling. Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

https://github.com/raspbernetes/k8s-cluster-installation/pull/92/files Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

* Revise requirements to correctly point to modules. * Disable hubble by default. Users can, and probably should do this on their [own](https://docs.cilium.io/en/v1.9/gettingstarted/k8s-install-kubeadm/#enable-hubble-for-cluster-wide-visibility) in the meantime we'll not support but leave the option there. * Add sysctl's for present systemd bug - cilium/cilium#10645 Signed-off-by: anthr76 <[email protected]>

Signed-off-by: anthr76 <[email protected]>

anthr76 · 2021-01-27T23:38:23Z

I think after review this PR can be considered ready to be merged

xunholy · 2021-01-30T02:01:58Z

ansible/group_vars/masters.yml

 # cluster_etcd_heartbeat_interval: 100
 # cluster_etcd_election_timeout: 1000
+# cluster_kube_proxy_enabled: true


Added this value twice

xunholy · 2021-01-30T02:03:25Z

ansible/roles/cni/meta/main.yml

@@ -0,0 +1,3 @@
+collections:


Can we prefix this file with ---

* Added var twice * `---` Signed-off-by: anthr76 <[email protected]>

carpenike added 2 commits October 5, 2020 11:33

enable cilium CNI option

c0e11b6

Signed-off-by: Ryan Holt <[email protected]>

fix some variables

c129195

Signed-off-by: Ryan Holt <[email protected]>

carpenike commented Oct 5, 2020

View reviewed changes

carpenike added 3 commits October 5, 2020 11:49

change variable name

f4c42be

Signed-off-by: Ryan Holt <[email protected]>

set permissions on manifests folder and ignore curl warning

22ba04b

Signed-off-by: Ryan Holt <[email protected]>

should have been inside args

64bd30b

Signed-off-by: Ryan Holt <[email protected]>

xunholy reviewed Oct 6, 2020

View reviewed changes

xunholy requested review from RobReus and crutonjohn October 6, 2020 01:41

crutonjohn reviewed Oct 11, 2020

View reviewed changes

carpenike and others added 7 commits November 16, 2020 21:45

add some creates pieces to commands

6e428e0

add some creates pieces to commands

c0002dc

Signed-off-by: Ryan Holt <[email protected]>

Merge branch 'add-cilium-cni-with-kube-proxy-option' of https://githu…

7d10fb0

…b.com/carpenike/k8s-cluster-installation into add-cilium-cni-with-kube-proxy-option

args needed to nest creates

a620425

Signed-off-by: Ryan Holt <[email protected]>

add another pipefail option

7ed428e

Updating variable role prefix and addressing some feedback

415ed8b

Signed-off-by: Michael Fornaro <[email protected]>

fix ansible lint issues

14dd29c

Signed-off-by: Michael Fornaro <[email protected]>

crutonjohn reviewed Nov 29, 2020

View reviewed changes

update helm values to be from a j2 template

645377e

Signed-off-by: Michael Fornaro <[email protected]>

xunholy force-pushed the add-cilium-cni-with-kube-proxy-option branch from e6ed4c0 to 645377e Compare November 29, 2020 21:35

xunholy requested review from RobReus and removed request for RobReus December 6, 2020 10:13

xunholy changed the base branch from master to main December 6, 2020 10:18

Merge branch 'main' into add-cilium-cni-with-kube-proxy-option

c3c2e1a

anthr76 added 3 commits December 19, 2020 15:46

Testing cilium on Kubic

d436a25

Signed-off-by: anthr76 <[email protected]>

Add multi-arch envoyproxy/envoy

9ffc4fd

Opt to use newer offical image rather then 3rd party source. Signed-off-by: anthr76 <[email protected]>

Add quotes for endpoint routes

781cffc

Signed-off-by: anthr76 <[email protected]>

anthr76 added 11 commits January 24, 2021 12:23

Drop cri_plugin for now

46f471b

Also update inventory source and delegate to localhost.. also for now. Change galaxy collection to new name. https://github.com/ansible-collections/community.kubernetes/issues/221 Signed-off-by: anthr76 <[email protected]>

Install helm as a kubernete dependency

ed97639

Signed-off-by: anthr76 <[email protected]>

Merge remote-tracking branch 'upstream/main' into add-cilium-cni-with…

b6508ee

…-kube-proxy-option Signed-off-by: anthr76 <[email protected]>

Remove hostname

4594501

Set in a more elegant multi-os manner later. Signed-off-by: anthr76 <[email protected]>

Add community.general.modprobe as a requirement

a05abcd

Signed-off-by: anthr76 <[email protected]>

Drop FQCN, and test kubelt j2 logic

5cfcf04

https://docs.cilium.io/en/v1.6/kubernetes/requirements/#enable-automatic-node-cidr-allocation-recommended ansible/ansible#71824 Signed-off-by: anthr76 <[email protected]>

Enable automatic node CIDR allocation

2bf94ae

https://docs.cilium.io/en/v1.6/kubernetes/requirements/#enable-automatic-node-cidr-allocation-recommended Signed-off-by: anthr76 <[email protected]>

allocate-node-cidrs is enabled by default

a97395a

Signed-off-by: anthr76 <[email protected]>

Disable envoy version check

4040826

Signed-off-by: anthr76 <[email protected]>

Template out some variable values and disable BPF by default

a8ebde2

Signed-off-by: anthr76 <[email protected]>

anthr76 force-pushed the add-cilium-cni-with-kube-proxy-option branch from 902e8fa to a8ebde2 Compare January 25, 2021 22:26

anthr76 removed their assignment Jan 25, 2021

anthr76 added 8 commits January 25, 2021 17:32

Remove old packages from Kubernetes role

904cbae

This was dropped due to the latest merge and made it into the working tree raspbernetes#84 Signed-off-by: anthr76 <[email protected]>

Kube-Proxy revisions

a439b8c

Disable kube-proxy by default for cilium. Revise var handling. Signed-off-by: anthr76 <[email protected]>

Revise eBPF mount

80f2054

Signed-off-by: anthr76 <[email protected]>

Change default variables

3f5f9ee

Signed-off-by: anthr76 <[email protected]>

fix docker restart handler

791621c

https://github.com/raspbernetes/k8s-cluster-installation/pull/92/files Signed-off-by: anthr76 <[email protected]>

Begin adding sysctl fix and enable hubble by default

51a5b1f

Signed-off-by: anthr76 <[email protected]>

Scope out cilium variables to examples.

c95d72e

Signed-off-by: anthr76 <[email protected]>

xunholy reviewed Jan 30, 2021

View reviewed changes

ansible/roles/cni/meta/main.yml

@@ -0,0 +1,3 @@

collections:

Copy link

Member

xunholy Jan 30, 2021

Choose a reason for hiding this comment

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

Can we prefix this file with ---

Small cleanups

fa4005f

* Added var twice * `---` Signed-off-by: anthr76 <[email protected]>

xunholy approved these changes Jan 30, 2021

View reviewed changes

xunholy merged commit d162e72 into raspbernetes:main Jan 30, 2021

anthr76 mentioned this pull request Mar 2, 2021

Replace pip installation with APT for python-openshift #114

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable cilium CNI option #72

enable cilium CNI option #72

carpenike commented Oct 5, 2020 •

edited by xunholy

Loading

carpenike Oct 5, 2020

crutonjohn Oct 11, 2020

carpenike Nov 17, 2020

crutonjohn Nov 28, 2020

carpenike commented Oct 5, 2020

xunholy left a comment

xunholy Oct 6, 2020

crutonjohn Oct 11, 2020

carpenike Nov 17, 2020

crutonjohn Nov 28, 2020

xunholy Oct 6, 2020

carpenike Nov 17, 2020

xunholy Nov 17, 2020

crutonjohn left a comment

crutonjohn Oct 11, 2020

crutonjohn left a comment

anthr76 commented Dec 18, 2020

carpenike commented Jan 25, 2021

anthr76 commented Jan 25, 2021 •

edited

Loading

anthr76 commented Jan 27, 2021

xunholy Jan 30, 2021

xunholy Jan 30, 2021

enable cilium CNI option #72

enable cilium CNI option #72

Conversation

carpenike commented Oct 5, 2020 • edited by xunholy Loading

Description

Type Of Change

Issue Ref (Optional)

Notes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpenike commented Oct 5, 2020

xunholy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crutonjohn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crutonjohn left a comment

Choose a reason for hiding this comment

anthr76 commented Dec 18, 2020

carpenike commented Jan 25, 2021

anthr76 commented Jan 25, 2021 • edited Loading

anthr76 commented Jan 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carpenike commented Oct 5, 2020 •

edited by xunholy

Loading

anthr76 commented Jan 25, 2021 •

edited

Loading