✨ Support for using both an external control plane and automatic webhooks together #1265
Conversation
…hooks together. This requires pre-filling LocalServingHost and possibly LocalServingExternalName with the right listen interface and external name so that the external control plane can reach the webhook server launched by envtest.
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @coderanger. 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 Once the patch is verified, the new status will be reflected by the 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. |
k8s-ci-robot
added
the
needs-ok-to-test
k8s-ci-robot
added
the
size/L
| return "", fmt.Errorf("unable to grab random port for serving webhooks on: %v", err) | ||
| } | ||
| o.LocalServingPort = port | ||
| o.LocalServingHost = host |
There was a problem hiding this comment.
What do you think about reusing this field instead? It would be generated IF not provided.
There was a problem hiding this comment.
You mean re: adding the new struct field? LocalServingHost has to be a name or IP for the webhook server bind to, in most cases I would expect it to be set to 0.0.0.0 when doing this style of test. The ExternalName field is what ends up written into the webhook definition and needs to be the externally visible name for the remote kube-apiserver to reach us on.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| port, host, err := addr.Suggest() | ||
| if err != nil { | ||
| return "", fmt.Errorf("unable to grab random port for serving webhooks on: %v", err) | ||
| if o.LocalServingPort == 0 { |
There was a problem hiding this comment.
Is there any case where it is sensible to have the port be 0 and use and external name?
There was a problem hiding this comment.
Like to actually set it to 0 in the OS? I don't think so, the reason for this check is the reverse, allowing passing in a fixed port if you are willing to take ownership of any collisions (ex. running just one suite in a CI job) while leaving the default still be "pick me a random port" like the OS would do, but without reuse collisions.
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: coderanger, pwittrock 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
This requires pre-filling LocalServingHost and possibly LocalServingExternalName with the right listen interface and external name so that the external control plane can reach the webhook server launched by envtest.
Using this successfully for local integration tests against kind and k3s. It does change some APIs but the only compat change is on addr.Suggest and that's explicitly an internal API so I think that's okay?