ImpersonationConfigController uses servicesinformer

This is a more reliable way to determine whether the load balancer
is already running.
Also added more unit tests for the load balancer.

Signed-off-by: Ryan Richard <richardry@vmware.com>

internal/controller/impersonatorconfig/impersonator_config.go

@@ -37,13 +37,13 @@ type impersonatorConfigController struct {
 	configMapResourceName            string
 	k8sClient                        kubernetes.Interface
 	configMapsInformer               corev1informers.ConfigMapInformer
+	servicesInformer                 corev1informers.ServiceInformer
 	generatedLoadBalancerServiceName string
 	labels                           map[string]string
 	startTLSListenerFunc             StartTLSListenerFunc
 	httpHandlerFactory               func() (http.Handler, error)
 
 	server               *http.Server
-	loadBalancer         *v1.Service
 	hasControlPlaneNodes *bool
 }
 
@@ -54,6 +54,7 @@ func NewImpersonatorConfigController(
 	configMapResourceName string,
 	k8sClient kubernetes.Interface,
 	configMapsInformer corev1informers.ConfigMapInformer,
+	servicesInformer corev1informers.ServiceInformer,
 	withInformer pinnipedcontroller.WithInformerOptionFunc,
 	withInitialEvent pinnipedcontroller.WithInitialEventOptionFunc,
 	generatedLoadBalancerServiceName string,
@@ -69,6 +70,7 @@ func NewImpersonatorConfigController(
 				configMapResourceName:            configMapResourceName,
 				k8sClient:                        k8sClient,
 				configMapsInformer:               configMapsInformer,
+				servicesInformer:                 servicesInformer,
 				generatedLoadBalancerServiceName: generatedLoadBalancerServiceName,
 				labels:                           labels,
 				startTLSListenerFunc:             startTLSListenerFunc,
@@ -80,6 +82,11 @@ func NewImpersonatorConfigController(
 			pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(configMapResourceName, namespace),
 			controllerlib.InformerOption{},
 		),
+		withInformer(
+			servicesInformer,
+			pinnipedcontroller.NameAndNamespaceExactMatchFilterFactory(generatedLoadBalancerServiceName, namespace),
+			controllerlib.InformerOption{},
+		),
 		// Be sure to run once even if the ConfigMap that the informer is watching doesn't exist.
 		withInitialEvent(controllerlib.Key{
 			Namespace: namespace,
@@ -128,26 +135,22 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error {
 		plog.Debug("Queried for control plane nodes", "foundControlPlaneNodes", hasControlPlaneNodes)
 	}
 
-	if (config.Mode == impersonator.ModeAuto && !*c.hasControlPlaneNodes) || config.Mode == impersonator.ModeEnabled {
+	if c.shouldHaveImpersonator(config) {
-		if err = c.startImpersonator(); err != nil {
+		if err = c.ensureImpersonatorIsStarted(); err != nil {
 			return err
 		}
 	} else {
-		if err = c.stopImpersonator(); err != nil {
+		if err = c.ensureImpersonatorIsStopped(); err != nil {
 			return err
 		}
 	}
 
-	// start the load balancer only if:
+	if c.shouldHaveLoadBalancer(config) {
-	// - the impersonator is running
+		if err = c.ensureLoadBalancerIsStarted(ctx.Context); err != nil {
-	// - the cluster is cloud hosted
-	// - there is no endpoint specified in the config
-	if c.server != nil && !*c.hasControlPlaneNodes && config.Endpoint == "" {
-		if err = c.startLoadBalancer(ctx.Context); err != nil {
 			return err
 		}
 	} else {
-		if err = c.stopLoadBalancer(ctx.Context); err != nil {
+		if err = c.ensureLoadBalancerIsStopped(ctx.Context); err != nil {
 			return err
 		}
 	}
@@ -155,7 +158,19 @@ func (c *impersonatorConfigController) Sync(ctx controllerlib.Context) error {
 	return nil
 }
 
-func (c *impersonatorConfigController) stopImpersonator() error {
+func (c *impersonatorConfigController) shouldHaveImpersonator(config *impersonator.Config) bool {
+	return (config.Mode == impersonator.ModeAuto && !*c.hasControlPlaneNodes) || config.Mode == impersonator.ModeEnabled
+}
+
+func (c *impersonatorConfigController) shouldHaveLoadBalancer(config *impersonator.Config) bool {
+	// start the load balancer only if:
+	// - the impersonator is running
+	// - the cluster is cloud hosted
+	// - there is no endpoint specified in the config
+	return c.server != nil && !*c.hasControlPlaneNodes && config.Endpoint == ""
+}
+
+func (c *impersonatorConfigController) ensureImpersonatorIsStopped() error {
 	if c.server != nil {
 		plog.Info("Stopping impersonation proxy", "port", impersonationProxyPort)
 		err := c.server.Close()
@@ -167,7 +182,7 @@ func (c *impersonatorConfigController) stopImpersonator() error {
 	return nil
 }
 
-func (c *impersonatorConfigController) startImpersonator() error {
+func (c *impersonatorConfigController) ensureImpersonatorIsStarted() error {
 	if c.server != nil {
 		return nil
 	}
@@ -210,22 +225,47 @@ func (c *impersonatorConfigController) startImpersonator() error {
 	return nil
 }
 
-func (c *impersonatorConfigController) stopLoadBalancer(ctx context.Context) error {
+func (c *impersonatorConfigController) isLoadBalancerRunning() (bool, error) {
-	if c.loadBalancer != nil {
+	_, err := c.servicesInformer.Lister().Services(c.namespace).Get(c.generatedLoadBalancerServiceName)
-		err := c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{})
+	notFound := k8serrors.IsNotFound(err)
-		if err != nil {
+	if notFound {
-			return err
+		return false, nil
-		}
 	}
-	return nil
+	if err != nil {
+		return false, err
+	}
+	return true, nil
 }
 
-func (c *impersonatorConfigController) startLoadBalancer(ctx context.Context) error {
+func (c *impersonatorConfigController) ensureLoadBalancerIsStopped(ctx context.Context) error {
-	if c.loadBalancer != nil {
+	running, err := c.isLoadBalancerRunning()
+	if err != nil {
+		return err
+	}
+	if !running {
 		return nil
 	}
 
-	appNameLabel := c.labels["app"] // TODO what if this doesn't exist
+	plog.Info("Deleting load balancer for impersonation proxy",
+		"service", c.generatedLoadBalancerServiceName,
+		"namespace", c.namespace)
+	err = c.k8sClient.CoreV1().Services(c.namespace).Delete(ctx, c.generatedLoadBalancerServiceName, metav1.DeleteOptions{})
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (c *impersonatorConfigController) ensureLoadBalancerIsStarted(ctx context.Context) error {
+	running, err := c.isLoadBalancerRunning()
+	if err != nil {
+		return err
+	}
+	if running {
+		return nil
+	}
+	appNameLabel := c.labels["app"]
 	loadBalancer := v1.Service{
 		Spec: v1.ServiceSpec{
 			Type: "LoadBalancer",
@@ -244,10 +284,12 @@ func (c *impersonatorConfigController) startLoadBalancer(ctx context.Context) er
 			Labels:    c.labels,
 		},
 	}
-	createdLoadBalancer, err := c.k8sClient.CoreV1().Services(c.namespace).Create(ctx, &loadBalancer, metav1.CreateOptions{})
+	plog.Info("creating load balancer for impersonation proxy",
+		"service", c.generatedLoadBalancerServiceName,
+		"namespace", c.namespace)
+	_, err = c.k8sClient.CoreV1().Services(c.namespace).Create(ctx, &loadBalancer, metav1.CreateOptions{})
 	if err != nil {
 		return fmt.Errorf("could not create load balancer: %w", err)
 	}
-	c.loadBalancer = createdLoadBalancer
 	return nil
 }

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -19,10 +19,12 @@ import (
 	"github.com/sclevine/spec/report"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	kubeinformers "k8s.io/client-go/informers"
+	corev1informers "k8s.io/client-go/informers/core/v1"
 	kubernetesfake "k8s.io/client-go/kubernetes/fake"
 	coretesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
@@ -64,17 +66,22 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
 		var observableWithInformerOption *testutil.ObservableWithInformerOption
 		var observableWithInitialEventOption *testutil.ObservableWithInitialEventOption
 		var configMapsInformerFilter controllerlib.Filter
+		var servicesInformerFilter controllerlib.Filter
 
 		it.Before(func() {
 			r = require.New(t)
 			observableWithInformerOption = testutil.NewObservableWithInformerOption()
 			observableWithInitialEventOption = testutil.NewObservableWithInitialEventOption()
-			configMapsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().ConfigMaps()
+			sharedInformerFactory := kubeinformers.NewSharedInformerFactory(nil, 0)
+			configMapsInformer := sharedInformerFactory.Core().V1().ConfigMaps()
+			servicesInformer := sharedInformerFactory.Core().V1().Services()
+
 			_ = NewImpersonatorConfigController(
 				installedInNamespace,
 				configMapResourceName,
 				nil,
 				configMapsInformer,
+				servicesInformer,
 				observableWithInformerOption.WithInformer,
 				observableWithInitialEventOption.WithInitialEvent,
 				generatedLoadBalancerServiceName,
@@ -83,6 +90,7 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
 				nil,
 			)
 			configMapsInformerFilter = observableWithInformerOption.GetFilterForInformer(configMapsInformer)
+			servicesInformerFilter = observableWithInformerOption.GetFilterForInformer(servicesInformer)
 		})
 
 		when("watching ConfigMap objects", func() {
@@ -133,6 +141,54 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
 			})
 		})
 
+		when("watching Service objects", func() {
+			var subject controllerlib.Filter
+			var target, wrongNamespace, wrongName, unrelated *corev1.Service
+
+			it.Before(func() {
+				subject = servicesInformerFilter
+				target = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: generatedLoadBalancerServiceName, Namespace: installedInNamespace}}
+				wrongNamespace = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: generatedLoadBalancerServiceName, Namespace: "wrong-namespace"}}
+				wrongName = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: installedInNamespace}}
+				unrelated = &corev1.Service{ObjectMeta: metav1.ObjectMeta{Name: "wrong-name", Namespace: "wrong-namespace"}}
+			})
+
+			when("the target Service changes", func() {
+				it("returns true to trigger the sync method", func() {
+					r.True(subject.Add(target))
+					r.True(subject.Update(target, unrelated))
+					r.True(subject.Update(unrelated, target))
+					r.True(subject.Delete(target))
+				})
+			})
+
+			when("a Service from another namespace changes", func() {
+				it("returns false to avoid triggering the sync method", func() {
+					r.False(subject.Add(wrongNamespace))
+					r.False(subject.Update(wrongNamespace, unrelated))
+					r.False(subject.Update(unrelated, wrongNamespace))
+					r.False(subject.Delete(wrongNamespace))
+				})
+			})
+
+			when("a Service with a different name changes", func() {
+				it("returns false to avoid triggering the sync method", func() {
+					r.False(subject.Add(wrongName))
+					r.False(subject.Update(wrongName, unrelated))
+					r.False(subject.Update(unrelated, wrongName))
+					r.False(subject.Delete(wrongName))
+				})
+			})
+
+			when("a Service with a different name and a different namespace changes", func() {
+				it("returns false to avoid triggering the sync method", func() {
+					r.False(subject.Add(unrelated))
+					r.False(subject.Update(unrelated, unrelated))
+					r.False(subject.Delete(unrelated))
+				})
+			})
+		})
+
 		when("starting up", func() {
 			it("asks for an initial event because the ConfigMap may not exist yet and it needs to run anyway", func() {
 				r.Equal(&controllerlib.Key{
@@ -233,6 +289,13 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			}, 10*time.Second, time.Millisecond)
 		}
 
+		var waitForLoadBalancerToBeDeleted = func(informer corev1informers.ServiceInformer, name string) {
+			r.Eventually(func() bool {
+				_, err := informer.Lister().Services(installedInNamespace).Get(name)
+				return k8serrors.IsNotFound(err)
+			}, 10*time.Second, time.Millisecond)
+		}
+
 		// Defer starting the informers until the last possible moment so that the
 		// nested Before's can keep adding things to the informer caches.
 		var startInformersAndController = func() {
@@ -242,6 +305,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				configMapResourceName,
 				kubeAPIClient,
 				kubeInformers.Core().V1().ConfigMaps(),
+				kubeInformers.Core().V1().Services(),
 				controllerlib.WithInformer,
 				controllerlib.WithInitialEvent,
 				generatedLoadBalancerServiceName,
@@ -307,6 +371,31 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			))
 		}
 
+		var addLoadBalancerServiceToTracker = func(resourceName string, client *kubernetesfake.Clientset) {
+			loadBalancerService := &corev1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      resourceName,
+					Namespace: installedInNamespace,
+					// Note that this seems to be ignored by the informer during initial creation, so actually
+					// the informer will see this as resource version "". Leaving it here to express the intent
+					// that the initial version is version 0.
+					ResourceVersion: "0",
+				},
+				Spec: corev1.ServiceSpec{
+					Type: corev1.ServiceTypeLoadBalancer,
+				},
+			}
+			r.NoError(client.Tracker().Add(loadBalancerService))
+		}
+
+		var deleteLoadBalancerServiceFromTracker = func(resourceName string, client *kubernetesfake.Clientset) {
+			r.NoError(client.Tracker().Delete(
+				schema.GroupVersionResource{Version: "v1", Resource: "services"},
+				installedInNamespace,
+				resourceName,
+			))
+		}
+
 		var addNodeWithRoleToTracker = func(role string) {
 			r.NoError(kubeAPIClient.Tracker().Add(
 				&corev1.Node{
@@ -318,6 +407,34 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			))
 		}
 
+		var requireNodesListed = func(action coretesting.Action) {
+			r.Equal(
+				coretesting.NewListAction(
+					schema.GroupVersionResource{Version: "v1", Resource: "nodes"},
+					schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
+					"",
+					metav1.ListOptions{}),
+				action,
+			)
+		}
+
+		var requireLoadBalancerWasCreated = func(action coretesting.Action) {
+			createLoadBalancerAction := action.(coretesting.CreateAction)
+			r.Equal("create", createLoadBalancerAction.GetVerb())
+			createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service)
+			r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name)
+			r.Equal(installedInNamespace, createdLoadBalancerService.Namespace)
+			r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type)
+			r.Equal("app-name", createdLoadBalancerService.Spec.Selector["app"])
+			r.Equal(labels, createdLoadBalancerService.Labels)
+		}
+
+		var requireLoadBalancerDeleted = func(action coretesting.Action) {
+			deleteLoadBalancerAction := action.(coretesting.DeleteAction)
+			r.Equal("delete", deleteLoadBalancerAction.GetVerb())
+			r.Equal(generatedLoadBalancerServiceName, deleteLoadBalancerAction.GetName())
+		}
+
 		it.Before(func() {
 			r = require.New(t)
 
@@ -345,10 +462,29 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					addNodeWithRoleToTracker("control-plane")
 				})
 
-				it("does not start the impersonator", func() {
+				it("does not start the impersonator or load balancer", func() {
 					startInformersAndController()
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 					requireTLSServerWasNeverStarted()
+					r.Equal(1, len(kubeAPIClient.Actions()))
+					requireNodesListed(kubeAPIClient.Actions()[0])
+				})
+			})
+
+			when("there are visible control plane nodes and a loadbalancer", func() {
+				it.Before(func() {
+					addNodeWithRoleToTracker("control-plane")
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient)
+				})
+
+				it("does not start the impersonator, deletes the loadbalancer", func() {
+					startInformersAndController()
+					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+					requireTLSServerWasNeverStarted()
+					r.Equal(2, len(kubeAPIClient.Actions()))
+					requireNodesListed(kubeAPIClient.Actions()[0])
+					requireLoadBalancerDeleted(kubeAPIClient.Actions()[1])
 				})
 			})
 
@@ -364,17 +500,28 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				})
 
 				it("starts the load balancer automatically", func() {
-					// action 0: list nodes
+					r.Equal(2, len(kubeAPIClient.Actions()))
-					// action 1: create load balancer
+					requireNodesListed(kubeAPIClient.Actions()[0])
-					// that should be all
+					requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
-					createLoadBalancerAction := kubeAPIClient.Actions()[1].(coretesting.CreateAction)
+				})
-					r.Equal("create", createLoadBalancerAction.GetVerb())
+			})
-					createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service)
+
-					r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name)
+			when("there are not visible control plane nodes and a load balancer already exists", func() {
-					r.Equal(installedInNamespace, createdLoadBalancerService.Namespace)
+				it.Before(func() {
-					r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type)
+					addNodeWithRoleToTracker("worker")
-					r.Equal("app-name", createdLoadBalancerService.Spec.Selector["app"])
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
-					r.Equal(labels, createdLoadBalancerService.Labels)
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient)
+					startInformersAndController()
+					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+				})
+
+				it("automatically starts the impersonator", func() {
+					requireTLSServerIsRunning()
+				})
+
+				it("does not start the load balancer automatically", func() {
+					r.Equal(1, len(kubeAPIClient.Actions()))
+					requireNodesListed(kubeAPIClient.Actions()[0])
 				})
 			})
 		})
@@ -388,14 +535,12 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				startInformersAndController()
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 				r.Equal(2, len(kubeAPIClient.Actions()))
-				r.Equal(
+				requireNodesListed(kubeAPIClient.Actions()[0])
-					coretesting.NewListAction(
+				requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
-						schema.GroupVersionResource{Version: "v1", Resource: "nodes"},
+
-						schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Node"},
+				// update manually because the kubeAPIClient isn't connected to the informer in the tests
-						"",
+				addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
-						metav1.ListOptions{}),
+				waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0")
-					kubeAPIClient.Actions()[0],
-				)
 
 				r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 				r.Equal(1, startTLSListenerFuncWasCalled) // wasn't started a second time
@@ -456,6 +601,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 						startInformersAndController()
 						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 						requireTLSServerWasNeverStarted()
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						r.Equal(1, len(kubeAPIClient.Actions()))
 					})
 				})
 
@@ -468,6 +615,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 						startInformersAndController()
 						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 						requireTLSServerIsRunning()
+						requireNodesListed(kubeAPIClient.Actions()[0])
 						r.Equal(1, len(kubeAPIClient.Actions()))
 					})
 				})
@@ -483,33 +631,121 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					startInformersAndController()
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 					requireTLSServerWasNeverStarted()
+					requireNodesListed(kubeAPIClient.Actions()[0])
+					r.Equal(1, len(kubeAPIClient.Actions()))
 				})
 			})
 
 			when("the configuration is enabled mode", func() {
-				it.Before(func() {
+
-					addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled")
+				when("there are control plane nodes", func() {
-					addNodeWithRoleToTracker("control-plane")
+					it.Before(func() {
+						addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled")
+						addNodeWithRoleToTracker("control-plane")
+					})
+
+					it("starts the impersonator", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						requireTLSServerIsRunning()
+					})
+
+					it("returns an error when the tls listener fails to start", func() {
+						startTLSListenerFuncError = errors.New("tls error")
+						startInformersAndController()
+						r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error")
+					})
+
+					it("does not start the load balancer", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						r.Equal(1, len(kubeAPIClient.Actions()))
+						requireNodesListed(kubeAPIClient.Actions()[0])
+					})
 				})
 
-				it("starts the impersonator regardless of the visibility of control plane nodes", func() {
+				when("there are no control plane nodes but a loadbalancer already exists", func() {
-					startInformersAndController()
+					it.Before(func() {
-					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled")
-					requireTLSServerIsRunning()
+						addNodeWithRoleToTracker("worker")
+						addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
+						addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient)
+					})
+
+					it("starts the impersonator", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						requireTLSServerIsRunning()
+					})
+
+					it("returns an error when the tls listener fails to start", func() {
+						startTLSListenerFuncError = errors.New("tls error")
+						startInformersAndController()
+						r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error")
+					})
+
+					it("does not start the load balancer", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						r.Equal(1, len(kubeAPIClient.Actions()))
+						requireNodesListed(kubeAPIClient.Actions()[0])
+					})
 				})
 
-				it("returns an error when the tls listener fails to start", func() {
+				when("there are control plane nodes and a loadbalancer already exists", func() {
-					startTLSListenerFuncError = errors.New("tls error")
+					it.Before(func() {
-					startInformersAndController()
+						addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled")
-					r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error")
+						addNodeWithRoleToTracker("control-plane")
+						addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
+						addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeAPIClient)
+					})
+
+					it("starts the impersonator", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						requireTLSServerIsRunning()
+					})
+
+					it("returns an error when the tls listener fails to start", func() {
+						startTLSListenerFuncError = errors.New("tls error")
+						startInformersAndController()
+						r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error")
+					})
+
+					it("stops the load balancer", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						r.Equal(2, len(kubeAPIClient.Actions()))
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						requireLoadBalancerDeleted(kubeAPIClient.Actions()[1])
+					})
 				})
 
-				it("does not start the load balancer if there are control plane nodes", func() {
+				when("there are no control plane nodes and there is no load balancer", func() {
-					startInformersAndController()
+					it.Before(func() {
-					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						addImpersonatorConfigMapToTracker(configMapResourceName, "mode: enabled")
-					// action 0: list nodes
+						addNodeWithRoleToTracker("worker")
-					// that should be all
+					})
-					r.Equal(1, len(kubeAPIClient.Actions()))
+
+					it("starts the impersonator", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						requireTLSServerIsRunning()
+					})
+
+					it("returns an error when the tls listener fails to start", func() {
+						startTLSListenerFuncError = errors.New("tls error")
+						startInformersAndController()
+						r.EqualError(controllerlib.TestSync(t, subject, *syncContext), "tls error")
+					})
+
+					it("starts the load balancer", func() {
+						startInformersAndController()
+						r.NoError(controllerlib.TestSync(t, subject, *syncContext))
+						r.Equal(2, len(kubeAPIClient.Actions()))
+						requireNodesListed(kubeAPIClient.Actions()[0])
+						requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
+					})
 				})
 			})
 
@@ -524,33 +760,32 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 					requireTLSServerIsRunning()
-					// TODO extract this
+					r.Equal(2, len(kubeAPIClient.Actions()))
-					// action 0: list nodes
+					requireNodesListed(kubeAPIClient.Actions()[0])
-					// action 1: create load balancer
+					requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
-					// that should be all
+
-					createLoadBalancerAction := kubeAPIClient.Actions()[1].(coretesting.CreateAction)
+					// update manually because the kubeAPIClient isn't connected to the informer in the tests
-					r.Equal("create", createLoadBalancerAction.GetVerb())
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
-					createdLoadBalancerService := createLoadBalancerAction.GetObject().(*corev1.Service)
+					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0")
-					r.Equal(generatedLoadBalancerServiceName, createdLoadBalancerService.Name)
-					r.Equal(installedInNamespace, createdLoadBalancerService.Namespace)
-					r.Equal(corev1.ServiceTypeLoadBalancer, createdLoadBalancerService.Spec.Type)
-					r.Equal("app-name", createdLoadBalancerService.Spec.Selector["app"])
-					r.Equal(labels, createdLoadBalancerService.Labels)
 
 					updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: disabled", "1")
 					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1")
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 					requireTLSServerIsNoLongerRunning()
-					deleteLoadBalancerAction := kubeAPIClient.Actions()[2].(coretesting.DeleteAction)
+					r.Equal(3, len(kubeAPIClient.Actions()))
-					r.Equal("delete", deleteLoadBalancerAction.GetVerb())
+					requireLoadBalancerDeleted(kubeAPIClient.Actions()[2])
-					r.Equal(generatedLoadBalancerServiceName, deleteLoadBalancerAction.GetName())
+
+					deleteLoadBalancerServiceFromTracker(generatedLoadBalancerServiceName, kubeInformerClient)
+					waitForLoadBalancerToBeDeleted(kubeInformers.Core().V1().Services(), generatedLoadBalancerServiceName)
 
 					updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", "2")
 					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2")
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 					requireTLSServerIsRunning()
+					r.Equal(4, len(kubeAPIClient.Actions()))
+					requireLoadBalancerWasCreated(kubeAPIClient.Actions()[3])
 				})
 
 				when("there is an error while shutting down the server", func() {
@@ -572,7 +807,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				})
 			})
 
-			when("the endpoint switches from not specified, to specified, to not specified", func() {
+			when("the endpoint switches from specified, to not specified, to specified", func() {
 				it.Before(func() {
 					addImpersonatorConfigMapToTracker(configMapResourceName, here.Doc(`
 						mode: enabled
@@ -581,22 +816,24 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					addNodeWithRoleToTracker("worker")
 				})
 
-				it("starts, stops, restarts the loadbalancer", func() {
+				it("doesn't start, then creates, then deletes the load balancer", func() {
 					startInformersAndController()
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
 
-					loadBalancer, err := kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{})
+					r.Equal(1, len(kubeAPIClient.Actions()))
-					r.Nil(loadBalancer)
+					requireNodesListed(kubeAPIClient.Actions()[0])
-					r.EqualError(err, "services \"some-service-resource-name\" not found")
 
 					updateImpersonatorConfigMapInTracker(configMapResourceName, "mode: enabled", "1")
 					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "1")
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
-					loadBalancer, err = kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{})
+					r.Equal(2, len(kubeAPIClient.Actions()))
-					r.NotNil(loadBalancer)
+					requireLoadBalancerWasCreated(kubeAPIClient.Actions()[1])
-					r.NoError(err, "services \"some-service-resource-name\" not found")
+
+					// update manually because the kubeAPIClient isn't connected to the informer in the tests
+					addLoadBalancerServiceToTracker(generatedLoadBalancerServiceName, kubeInformerClient)
+					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().Services().Informer(), "0")
 
 					updateImpersonatorConfigMapInTracker(configMapResourceName, here.Doc(`
 						mode: enabled
@@ -605,9 +842,8 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 					waitForInformerCacheToSeeResourceVersion(kubeInformers.Core().V1().ConfigMaps().Informer(), "2")
 
 					r.NoError(controllerlib.TestSync(t, subject, *syncContext))
-					loadBalancer, err = kubeAPIClient.CoreV1().Services(installedInNamespace).Get(context.Background(), generatedLoadBalancerServiceName, metav1.GetOptions{})
+					r.Equal(3, len(kubeAPIClient.Actions()))
-					r.Nil(loadBalancer)
+					requireLoadBalancerDeleted(kubeAPIClient.Actions()[2])
-					r.EqualError(err, "services \"some-service-resource-name\" not found")
 				})
 			})
 		})

internal/controllermanager/prepare_controllers.go

@@ -289,6 +289,7 @@ func PrepareControllers(c *Config) (func(ctx context.Context), error) {
 				"pinniped-concierge-impersonation-proxy-config", // TODO this string should come from `c.NamesConfig`
 				client.Kubernetes,
 				informers.installationNamespaceK8s.Core().V1().ConfigMaps(),
+				informers.installationNamespaceK8s.Core().V1().Services(),
 				controllerlib.WithInformer,
 				controllerlib.WithInitialEvent,
 				"pinniped-concierge-impersonation-proxy-load-balancer", // TODO this string should come from `c.NamesConfig`