fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes #7909

lobkovilya · 2023-09-28T14:46:17Z

Today there is a possibility to change the Envoy Admin address to ::1 but admin_proxy_generator.go generates an admin cluster always with a 127.0.0.1 address.

This PR puts a specified admin address to proxy metadata and uses it when generating an admin cluster.

Checklist prior to review

Link to relevant issue as well as docs and UI issues -- Fix Wait For Dataplane Ready takes too long and times out at 3 minutes #7849
This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as a image registry) and it will work on Windows, system specific functions like syscall.Mkfifo have equivalent implementation on the other OS --
Tests (Unit test, E2E tests, manual test on universal and k8s) --
Do you need to update UPGRADE.md? --
Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label) --
Do you need to explicitly set a > Changelog: entry here or add a ci/ label to run fewer/more tests?

Signed-off-by: Ilya Lobkov <[email protected]>

cbugneac-nex · 2023-09-29T08:34:55Z

Hi @lobkovilya should we also mention that kuma-sidecar (kuma-dp) also needs to bind to IPv6 or this is for a separate PR ?

lobkovilya · 2023-09-29T08:45:01Z

Hi, kuma-sidecar is binding to the data plane proxy inbound address as you can see here (g.getAddress(proxy) returns it):

kuma/pkg/xds/generator/admin_proxy_generator.go

Line 87 in 21efbfb

    
           listener, err := envoy_listeners.NewInboundListenerBuilder(proxy.APIVersion, g.getAddress(proxy), adminPort, core_xds.SocketAddressProtocolTCP).

.

And I believe we already have IPv6 support for DPP inbounds, so it should work. The only issue here was using 127.0.0.1 in the envoy admin cluster and this PR changes this to take the address from the proxy's metadata.

pkg/xds/generator/admin_proxy_generator.go

pkg/xds/bootstrap/testdata/generator.metrics-config.kubernetes.golden.yaml

Signed-off-by: Ilya Lobkov <[email protected]>

github-actions · 2023-09-29T14:55:11Z

backporting to release-2.1 with action

backporting to release-2.3 with action
backporting to release-2.4 with action
backporting to release-2.0 with action

…veness probes (#7909) Put a specified admin address to proxy metadata and use it when generating an admin cluster. Signed-off-by: Ilya Lobkov <[email protected]>

lahabana · 2023-09-29T17:00:14Z

Couple of questions:

what happens in dual stack?
could we have not added an e2e test to make sure it works on both ipv4 and ipv6 in real life setups?

…veness probes (backport of #7909) (#7929) * fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (#7909) Put a specified admin address to proxy metadata and use it when generating an admin cluster. * test: fix flaky test * chore(kuma-cp): improve error message for invalid admin addresses Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Mike Beaumont <[email protected]> Co-authored-by: Ilya Lobkov <[email protected]> Co-authored-by: Mike Beaumont <[email protected]>

lobkovilya · 2023-10-02T09:18:40Z

what happens in dual stack?

Envoy binds to 127.0.0.1 by default, so dual stack worked well before this PR.

could we have not added an e2e test to make sure it works on both ipv4 and ipv6 in real life setups?

I'm not sure if we need a dedicated e2e test for this, but I can change one Zone CP when running kindipv6 to have:

KUMA_RUNTIME_KUBERNETES_INJECTOR_SIDECAR_CONTAINER_WAIT_FOR_DATAPLANE_READY: "true"
KUMA_BOOTSTRAP_SERVER_PARAMS_ADMIN_ADDRESS: "::1"

that way we can ensure these features are tested.

cbugneac-nex · 2023-10-02T09:41:46Z

Apologies, will this change be included in the next release, e.g. 2.4.2 ?

michaelbeaumont · 2023-10-02T09:43:34Z

@cbugneac-nex Yes: https://github.com/kumahq/kuma/tree/release-2.4

cbugneac-nex · 2023-10-02T09:49:08Z

Thanks @michaelbeaumont Don't want to look pushy, but when release of 2.4.2 is planned to happen ?
Reason: I'm blocked now with #7849 and would like to plan my work when I can get back to it.

lahabana · 2023-10-02T09:49:32Z

I like this option of using ::1 for our ipv6 tests yes @lobkovilya !

…veness probes (#7909) Put a specified admin address to proxy metadata and use it when generating an admin cluster. Signed-off-by: Ilya Lobkov <[email protected]>

…veness probes (backport of #7909) (#7927) Automatic cherry-pick of #7909 for branch release-2.3 Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Mike Beaumont <[email protected]>

…veness probes (backport of #7909) (#7930) Automatic cherry-pick of #7909 for branch release-2.0 Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Mike Beaumont <[email protected]>

…veness probes (backport of #7909) (#7928) * fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (#7909) Put a specified admin address to proxy metadata and use it when generating an admin cluster. * test(unit): fix flaky "should return error when admin address is not allowed" test (#7933) fix(kuma-cp): use SortedKeys function in error msg * make SortedKeys generic * chore(kuma-cp): improve error message for invalid admin addresses (#7934) * update golden files Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Mike Beaumont <[email protected]> Co-authored-by: Ilya Lobkov <[email protected]> Co-authored-by: Mike Beaumont <[email protected]>

…veness probes (backport of #7909) (#7926) * fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (#7909) Put a specified admin address to proxy metadata and use it when generating an admin cluster. * test(unit): fix flaky "should return error when admin address is not allowed" test (#7933) fix(kuma-cp): use SortedKeys function in error msg * make SortedKeys generic * chore(kuma-cp): improve error message for invalid admin addresses (#7934) * update golden files Signed-off-by: Ilya Lobkov <[email protected]> Signed-off-by: Mike Beaumont <[email protected]> Co-authored-by: Ilya Lobkov <[email protected]> Co-authored-by: Mike Beaumont <[email protected]>

lobkovilya added 2 commits September 28, 2023 16:09

fix(kuma-cp): add support for ipv6 loopback addr for envoy admin

1d0e74c

Signed-off-by: Ilya Lobkov <[email protected]>

fix(kuma-cp): default value if metadata field is not set

21efbfb

Signed-off-by: Ilya Lobkov <[email protected]>

lobkovilya changed the title ~~fix(kuma-cp): add support for ipv6 loopback addr for envoy admin~~ fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes Sep 29, 2023

lobkovilya marked this pull request as ready for review September 29, 2023 10:30

lobkovilya requested a review from a team as a code owner September 29, 2023 10:30

lobkovilya requested review from michaelbeaumont and jakubdyszkiewicz and removed request for a team September 29, 2023 10:30

michaelbeaumont reviewed Sep 29, 2023

View reviewed changes

pkg/xds/generator/admin_proxy_generator.go Show resolved Hide resolved

pkg/xds/bootstrap/testdata/generator.metrics-config.kubernetes.golden.yaml Show resolved Hide resolved

fix(kuma-cp): validate addresses, added tests

5699325

Signed-off-by: Ilya Lobkov <[email protected]>

michaelbeaumont approved these changes Sep 29, 2023

View reviewed changes

lobkovilya merged commit 8481ec2 into kumahq:master Sep 29, 2023

lobkovilya deleted the fix/envoy-admin-ipv6 branch September 29, 2023 14:50

lobkovilya added the backport label Sep 29, 2023

kumahq bot mentioned this pull request Sep 29, 2023

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (backport of #7909) #7926

Merged

This was referenced Sep 29, 2023

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (backport of #7909) #7927

Merged

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (backport of #7909) #7928

Merged

kumahq bot mentioned this pull request Sep 29, 2023

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (backport of #7909) #7929

Merged

kumahq bot mentioned this pull request Sep 29, 2023

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes (backport of #7909) #7930

Merged

lobkovilya mentioned this pull request Sep 29, 2023

test(unit): fix flaky "should return error when admin address is not allowed" test #7933

Merged

6 tasks

BrewTestBot mentioned this pull request Nov 15, 2023

kumactl 2.5.0 Homebrew/homebrew-core#154429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes #7909

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes #7909

lobkovilya commented Sep 28, 2023 •

edited

Loading

cbugneac-nex commented Sep 29, 2023

lobkovilya commented Sep 29, 2023

github-actions bot commented Sep 29, 2023 •

edited

Loading

lahabana commented Sep 29, 2023

lobkovilya commented Oct 2, 2023

cbugneac-nex commented Oct 2, 2023

michaelbeaumont commented Oct 2, 2023 •

edited

Loading

cbugneac-nex commented Oct 2, 2023

lahabana commented Oct 2, 2023

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes #7909

fix(kuma-cp): specifying IPv6 Envoy Admin address breaks readiness/liveness probes #7909

Conversation

lobkovilya commented Sep 28, 2023 • edited Loading

Checklist prior to review

cbugneac-nex commented Sep 29, 2023

lobkovilya commented Sep 29, 2023

github-actions bot commented Sep 29, 2023 • edited Loading

lahabana commented Sep 29, 2023

lobkovilya commented Oct 2, 2023

cbugneac-nex commented Oct 2, 2023

michaelbeaumont commented Oct 2, 2023 • edited Loading

cbugneac-nex commented Oct 2, 2023

lahabana commented Oct 2, 2023

lobkovilya commented Sep 28, 2023 •

edited

Loading

github-actions bot commented Sep 29, 2023 •

edited

Loading

michaelbeaumont commented Oct 2, 2023 •

edited

Loading