[jjo] add kube-router support #3339

jjo · 2018-09-18T14:29:38Z

add kube-router as another network_plugin choice
support most used kube-router flags via
kube_router_foo vars as other plugins
implement replacing kube-proxy (--run-service-proxy=true) via
kube_proxy_remove (boolean) fact
working around kubeadm lack of support for skipping kube-proxy
daemonset creation (Make kube-proxy optional (default to install) kubernetes/kubeadm#776) by removing it
after kubeadm run, added a caveat note to doc/kube-router.md
verified working both on kubeadm_enabled: true|false

jjo · 2018-09-18T14:31:30Z

k8s-ci-robot · 2018-09-18T14:31:37Z

@jjo: GitHub didn't allow me to request PR reviews from the following users: murali-reddy, andrewsykim.

Note that only kubernetes-incubator members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @murali-reddy @andrewsykim

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.

jjo · 2018-09-18T14:35:58Z

FYI I have it working over an openstack deploy, with

kube_network_plugin: kube-router
# [... default settings ...]
kube_proxy_mode: none
kube_router_run_service_proxy: true

Still has some rough edges:

requiring kube_proxy_mode: none for --run-service-proxy
(should be set_fact really)
CI test files done mostly copypasta-ed from other plugins,
need some knowledgeable review

woopstar

The readme file must also be updated with the network plugin and version.

roles/download/defaults/main.yml

roles/kubernetes-apps/network_plugin/kube-router/tasks/main.yml

roles/kubernetes/master/templates/kubeadm-config.v1alpha2.yaml.j2

woopstar · 2018-09-18T18:35:04Z

roles/network_plugin/kube-router/defaults/main.yml

+
+# Enables Service Proxy -- sets up IPVS for Kubernetes Services
+# TODO(jjo): this setting currently requires also:
+# - no kubeadm_enabled


If this is yet not supported, do a check in the pre-install stage to make sure it does not happen?

Thanks for the suggestion:

added assertion on not kubeadm_enabled

added set_fact to override kube_proxy_mode: none if kube-router is set to replace it

roles/network_plugin/kube-router/templates/kube-router.yml.j2

woopstar · 2018-09-18T18:37:32Z

roles/network_plugin/kube-router/templates/kube-router.yml.j2

+          readOnly: true
+      initContainers:
+      - name: install-cni
+        image: busybox


busybox image is in the downloads file too. Please update to ansible var

Done, although I added a busybox_image_repo (_tag)
as I don't feel like using test_image_repo (and _tag) for
critical kube-router pods functionality.

jjo · 2018-09-19T19:07:57Z

Thanks @woopstar for the thorough review, addressed your comments, PTAL
(fyi had to rebase for some conflicts).

woopstar · 2018-09-20T08:17:49Z

roles/kubernetes/preinstall/tasks/0020-verify-settings.yml

@@ -44,6 +44,12 @@
    - { name: openstack_lbaas_enabled, value: "{{ openstack_lbaas_enabled }}" }
  ignore_errors: "{{ ignore_assert_errors }}"

+- name: Stop if kube-router chosen as network_plugin and kubeadm_enabled (not yet supported/verified)


This is actually a problem. We are about to switch to use kubeadm as default deployment as you can see in #3301

Cool, thanks for the note -- I've reworked logic, and verified ok
also with kubeadm_enabled installs (noting a caveat re: kubeadm
lack of support for skipping kube-proxy deploy).

jjo · 2018-09-20T23:43:46Z

Thanks again @woopstar for the re^2view, reworked it for kubeadm_enabled
support, PTAL.

Atoms · 2018-09-21T06:22:13Z

ci check this

roles/download/defaults/main.yml

jjo · 2018-09-22T16:21:05Z

Addressed latest @woopstar comments, tnx again.

ant31 · 2018-10-04T17:22:45Z

why are the job allow to failed ?

methodx · 2018-10-08T13:08:55Z

Hi @jjo
Please, check my PR.

Should be handy for kubernetes version >=v1.12.0

jjo · 2018-10-09T01:26:40Z

Hi @jjo
Please, check my PR.

Should be handy for kubernetes version >=v1.12.0

Thanks @methodx!, adapted manifest tolerations to match other CNIs,
see 8753a01, leaving feedback
at your PR re: other changes.

woopstar · 2018-10-15T12:16:44Z

CI tests all good.

Can you squash commits.

And also add PriorityClass to the deployment - see #3361

Think we can merge after that, then.

ant31 · 2018-10-15T18:49:46Z

@woopstar we have autosquash now

jjo · 2018-10-15T19:04:22Z

@woopstar: added PriorityClass to kube-router DaemonSet,
lemme know if you want me to squash commits, iiuc from @ant31
wouldn't be needed (but obviously fine to do so nevertheless).

Fixes cloudnativelabs/kube-router#147. * add kube-router as another network_plugin choice * support most used kube-router flags via `kube_router_foo` vars as other plugins * implement replacing kube-proxy (--run-service-proxy=true) via `kube_proxy_mode: none`, verified in a _non kubeadm_enabled_ install, should also work for recent kubeadm releases via `skipKubeProxyInstall: true` config

- verify it working ok w/the kubeadm_enabled and kube_router_run_service_proxy true or false - introduce `kube_proxy_remove` fact, to decouple logic from kube_proxy_mode (which affects kubeadm configmap settings, thus no-good to ab-use it to 'none')

…e_proxy

…s on my and existing changes

woopstar · 2018-10-16T14:13:53Z

/lgtm
/approve

k8s-ci-robot · 2018-10-16T14:13:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: woopstar

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

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

k8s-ci-robot requested review from ArchiFleKs and jjungnickel September 18, 2018 14:29

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 Sep 18, 2018

jjo mentioned this pull request Sep 18, 2018

integrate with kubespray cloudnativelabs/kube-router#147

Closed

woopstar suggested changes Sep 18, 2018

View reviewed changes

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 19, 2018

jjo force-pushed the jjo-add-kube-router-support branch from c542723 to 1fad254 Compare September 19, 2018 19:07

jjo force-pushed the jjo-add-kube-router-support branch from 1fad254 to d784960 Compare September 19, 2018 19:09

woopstar reviewed Sep 20, 2018

View reviewed changes

woopstar added this to the 2.7 milestone Sep 20, 2018

jjo force-pushed the jjo-add-kube-router-support branch 2 times, most recently from eb93517 to bd4442f Compare September 20, 2018 23:42

jjo force-pushed the jjo-add-kube-router-support branch from 4111856 to 0ce4454 Compare September 20, 2018 23:53

woopstar reviewed Sep 21, 2018

View reviewed changes

roles/download/defaults/main.yml Outdated Show resolved Hide resolved

woopstar reviewed Sep 21, 2018

View reviewed changes

roles/download/defaults/main.yml Outdated Show resolved Hide resolved

jjo force-pushed the jjo-add-kube-router-support branch 2 times, most recently from 70d9193 to e6c3291 Compare September 22, 2018 16:19

woopstar modified the milestones: 2.7, v2.8 Sep 26, 2018

jjo force-pushed the jjo-add-kube-router-support branch 2 times, most recently from c4ee8ef to fba686a Compare September 29, 2018 22:35

jjo force-pushed the jjo-add-kube-router-support branch from 241bbbb to 8753a01 Compare October 9, 2018 01:24

jjo force-pushed the jjo-add-kube-router-support branch from 8753a01 to 5395c6f Compare October 11, 2018 02:24

woopstar approved these changes Oct 15, 2018

View reviewed changes

jjo added 12 commits October 15, 2018 16:13

[jjo] address PR#3339 review from @woopstar

db23b08

add busybox image used by kube-router to downloads

5b866d5

fix busybox download groups key

3ce4872

improve kube-router.md re: kubeadm_enabled and kube_router_run_servic…

712edbe

…e_proxy

address @woopstar latest review

5eca82c

add inventory/sample/group_vars/k8s-cluster/k8s-net-kube-router.yml

cf9c659

fix kube_router_run_service_proxy conditional for kube-proxy removal

4d198c0

fix kube_proxy_remove fact (w/ |bool), add some needed kube-proxy tag…

7a2eecf

…s on my and existing changes

update kube-router tolerations for 1.12 compatibility

74e0115

add PriorityClass to kube-router DaemonSet

0a083f6

jjo force-pushed the jjo-add-kube-router-support branch from 63c72ad to 0a083f6 Compare October 15, 2018 19:15

woopstar removed request for jjungnickel and ArchiFleKs October 16, 2018 12:30

k8s-ci-robot assigned woopstar Oct 16, 2018

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

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

k8s-ci-robot merged commit a5edd0d into kubernetes-sigs:master Oct 16, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[jjo] add kube-router support #3339

[jjo] add kube-router support #3339

jjo commented Sep 18, 2018 •

edited

Loading

jjo commented Sep 18, 2018

k8s-ci-robot commented Sep 18, 2018

jjo commented Sep 18, 2018

woopstar left a comment

woopstar Sep 18, 2018

jjo Sep 19, 2018

woopstar Sep 18, 2018

jjo Sep 19, 2018

jjo commented Sep 19, 2018 •

edited

Loading

woopstar Sep 20, 2018

jjo Sep 20, 2018

jjo commented Sep 20, 2018

Atoms commented Sep 21, 2018

jjo commented Sep 22, 2018

ant31 commented Oct 4, 2018

methodx commented Oct 8, 2018

jjo commented Oct 9, 2018

woopstar commented Oct 15, 2018

ant31 commented Oct 15, 2018

jjo commented Oct 15, 2018

woopstar commented Oct 16, 2018

k8s-ci-robot commented Oct 16, 2018

[jjo] add kube-router support #3339

[jjo] add kube-router support #3339

Conversation

jjo commented Sep 18, 2018 • edited Loading

jjo commented Sep 18, 2018

k8s-ci-robot commented Sep 18, 2018

jjo commented Sep 18, 2018

woopstar left a comment

Choose a reason for hiding this comment

woopstar Sep 18, 2018

Choose a reason for hiding this comment

jjo Sep 19, 2018

Choose a reason for hiding this comment

woopstar Sep 18, 2018

Choose a reason for hiding this comment

jjo Sep 19, 2018

Choose a reason for hiding this comment

jjo commented Sep 19, 2018 • edited Loading

woopstar Sep 20, 2018

Choose a reason for hiding this comment

jjo Sep 20, 2018

Choose a reason for hiding this comment

jjo commented Sep 20, 2018

Atoms commented Sep 21, 2018

jjo commented Sep 22, 2018

ant31 commented Oct 4, 2018

methodx commented Oct 8, 2018

jjo commented Oct 9, 2018

woopstar commented Oct 15, 2018

ant31 commented Oct 15, 2018

jjo commented Oct 15, 2018

woopstar commented Oct 16, 2018

k8s-ci-robot commented Oct 16, 2018

jjo commented Sep 18, 2018 •

edited

Loading

jjo commented Sep 19, 2018 •

edited

Loading