Merge pull request #2389 from vmware-tanzu/agentConfig_priorityClassName

New configuration option for Concierge: kube cert agent `priorityClassName`

Author: joshuatcasey

deploy/concierge/deployment.yaml

@@ -3,7 +3,16 @@
 
 #@ load("@ytt:data", "data")
 #@ load("@ytt:json", "json")
-#@ load("helpers.lib.yaml", "defaultLabel", "labels", "deploymentPodLabel", "namespace", "defaultResourceName", "defaultResourceNameWithSuffix", "getAndValidateLogLevel", "pinnipedDevAPIGroupWithPrefix")
+#@ load("helpers.lib.yaml",
+#@   "defaultLabel",
+#@   "labels",
+#@   "deploymentPodLabel",
+#@   "namespace",
+#@   "defaultResourceName",
+#@   "defaultResourceNameWithSuffix",
+#@   "getAndValidateLogLevel",
+#@   "pinnipedDevAPIGroupWithPrefix",
+#@ )
 #@ load("@ytt:template", "template")
 
 #@ if not data.values.into_namespace:
@@ -84,6 +93,7 @@ data:
     labels: (@= json.encode(labels()).rstrip() @)
     kubeCertAgent:
       namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @)
+      priorityClassName: (@= data.values.kube_cert_agent_priority_class_name @)
       (@ if data.values.kube_cert_agent_image: @)
       image: (@= data.values.kube_cert_agent_image @)
       (@ else: @)
@@ -95,12 +105,12 @@ data:
       (@ end @)
       (@ if data.values.image_pull_dockerconfigjson: @)
       imagePullSecrets:
-        - image-pull-secret
+      - image-pull-secret
       (@ end @)
     (@ if data.values.log_level: @)
     log:
       level: (@= getAndValidateLogLevel() @)
-    (@ end @)
+      (@ end @)
     tls:
       onedottwo:
         allowedCiphers: (@= str(data.values.allowed_ciphers_for_tls_onedottwo) @)

deploy/concierge/helpers.lib.yaml

@@ -1,4 +1,4 @@
-#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
 #! SPDX-License-Identifier: Apache-2.0
 
 #@ load("@ytt:data", "data")

deploy/concierge/values.yaml

@@ -1,4 +1,4 @@
-#! Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
+#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
 #! SPDX-License-Identifier: Apache-2.0
 
 #@ def validate_strings_map(obj):
@@ -68,15 +68,24 @@ image_digest: ""
 image_tag: latest
 
 #@schema/title "Kube Cert Agent image"
-#@ kube_cert_agent_image = "Optionally specify a different image for the 'kube-cert-agent' pod which is scheduled \
+#@ kube_cert_agent_image_desc = "Optionally specify a different image for the 'kube-cert-agent' pod which is scheduled \
 #@ on the control plane. This image needs only to include `sleep` and `cat` binaries. \
 #@ By default, the same image specified for image_repo/image_digest/image_tag will be re-used."
-#@schema/desc kube_cert_agent_image
+#@schema/desc kube_cert_agent_image_desc
 #@schema/examples ("Image including tag or digest", "ghcr.io/vmware-tanzu/pinniped/pinniped-server:latest")
 #@schema/nullable
 #@schema/validation min_len=1
 kube_cert_agent_image: ""
 
+#@schema/title "Kube Cert Agent Priority Class Name"
+#@ kube_cert_agent_priority_class_name_desc = "Optionally specify a PriorityClassName for the 'kube-cert-agent' pod. \
+#@ See https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/ for more details. \
+#@ By default, this is the empty string."
+#@schema/desc kube_cert_agent_priority_class_name_desc
+#@schema/examples ("name of a PriorityClass object", "high-priority")
+#@schema/validation min_len=0
+kube_cert_agent_priority_class_name: ""
+
 #@schema/title "Image pull dockerconfigjson"
 #@ image_pull_dockerconfigjson_desc = "A base64 encoded secret to be used when pulling the `image_repo` container image. \
 #@ Can be used when the image_repo is a private registry. Typically, the value would be the output of: \

deploy/supervisor/deployment.yaml

@@ -1,4 +1,4 @@
-#! Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+#! Copyright 2020-2025 the Pinniped contributors. All Rights Reserved.
 #! SPDX-License-Identifier: Apache-2.0
 
 #@ load("@ytt:data", "data")
@@ -13,7 +13,7 @@
 #@   "pinnipedDevAPIGroupWithPrefix",
 #@   "getPinnipedConfigMapData",
 #@   "hasUnixNetworkEndpoint",
-#@  )
+#@ )
 #@ load("@ytt:template", "template")
 
 #@ if not data.values.into_namespace:

hack/module.sh

@@ -9,7 +9,7 @@ ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
 
 function usage() {
   echo "Error: <task> must be specified"
-  echo "       module.sh <task> [tidy, lint, test, unittest]"
+  echo "       module.sh <task> [tidy, lint, unit, unittest]"
   exit 1
 }
 
@@ -49,10 +49,12 @@ function main() {
     # Temporarily avoid using the race detector for the impersonator package due to https://github.com/kubernetes/kubernetes/issues/128548
     KUBE_CACHE_MUTATION_DETECTOR=${kube_cache_mutation_detector} \
       KUBE_PANIC_WATCH_DECODE_ERROR=${kube_panic_watch_decode_error} \
+      CGO_ENABLED=0 \
       go test -short -race $(go list ./... | grep -v internal/concierge/impersonator)
     # TODO: change this back to using the race detector everywhere
     KUBE_CACHE_MUTATION_DETECTOR=${kube_cache_mutation_detector} \
       KUBE_PANIC_WATCH_DECODE_ERROR=${kube_panic_watch_decode_error} \
+      CGO_ENABLED=0 \
       go test -short ./internal/concierge/impersonator
     ;;
   'generate')

internal/config/concierge/config.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"strings"
 
+	"k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/yaml"
 
@@ -85,6 +86,10 @@ func FromPath(ctx context.Context, path string, setAllowedCiphers ptls.SetAllowe
 		return nil, fmt.Errorf("validate names: %w", err)
 	}
 
+	if err := validateKubeCertAgent(&config.KubeCertAgentConfig); err != nil {
+		return nil, fmt.Errorf("validate kubeCertAgent: %w", err)
+	}
+
 	if err := plog.ValidateAndSetLogLevelAndFormatGlobally(ctx, config.Log); err != nil {
 		return nil, fmt.Errorf("validate log level: %w", err)
 	}
@@ -186,6 +191,22 @@ func validateNames(names *NamesConfigSpec) error {
 	return nil
 }
 
+func validateKubeCertAgent(agentConfig *KubeCertAgentSpec) error {
+	if len(agentConfig.PriorityClassName) == 0 {
+		// Optional, so empty is valid.
+		return nil
+	}
+
+	// See https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/#priorityclass
+	// for PriorityClassName rules.
+	errStrings := validation.IsDNS1123Subdomain(agentConfig.PriorityClassName)
+	if len(errStrings) > 0 {
+		// Always good enough to return the first error. IsDNS1123Subdomain only has two errors that it can return.
+		return fmt.Errorf("invalid priorityClassName: %s", errStrings[0])
+	}
+	return nil
+}
+
 func validateAPI(apiConfig *APIConfigSpec) error {
 	if *apiConfig.ServingCertificateConfig.DurationSeconds < *apiConfig.ServingCertificateConfig.RenewBeforeSeconds {
 		return constable.Error("durationSeconds cannot be smaller than renewBeforeSeconds")

internal/config/concierge/config_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -17,6 +18,9 @@ import (
 )
 
 func TestFromPath(t *testing.T) {
+	stringOfLength253 := strings.Repeat("a", 253)
+	stringOfLength254 := strings.Repeat("a", 254)
+
 	tests := []struct {
 		name                string
 		yaml                string
@@ -26,7 +30,7 @@ func TestFromPath(t *testing.T) {
 	}{
 		{
 			name: "Fully filled out",
-			yaml: here.Doc(`
+			yaml: here.Docf(`
 				---
 				discovery:
 				  url: https://some.discovery/url
@@ -64,6 +68,7 @@ func TestFromPath(t *testing.T) {
 				  namePrefix: kube-cert-agent-name-prefix-
 				  image: kube-cert-agent-image
 				  imagePullSecrets: [kube-cert-agent-image-pull-secret]
+				  priorityClassName: %s
 				log:
 				  level: debug
 				tls:
@@ -74,7 +79,7 @@ func TestFromPath(t *testing.T) {
 					- TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305
 				audit:
 				  logUsernamesAndGroups: enabled
-			`),
+			`, stringOfLength253),
 			wantConfig: &Config{
 				DiscoveryInfo: DiscoveryInfoSpec{
 					URL: ptr.To("https://some.discovery/url"),
@@ -112,9 +117,10 @@ func TestFromPath(t *testing.T) {
 					"myLabelKey2": "myLabelValue2",
 				},
 				KubeCertAgentConfig: KubeCertAgentSpec{
-					NamePrefix:       ptr.To("kube-cert-agent-name-prefix-"),
-					Image:            ptr.To("kube-cert-agent-image"),
-					ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"},
+					NamePrefix:        ptr.To("kube-cert-agent-name-prefix-"),
+					Image:             ptr.To("kube-cert-agent-image"),
+					ImagePullSecrets:  []string{"kube-cert-agent-image-pull-secret"},
+					PriorityClassName: stringOfLength253,
 				},
 				Log: plog.LogSpec{
 					Level: plog.LevelDebug,
@@ -173,6 +179,7 @@ func TestFromPath(t *testing.T) {
 				  namePrefix: kube-cert-agent-name-prefix-
 				  image: kube-cert-agent-image
 				  imagePullSecrets: [kube-cert-agent-image-pull-secret]
+				  priorityClassName: kube-cert-agent-priority-class-name
 				log:
 				  level: all
 				  format: json
@@ -216,9 +223,10 @@ func TestFromPath(t *testing.T) {
 					"myLabelKey2": "myLabelValue2",
 				},
 				KubeCertAgentConfig: KubeCertAgentSpec{
-					NamePrefix:       ptr.To("kube-cert-agent-name-prefix-"),
-					Image:            ptr.To("kube-cert-agent-image"),
-					ImagePullSecrets: []string{"kube-cert-agent-image-pull-secret"},
+					NamePrefix:        ptr.To("kube-cert-agent-name-prefix-"),
+					Image:             ptr.To("kube-cert-agent-image"),
+					ImagePullSecrets:  []string{"kube-cert-agent-image-pull-secret"},
+					PriorityClassName: "kube-cert-agent-priority-class-name",
 				},
 				Log: plog.LogSpec{
 					Level:  plog.LevelAll,
@@ -703,6 +711,50 @@ func TestFromPath(t *testing.T) {
 			`),
 			wantError: "validate audit: invalid logUsernamesAndGroups format, valid choices are 'enabled', 'disabled', or empty string (equivalent to 'disabled')",
 		},
+		{
+			name: "invalid kubeCertAgent.priorityClassName length",
+			yaml: here.Docf(`
+				---
+				names:
+				  servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
+				  credentialIssuer: pinniped-config
+				  apiService: pinniped-api
+				  impersonationLoadBalancerService: impersonationLoadBalancerService-value
+				  impersonationClusterIPService: impersonationClusterIPService-value
+				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
+				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  agentServiceAccount: agentServiceAccount-value
+				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
+				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
+				kubeCertAgent:
+				  priorityClassName: %s
+			`, stringOfLength254),
+			wantError: "validate kubeCertAgent: invalid priorityClassName: must be no more than 253 characters",
+		},
+		{
+			name: "invalid kubeCertAgent.priorityClassName format",
+			yaml: here.Doc(`
+				---
+				names:
+				  servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
+				  credentialIssuer: pinniped-config
+				  apiService: pinniped-api
+				  impersonationLoadBalancerService: impersonationLoadBalancerService-value
+				  impersonationClusterIPService: impersonationClusterIPService-value
+				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
+				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  impersonationSignerSecret: impersonationSignerSecret-value
+				  agentServiceAccount: agentServiceAccount-value
+				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
+				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
+				kubeCertAgent:
+				  priorityClassName: thisIsNotAValidPriorityClassName
+			`),
+			wantError: `validate kubeCertAgent: invalid priorityClassName: a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is '[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*')`,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {

internal/config/concierge/types.go

@@ -108,4 +108,7 @@ type KubeCertAgentSpec struct {
 	// ImagePullSecrets is a list of names of Kubernetes Secret objects that will be used as
 	// ImagePullSecrets on the kube-cert-agent pods.
 	ImagePullSecrets []string
+
+	// PriorityClassName optionally sets the PriorityClassName for the agent's pod.
+	PriorityClassName string `json:"priorityClassName"`
 }

internal/controller/kubecertagent/kubecertagent.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 // Package kubecertagent provides controllers that ensure a pod (the kube-cert-agent), is
@@ -101,6 +101,9 @@ type AgentConfig struct {
 	// DiscoveryURLOverride is the Kubernetes server endpoint to report in the CredentialIssuer, overriding any
 	// value discovered in the kube-public/cluster-info ConfigMap.
 	DiscoveryURLOverride *string
+
+	// PriorityClassName optionally sets the PriorityClassName for the agent's pod.
+	PriorityClassName string
 }
 
 // Only select using the unique label which will not match the pods of any other Deployment.
@@ -429,12 +432,14 @@ func (c *agentController) createOrUpdateDeployment(ctx context.Context, newestCo
 	updatedDeployment.ObjectMeta = mergeLabelsAndAnnotations(updatedDeployment.ObjectMeta, expectedDeployment.ObjectMeta)
 	desireSelectorUpdate := !apiequality.Semantic.DeepEqual(updatedDeployment.Spec.Selector, existingDeployment.Spec.Selector)
 	desireTemplateLabelsUpdate := !apiequality.Semantic.DeepEqual(updatedDeployment.Spec.Template.Labels, existingDeployment.Spec.Template.Labels)
+	// The user might want to set PriorityClassName back to the default value of empty string. DeepDerivative() won't detect this case below.
+	desirePriorityClassNameUpdate := updatedDeployment.Spec.Template.Spec.PriorityClassName != existingDeployment.Spec.Template.Spec.PriorityClassName
 
 	// If the existing Deployment already matches our desired spec, we're done.
 	if apiequality.Semantic.DeepDerivative(updatedDeployment, existingDeployment) {
 		// DeepDerivative allows the map fields of updatedDeployment to be a subset of existingDeployment,
 		// but we want to check that certain of those map fields are exactly equal before deciding to skip the update.
-		if !desireSelectorUpdate && !desireTemplateLabelsUpdate {
+		if !desireSelectorUpdate && !desireTemplateLabelsUpdate && !desirePriorityClassNameUpdate {
 			return nil // already equal enough, so skip update
 		}
 	}
@@ -661,7 +666,8 @@ func (c *agentController) newAgentDeployment(controllerManagerPod *corev1.Pod) *
 						RunAsUser:  ptr.To[int64](0),
 						RunAsGroup: ptr.To[int64](0),
 					},
-					HostNetwork: controllerManagerPod.Spec.HostNetwork,
+					HostNetwork:       controllerManagerPod.Spec.HostNetwork,
+					PriorityClassName: c.cfg.PriorityClassName,
 				},
 			},