GoogleCloudPlatform / netd

netd: GKE Networking Daemonset
Apache License 2.0
54 stars 41 forks source link

Correctly initialize with an IPv6 API server endpoint. #302

Closed krzykwas closed 5 months ago

krzykwas commented 5 months ago

If $KUBERNETES_SERVICE_HOST is an IPv6 address, wrap it with [], so a curl command to /api/v1/nodes succeeds.

krzykwas commented 5 months ago

/assign @aojea /assign @marqc

google-oss-prow[bot] commented 5 months ago

@krzykwas: GitHub didn't allow me to assign the following users: marqc.

Note that only GoogleCloudPlatform members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time. For more information please see the contributor guide

In response to [this](https://github.com/GoogleCloudPlatform/netd/pull/302#issuecomment-2072357577): >/assign @aojea >/assign @marqc Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
krzykwas commented 5 months ago

Can you add a test case with IPv4 and IPv6 KUBERNETES_SERVICE_HOST and verification that a correct curl arguments are passed, generally a clone of this test case https://github.com/GoogleCloudPlatform/netd/blob/master/scripts/testcase/testcase-basic-v2.sh#L1 https://github.com/GoogleCloudPlatform/netd/blob/master/scripts/testcase/testcase-basic-v2.sh#L23

Done

google-oss-prow[bot] commented 5 months ago

@marqc: changing LGTM is restricted to collaborators

In response to [this](https://github.com/GoogleCloudPlatform/netd/pull/302#pullrequestreview-2020251320): > Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
aojea commented 5 months ago

/lgtm /assign @jingyuanliang

jingyuanliang commented 5 months ago

/lgtm /approve

google-oss-prow[bot] commented 5 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingyuanliang, krzykwas, marqc

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](https://github.com/GoogleCloudPlatform/netd/blob/master/OWNERS)~~ [jingyuanliang] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
jingyuanliang commented 5 months ago

cc @yiningou - cl/624106013 needs similar thing.

jingyuanliang commented 5 months ago

I'm not sure if ip-masq-agent will still be relevant in the current ipv6-only design (v6 folks could you advise?) but at least we shouldn't let it crash when it runs over the curl lines.

aojea commented 5 months ago

I'm not sure if ip-masq-agent will still be relevant in the current ipv6-only design (v6 folks could you advise?) but at least we shouldn't let it crash when it runs over the curl lines.

where is the ip-masq-agent using curl?

jingyuanliang commented 4 months ago

where is the ip-masq-agent using curl?

No it's not the kubernetes-sigs/ip-masq-agent but our enablement approach for cilium's ip-masq-agent.

aojea commented 4 months ago

all golang code must use the net.JoinHostPort() helper to build urls, that do the right thing https://pkg.go.dev/net#JoinHostPort

jingyuanliang commented 4 months ago

Good to know but it's another piece of bash script there...