feat: Move Pdb checks to State server

In certain Networing scenarios, if the webhook is running in the host
network it might not be able to connect to the other servers.

For this reason we're isolating all network calls in between servers to
not happen from the webhook pod.

Signed-off-by: Samuel Torres <[email protected]>

Author: samuel-form3

charts/x-pdb/templates/_helpers.tpl

@@ -98,3 +98,11 @@ Create the name of the service account to use
 {{- default "default" .Values.serviceAccount.name }}
 {{- end }}
 {{- end }}
+
+
+{{/*
+Name of the service for the state service
+*/}}
+{{- define "x-pdb.stateServiceName" -}}
+{{ default (printf "%s-state" (include "x-pdb.fullname" .)) .Values.state.service.nameOverride }}
+{{- end }}

charts/x-pdb/templates/certificates.yaml

@@ -38,7 +38,13 @@ metadata:
     {{- include "x-pdb.stateLabels" . | nindent 4 }}
 spec:
   dnsNames:
-    {{- toYaml .Values.certificates.state.certManager.dnsNames | nindent 4 }}
+    - {{ include "x-pdb.stateServiceName" . }}
+    - {{ include "x-pdb.stateServiceName" . }}.{{ include "x-pdb.namespace" . }}
+    - {{ include "x-pdb.stateServiceName" . }}.{{ include "x-pdb.namespace" . }}.svc
+    - {{ include "x-pdb.stateServiceName" . }}.{{ include "x-pdb.namespace" . }}.svc.cluster.local
+    {{- range $name:= .Values.certificates.state.certManager.dnsNames }}
+    - {{ $name }}
+    {{- end }}
   ipAddresses:
     {{- toYaml .Values.certificates.state.certManager.ipAddresses | nindent 4 }}
   issuerRef:

charts/x-pdb/templates/state/deployment.yaml

@@ -47,6 +47,7 @@ spec:
           - "--zap-log-level={{ .Values.state.log.level }}"
           - "--state-certs-dir=/tmp/state-cert"
           - "--state-port={{ .Values.state.port }}"
+          - "--remote-state-endpoints={{- join "," .Values.state.remoteStateEndpoints }}"
           - "--metrics-bind-address=:{{ .Values.state.metricsPort }}"
           - "--health-probe-bind-address=:{{ .Values.state.healthProbePort }}"
           {{- range $value := .Values.state.extraArgs }}

charts/x-pdb/templates/state/service.yaml

@@ -1,7 +1,7 @@
 apiVersion: v1
 kind: Service
 metadata:
-  name: {{ default (printf "%s-state" (include "x-pdb.fullname" .)) .Values.state.service.nameOverride }}
+  name: {{ include "x-pdb.stateServiceName" . }}
   namespace: {{ include "x-pdb.namespace" . }}
   labels:
     {{- include "x-pdb.stateLabels" . | nindent 4 }}

charts/x-pdb/templates/webhook/deployment.yaml

@@ -45,7 +45,7 @@ spec:
           args:
           - "--cluster-id={{ .Values.clusterID }}"
           - "--zap-log-level={{ .Values.webhook.log.level }}"
-          - "--remote-endpoints={{- join "," .Values.webhook.remoteEndpoints }}"
+          - "--local-state-endpoint={{ include "x-pdb.stateServiceName" . }}.{{ include "x-pdb.namespace" . }}.svc.cluster.local:443"
           - "--webhook-certs-dir=/tmp/webhook-cert"
           - "--state-certs-dir=/tmp/state-cert"
           - "--webhook-port={{ .Values.webhook.port }}"

charts/x-pdb/values.yaml

@@ -17,6 +17,7 @@ state:
   metricsPort: 8080
   log:
     level: info
+  remoteStateEndpoints: []
   extraArgs: []
     # - "--dry-run=true"
   hostNetwork: false

cmd/state/main.go

@@ -17,8 +17,11 @@ limitations under the License.
 package main
 
 import (
+	"errors"
 	"flag"
+	"fmt"
 	"os"
+	"strings"
 
 	"github.com/form3tech-oss/x-pdb/internal/lock"
 	"github.com/form3tech-oss/x-pdb/internal/pdb"
@@ -56,19 +59,21 @@ func init() {
 }
 
 func main() {
+	var clusterID string
+	var dryRun bool
+	var kubeContext string
+	var leaseNamespace string
 	var metricsAddr string
+	var podID string
 	var probeAddr string
+	var remoteStateEndpoints string
 	var stateCertsDir string
 	var statePort int
-	var leaseNamespace string
-	var podID string
-	var kubeContext string
-	var clusterID string
-	var dryRun bool
 	flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", "The address the metric endpoint binds to.")
 	flag.StringVar(&probeAddr, "health-probe-bind-address", ":8081", "The address the probe endpoint binds to.")
 	flag.StringVar(&stateCertsDir, "state-certs-dir", "", "The directory that contains state server certificates")
 	flag.IntVar(&statePort, "state-port", 9643, "The state server binding port")
+	flag.StringVar(&remoteStateEndpoints, "remote-state-endpoints", "", "The list of endpoints of the remote pdb controllers")
 	flag.StringVar(&leaseNamespace, "namespace", "kube-system", "the namespace in which the controller runs in")
 	flag.StringVar(&podID, "pod-id", os.Getenv("HOSTNAME"),
 		"The ID of the pod x-pdb pod. Used as prefix for the lease-holder-identity to obtain locks across clusters.",
@@ -113,6 +118,12 @@ func main() {
 		os.Exit(1)
 	}
 
+	remoteEndpointsList, err := parseEndpoints(remoteStateEndpoints)
+	if err != nil {
+		setupLog.Error(err, "unable to parse remote endpoints")
+		os.Exit(1)
+	}
+
 	stateClientPool := stateclient.NewClientPool(signalHandler, &logger, stateCertsDir)
 
 	lockService := lock.NewService(
@@ -121,7 +132,7 @@ func main() {
 		mgr.GetAPIReader(),
 		stateClientPool,
 		leaseNamespace,
-		[]string{},
+		remoteEndpointsList,
 	)
 
 	scaleFinder := pdb.NewScaleFinder(mgr.GetClient(), cli.DiscoveryClient)
@@ -131,14 +142,12 @@ func main() {
 		scaleFinder,
 		stateClientPool,
 		leaseNamespace,
-		[]string{})
+		remoteEndpointsList)
 
-	{
-		stateServer := stateserver.NewServer(pdbService, lockService, &logger, statePort, stateCertsDir)
-		if err := mgr.Add(stateServer); err != nil {
-			setupLog.Error(err, "unable to create state server")
-			os.Exit(1)
-		}
+	stateServer := stateserver.NewServer(mgr.GetClient(), pdbService, lockService, &logger, statePort, stateCertsDir)
+	if err := mgr.Add(stateServer); err != nil {
+		setupLog.Error(err, "unable to create state server")
+		os.Exit(1)
 	}
 
 	// +kubebuilder:scaffold:builder
@@ -157,3 +166,26 @@ func main() {
 		os.Exit(1)
 	}
 }
+
+func parseEndpoints(endpointString string) ([]string, error) {
+	//nolint:prealloc
+	var endpoints []string
+	var errs []error
+	splitEndpoints := strings.Split(endpointString, ",")
+
+	if len(splitEndpoints) == 1 && splitEndpoints[0] == "" {
+		return endpoints, nil
+	}
+
+	for _, ep := range splitEndpoints {
+		sanitizedEndpoint := strings.TrimSpace(ep)
+
+		if sanitizedEndpoint == "" {
+			errs = append(errs, fmt.Errorf("endpoint cannot be empty"))
+			continue
+		}
+
+		endpoints = append(endpoints, sanitizedEndpoint)
+	}
+	return endpoints, errors.Join(errs...)
+}

cmd/webhook/main.go

@@ -17,14 +17,10 @@ limitations under the License.
 package main
 
 import (
-	"errors"
 	"flag"
-	"fmt"
 	"os"
-	"strings"
 
 	"github.com/form3tech-oss/x-pdb/internal/disruptionprobe"
-	"github.com/form3tech-oss/x-pdb/internal/lock"
 	"github.com/form3tech-oss/x-pdb/internal/pdb"
 	"github.com/form3tech-oss/x-pdb/internal/preactivities"
 	stateclient "github.com/form3tech-oss/x-pdb/internal/state/client"
@@ -68,7 +64,7 @@ func main() {
 	var webhookCertsDir string
 	var webhookPort int
 	var stateCertsDir string
-	var remoteEndpoints string
+	var localStateEndpoint string
 	var leaseNamespace string
 	var podID string
 	var kubeContext string
@@ -79,7 +75,7 @@ func main() {
 	flag.StringVar(&webhookCertsDir, "webhook-certs-dir", "", "The directory that contains webhook certificates")
 	flag.IntVar(&webhookPort, "webhook-port", 9443, "The webhook binding port")
 	flag.StringVar(&stateCertsDir, "state-certs-dir", "", "The directory that contains state server certificates")
-	flag.StringVar(&remoteEndpoints, "remote-endpoints", "", "The list of endpoints of the remote pdb controllers")
+	flag.StringVar(&localStateEndpoint, "local-state-endpoint", "x-pdb", "The address the probe endpoint binds to.")
 	flag.StringVar(&leaseNamespace, "namespace", "kube-system", "the namespace in which the controller runs in")
 	flag.StringVar(&podID, "pod-id", os.Getenv("HOSTNAME"),
 		"The ID of the pod x-pdb pod. Used as prefix for the lease-holder-identity to obtain locks across clusters.",
@@ -124,23 +120,13 @@ func main() {
 		os.Exit(1)
 	}
 
-	remoteEndpointsList, err := parseEndpoints(remoteEndpoints)
+	stateClientPool := stateclient.NewClientPool(signalHandler, &logger, stateCertsDir)
+	stateClient, err := stateClientPool.Get(localStateEndpoint)
 	if err != nil {
-		setupLog.Error(err, "unable to parse remote endpoints")
+		setupLog.Error(err, "unable to get a state client")
 		os.Exit(1)
 	}
 
-	stateClientPool := stateclient.NewClientPool(signalHandler, &logger, stateCertsDir)
-
-	lockService := lock.NewService(
-		&logger,
-		mgr.GetClient(),
-		mgr.GetAPIReader(),
-		stateClientPool,
-		leaseNamespace,
-		remoteEndpointsList,
-	)
-
 	disruptionProbeClientPool := disruptionprobe.NewClientPool(signalHandler, &logger, stateCertsDir)
 	disruptionProbeService := disruptionprobe.NewService(&logger, disruptionProbeClientPool)
 
@@ -151,37 +137,35 @@ func main() {
 		scaleFinder,
 		stateClientPool,
 		leaseNamespace,
-		remoteEndpointsList)
+		[]string{})
 
 	preactivitiesService := preactivities.NewService(logger, mgr.GetClient())
 
-	{
-		hookServer := &webhook.DefaultServer{
-			Options: webhook.Options{
-				Port:    webhookPort,
-				CertDir: webhookCertsDir,
-			},
-		}
-		if err := mgr.Add(hookServer); err != nil {
-			setupLog.Error(err, "unable to create pod mutator webhook server")
-			os.Exit(1)
-		}
-		decoder := admission.NewDecoder(mgr.GetScheme())
-		podValidationWebhook := webhooks.NewPodValidationWebhook(
-			mgr.GetClient(),
-			logger,
-			decoder,
-			mgr.GetEventRecorderFor("x-pdb"),
-			clusterID,
-			podID,
-			dryRun,
-			pdbService,
-			lockService,
-			disruptionProbeService,
-			preactivitiesService,
-		)
-		hookServer.Register("/validate", &webhook.Admission{Handler: podValidationWebhook})
+	hookServer := &webhook.DefaultServer{
+		Options: webhook.Options{
+			Port:    webhookPort,
+			CertDir: webhookCertsDir,
+		},
+	}
+	if err := mgr.Add(hookServer); err != nil {
+		setupLog.Error(err, "unable to create pod mutator webhook server")
+		os.Exit(1)
 	}
+	decoder := admission.NewDecoder(mgr.GetScheme())
+	podValidationWebhook := webhooks.NewPodValidationWebhook(
+		mgr.GetClient(),
+		logger,
+		decoder,
+		mgr.GetEventRecorderFor("x-pdb"),
+		clusterID,
+		podID,
+		dryRun,
+		stateClient,
+		pdbService,
+		disruptionProbeService,
+		preactivitiesService,
+	)
+	hookServer.Register("/validate", &webhook.Admission{Handler: podValidationWebhook})
 
 	// +kubebuilder:scaffold:builder
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
@@ -199,26 +183,3 @@ func main() {
 		os.Exit(1)
 	}
 }
-
-func parseEndpoints(endpointString string) ([]string, error) {
-	//nolint:prealloc
-	var endpoints []string
-	var errs []error
-	splitEndpoints := strings.Split(endpointString, ",")
-
-	if len(splitEndpoints) == 1 && splitEndpoints[0] == "" {
-		return endpoints, nil
-	}
-
-	for _, ep := range splitEndpoints {
-		sanitizedEndpoint := strings.TrimSpace(ep)
-
-		if sanitizedEndpoint == "" {
-			errs = append(errs, fmt.Errorf("endpoint cannot be empty"))
-			continue
-		}
-
-		endpoints = append(endpoints, sanitizedEndpoint)
-	}
-	return endpoints, errors.Join(errs...)
-}

hack/env/xpdb-values.yaml

@@ -7,7 +7,11 @@ webhook:
   extraArgs:
   - --zap-stacktrace-level=panic
   automountServiceAccountToken: true
+  log:
+    level: debug
 state:
+  log:
+    level: debug
   image:
     tag: "latest"
   extraArgs:

hack/install-xpdb.sh

@@ -38,7 +38,7 @@ fi
 helm upgrade -i x-pdb ./charts/x-pdb \
   -f "hack/env/xpdb-values.yaml" \
   --set clusterID="${CLUSTER}" \
-  --set webhook.remoteEndpoints="$remote_endpoints" \
+  --set state.remoteStateEndpoints="$remote_endpoints" \
   --set certificates.state.certManager.ipAddresses="{$this_address}" \
   --set state.service.loadBalancerIP="$this_address" \
   --kube-context="${CONTEXT}"