Add Cilium as CNI plugin #4224

nebril · 2018-01-08T15:46:16Z

This change adds Cilium as an option to provide networking for kops-deployed cluster.

For Cilium to run you need an image with kernel >=4.8, I am testing on CoreOS image: 595879546273/CoreOS-stable-1576.4.0-hvm

nebril · 2018-01-18T13:39:45Z

You can spin up cluster with cilium networking as follows:

kops create cluster \
--zones <zones, e.g. "eu-central-1a"> \
--image 595879546273/CoreOS-stable-1576.4.0-hvm \
--networking cilium \
--override "cluster.spec.etcdClusters[*].version=3.1.11" \
--kubernetes-version 1.8.7

image uses CoreOS to make sure kernel is new enough for Cilium to work properly
Cilium requires etcd>= 3.1.0 to work properly.
I tested it only with k8s 1.8 for now, seems to be working properly.

nebril · 2018-01-18T13:46:35Z

/retest

chrislovecnm · 2018-01-19T18:18:58Z

/retest

chrislovecnm · 2018-01-20T00:26:56Z

So how can we ensure that the user is running the correct kernel, which btw is not tested with e2e testing.

nebril · 2018-01-22T13:47:19Z

@chrislovecnm you mean at the point of creating the cluster? I am trying to figure out if filtering output of aws ec2 describe-images with kernel-id filter can help here.

Seems like images with aki-184c7a05 KernelID (which is what CoreOS image uses) should work. I don't know how to make a connection between kernel version and KernelID which would work for future KernelIDs, but there are only 5 unique KernelIDs in output of aws ec2 describe-images (which I guess consists all public images available for use).

I thought that aws ec2 describe-image-attribute might help, but it seems like most public images deny the right to describe image attributes publicly.

Would stating in the docs explicitly that you have to run proper kernel in order for Cilium to work correctly be enough?

nebril · 2018-01-26T10:56:54Z

@chrislovecnm ping. Do you have any other feedback about this PR?

chrislovecnm · 2018-01-26T18:36:54Z

@nebril overall the PR looks good, I just completed a review, not sure how to handle the kernel stuff. I apologize how long the review process has been.

@justinsb thoughts about how to handle the kernel dependency? I am wondering if we should output a message or just update the docs? For Cilium to run you need an image with kernel >=4.8, and I do not think we have any way to check for that. Also, I know that kernel version is significantly ahead of the e2e testing. What advice do you have?

chrislovecnm

A bunch of the dates in files will drop out if you rebase, I think. Otherwise, if there is no code changes, please back out the date changes.

Thanks for the new plugin, I think we have 9 now! As I mention on all new CNI plugins, please strive to keep the plugin updated, as we rely on the community for those updates.

Thanks again!

chrislovecnm · 2018-01-26T18:37:32Z

pkg/apis/kops/v1alpha1/zz_generated.conversion.go

@@ -1,7 +1,7 @@
 // +build !ignore_autogenerated

 /*
-Copyright 2017 The Kubernetes Authors.
+Copyright 2018 The Kubernetes Authors.


Can you rebase, and then probably want to re-run make apimachinery. I think these changes will be removed.

chrislovecnm · 2018-01-26T18:39:35Z

upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.8.yaml.template

+  name: cilium-config
+  namespace: kube-system
+data:
+  # This etcd-config contains the etcd endpoints of your cluster. If you use


This seems to be using http etcd. There is an update for Calico to use TLS that is going in. You can either change the PR to output a message that you are using unencrypted etcd communication, and the etcd port is open, or figure out TLS.

chrislovecnm · 2018-01-26T18:41:05Z

upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.8.yaml.template

@@ -0,0 +1,285 @@
+kind: ConfigMap


Should we warn the user when they try to use cilium with k8s 1.7 for instance? It will not work.

chrislovecnm · 2018-01-26T18:41:36Z

upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.8.yaml.template

+        prometheus.io/port: "9090"
+    spec:
+      serviceAccountName: cilium
+      containers:


I would encourage adding the resource limits that say the weave manifest has. Or other applicable resource limits.

chrislovecnm · 2018-01-26T18:42:09Z

upup/models/cloudup/resources/addons/networking.cilium.io/k8s-1.8.yaml.template

+    spec:
+      serviceAccountName: cilium
+      containers:
+      - image: cilium/cilium:stable


Is there a version we can use? Like a numbered version? We have had problems with the latest or other non-number versioned tags, where the manifest needs to be updated to the latest version of the CNI provider.

chrislovecnm · 2018-01-26T18:47:17Z

Cilium requires etcd>= 3.1.0 to work properly.

We need to validate that as well ...

nebril · 2018-01-29T12:55:49Z

@chrislovecnm Thanks for great review, I will try to address your comments and will let you know when it's done.

k8s-github-robot · 2018-02-04T08:04:24Z

@nebril PR needs rebase

nebril · 2018-03-20T11:26:23Z

@chrislovecnm can you re-review this?

I would encourage adding the resource limits that say the weave manifest has. Or other applicable resource limits.

The issue I have with this comment is that Cilium is a CNI plugin which is basically the Kubernetes component, so limiting its resources seems like limiting for example kubelet or other k8s components. Are you sure this is required?

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

justinsb · 2018-03-26T14:09:37Z

/lgtm

Thanks @nebril

k8s-ci-robot · 2018-03-26T14:09:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb, nebril

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

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

justinsb · 2018-03-26T14:18:55Z

I think this is fine to merge as an option.

Just FYI @nebril I think using etcd directly is basically frowned upon - it's better to talk to the kubernetes API. Flannel and calico both started by talking directly to etcd and have moved / are moving to using the API instead.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 8, 2018

nebril force-pushed the cilium-support branch from f267be5 to 4de81e6 Compare January 8, 2018 16:26

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 8, 2018

nebril force-pushed the cilium-support branch from 4de81e6 to 64ce483 Compare January 9, 2018 10:22

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 9, 2018

nebril force-pushed the cilium-support branch from 64ce483 to e6ade79 Compare January 9, 2018 11:04

nebril force-pushed the cilium-support branch from e6ade79 to 0bfa5bc Compare January 18, 2018 13:29

nebril changed the title ~~Add Cilium as CNI plugin [WIP]~~ Add Cilium as CNI plugin Jan 18, 2018

nebril mentioned this pull request Jan 18, 2018

kops integration & guide cilium/cilium#2372

Closed

chrislovecnm reviewed Jan 26, 2018

View reviewed changes

nebril force-pushed the cilium-support branch from 0bfa5bc to ea5a8e8 Compare January 29, 2018 12:54

nebril force-pushed the cilium-support branch from ea5a8e8 to 59b698d Compare January 30, 2018 14:53

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2018

justinsb added this to the 1.9 milestone Feb 21, 2018

nebril force-pushed the cilium-support branch from 59b698d to e9fd36d Compare March 15, 2018 15:36

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2018

nebril added 5 commits March 20, 2018 13:07

Add Cilium as CNI plugin

bca52de

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

make apimachinery generated changes

4575fc2

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

Openapi generated changes for Cilium networking

52e0a42

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

Validate etcd and k8s version for Cilium

ed67c01

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

Update Cilium template to working state

84b522d

Signed-off-by: Maciej Kwiek <maciej@covalent.io>

nebril force-pushed the cilium-support branch from 3b67a0e to 84b522d Compare March 20, 2018 12:23

k8s-ci-robot assigned justinsb Mar 26, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 26, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 26, 2018

k8s-ci-robot merged commit fc1bed4 into kubernetes:master Mar 26, 2018

nebril mentioned this pull request Apr 30, 2018

Add support for Cilium CNI plugin #4087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Cilium as CNI plugin #4224

Add Cilium as CNI plugin #4224

nebril commented Jan 8, 2018 •

edited

Loading

nebril commented Jan 18, 2018 •

edited

Loading

nebril commented Jan 18, 2018

chrislovecnm commented Jan 19, 2018

chrislovecnm commented Jan 20, 2018

nebril commented Jan 22, 2018 •

edited

Loading

nebril commented Jan 26, 2018

chrislovecnm commented Jan 26, 2018 •

edited

Loading

chrislovecnm left a comment

chrislovecnm Jan 26, 2018

nebril Jan 29, 2018

chrislovecnm Jan 26, 2018

chrislovecnm Jan 26, 2018

chrislovecnm Jan 26, 2018

chrislovecnm Jan 26, 2018

chrislovecnm commented Jan 26, 2018

nebril commented Jan 29, 2018

k8s-github-robot commented Feb 4, 2018

nebril commented Mar 20, 2018

justinsb commented Mar 26, 2018

k8s-ci-robot commented Mar 26, 2018

justinsb commented Mar 26, 2018

Add Cilium as CNI plugin #4224

Add Cilium as CNI plugin #4224

Conversation

nebril commented Jan 8, 2018 • edited Loading

nebril commented Jan 18, 2018 • edited Loading

nebril commented Jan 18, 2018

chrislovecnm commented Jan 19, 2018

chrislovecnm commented Jan 20, 2018

nebril commented Jan 22, 2018 • edited Loading

nebril commented Jan 26, 2018

chrislovecnm commented Jan 26, 2018 • edited Loading

chrislovecnm left a comment

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

nebril Jan 29, 2018

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

chrislovecnm Jan 26, 2018

Choose a reason for hiding this comment

chrislovecnm commented Jan 26, 2018

nebril commented Jan 29, 2018

k8s-github-robot commented Feb 4, 2018

nebril commented Mar 20, 2018

justinsb commented Mar 26, 2018

k8s-ci-robot commented Mar 26, 2018

justinsb commented Mar 26, 2018

nebril commented Jan 8, 2018 •

edited

Loading

nebril commented Jan 18, 2018 •

edited

Loading

nebril commented Jan 22, 2018 •

edited

Loading

chrislovecnm commented Jan 26, 2018 •

edited

Loading