Merge pull request kubernetes#2066 from andrewsykim/kep-1864

KEP-1864: add prod readiness review and beta milestone

Author: k8s-ci-robot

keps/prod-readiness/sig-network/1864.yaml

@@ -0,0 +1,3 @@
+kep-number: 1864
+beta:
+  approver: "@johnbelamaric"

keps/sig-network/1864-disable-lb-node-ports/README.md

@@ -12,9 +12,17 @@
   - [Test Plan](#test-plan)
   - [Graduation Criteria](#graduation-criteria)
   - [Alpha](#alpha)
+  - [Beta](#beta)
   - [GA](#ga)
   - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
   - [Version Skew Strategy](#version-skew-strategy)
+- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire)
+  - [Feature Enablement and Rollback](#feature-enablement-and-rollback)
+  - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning)
+  - [Monitoring Requirements](#monitoring-requirements)
+  - [Dependencies](#dependencies)
+  - [Scalability](#scalability)
+  - [Troubleshooting](#troubleshooting)
 - [Implementation History](#implementation-history)
 - [Drawbacks](#drawbacks)
 - [Alternatives](#alternatives)
@@ -27,11 +35,11 @@
 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
+- [X] (R) KEP approvers have approved the KEP status as `implementable`
+- [X] (R) Design details are appropriately documented
+- [X] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
+- [X] (R) Graduation criteria is in place
+- [X] (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]
@@ -115,12 +123,19 @@ E2E tests:
 
 ### Alpha
 
-Adds new field `allocateLoadBalancerNodePorts` to Service but not implemented, this allows for rollback.
+* Adds new field `allocateLoadBalancerNodePorts` to Service, but the field is dropped unless an existing Service has the field set already.
+* Only allow the field `allocateLoadBalancerNodePorts` to be set when the feature gate is on.
+* There are sufficient unit tests exercising API strategy  with the feature gate enabled / disabled.
 
-### GA
+### Beta
+
+* E2E tests checking that node ports do not get allocated when `service.spec.allocateLoadBalancerNodePorts=false`.
+* Feature gate is on by default.
 
-Feature is enabled when field is set.
+### GA
 
+* Feature gate is on by default and locked.
+* To safely handle rollback, there has been at least 1 release prior where apiserver understands the new field (covered in alpha).
 
 ### Upgrade / Downgrade Strategy
 
@@ -136,6 +151,172 @@ re-enabling node port should not cause any traffic disruptions.
 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.
 
+## Production Readiness Review Questionnaire
+
+### Feature Enablement and Rollback
+
+_This section must be completed when targeting alpha to a release._
+
+* **How can this feature be enabled / disabled in a live cluster?**
+  - [X] Feature gate (also fill in values in `kep.yaml`)
+    - Feature gate name: ServiceLBNodePortControl
+    - Components depending on the feature gate: kube-apiserver
+  - [ ] Other
+    - Describe the mechanism:
+    - Will enabling / disabling the feature require downtime of the control
+      plane?
+    - Will enabling / disabling the feature require downtime or reprovisioning
+      of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled).
+
+* **Does enabling the feature change any default behavior?**
+
+No, enabling the feature gate but not setting `spec.allocateLoadBalancerNodePorts` will not
+change any default behaviors in Service.
+
+* **Can the feature be disabled once it has been enabled (i.e. can we roll back
+  the enablement)?**
+
+  Yes, if the feature gate is disabled, new Services cannot use the new field, but existing Services
+  already using the field will continue to have it set. Updates to existing fields are allowed.
+
+* **What happens if we reenable the feature if it was previously rolled back?**
+
+  The existing value for `spec.allocateLoadBalancerNodePorts` will remain intact since API strategy
+  will not drop fields if existing resources have it set.
+
+* **Are there any tests for feature enablement/disablement?**
+
+  Yes, there will be unit tests for the Service API strategy which exercises the behavior
+  with the feature gate enabled and disabled.
+
+### Rollout, Upgrade and Rollback Planning
+
+_This section must be completed when targeting beta graduation to a release._
+
+* **How can a rollout fail? Can it impact already running workloads?**
+
+* By default this should not impact any existing Services since we are not changing any default behaviors.
+* Enabling this feature on new clusters can impact workloads if load balancers depend on node ports without users
+being aware.
+
+* **What specific metrics should inform a rollback?**
+
+Metrics for node port counts will vary for Service LoadBalancers that set `spec.allocateLoadBalancerNodeports=false`.
+If load balancers are misbehaving at the same time node port allocation metric is decreasing, the user may want to
+consider rolling back this feature.
+
+* **Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?**
+
+No, upgrade->downgrade->upgrade has not been tested yet. Like any new API field, on downgrade
+any existing Services using the field will continue to have the field set. For these Services,
+they will not have node ports allocated. New Services cannot use the new field unless the feature
+gate is enabled in the old version when the feature was alpha.
+
+Manual validation of this behavior should be done prior to promoting this feature to beta.
+
+* **Is the rollout accompanied by any deprecations and/or removals of features, APIs,
+fields of API types, flags, etc.?**
+
+No.
+
+### Monitoring Requirements
+
+_This section must be completed when targeting beta graduation to a release._
+
+* **How can an operator determine if the feature is in use by workloads?**
+
+Service should have `spec.allocateLoadBalancerNodePorts=false` and Service LoadBalancers will not have node ports allocated.
+
+* **What are the SLIs (Service Level Indicators) an operator can use to determine
+the health of the service?**
+
+N/A
+
+* **What are the reasonable SLOs (Service Level Objectives) for the above SLIs?**
+
+N/A
+
+* **Are there any missing metrics that would be useful to have to improve observability
+of this feature?**
+
+N/A
+
+### Dependencies
+
+_This section must be completed when targeting beta graduation to a release._
+
+* **Does this feature depend on any specific services running in the cluster?**
+
+This feature is dependent on the Service LoadBalancer implementation of a cluster. This feature
+should only be used if the load balancer implementation does not need node ports for the load balancer
+data path.
+
+
+### Scalability
+
+_For alpha, this section is encouraged: reviewers should consider these questions
+and attempt to answer them._
+
+_For beta, this section is required: reviewers must answer these questions._
+
+_For GA, this section is required: approvers should be able to confirm the
+previous answers based on experience in the field._
+
+* **Will enabling / using this feature result in any new API calls?**
+  Describe them, providing:
+
+No, enabling this feature should actually reduce the number of operations, since
+the feature is to disable an existing behavior with node ports.
+
+* **Will enabling / using this feature result in introducing new API types?**
+
+No
+
+* **Will enabling / using this feature result in any new calls to the cloud
+provider?**
+
+No
+
+* **Will enabling / using this feature result in increasing size or count of
+the existing API objects?**
+
+No
+
+* **Will enabling / using this feature result in increasing time taken by any
+operations covered by [existing SLIs/SLOs]?**
+
+No
+
+* **Will enabling / using this feature result in non-negligible increase of
+resource usage (CPU, RAM, disk, IO, ...) in any components?**
+
+No
+
+### Troubleshooting
+
+The Troubleshooting section currently serves the `Playbook` role. We may consider
+splitting it into a dedicated `Playbook` document (potentially with some monitoring
+details). For now, we leave it here.
+
+_This section must be completed when targeting beta graduation to a release._
+
+* **How does this feature react if the API server and/or etcd is unavailable?**
+
+Not any different from when node ports are used for load balancers.
+
+* **What are other known failure modes?**
+
+If `service.spec.allocateLoadBalancerNodePorts=false` but the load balancer implementation does depend on node ports.
+
+* **What steps should be taken if SLOs are not being met to determine the problem?**
+
+In a scenario where a user sets `service.spec.allocateLoadBalancerNodePorts=false` but the load balancer does require node ports,
+the user can re-enable node ports for a Service by setting `service.spec.allocateLoadBalancerNodePorts` back to `true`.
+This will trigger node port allocation from kube-apiserver.
+
+[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md
+[existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos
+
 ## Implementation History
 
 - 2020-06-17: KEP is proposed as implementable

keps/sig-network/1864-disable-lb-node-ports/kep.yaml

@@ -17,16 +17,21 @@ prr-approvers:
   - "@johnbelamaric"
 
 # The target maturity stage in the current dev cycle for this KEP.
-stage: stable
+stage: beta
 
 # 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"
+latest-milestone: "v1.21"
 
 # The milestone at which this feature was, or is targeted to be, at each stage.
 milestone:
   alpha: "v1.20"
   beta: "v1.21"
   stable: "v1.22"
 
+feature-gates:
+  - name: ServiceLBNodePortControl
+    components:
+      - kube-apiserver
+disable-supported: true