Apply suggestions from code review

kubernetes · Jan 10, 2022 · ff7cd20 · ff7cd20
1 parent abd6684
commit ff7cd20
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 9 deletions.
diff --git a/keps/sig-network/3070-reserved-service-ip-range/README.md b/keps/sig-network/3070-reserved-service-ip-range/README.md
@@ -224,14 +224,14 @@ Example 1 Large Subnet:
 - Range Size: 2^16 - 2 = 65534
 - Band Offset: min(max(16,65536/16),256) = min(4096,256) = 256
 - Static band start: 192.168.0.1
-- Static band ends: 192.168.1.1
+- Static band ends: 192.168.1.0
 
            ┌─────────────┬─────────────────────────────────────────────┐
            │   static    │                    dynamic                  │
            └─────────────┴─────────────────────────────────────────────┘
 
            ◄────────────► ◄────────────────────────────────────────────►
-    192.168.0.1     192.168.1.1                                 192.168.255.254
+    192.168.0.1     192.168.1.0                                 192.168.255.254
 
 Example 2:
 
@@ -314,14 +314,14 @@ Same as with upgrade, there is no change in the bitmaps, keeping the feature 100
 - [x] Feature gate (also fill in values in `kep.yaml`)
   - Feature gate name: ServiceIPStaticSubrange
   - Components depending on the feature gate: kube-apiserver
-- [x] Other
+- [ ] Other
   - Describe the mechanism:
   - The feature require to restart the apiserver to use the new allocation strategy for Services, however,
   it will not require downtime in multi-control-plane environments.
     
 ###### Does enabling the feature change any default behavior?
 
-Dynamic allocated IPs will not be chosen from the first X values of the Service IP range, where X is obtained from the formula
+Dynamic allocated IPs will not be initially chosen from the first X values of the Service IP range, where X is obtained from the formula
 described in [Proposed Services ClusterIPs allocation model](#proposed-services-clusterips-allocation-model)
 This change is transparent to the user, since the IP is and was already random, it doesn't matter from which range the IP is obtained.
 
@@ -362,26 +362,29 @@ The increase of the errors metrics or the trend of the allocated_ips and availab
 
 ### Monitoring Requirements
 
-Current allocator metrics will be expanded with a new label to each metric containing information about the
+Following allocator metrics will be expanded with a new label to each metric containing information about the
 type of allocation requested (dynamic or static):
 
-  - allocated_ips
-  - available_ips
   - allocation_total
   - allocation_errors_total
 
+The other allocator the metrics can not use the additional label because, when an ip is released, there is not information on how the ip was allocated.
+
+  - allocated_ips
+  - available_ips
+
 ###### How can an operator determine if the feature is in use by workloads?
 
 A new metric will be added containing the information of the range configuration in the allocator.
 
 ```go
 	allocatorInfo = metrics.NewGaugeVec(
 		&metrics.GaugeOpts{
-			Name:           "kubernetes_allocator_info",
+			Name:           "allocator_info",
 			Help:           "A metric with a constant '1' value labeled by Service CIDR, ",
 			StabilityLevel: metrics.ALPHA,
 		},
-		[]string{"cidr", "address_min", "address_max", "dynamic_range_offset"},
+		[]string{"cidr", "allocator_size", "dynamic_range_offset"},
 	)
 ```
 

diff --git a/keps/sig-network/3070-reserved-service-ip-range/kep.yaml b/keps/sig-network/3070-reserved-service-ip-range/kep.yaml
@@ -41,3 +41,4 @@ metrics:
   - available_ips
   - allocation_total
   - allocation_errors_total
+  - allocator_info