From 8a83b1393e03fbdf26402c39573eba63f5482b89 Mon Sep 17 00:00:00 2001 From: Andrew Sy Kim Date: Fri, 1 Nov 2019 20:38:07 -0400 Subject: [PATCH] optionally disable node ports for Service Type=LoadBalancer Signed-off-by: Andrew Sy Kim --- .../1864-disable-lb-node-ports/README.md | 159 ++++++++++++++++++ .../1864-disable-lb-node-ports/kep.yaml | 30 ++++ 2 files changed, 189 insertions(+) create mode 100644 keps/sig-network/1864-disable-lb-node-ports/README.md create mode 100644 keps/sig-network/1864-disable-lb-node-ports/kep.yaml diff --git a/keps/sig-network/1864-disable-lb-node-ports/README.md b/keps/sig-network/1864-disable-lb-node-ports/README.md new file mode 100644 index 00000000000..0b7fbc6e587 --- /dev/null +++ b/keps/sig-network/1864-disable-lb-node-ports/README.md @@ -0,0 +1,159 @@ +# KEP-1864: Optionally Disable Node Ports for Service Type=LoadBalancer + + +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [Risks and Mitigations](#risks-and-mitigations) +- [Design Details](#design-details) + - [Test Plan](#test-plan) + - [Graduation Criteria](#graduation-criteria) + - [Alpha](#alpha) + - [GA](#ga) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) + - [APIServer Flag](#apiserver-flag) + - [Service Type=LoadBalancerWithoutNodePorts](#service-typeloadbalancerwithoutnodeports) + + +## Release Signoff Checklist + +Items marked with (R) are required *prior to targeting to a milestone / release*. + +- [X] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (R) Design details are appropriately documented +- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input +- [ ] (R) Graduation criteria is in place +- [ ] (R) Production readiness review completed +- [ ] Production readiness review approved +- [ ] "Implementation History" section is up-to-date for milestone +- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [ ] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes + + + +[kubernetes.io]: https://kubernetes.io/ +[kubernetes/enhancements]: https://git.k8s.io/enhancements +[kubernetes/kubernetes]: https://git.k8s.io/kubernetes +[kubernetes/website]: https://git.k8s.io/website + +## Summary + +Today, a Kubernetes Service Type=LoadBalancer will always allocate a node port for every service port. Though most implementations of Service Type=LoadBalancer do require node ports, there are several implementations that do not. This KEP proposes to add a new field to Service to opt out of node port allocation for loadbalancers. + +## Motivation + +There are several implementations of Service Type=LoadBalancer API today that do not consume the node ports automatically allocated by Kubernetes. Some examples include but are not limited to: +* [MetalLB](https://github.com/danderson/metallb) +* [kube-router](https://github.com/cloudnativelabs/kube-router) + +The drawbacks of allocating unused node ports are that: +* the number of load balancers is now limited to the number of available node ports. Less if each load balancer consumes multiple node ports. +* node ports are exposed for an LB even though they are not used -- unecessarily exposed ports can fail regulatory/compliance requirements + +For clusters that have integrations for Service Type=LoadBalancer but don't require node ports should have the option to disable node port allocation. + +### Goals + +* Allow users to optionally disable node port for Service Type=LoadBalancer +* Service LoadBalancer node ports can safely be re-enabled in a cluster + +### Non-Goals + +* Changing default values/behavior for Service Type=LoadBalancer + +## Proposal + +Add a new field `allocateLoadBalancerNodePorts` to `Service.Spec` that allows a user to opt out of node ports for Service Type=LoadBalancer. + - if `allocateLoadBalancerNodePort: true`, allocate node ports for Service Type=LB, this is the existing behavior today and will be the default value. + - if `allocateLoadBalancerNodePort: false`, stop allocating new node ports but don't deallocate existing node ports. + - if `allocateLoadBalancerNodePort: false`, and a user wants to disable node ports on existing Services, they need to remove the nodePort field explicitly. + +When a user relies on `kubectl apply` to fill in the node port field, the node port would never be deallocated since the existing node ports from the +server will always be merged prior to update. A user must send an explicit update request for the Service using something like `kubectl edit svc` or +building a controller to update all Services in a cluster. + +### Risks and Mitigations + +A user may unknowingly disable node ports while it is serving traffic for their pods. The chances of this should be significantly reduced since +node ports are not automatically deallocated when a user sets `allocateLoadBalancerNodePort: false`. The additional step to disable node ports +should ensure users are aware of the consequences of this change. + +## Design Details + +API changes to Service: +* Add a new field `spec.allocateLoadBalancerNodePorts: true|false`. +* `allocateLoadBalancerNodePorts` defaults to true, preserving existing behavior for Service Type=LoadBalancer. +* On create, if `allocateLoadBalancerNodePorts: false`, don't allocate node ports. +* On update, if `allocateLoadBalancerNodePorts: false` don't allocate new node ports but do not deallocate existing node ports if set. +* On update, set `nodePort: 0` (by removing the field) to deallocate a node port, a new port will not be re-allocated if `allocateLoadBalancerNodePorts: false`. +* On delete, node ports are deallocated regardless of `allocateLoadBalancerNodePorts`. + +### Test Plan + +Unit tests: +* unit tests for the allocate/deallocation of node ports based on the value of `allocateLoadBalancerNodePorts`. +* validate the default value for `allocateLoadBalanceNodePorts` on Service. + +E2E tests: +* The default behavior for `allocateLoadBalancerNodePorts` does not break any existing e2e tests. +* e2e tests for disabling node ports on a Service (testing the network data path may be difficult since testing platforms uses node ports). +* e2e tests for disabling node ports on an existing Service, ensure the node ports are preserved. +* e2e tests to explicitly disable node ports with `allocateLoadBalancerNodePorts: false`. +* e2e tests to re-enable `allocateLoadBalancerNodeports` for a Service with node ports disabled. + +### Graduation Criteria + +### Alpha + +Adds new field `allocateLoadBalancerNodePorts` to Service but not implemented, this allows for rollback. + +### GA + +Feature is enabled when field is set. + + +### Upgrade / Downgrade Strategy + +Upgrade should be trivial since kube-proxy's behavior of the Service is determined by whether a node port is set. +For existing Services that are upgraded, `nodePort` will continue to be set (even if `allocateLoadBalancerNodePort: false`). + +On downgrade, if `allocateLoadBalancerNodePorts: false`, the worse case is that a Service which was intended to have +node ports disabled will now have node ports re-enabled. Assuming the loadbalancer implementation never relied on the node port, +re-enabling node port should not cause any traffic disruptions. + +### Version Skew Strategy + +Version skew from the control plane to kube-proxy should be trivial since kube-proxy's behavior is driven by the `nodePort` field +and not the `allocateLoadBalancerNodePorts` field. + +## Implementation History + +- 2020-06-17: KEP is proposed as implementable + +## Drawbacks + +More fields == more complexity in Service. + +## Alternatives + +### APIServer Flag + +We can add an apiserver flag that toggles the node port behavior for Service Type=LoadBalancer. This requires the cluster admin to +understand the Service Type=LoadBalancer implementation and set this flag accordingly. In general we should try to avoid global flags +if it's feasible to drive the behavior through the API. + +### Service Type=LoadBalancerWithoutNodePorts + +A new Service Type like `LoadBalancerWithoutNodePorts` can be added, which would have similar semantics as Service Type=LoadBalancer +without node ports. This wouldn't be a good experience for existing consumers of Service Type=LoadBalancer that now have to watch a new Type. + diff --git a/keps/sig-network/1864-disable-lb-node-ports/kep.yaml b/keps/sig-network/1864-disable-lb-node-ports/kep.yaml new file mode 100644 index 00000000000..ac2d80a53b9 --- /dev/null +++ b/keps/sig-network/1864-disable-lb-node-ports/kep.yaml @@ -0,0 +1,30 @@ +title: Optionally Disable Node Ports for Service Type=LoadBalancer +kep-number: 1864 +authors: + - "@andrewsykim" +owning-sig: sig-network +participating-sigs: + - sig-cloud-provider +status: implementable +creation-date: 2020-06-21 +reviewers: + - "@thockin" + - "@uablrek" + - "@rikatz" +approvers: + - "@thockin" +prr-approvers: + - "@johnbelamaric" + +# The target maturity stage in the current dev cycle for this KEP. +stage: stable + +# The most recent milestone for which work toward delivery of this KEP has been +# done. This can be the current (upcoming) milestone, if it is being actively +# worked on. +latest-milestone: "v1.20" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + stable: "v1.20" +