perf: move lbp configuration up to the annotation level

Author: elijah-rou

config/core/300-resources/configuration.yaml

@@ -1027,12 +1027,6 @@ spec:
                             description: This is accessible behind a feature flag - kubernetes.podspec-init-containers
                             type: object
                             x-kubernetes-preserve-unknown-fields: true
-                        loadBalancingPolicy:
-                          description: |-
-                            LoadBalancingPolicy is the load balancing algorithm used by the
-                            activator to route requests to application pods. If unspecified,
-                            a suggested default is applied depending on ContainerConcurrency
-                          type: string
                         nodeSelector:
                           description: |-
                             This is accessible behind a feature flag - kubernetes.podspec-nodeselector

config/core/300-resources/revision.yaml

@@ -1003,12 +1003,6 @@ spec:
                     description: This is accessible behind a feature flag - kubernetes.podspec-init-containers
                     type: object
                     x-kubernetes-preserve-unknown-fields: true
-                loadBalancingPolicy:
-                  description: |-
-                    LoadBalancingPolicy is the load balancing algorithm used by the
-                    activator to route requests to application pods. If unspecified,
-                    a suggested default is applied depending on ContainerConcurrency
-                  type: string
                 nodeSelector:
                   description: |-
                     This is accessible behind a feature flag - kubernetes.podspec-nodeselector

config/core/300-resources/service.yaml

@@ -1045,12 +1045,6 @@ spec:
                             description: This is accessible behind a feature flag - kubernetes.podspec-init-containers
                             type: object
                             x-kubernetes-preserve-unknown-fields: true
-                        loadBalancingPolicy:
-                          description: |-
-                            LoadBalancingPolicy is the load balancing algorithm used by the
-                            activator to route requests to application pods. If unspecified,
-                            a suggested default is applied depending on ContainerConcurrency
-                          type: string
                         nodeSelector:
                           description: |-
                             This is accessible behind a feature flag - kubernetes.podspec-nodeselector

docs/serving-api.md

@@ -948,20 +948,6 @@ to stay open while not receiving any bytes from the user’s application. If
 unspecified, a system default will be provided.</p>
 </td>
 </tr>
-<tr>
-<td>
-<code>loadBalancingPolicy</code><br/>
-<em>
-string
-</em>
-</td>
-<td>
-<em>(Optional)</em>
-<p>LoadBalancingPolicy is the load balancing algorithm used by the
-activator to route requests to application pods. If unspecified,
-a suggested default is applied depending on ContainerConcurrency</p>
-</td>
-</tr>
 </table>
 </td>
 </tr>
@@ -1452,20 +1438,6 @@ to stay open while not receiving any bytes from the user&rsquo;s application. If
 unspecified, a system default will be provided.</p>
 </td>
 </tr>
-<tr>
-<td>
-<code>loadBalancingPolicy</code><br/>
-<em>
-string
-</em>
-</td>
-<td>
-<em>(Optional)</em>
-<p>LoadBalancingPolicy is the load balancing algorithm used by the
-activator to route requests to application pods. If unspecified,
-a suggested default is applied depending on ContainerConcurrency</p>
-</td>
-</tr>
 </tbody>
 </table>
 <h3 id="serving.knative.dev/v1.RevisionStatus">RevisionStatus
@@ -1694,20 +1666,6 @@ to stay open while not receiving any bytes from the user&rsquo;s application. If
 unspecified, a system default will be provided.</p>
 </td>
 </tr>
-<tr>
-<td>
-<code>loadBalancingPolicy</code><br/>
-<em>
-string
-</em>
-</td>
-<td>
-<em>(Optional)</em>
-<p>LoadBalancingPolicy is the load balancing algorithm used by the
-activator to route requests to application pods. If unspecified,
-a suggested default is applied depending on ContainerConcurrency</p>
-</td>
-</tr>
 </table>
 </td>
 </tr>

pkg/activator/net/throttler.go

@@ -601,9 +601,14 @@ func (t *Throttler) getOrCreateRevisionThrottler(revID types.NamespacedName) (*r
 		if err != nil {
 			return nil, err
 		}
+		// Get load balancing policy from annotation
+		var lbPolicy *string
+		if _, v, ok := serving.LoadBalancingPolicyAnnotation.Get(rev.GetAnnotations()); ok && v != "" {
+			lbPolicy = &v
+		}
 		revThrottler = newRevisionThrottler(
 			revID,
-			rev.Spec.LoadBalancingPolicy,
+			lbPolicy,
 			int(rev.Spec.GetContainerConcurrency()),
 			pkgnet.ServicePortName(rev.GetProtocol()),
 			queue.BreakerParams{QueueDepth: breakerQueueDepth, MaxConcurrency: revisionMaxConcurrency},
@@ -626,12 +631,17 @@ func (t *Throttler) revisionUpdated(obj any) {
 		t.logger.Errorw("Failed to get revision throttler for revision",
 			zap.Error(err), zap.String(logkey.Key, revID.String()))
 	} else if rt != nil {
-		// Update the lbPolicy dynamically if the revision's spec policy changed
+		// Update the lbPolicy dynamically if the revision's annotation policy changed
 		containerConcurrency := rev.Spec.GetContainerConcurrency()
 		if containerConcurrency < 0 {
 			containerConcurrency = 0
 		}
-		newPolicy, name := pickLBPolicy(rev.Spec.LoadBalancingPolicy, nil, int(containerConcurrency), t.logger)
+		// Get load balancing policy from annotation
+		var lbPolicy *string
+		if _, v, ok := serving.LoadBalancingPolicyAnnotation.Get(rev.GetAnnotations()); ok && v != "" {
+			lbPolicy = &v
+		}
+		newPolicy, name := pickLBPolicy(lbPolicy, nil, int(containerConcurrency), t.logger)
 		// Use atomic store for lock-free access in the hot request path
 		rt.lbPolicy.Store(newPolicy)
 		// Safe conversion: containerConcurrency is guaranteed to be non-negative after the check above

pkg/activator/net/throttler_test.go

@@ -1222,9 +1222,11 @@ func TestThrottlerUsesRevisionLoadBalancingPolicy(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test-revision-rc2",
 				Namespace: "test-namespace",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "random-choice-2",
+				},
 			},
 			Spec: v1.RevisionSpec{
-				LoadBalancingPolicy:  stringPtr("random-choice-2"),
 				ContainerConcurrency: ptr.Int64(10),
 			},
 		},
@@ -1235,9 +1237,11 @@ func TestThrottlerUsesRevisionLoadBalancingPolicy(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test-revision-rr",
 				Namespace: "test-namespace",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "round-robin",
+				},
 			},
 			Spec: v1.RevisionSpec{
-				LoadBalancingPolicy:  stringPtr("round-robin"),
 				ContainerConcurrency: ptr.Int64(10),
 			},
 		},
@@ -1248,9 +1252,11 @@ func TestThrottlerUsesRevisionLoadBalancingPolicy(t *testing.T) {
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "test-revision-lc",
 				Namespace: "test-namespace",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "least-connections",
+				},
 			},
 			Spec: v1.RevisionSpec{
-				LoadBalancingPolicy:  stringPtr("least-connections"),
 				ContainerConcurrency: ptr.Int64(10),
 			},
 		},
@@ -1354,9 +1360,12 @@ func TestDynamicLoadBalancingPolicyUpdate(t *testing.T) {
 		t.Fatalf("Unexpected distribution before update: %v", selections)
 	}
 
-	// Update the revision to set round-robin via spec and invoke revisionUpdated
+	// Update the revision to set round-robin via annotation and invoke revisionUpdated
 	rev = rev.DeepCopy()
-	rev.Spec.LoadBalancingPolicy = stringPtr("round-robin")
+	if rev.Annotations == nil {
+		rev.Annotations = make(map[string]string)
+	}
+	rev.Annotations["serving.knative.dev/load-balancing-policy"] = "round-robin"
 	// Update informer store and call revisionUpdated
 	revisions.Informer().GetIndexer().Update(rev)
 	throttler.revisionUpdated(rev)

pkg/apis/serving/register.go

@@ -144,6 +144,10 @@ const (
 
 	// ProgressDeadlineAnnotationKey is the label key for the per revision progress deadline to set for the deployment
 	ProgressDeadlineAnnotationKey = GroupName + "/progress-deadline"
+
+	// LoadBalancingPolicyKey is the annotation key for specifying the load balancing algorithm
+	// used by the activator to route requests to application pods.
+	LoadBalancingPolicyKey = GroupName + "/load-balancing-policy"
 )
 
 var (
@@ -202,4 +206,7 @@ var (
 	ProgressDeadlineAnnotation = kmap.KeyPriority{
 		ProgressDeadlineAnnotationKey,
 	}
+	LoadBalancingPolicyAnnotation = kmap.KeyPriority{
+		LoadBalancingPolicyKey,
+	}
 )

pkg/apis/serving/v1/revision_types.go

@@ -100,12 +100,6 @@ type RevisionSpec struct {
 	// unspecified, a system default will be provided.
 	// +optional
 	IdleTimeoutSeconds *int64 `json:"idleTimeoutSeconds,omitempty"`
-
-	// LoadBalancingPolicy is the load balancing algorithm used by the
-	// activator to route requests to application pods. If unspecified,
-	// a suggested default is applied depending on ContainerConcurrency
-	// +optional
-	LoadBalancingPolicy *string `json:"loadBalancingPolicy,omitempty"`
 }
 
 const (

pkg/apis/serving/v1/revision_validation.go

@@ -37,6 +37,7 @@ import (
 func (r *Revision) Validate(ctx context.Context) *apis.FieldError {
 	errs := serving.ValidateObjectMetadata(ctx, r.GetObjectMeta(), true).Also(
 		r.ValidateLabels().ViaField("labels")).ViaField("metadata")
+	errs = errs.Also(validateLoadBalancingPolicyAnnotation(r.GetAnnotations()).ViaField("metadata.annotations"))
 	errs = errs.Also(r.Status.Validate(apis.WithinStatus(ctx)).ViaField("status"))
 
 	if apis.IsInUpdate(ctx) {
@@ -72,6 +73,7 @@ func (rts *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError
 	errs = errs.Also(validateRevisionName(ctx, rts.Name, rts.GenerateName))
 	errs = errs.Also(validateQueueSidecarResourceAnnotations(rts.Annotations).ViaField("metadata.annotations"))
 	errs = errs.Also(validateProgressDeadlineAnnotation(rts.Annotations).ViaField("metadata.annotations"))
+	errs = errs.Also(validateLoadBalancingPolicyAnnotation(rts.Annotations).ViaField("metadata.annotations"))
 	return errs
 }
 
@@ -117,10 +119,6 @@ func (rs *RevisionSpec) Validate(ctx context.Context) *apis.FieldError {
 		errs = errs.Also(serving.ValidateContainerConcurrency(ctx, rs.ContainerConcurrency).ViaField("containerConcurrency"))
 	}
 
-	if rs.LoadBalancingPolicy != nil {
-		errs = errs.Also(serving.ValidateLoadBalancingPolicy(ctx, rs.LoadBalancingPolicy).ViaField("loadBalancingPolicy"))
-	}
-
 	return errs
 }
 
@@ -247,3 +245,14 @@ func validateProgressDeadlineAnnotation(annos map[string]string) *apis.FieldErro
 	}
 	return nil
 }
+
+// validateLoadBalancingPolicyAnnotation validates the load balancing policy annotation.
+func validateLoadBalancingPolicyAnnotation(annos map[string]string) *apis.FieldError {
+	if k, v, _ := serving.LoadBalancingPolicyAnnotation.Get(annos); v != "" {
+		if v != "round-robin" && v != "random-choice-2" && v != "least-connections" && v != "first-available" {
+			return apis.ErrInvalidValue(
+				v, k, "load balancing policy should be one of `random-choice-2`, `round-robin`, `least-connections` or `first-available`")
+		}
+	}
+	return nil
+}

pkg/apis/serving/v1/revision_validation_test.go

@@ -80,14 +80,16 @@ func TestRevisionValidation(t *testing.T) {
 		r: &Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "valid-lb-policy",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "round-robin",
+				},
 			},
 			Spec: RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: ptr.String("round-robin"),
 			},
 		},
 		want: nil,
@@ -96,14 +98,16 @@ func TestRevisionValidation(t *testing.T) {
 		r: &Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "valid-lb-policy",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "random-choice-2",
+				},
 			},
 			Spec: RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: ptr.String("random-choice-2"),
 			},
 		},
 		want: nil,
@@ -112,14 +116,16 @@ func TestRevisionValidation(t *testing.T) {
 		r: &Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "valid-lb-policy",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "least-connections",
+				},
 			},
 			Spec: RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: ptr.String("least-connections"),
 			},
 		},
 		want: nil,
@@ -128,14 +134,16 @@ func TestRevisionValidation(t *testing.T) {
 		r: &Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "valid-lb-policy",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "first-available",
+				},
 			},
 			Spec: RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: ptr.String("first-available"),
 			},
 		},
 		want: nil,
@@ -144,18 +152,20 @@ func TestRevisionValidation(t *testing.T) {
 		r: &Revision{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "invalid-lb-policy",
+				Annotations: map[string]string{
+					"serving.knative.dev/load-balancing-policy": "random",
+				},
 			},
 			Spec: RevisionSpec{
 				PodSpec: corev1.PodSpec{
 					Containers: []corev1.Container{{
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: ptr.String("random"),
 			},
 		},
 		want: apis.ErrInvalidValue(
-			"random", "spec.loadBalancingPolicy",
+			"random", "metadata.annotations.serving.knative.dev/load-balancing-policy",
 			"load balancing policy should be one of `random-choice-2`, `round-robin`, `least-connections` or `first-available`"),
 	}, {
 		name: "nil load balancing policy is valid",
@@ -169,7 +179,6 @@ func TestRevisionValidation(t *testing.T) {
 						Image: "busybox",
 					}},
 				},
-				LoadBalancingPolicy: nil,
 			},
 		},
 		want: nil,