refactor: Use controller manager to start runtime hooks server (#134)

Author: jimmidyson

charts/capi-runtime-extensions/README.md

@@ -28,21 +28,13 @@ A Helm chart for capi-runtime-extensions
 | certificates.issuer.kind | string | `"Issuer"` |  |
 | certificates.issuer.name | string | `""` |  |
 | certificates.issuer.selfSigned | bool | `true` |  |
-| controllers.enableLeaderElection | bool | `false` |  |
 | deployment.replicas | int | `1` |  |
 | env | object | `{}` |  |
-| handlers.AuditPolicyPatch.enabled | bool | `true` |  |
-| handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content | string | `""` |  |
-| handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.name | string | `"calico-cni-installation-dockercluster"` |  |
-| handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.create | bool | `true` |  |
-| handlers.CalicoCNI.defaultPodSubnet | string | `"192.168.0.0/16"` |  |
-| handlers.CalicoCNI.defaultTigeraOperatorConfigMap.name | string | `"tigera-operator"` |  |
-| handlers.CalicoCNI.enabled | bool | `true` |  |
-| handlers.ExtraAPIServerCertSANsPatch.enabled | bool | `true` |  |
-| handlers.ExtraAPIServerCertSANsVars.enabled | bool | `true` |  |
-| handlers.HTTPProxyPatch.enabled | bool | `true` |  |
-| handlers.HTTPProxyVars.enabled | bool | `true` |  |
-| handlers.ServiceLoadBalancerGC.enabled | bool | `true` |  |
+| hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content | string | `""` |  |
+| hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.name | string | `"calico-cni-installation-dockercluster"` |  |
+| hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.create | bool | `true` |  |
+| hooks.CalicoCNI.defaultPodSubnet | string | `"192.168.0.0/16"` |  |
+| hooks.CalicoCNI.defaultTigeraOperatorConfigMap.name | string | `"tigera-operator"` |  |
 | image.pullPolicy | string | `"IfNotPresent"` |  |
 | image.repository | string | `"ghcr.io/d2iq-labs/capi-runtime-extensions"` |  |
 | image.tag | string | `""` |  |

charts/capi-runtime-extensions/templates/cni/calico/manifests/docker/installation.yaml

@@ -1,15 +1,15 @@
 # Copyright 2023 D2iQ, Inc. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
 
-{{- if and .Values.handlers.CalicoCNI.enabled .Values.handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.create }}
+{{- if .Values.hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.create }}
 apiVersion: v1
 kind: ConfigMap
 metadata:
-  name: '{{ .Values.handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.name }}'
+  name: '{{ .Values.hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.name }}'
 data:
   calico-installation: |
-{{- if .Values.handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content -}}
-    {{ .Values.handlers.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content | nindent 4}}
+{{- if .Values.hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content -}}
+    {{ .Values.hooks.CalicoCNI.defaultInstallationConfigMaps.DockerCluster.configMap.content | nindent 4}}
 {{- else -}}
     # This section includes base Calico installation configuration.
     # For more information, see: https://docs.projectcalico.org/reference/installation/api
@@ -25,7 +25,7 @@ data:
         # Note: The ipPools section cannot be modified post-install.
         ipPools:
         - blockSize: 26
-          cidr: {{ .Values.handlers.CalicoCNI.defaultPodSubnet }}
+          cidr: {{ .Values.hooks.CalicoCNI.defaultPodSubnet }}
           encapsulation: VXLANCrossSubnet
           natOutgoing: Enabled
           nodeSelector: all()

charts/capi-runtime-extensions/templates/cni/calico/manifests/tigera-operator-configmap.yaml

@@ -8,4 +8,4 @@ data:
 kind: ConfigMap
 metadata:
   creationTimestamp: null
-  name: '{{ .Values.handlers.CalicoCNI.defaultTigeraOperatorConfigMap.name }}'
+  name: '{{ .Values.hooks.CalicoCNI.defaultTigeraOperatorConfigMap.name }}'

charts/capi-runtime-extensions/templates/deployment.yaml

@@ -28,12 +28,8 @@ spec:
         image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default $.Chart.AppVersion }}"
         imagePullPolicy: "{{ .Values.image.pullPolicy }}"
         args:
-        - --controllermanager.leader-elect={{ if gt (.Values.deployment.replicas | int) 1 }}true{{ else }}{{ .Values.controllers.enableLeaderElection }}{{ end }}
-        - --runtimehooks.cert-dir=/runtimehooks-certs/
-        {{- range $key, $value := .Values.handlers }}{{ if $value.enabled }}
-        - --runtimehooks.enabled-handlers={{ $key }}
-        {{ end }}{{- end }}
-        - --runtimehooks.calicocni.defaultsNamespace=$(POD_NAMESPACE)
+        - --webhook-cert-dir=/runtimehooks-certs/
+        - --calicocni.defaultsNamespace=$(POD_NAMESPACE)
         {{- range $key, $value := .Values.extraArgs }}
         - --{{ $key }}={{ $value }}
         {{- end }}

charts/capi-runtime-extensions/templates/role.yaml

@@ -18,6 +18,14 @@ rules:
   - patch
   - update
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - secrets
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - addons.cluster.x-k8s.io
   resources:

charts/capi-runtime-extensions/values.yaml

@@ -1,9 +1,8 @@
 # Copyright 2023 D2iQ, Inc. All rights reserved.
 # SPDX-License-Identifier: Apache-2.0
 
-handlers:
+hooks:
   CalicoCNI:
-    enabled: true
     defaultPodSubnet: 192.168.0.0/16
     defaultTigeraOperatorConfigMap:
       name: tigera-operator
@@ -13,25 +12,10 @@ handlers:
         configMap:
           name: calico-cni-installation-dockercluster
           content: ""
-  ServiceLoadBalancerGC:
-    enabled: true
-  HTTPProxyVars:
-    enabled: true
-  HTTPProxyPatch:
-    enabled: true
-  AuditPolicyPatch:
-    enabled: true
-  ExtraAPIServerCertSANsVars:
-    enabled: true
-  ExtraAPIServerCertSANsPatch:
-    enabled: true
 
 deployment:
   replicas: 1
 
-controllers:
-  enableLeaderElection: false
-
 image:
   repository: ghcr.io/d2iq-labs/capi-runtime-extensions
   tag: ""

cmd/capi-runtime-extensions/main.go

@@ -5,12 +5,12 @@ package main
 
 import (
 	"flag"
+	"fmt"
 	"net/http"
 	"os"
 	"time"
 
 	"github.com/spf13/pflag"
-	"golang.org/x/sync/errgroup"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
@@ -22,10 +22,11 @@ import (
 	capiv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	crsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1beta1"
 	ctrl "sigs.k8s.io/controller-runtime"
-	ctrclient "sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 
 	"github.com/d2iq-labs/capi-runtime-extensions/common/pkg/server"
-	"github.com/d2iq-labs/capi-runtime-extensions/internal/controllermanager"
 	"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/cni/calico"
 	"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/extraapiservercertsans"
 	"github.com/d2iq-labs/capi-runtime-extensions/pkg/handlers/httpproxy"
@@ -53,42 +54,42 @@ func main() {
 	// Creates a logger to be used during the main func.
 	setupLog := ctrl.Log.WithName("main")
 
-	controllers := controllermanager.New()
-
 	scheme := runtime.NewScheme()
 	utilruntime.Must(clientgoscheme.AddToScheme(scheme))
 	utilruntime.Must(crsv1.AddToScheme(scheme))
 	utilruntime.Must(capiv1.AddToScheme(scheme))
 
-	// Gets a client to access the Kubernetes cluster where this RuntimeExtension will be deployed to
-	restConfig, err := ctrl.GetConfig()
-	if err != nil {
-		setupLog.Error(err, "error getting config for the cluster")
-		os.Exit(1)
+	mgrOptions := &ctrl.Options{
+		Scheme: scheme,
+		Metrics: metricsserver.Options{
+			BindAddress: ":8080",
+		},
+		HealthProbeBindAddress: ":8081",
+		LeaderElection:         false,
 	}
 
-	client, err := ctrclient.New(restConfig, ctrclient.Options{Scheme: scheme})
-	if err != nil {
-		setupLog.Error(err, "error creating client to the cluster")
-		os.Exit(1)
-	}
+	pflag.CommandLine.StringVar(
+		&mgrOptions.Metrics.BindAddress,
+		"metrics-bind-address",
+		mgrOptions.Metrics.BindAddress,
+		"The address the metric endpoint binds to.",
+	)
+
+	pflag.CommandLine.StringVar(
+		&mgrOptions.HealthProbeBindAddress,
+		"health-probe-bind-address",
+		mgrOptions.HealthProbeBindAddress,
+		"The address the probe endpoint binds to.",
+	)
 
 	calicoCNIConfig := &calico.CalicoCNIConfig{}
 
-	runtimeWebhookServer := server.NewServer(
-		servicelbgc.New(client),
-		calico.New(client, calicoCNIConfig),
-		httpproxy.NewVariable(),
-		httpproxy.NewPatch(client),
-		extraapiservercertsans.NewVariable(),
-		extraapiservercertsans.NewPatch(),
-	)
+	runtimeWebhookServerOpts := server.NewServerOptions()
 
 	// Initialize and parse command line flags.
 	initFlags(pflag.CommandLine)
-	runtimeWebhookServer.AddFlags("runtimehooks", pflag.CommandLine)
-	controllers.AddFlags("controllermanager", pflag.CommandLine)
-	calicoCNIConfig.AddFlags("runtimehooks.calicocni", pflag.CommandLine)
+	runtimeWebhookServerOpts.AddFlags(pflag.CommandLine)
+	calicoCNIConfig.AddFlags("calicocni", pflag.CommandLine)
 	pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc)
 	pflag.CommandLine.AddGoFlagSet(flag.CommandLine)
 	pflag.Parse()
@@ -120,26 +121,45 @@ func main() {
 	}
 
 	signalCtx := ctrl.SetupSignalHandler()
-	g, ctx := errgroup.WithContext(signalCtx)
-
-	g.Go(func() error {
-		err := runtimeWebhookServer.Start(ctx)
-		if err != nil {
-			setupLog.Error(err, "unable to start runtime hooks wehook server")
-		}
-		return err
-	})
-
-	g.Go(func() error {
-		err := controllers.Start(ctx)
-		if err != nil {
-			setupLog.Error(err, "unable to start controller manager")
-		}
-		return err
-	})
-
-	if err := g.Wait(); err != nil {
-		setupLog.Error(err, "failed to run successfully")
+
+	mgr, err := newManager(mgrOptions)
+	if err != nil {
+		setupLog.Error(err, "failed to create a new controller manager")
 		os.Exit(1)
 	}
+
+	runtimeWebhookServer := server.NewServer(
+		runtimeWebhookServerOpts,
+		servicelbgc.New(mgr.GetClient()),
+		calico.New(mgr.GetClient(), calicoCNIConfig),
+		httpproxy.NewVariable(),
+		httpproxy.NewPatch(mgr.GetClient()),
+		extraapiservercertsans.NewVariable(),
+		extraapiservercertsans.NewPatch(),
+	)
+	if err := mgr.Add(runtimeWebhookServer); err != nil {
+		setupLog.Error(err, "unable to add runtime webhook server runnable to controller manager")
+		os.Exit(1)
+	}
+
+	if err := mgr.Start(signalCtx); err != nil {
+		setupLog.Error(err, "unable to start controller manager")
+		os.Exit(1)
+	}
+}
+
+func newManager(opts *manager.Options) (ctrl.Manager, error) {
+	mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), *opts)
+	if err != nil {
+		return nil, fmt.Errorf("unable to create manager: %w", err)
+	}
+
+	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
+		return nil, fmt.Errorf("unable to set up health check: %w", err)
+	}
+	if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
+		return nil, fmt.Errorf("unable to set up ready check: %w", err)
+	}
+
+	return mgr, nil
 }

common/pkg/server/server.go

@@ -5,7 +5,6 @@ package server
 
 import (
 	"context"
-	"slices"
 	"strings"
 
 	"github.com/spf13/pflag"
@@ -20,51 +19,43 @@ import (
 )
 
 type Server struct {
-	allExtensionHandlers []handlers.Named
-
-	webhookPort    int
-	webhookCertDir string
-
 	catalog *runtimecatalog.Catalog
+	hooks   []handlers.Named
 
-	enabledHandlers []string
+	opts *ServerOptions
 }
 
-func NewServer(extensionHandlers ...handlers.Named) *Server {
+func NewServer(opts *ServerOptions, hooks ...handlers.Named) *Server {
 	// catalog contains all information about RuntimeHooks.
 	catalog := runtimecatalog.New()
 
 	_ = runtimehooksv1.AddToCatalog(catalog)
 
 	return &Server{
-		allExtensionHandlers: extensionHandlers,
-		catalog:              catalog,
-		webhookPort:          9443,
-		webhookCertDir:       "/runtimehooks-certs/",
+		catalog: catalog,
+		opts:    opts,
+		hooks:   hooks,
 	}
 }
 
-func (s *Server) AddFlags(prefix string, fs *pflag.FlagSet) {
-	fs.IntVar(&s.webhookPort, prefix+".port", s.webhookPort, "Webhook Server port")
+type ServerOptions struct {
+	webhookPort    int
+	webhookCertDir string
+}
+
+func NewServerOptions() *ServerOptions {
+	return &ServerOptions{}
+}
+
+func (s *ServerOptions) AddFlags(fs *pflag.FlagSet) {
+	fs.IntVar(&s.webhookPort, "webhook-port", s.webhookPort, "Webhook Server port")
 
 	fs.StringVar(
 		&s.webhookCertDir,
-		prefix+".cert-dir",
+		"webhook-cert-dir",
 		s.webhookCertDir,
 		"Runtime hooks server cert dir.",
 	)
-
-	handlerNames := make([]string, 0, len(s.allExtensionHandlers))
-	for _, h := range s.allExtensionHandlers {
-		handlerNames = append(handlerNames, h.Name())
-	}
-
-	fs.StringSliceVar(
-		&s.enabledHandlers,
-		prefix+".enabled-handlers",
-		handlerNames,
-		"list of all enabled handlers",
-	)
 }
 
 func (s *Server) Start(ctx context.Context) error {
@@ -74,20 +65,16 @@ func (s *Server) Start(ctx context.Context) error {
 	// Create a http server for serving runtime extensions
 	webhookServer, err := server.New(server.Options{
 		Catalog: s.catalog,
-		Port:    s.webhookPort,
-		CertDir: s.webhookCertDir,
+		Port:    s.opts.webhookPort,
+		CertDir: s.opts.webhookCertDir,
 	})
 	if err != nil {
 		setupLog.Error(err, "error creating webhook server")
 		return err
 	}
 
-	for idx := range s.allExtensionHandlers {
-		h := s.allExtensionHandlers[idx]
-
-		if !slices.Contains(s.enabledHandlers, h.Name()) {
-			continue
-		}
+	for idx := range s.hooks {
+		h := s.hooks[idx]
 
 		if t, ok := h.(lifecycle.BeforeClusterCreate); ok {
 			if err := webhookServer.AddExtensionHandler(server.ExtensionHandler{