Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add secondary iface to KIND network #26338

Merged
merged 1 commit into from Jul 20, 2023
Merged

Conversation

ysksuzuki
Copy link
Member

@ysksuzuki ysksuzuki commented Jun 18, 2023

Add secondary iface to KIND network, so that we can test the NodePort via secondary iface.

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!

Part of #18846

@ysksuzuki ysksuzuki requested review from a team as code owners June 18, 2023 12:33
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 18, 2023
@ysksuzuki ysksuzuki marked this pull request as draft June 18, 2023 12:46
@ysksuzuki
Copy link
Member Author

conformance-e2e on this PR has passed
https://github.com/cilium/cilium/actions/runs/5303659307

@ysksuzuki
Copy link
Member Author

ysksuzuki commented Jun 18, 2023

Tested the secondary iface connectivity manually

// DSR + native routing + devices eth+(eth0, eth1)
cilium install --wait \
    --chart-directory=/home/yusuke/go/src/github.com/cilium/cilium/install/kubernetes/cilium \
    --helm-set=image.repository=quay.io/ysksuzuki/cilium-dev \
    --helm-set=image.useDigest=false \
    --helm-set=image.tag=main.0 \
    --helm-set=operator.image.repository=quay.io/ysksuzuki/operator \
    --helm-set=operator.image.suffix="" \
    --helm-set=operator.image.tag=main.0 \
    --helm-set=operator.image.useDigest=false \
    --helm-set=hubble.relay.image.repository=quay.io/ysksuzuki/hubble-relay \
    --helm-set=hubble.relay.image.tag=main.0 \
    --helm-set=hubble.relay.image.useDigest=false \
    --helm-set=hubble.eventBufferCapacity=65535 \
    --rollback=false \
    --config monitor-aggregation=none \
    --nodes-without-cilium=kind-worker3 \
    --helm-set-string=kubeProxyReplacement=strict \
    --helm-set-string=routingMode=native \
    --helm-set-string=autoDirectNodeRoutes=true \
    --helm-set-string=ipv4NativeRoutingCIDR=10.244.0.0/16 \
    --helm-set-string=ipv6NativeRoutingCIDR=fd00:10:244::/56 \
    --helm-set-string=loadBalancer.acceleration=testing-only \
    --helm-set-string=loadBalancer.mode=dsr \
    --helm-set-string=loadBalancer.dsrDispatch=opt \
    --helm-set=bpf.masquerade=true \
    --helm-set=enableIPv6Masquerade=false \
    --helm-set=debug.enabled=true \
    --helm-set=ipv6.enabled=true \
    --helm-set-string=devices=eth+

// SNAT + vxlan + devices eth+(eth0, eth1)
cilium install --wait \
    --chart-directory=/home/yusuke/go/src/github.com/cilium/cilium/install/kubernetes/cilium \
    --helm-set=image.repository=quay.io/ysksuzuki/cilium-dev \
    --helm-set=image.useDigest=false \
    --helm-set=image.tag=main.0 \
    --helm-set=operator.image.repository=quay.io/ysksuzuki/operator \
    --helm-set=operator.image.suffix="" \
    --helm-set=operator.image.tag=main.0 \
    --helm-set=operator.image.useDigest=false \
    --helm-set=hubble.relay.image.repository=quay.io/ysksuzuki/hubble-relay \
    --helm-set=hubble.relay.image.tag=main.0 \
    --helm-set=hubble.relay.image.useDigest=false \
    --helm-set=hubble.eventBufferCapacity=65535 \
    --rollback=false \
    --config monitor-aggregation=none \
    --nodes-without-cilium=kind-worker3 \
    --helm-set-string=kubeProxyReplacement=strict \
    --helm-set-string=routingMode=tunnel \
    --helm-set-string=loadBalancer.acceleration=testing-only \
    --helm-set-string=loadBalancer.mode=snat \
    --helm-set=bpf.masquerade=true \
    --helm-set=enableIPv6Masquerade=false \
    --helm-set=debug.enabled=true \
    --helm-set=ipv6.enabled=true \
    --helm-set-string=devices=eth+

$ docker exec kind-control-plane ip a
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
    inet 127.0.0.1/8 scope host lo
       valid_lft forever preferred_lft forever
    inet6 ::1/128 scope host
       valid_lft forever preferred_lft forever
2: cilium_net@cilium_host: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether ee:59:13:9e:c4:9e brd ff:ff:ff:ff:ff:ff
    inet6 fe80::ec59:13ff:fe9e:c49e/64 scope link
       valid_lft forever preferred_lft forever
3: cilium_host@cilium_net: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 6a:5b:ac:34:40:c4 brd ff:ff:ff:ff:ff:ff
    inet 10.244.0.28/32 scope global cilium_host
       valid_lft forever preferred_lft forever
    inet6 fd00:10:244::5532/64 scope global
       valid_lft forever preferred_lft forever
    inet6 fe80::685b:acff:fe34:40c4/64 scope link
       valid_lft forever preferred_lft forever
5: lxc_health@if4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether 1a:d7:f2:97:93:a8 brd ff:ff:ff:ff:ff:ff link-netnsid 1
    inet6 fe80::18d7:f2ff:fe97:93a8/64 scope link
       valid_lft forever preferred_lft forever
28: eth0@if29: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdpgeneric/id:2426 qdisc noqueue state UP group default qlen 1000
    link/ether 02:42:ac:14:00:04 brd ff:ff:ff:ff:ff:ff link-netnsid 0
    inet 172.20.0.4/16 brd 172.20.255.255 scope global eth0
       valid_lft forever preferred_lft forever
    inet6 fc00:c111::4/64 scope global nodad
       valid_lft forever preferred_lft forever
    inet6 fe80::42:acff:fe14:4/64 scope link
       valid_lft forever preferred_lft forever
34: eth1@if35: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 xdpgeneric/id:2484 qdisc noqueue state UP group default qlen 1000
    link/ether 02:42:ac:15:00:03 brd ff:ff:ff:ff:ff:ff link-netnsid 0
    inet 172.21.0.3/16 brd 172.21.255.255 scope global eth1
       valid_lft forever preferred_lft forever
    inet6 fc00:c112::3/64 scope global nodad
       valid_lft forever preferred_lft forever
    inet6 fe80::42:acff:fe15:3/64 scope link
       valid_lft forever preferred_lft forever

$ kubectl  -n cilium-test get svc
NAME              TYPE       CLUSTER-IP      EXTERNAL-IP   PORT(S)          AGE
echo-other-node   NodePort   10.96.11.5      <none>        8080:30476/TCP   8m24s
echo-same-node    NodePort   10.96.105.203   <none>        8080:31186/TCP   8m24s

// Secondary IPv4
$ docker exec kind-worker3 curl http://172.21.0.3:30476
<html>
  <head>
    <link
      rel="stylesheet"
      href="https://use.fontawesome.com/releases/v5.8.2/css/all.css"
      integrity="sha384-oS3vJWv+0UjzBfQzYUhtDYW+Pj2yciDJxpsK1OYPAYjqT085Qq/1cq5FLXAZQ7Ay"
      crossorigin="anonymous"

// Secondary IPv6
$ docker exec kind-worker3 curl http://[fc00:c112::3]:30476
<html>
  <head>
    <link
      rel="stylesheet"
      href="https://use.fontawesome.com/releases/v5.8.2/css/all.css"
      integrity="sha384-oS3vJWv+0UjzBfQzYUhtDYW+Pj2yciDJxpsK1OYPAYjqT085Qq/1cq5FLXAZQ7Ay"
      crossorigin="anonymous"

@ysksuzuki ysksuzuki force-pushed the pr/ysksuzuki/add-secondary-if-kind branch from a3f80af to 9a4b4b2 Compare June 19, 2023 06:10
Copy link
Member

@bimmlerd bimmlerd left a comment

Choose a reason for hiding this comment

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

(sorry for the early review, didn't realise this was still in draft)

Should we be gating this behind a flag? I guess most people don't need a secondary interface in their dev cycle, and I see value in keeping the default as simple as possible.

Other than nits, implementation looks good though.

for ifc in /sys/class/net/"${bridge_dev}"/brif/*; do
ifc="${ifc#"/sys/class/net/${bridge_dev}/brif/"}"
for ifc in /sys/class/net/"${bridge_dev}"*/brif/*; do
ifc=$(echo $ifc | sed "s:/sys/class/net/${bridge_dev}.*/brif/::")
Copy link
Member

Choose a reason for hiding this comment

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

let's use $SED here, which should point to GNU sed on macos too

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch! Thanks

@@ -11,9 +11,11 @@ fi

default_cluster_name="kind"
default_network="kind-cilium"
default_secondary_network="${default_network}-secondary"
Copy link
Member

@bimmlerd bimmlerd Jun 19, 2023

Choose a reason for hiding this comment

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

maybe just secondary instead of default_secondary?

@ysksuzuki
Copy link
Member Author

ysksuzuki commented Jun 19, 2023

Should we be gating this behind a flag? I guess most people don't need a secondary interface in their dev cycle, and I see value in keeping the default as simple as possible.

Make sense. I will add a flag, e.g. --secondary, and set up a simple single-network by default.

@ysksuzuki ysksuzuki force-pushed the pr/ysksuzuki/add-secondary-if-kind branch 2 times, most recently from 04153bd to c3c9263 Compare July 14, 2023 06:20
@ysksuzuki
Copy link
Member Author

/test

@ysksuzuki ysksuzuki marked this pull request as ready for review July 14, 2023 07:41
@ysksuzuki ysksuzuki requested review from bimmlerd and brb July 14, 2023 07:42
@nbusseneau nbusseneau added release-note/ci This PR makes changes to the CI. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 18, 2023
@ysksuzuki
Copy link
Member Author

Merging #26856 made the conflict. I have to rebase my branch.

@ysksuzuki ysksuzuki force-pushed the pr/ysksuzuki/add-secondary-if-kind branch from c3c9263 to dbf9ef1 Compare July 19, 2023 11:29
@ysksuzuki
Copy link
Member Author

/test

@ysksuzuki
Copy link
Member Author

Hi @brb , please take a look at this PR

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 19, 2023
@ysksuzuki ysksuzuki force-pushed the pr/ysksuzuki/add-secondary-if-kind branch from dbf9ef1 to e8653ef Compare July 19, 2023 23:49
@ysksuzuki ysksuzuki force-pushed the pr/ysksuzuki/add-secondary-if-kind branch from e8653ef to f096a89 Compare July 20, 2023 00:01
Add secondary iface to KIND network, so that we can test the NodePort
via secondary iface.

Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
@ysksuzuki
Copy link
Member Author

/test

@ysksuzuki ysksuzuki requested a review from brb July 20, 2023 02:13
Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@brb brb merged commit d675d6f into main Jul 20, 2023
176 checks passed
@brb brb deleted the pr/ysksuzuki/add-secondary-if-kind branch July 20, 2023 05:25
@tklauser tklauser added the needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch label Aug 23, 2023
@tklauser tklauser mentioned this pull request Aug 24, 2023
9 tasks
@tklauser tklauser added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Aug 24, 2023
@joestringer joestringer added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Aug 25, 2023
@brb brb mentioned this pull request Sep 4, 2023
10 tasks
@qmonnet qmonnet added the backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. label Jan 4, 2024
@qmonnet qmonnet mentioned this pull request Jan 4, 2024
5 tasks
@github-actions github-actions bot added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/CI-improvement Topic or proposal to improve the Continuous Integration workflow backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants