Fix cilium strict kube proxy replacement in HA #6473

MrFreezeex · 2020-07-30T11:52:37Z

Signed-off-by: Arthur Outhenin-Chalandre arthur@cri.epita.fr

What type of PR is this?

/kind bug

What this PR does / why we need it:

Fix cilium kube-proxy less install with an HA setup.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

k8s-ci-robot · 2020-07-30T11:52:45Z

Hi @MrFreezeex. 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.

MrFreezeex · 2020-07-30T13:50:28Z

This should do the job @mmack.

I replaced the test to execute in HA mode and fixed the bug that @mmack pointed out in #6334.
You may re-execute the job, but here is the modified testcase that passed https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/663726571

roles/kubespray-defaults/defaults/main.yaml

roles/network_plugin/cilium/templates/cilium-deploy.yml.j2

roles/kubespray-defaults/defaults/main.yaml

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

MrFreezeex · 2020-08-01T01:23:49Z

I didn't know about the urlsplit filter thanks @EppO !

I refactored a bit the PR to use only one new variable hopefully It's better like that. I have to use a new variable (or another trick) because If no loadbalancer is configured the kube_apiserver_endpoint will point to localhost which is not a valid endpoint for workers. I don't think there is another way to set the KUBERNETES_SERVICE_HOST environment to each node individually. If you have a better suggestion let me know!

The succeded cilium job is here https://gitlab.com/kargo-ci/kubernetes-sigs-kubespray/-/jobs/666510925 (same code, I just removed the commit to trigger It first).

EppO · 2020-08-01T01:48:12Z

Important: Replace API_SERVER_IP and API_SERVER_PORT below with the concrete control-plane node IP address and the kube-apiserver port number reported by kubeadm init (usually, it is port 6443).

Specifying this is necessary as kubeadm init is run explicitly without setting up kube-proxy and as a consequence while it exports KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT with a ClusterIP of the kube-apiserver service to the environment, there is no kube-proxy in our setup provisioning that service. The Cilium agent therefore needs to be made aware of this information through below configuration.

ok now I get your point. But that means all cilium agents (deployed on every node, right?) will point to the first master if no external LB is defined. We really need a cilium doc to list this kind of limitations. Could you start a docs/cilium.md in your PR with just that "warning". We'll enhance the doc outside of that PR. Just don't want to forget about it

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

MrFreezeex · 2020-08-01T11:56:43Z

I added docs about this limitation.

EppO · 2020-08-03T18:58:18Z

/ok-to-test
/lgtm

EppO

just some makeup changes and it'll be all good!
Thanks for your PR

docs/cilium.md

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

EppO · 2020-08-04T14:06:49Z

/lgtm

EppO · 2020-08-05T21:50:48Z

/assign @Miouge1

Miouge1 · 2020-08-06T07:11:29Z

/approve

k8s-ci-robot · 2020-08-06T07:11:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miouge1, MrFreezeex

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

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: (30 commits) Minor Ambassador docs updates (kubernetes-sigs#6503) Fix cilium strict kube proxy replacement in HA (kubernetes-sigs#6473) Upgrade JetStack Cert-Manager to v0.15.2 (kubernetes-sigs#6414) Fix E306 in tests/ (kubernetes-sigs#6495) Fix E306 in roles/kubernetes (kubernetes-sigs#6500) Allows tls verify skip on webhook auth url (kubernetes-sigs#6472) Fix E306 in scripts/ (kubernetes-sigs#6496) Correct sample inventory to pass yamllint (kubernetes-sigs#6499) Option for MetalLB to talk BGP (kubernetes-sigs#6383) bootstrap-os for remove-node (kubernetes-sigs#6154) Quoted type constraints are deprecated (kubernetes-sigs#6497) Update base image to v2.13.3 (kubernetes-sigs#6494) Fix Flexvolume mount in Openstack Controller (kubernetes-sigs#6480) Remove hvac dependency (kubernetes-sigs#6476) Create a PodDisruptionBudget for the Cinder CSI controllerplugin (kubernetes-sigs#6385) Upgrade molecule to v3 (kubernetes-sigs#6468) Remove workaround for kubeadm upgrade (kubernetes-sigs#6478) Update kube-router to 1.0.1 and kube-ovn to 1.3.0 (kubernetes-sigs#6479) fix src for audit webhook config yaml (kubernetes-sigs#6470) crio: align template crio.conf with upstream (kubernetes-sigs#6432) ...

* Update the cilium svc proxy test to HA mode Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr> * Fix cilium strict kube-proxy in HA Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr> * Add a single global endpoint variable Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr> * Add cilium docs about kube-proxy replacement Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr> * Fix issues in docs Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 30, 2020

k8s-ci-robot requested review from bozzo and floryut July 30, 2020 11:52

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 30, 2020

MrFreezeex changed the title ~~tests: check if HA strict is working~~ tests: check if HA cilium kube proxy strict mode is working Jul 30, 2020

MrFreezeex force-pushed the test-cilium-ha branch from bb3b520 to a097175 Compare July 30, 2020 11:54

MrFreezeex changed the title ~~tests: check if HA cilium kube proxy strict mode is working~~ WIP: check if HA cilium kube proxy strict mode is working Jul 30, 2020

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2020

MrFreezeex mentioned this pull request Jul 30, 2020

Adding kube-proxy-replacement support in cilium #6334

Merged

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 30, 2020

MrFreezeex force-pushed the test-cilium-ha branch 2 times, most recently from ba1cdf2 to 395393a Compare July 30, 2020 13:46

MrFreezeex changed the title ~~WIP: check if HA cilium kube proxy strict mode is working~~ Fix cilium strict kube proxy replacement in HA Jul 30, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2020

MrFreezeex commented Jul 30, 2020

View reviewed changes

roles/kubespray-defaults/defaults/main.yaml Outdated Show resolved Hide resolved

EppO suggested changes Jul 31, 2020

View reviewed changes

MrFreezeex added 3 commits August 1, 2020 02:43

Update the cilium svc proxy test to HA mode

2d36685

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

Fix cilium strict kube-proxy in HA

657e123

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

Add a single global endpoint variable

212baf4

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

MrFreezeex force-pushed the test-cilium-ha branch from 395393a to 212baf4 Compare August 1, 2020 00:52

k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Aug 1, 2020

MrFreezeex force-pushed the test-cilium-ha branch from 38e73af to 212baf4 Compare August 1, 2020 01:23

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 1, 2020

Add cilium docs about kube-proxy replacement

24feea3

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

MrFreezeex force-pushed the test-cilium-ha branch from 99984f8 to 24feea3 Compare August 1, 2020 11:55

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 3, 2020

k8s-ci-robot assigned EppO Aug 3, 2020

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

EppO suggested changes Aug 3, 2020

View reviewed changes

docs/cilium.md Outdated Show resolved Hide resolved

Fix issues in docs

4702da8

Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2020

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

k8s-ci-robot assigned Miouge1 Aug 5, 2020

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

k8s-ci-robot merged commit 35682b5 into kubernetes-sigs:master Aug 6, 2020

MrFreezeex deleted the test-cilium-ha branch August 6, 2020 10:50

stress-t mentioned this pull request Jan 29, 2021

Improving PR 6473 (cilium strict kube proxy replacement in HA) #7231

Closed

stress-t mentioned this pull request Feb 6, 2021

Improving PR 6473 #7259

Merged

MrFreezeex mentioned this pull request Mar 14, 2023

REQUEST: New membership for MrFreezeex kubernetes/org#4089

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix cilium strict kube proxy replacement in HA #6473

Fix cilium strict kube proxy replacement in HA #6473

MrFreezeex commented Jul 30, 2020 •

edited

Loading

k8s-ci-robot commented Jul 30, 2020

MrFreezeex commented Jul 30, 2020 •

edited

Loading

MrFreezeex commented Aug 1, 2020 •

edited

Loading

EppO commented Aug 1, 2020 •

edited

Loading

MrFreezeex commented Aug 1, 2020

EppO commented Aug 3, 2020

EppO left a comment •

edited

Loading

EppO commented Aug 4, 2020

EppO commented Aug 5, 2020

Miouge1 commented Aug 6, 2020

k8s-ci-robot commented Aug 6, 2020

Fix cilium strict kube proxy replacement in HA #6473

Fix cilium strict kube proxy replacement in HA #6473

Conversation

MrFreezeex commented Jul 30, 2020 • edited Loading

k8s-ci-robot commented Jul 30, 2020

MrFreezeex commented Jul 30, 2020 • edited Loading

MrFreezeex commented Aug 1, 2020 • edited Loading

EppO commented Aug 1, 2020 • edited Loading

MrFreezeex commented Aug 1, 2020

EppO commented Aug 3, 2020

EppO left a comment • edited Loading

Choose a reason for hiding this comment

EppO commented Aug 4, 2020

EppO commented Aug 5, 2020

Miouge1 commented Aug 6, 2020

k8s-ci-robot commented Aug 6, 2020

MrFreezeex commented Jul 30, 2020 •

edited

Loading

MrFreezeex commented Jul 30, 2020 •

edited

Loading

MrFreezeex commented Aug 1, 2020 •

edited

Loading

EppO commented Aug 1, 2020 •

edited

Loading

EppO left a comment •

edited

Loading