Merge pull request crossplane#5540 from Argannor/feat/configurable-ports

feat/configurable ports

Author: jbw976

cluster/charts/crossplane/README.md

@@ -85,6 +85,7 @@ and their default values.
 | `imagePullSecrets` | The imagePullSecret names to add to the Crossplane ServiceAccount. | `[]` |
 | `leaderElection` | Enable [leader election](https://docs.crossplane.io/latest/concepts/pods/#leader-election) for the Crossplane pod. | `true` |
 | `metrics.enabled` | Enable Prometheus path, port and scrape annotations and expose port 8080 for both the Crossplane and RBAC Manager pods. | `false` |
+| `metrics.port` | The port the metrics server listens on. | `""` |
 | `nodeSelector` | Add `nodeSelectors` to the Crossplane pod deployment. | `{}` |
 | `packageCache.configMap` | The name of a ConfigMap to use as the package cache. Disables the default package cache `emptyDir` Volume. | `""` |
 | `packageCache.medium` | Set to `Memory` to hold the package cache in a RAM backed file system. Useful for Crossplane development. | `""` |
@@ -105,6 +106,7 @@ and their default values.
 | `rbacManager.skipAggregatedClusterRoles` | Don't install aggregated Crossplane ClusterRoles. | `false` |
 | `rbacManager.tolerations` | Add `tolerations` to the RBAC Manager pod deployment. | `[]` |
 | `rbacManager.topologySpreadConstraints` | Add `topologySpreadConstraints` to the RBAC Manager pod deployment. | `[]` |
+| `readiness.port` | The port the readyz server listens on. | `""` |
 | `registryCaBundleConfig.key` | The ConfigMap key containing a custom CA bundle to enable fetching packages from registries with unknown or untrusted certificates. | `""` |
 | `registryCaBundleConfig.name` | The ConfigMap name containing a custom CA bundle to enable fetching packages from registries with unknown or untrusted certificates. | `""` |
 | `replicas` | The number of Crossplane pod `replicas` to deploy. | `1` |
@@ -132,6 +134,7 @@ and their default values.
 | `tolerations` | Add `tolerations` to the Crossplane pod deployment. | `[]` |
 | `topologySpreadConstraints` | Add `topologySpreadConstraints` to the Crossplane pod deployment. | `[]` |
 | `webhooks.enabled` | Enable webhooks for Crossplane and installed Provider packages. | `true` |
+| `webhooks.port` | The port the webhook server listens on. | `""` |
 
 ### Command Line
 

cluster/charts/crossplane/templates/deployment.yaml

@@ -141,14 +141,14 @@ spec:
             port: readyz
         ports:
         - name: readyz
-          containerPort: 8081
+          containerPort: {{ .Values.readiness.port | default 8081 }}
         {{- if .Values.metrics.enabled }}
         - name: metrics
-          containerPort: 8080
+          containerPort: {{ .Values.metrics.port | default 8080 }}
         {{- end }}
         {{- if .Values.webhooks.enabled }}
         - name: webhooks
-          containerPort: 9443
+          containerPort: {{ .Values.webhooks.port | default 9443 }}
         {{- end }}
         {{- with .Values.securityContextCrossplane }}
         securityContext:
@@ -185,6 +185,18 @@ spec:
           - name: "WEBHOOK_ENABLED"
             value: "false"
           {{- end }}
+          {{- if and .Values.webhooks.enabled .Values.webhooks.port }}
+          - name: "WEBHOOK_PORT"
+            value: "{{ .Values.webhooks.port }}"
+          {{- end}}
+          {{- if and .Values.metrics.enabled .Values.metrics.port }}
+          - name: "METRICS_PORT"
+            value: "{{ .Values.metrics.port }}"
+          {{- end}}
+          {{- if .Values.readiness.port }}
+          - name: "HEALTH_PROBE_PORT"
+            value: "{{ .Values.readiness.port }}"
+          {{- end}}
           - name: "TLS_SERVER_SECRET_NAME"
             value: crossplane-tls-server
           - name: "TLS_SERVER_CERTS_DIR"

cluster/charts/crossplane/templates/service.yaml

@@ -21,5 +21,5 @@ spec:
   ports:
   - protocol: TCP
     port: 9443
-    targetPort: 9443
+    targetPort: {{ .Values.webhooks.port | default 9443 }}
 {{- end }}

cluster/charts/crossplane/values.yaml

@@ -80,6 +80,8 @@ service:
 webhooks:
   # -- Enable webhooks for Crossplane and installed Provider packages.
   enabled: true
+  # -- The port the webhook server listens on.
+  port: ""
 
 rbacManager:
   # -- Deploy the RBAC Manager pod and its required roles.
@@ -167,6 +169,12 @@ securityContextRBACManager:
 metrics:
   # -- Enable Prometheus path, port and scrape annotations and expose port 8080 for both the Crossplane and RBAC Manager pods.
   enabled: false
+  # -- The port the metrics server listens on.
+  port: ""
+
+readiness:
+  # -- The port the readyz server listens on.
+  port: ""
 
 # -- Add custom environmental variables to the Crossplane pod deployment.
 # Replaces any `.` in a variable name with `_`. For example, `SAMPLE.KEY=value1` becomes `SAMPLE_KEY=value1`.

cmd/crossplane/core/core.go

@@ -38,6 +38,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	"github.com/crossplane/crossplane-runtime/pkg/certificates"
@@ -105,6 +106,10 @@ type startCommand struct {
 	WebhookEnabled                      bool `default:"true"  env:"WEBHOOK_ENABLED"                        help:"Enable webhook configuration."`
 	AutomaticDependencyDowngradeEnabled bool `default:"false" env:"AUTOMATIC_DEPENDENCY_DOWNGRADE_ENABLED" help:"Enable automatic dependency version downgrades. This configuration requires the 'EnableDependencyVersionUpgrades' feature flag to be enabled."`
 
+	WebhookPort     int `default:"9443" env:"WEBHOOK_PORT"      help:"The port the webhook server listens on."`
+	MetricsPort     int `default:"8080" env:"METRICS_PORT"      help:"The port the metrics server listens on."`
+	HealthProbePort int `default:"8081" env:"HEALTH_PROBE_PORT" help:"The port the health probe endpoint listens on."`
+
 	TLSServerSecretName string `env:"TLS_SERVER_SECRET_NAME" help:"The name of the TLS Secret that will store Crossplane's server certificate."`
 	TLSServerCertsDir   string `env:"TLS_SERVER_CERTS_DIR"   help:"The path of the folder which will store TLS server certificate of Crossplane."`
 	TLSClientSecretName string `env:"TLS_CLIENT_SECRET_NAME" help:"The name of the TLS Secret that will be store Crossplane's client certificate."`
@@ -165,15 +170,21 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error { //noli
 					t.MinVersion = tls.VersionTLS13
 				},
 			},
+			Port: c.WebhookPort,
 		}),
 		Client: client.Options{
 			Cache: &client.CacheOptions{
 				DisableFor:   []client.Object{&corev1.Secret{}},
 				Unstructured: false, // this is the default to not cache unstructured objects
 			},
 		},
+
 		EventBroadcaster: eb,
 
+		Metrics: metricsserver.Options{
+			BindAddress: fmt.Sprintf(":%d", c.MetricsPort),
+		},
+
 		// controller-runtime uses both ConfigMaps and Leases for leader
 		// election by default. Leases expire after 15 seconds, with a
 		// 10 second renewal deadline. We've observed leader loss due to
@@ -189,7 +200,7 @@ func (c *startCommand) Run(s *runtime.Scheme, log logging.Logger) error { //noli
 		RenewDeadline:                 func() *time.Duration { d := 50 * time.Second; return &d }(),
 
 		PprofBindAddress:       c.Profile,
-		HealthProbeBindAddress: ":8081",
+		HealthProbeBindAddress: fmt.Sprintf(":%d", c.HealthProbePort),
 	})
 	if err != nil {
 		return errors.Wrap(err, "cannot create manager")

internal/controller/pkg/revision/runtime.go

@@ -22,7 +22,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/crossplane/crossplane-runtime/pkg/meta"
 
@@ -44,6 +43,7 @@ const (
 
 	// See https://github.com/grpc/grpc/blob/v1.58.0/doc/naming.md
 	grpcPortName       = "grpc"
+	grpcPort           = 9443
 	servicePort        = 9443
 	serviceEndpointFmt = "dns:///%s.%s:%d"
 
@@ -283,14 +283,7 @@ func (b *RuntimeManifestBuilder) Service(overrides ...ServiceOverride) *corev1.S
 		// Overrides that we are opinionated about.
 		ServiceWithNamespace(b.namespace),
 		ServiceWithOwnerReferences([]metav1.OwnerReference{meta.AsController(meta.TypedReferenceTo(b.revision, b.revision.GetObjectKind().GroupVersionKind()))}),
-		ServiceWithSelectors(b.podSelectors()),
-		ServiceWithAdditionalPorts([]corev1.ServicePort{
-			{
-				Protocol:   corev1.ProtocolTCP,
-				Port:       servicePort,
-				TargetPort: intstr.FromInt32(servicePort),
-			},
-		}))
+		ServiceWithSelectors(b.podSelectors()))
 
 	// We append the overrides passed to the function last so that they can
 	// override the above ones.

internal/controller/pkg/revision/runtime_function.go

@@ -25,6 +25,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/crossplane/crossplane-runtime/pkg/errors"
@@ -94,7 +95,8 @@ func (h *FunctionHooks) Pre(ctx context.Context, _ runtime.Object, pr v1.Package
 	if !ok {
 		return errors.Errorf("cannot apply function package hooks to %T", pr)
 	}
-	fRev.Status.Endpoint = fmt.Sprintf(serviceEndpointFmt, svc.Name, svc.Namespace, servicePort)
+
+	fRev.Status.Endpoint = fmt.Sprintf(serviceEndpointFmt, svc.Name, svc.Namespace, grpcPort)
 
 	secServer := build.TLSServerSecret()
 	if err := h.client.Apply(ctx, secServer); err != nil {
@@ -183,7 +185,7 @@ func functionDeploymentOverrides(image string) []DeploymentOverride {
 		DeploymentRuntimeWithAdditionalPorts([]corev1.ContainerPort{
 			{
 				Name:          grpcPortName,
-				ContainerPort: servicePort,
+				ContainerPort: grpcPort,
 			},
 		}),
 	}
@@ -199,6 +201,14 @@ func functionServiceOverrides() []ServiceOverride {
 		// FunctionComposer) can load balance across the endpoints.
 		// https://kubernetes.io/docs/concepts/services-networking/service/#headless-services
 		ServiceWithClusterIP(corev1.ClusterIPNone),
+		ServiceWithAdditionalPorts([]corev1.ServicePort{
+			{
+				Name:       grpcPortName,
+				Protocol:   corev1.ProtocolTCP,
+				Port:       grpcPort,
+				TargetPort: intstr.FromString(grpcPortName),
+			},
+		}),
 	}
 }
 

internal/controller/pkg/revision/runtime_override_options.go

@@ -17,6 +17,8 @@ limitations under the License.
 package revision
 
 import (
+	"strconv"
+
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -99,9 +101,16 @@ func DeploymentWithOptionalPodScrapeAnnotations() DeploymentOverride {
 		if d.Spec.Template.Annotations == nil {
 			d.Spec.Template.Annotations = map[string]string{}
 		}
+		metricsPort := strconv.Itoa(metricsPortNumber)
+		for _, port := range d.Spec.Template.Spec.Containers[0].Ports {
+			if port.Name == metricsPortName {
+				metricsPort = strconv.Itoa(int(port.ContainerPort))
+				break
+			}
+		}
 		if _, ok := d.Spec.Template.Annotations["prometheus.io/scrape"]; !ok {
 			d.Spec.Template.Annotations["prometheus.io/scrape"] = "true"
-			d.Spec.Template.Annotations["prometheus.io/port"] = "8080"
+			d.Spec.Template.Annotations["prometheus.io/port"] = metricsPort
 			d.Spec.Template.Annotations["prometheus.io/path"] = "/metrics"
 		}
 	}
@@ -223,10 +232,19 @@ func DeploymentRuntimeWithAdditionalEnvironments(env []corev1.EnvVar) Deployment
 }
 
 // DeploymentRuntimeWithAdditionalPorts adds additional ports to the runtime
-// container of a Deployment.
+// container of a Deployment. A port will be added only if a port of the same name doesn't already exist.
 func DeploymentRuntimeWithAdditionalPorts(ports []corev1.ContainerPort) DeploymentOverride {
 	return func(d *appsv1.Deployment) {
-		d.Spec.Template.Spec.Containers[0].Ports = append(d.Spec.Template.Spec.Containers[0].Ports, ports...)
+		names := make(map[string]bool)
+		for _, p := range d.Spec.Template.Spec.Containers[0].Ports {
+			names[p.Name] = true
+		}
+		for _, p := range ports {
+			if _, ok := names[p.Name]; ok {
+				continue
+			}
+			d.Spec.Template.Spec.Containers[0].Ports = append(d.Spec.Template.Spec.Containers[0].Ports, p)
+		}
 	}
 }
 
@@ -401,10 +419,19 @@ func ServiceWithSelectors(selectors map[string]string) ServiceOverride {
 	}
 }
 
-// ServiceWithAdditionalPorts adds additional ports to a Service.
+// ServiceWithAdditionalPorts adds additional ports to a Service if no port with the same name was found.
 func ServiceWithAdditionalPorts(ports []corev1.ServicePort) ServiceOverride {
 	return func(s *corev1.Service) {
-		s.Spec.Ports = append(s.Spec.Ports, ports...)
+		names := make(map[string]bool)
+		for _, p := range s.Spec.Ports {
+			names[p.Name] = true
+		}
+		for _, p := range ports {
+			if _, ok := names[p.Name]; ok {
+				continue
+			}
+			s.Spec.Ports = append(s.Spec.Ports, p)
+		}
 	}
 }
 

internal/controller/pkg/revision/runtime_override_options_test.go

@@ -211,3 +211,108 @@ func TestDeploymentWithRuntimeContainer(t *testing.T) {
 		})
 	}
 }
+
+func TestDeploymentRuntimeWithAdditionalPorts(t *testing.T) {
+	type args struct {
+		deployment *appsv1.Deployment
+		ports      []corev1.ContainerPort
+	}
+	type want struct {
+		deployment *appsv1.Deployment
+	}
+
+	cases := map[string]struct {
+		reason string
+		args   args
+		want   want
+	}{
+		"NoPorts": {
+			reason: "Should add the given ports if no ports are set",
+			args: args{
+				deployment: &appsv1.Deployment{
+					Spec: appsv1.DeploymentSpec{
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{
+									{},
+								},
+							},
+						},
+					},
+				},
+				ports: []corev1.ContainerPort{
+					{ContainerPort: 80, Name: "http"},
+					{ContainerPort: 443, Name: "https"},
+				},
+			},
+			want: want{
+				deployment: &appsv1.Deployment{
+					Spec: appsv1.DeploymentSpec{
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{
+									{
+										Ports: []corev1.ContainerPort{
+											{ContainerPort: 80, Name: "http"},
+											{ContainerPort: 443, Name: "https"},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		"DontOverridePorts": {
+			reason: "Should add only new ports and not override existing ports",
+			args: args{
+				deployment: &appsv1.Deployment{
+					Spec: appsv1.DeploymentSpec{
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{
+									{
+										Ports: []corev1.ContainerPort{
+											{ContainerPort: 8080, Name: "http"},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				ports: []corev1.ContainerPort{
+					{ContainerPort: 80, Name: "http"},
+					{ContainerPort: 443, Name: "https"},
+				},
+			},
+			want: want{
+				deployment: &appsv1.Deployment{
+					Spec: appsv1.DeploymentSpec{
+						Template: corev1.PodTemplateSpec{
+							Spec: corev1.PodSpec{
+								Containers: []corev1.Container{
+									{
+										Ports: []corev1.ContainerPort{
+											{ContainerPort: 8080, Name: "http"},
+											{ContainerPort: 443, Name: "https"},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			DeploymentRuntimeWithAdditionalPorts(tc.args.ports)(tc.args.deployment)
+			if diff := cmp.Diff(tc.want.deployment, tc.args.deployment); diff != "" {
+				t.Errorf("\n%s\nDeploymentRuntimeWithAdditionalPorts(...): -want, +got:\n%s", tc.reason, diff)
+			}
+		})
+	}
+}