From 6d047c151f0019e0795d39b3c6622a3d7c29516f Mon Sep 17 00:00:00 2001
From: Andrew Keesler <akeesler@vmware.com>
Date: Wed, 23 Sep 2020 11:30:03 -0400
Subject: [PATCH] Fix kubecertagent deleter test to reconcile on pod template
 fields

I think we want to reconcile on these pod template fields so that if
someone were to redeploy Pinniped with a new image for the agent, the
agent would get updated immediately. Before this change, the agent image
wouldn't get updated until the agent pod was deleted.
---
 .../controller/kubecertagent/deleter_test.go  | 486 ++++++++++--------
 .../controller/kubecertagent/kubecertagent.go |  13 +
 2 files changed, 283 insertions(+), 216 deletions(-)

diff --git a/internal/controller/kubecertagent/deleter_test.go b/internal/controller/kubecertagent/deleter_test.go
index cd076f8a..bbf2277d 100644
--- a/internal/controller/kubecertagent/deleter_test.go
+++ b/internal/controller/kubecertagent/deleter_test.go
@@ -126,7 +126,7 @@ func TestDeleterControllerSync(t *testing.T) {
 			Resource: "pods",
 		}
 
-		// fnv 32a hash of controller-manager uid
+		// fnv 32a hash of "some-controller-manager-uid"
 		controllerManagerPodHash := "fbb0addd"
 		agentPod := agentPodTemplate.DeepCopy()
 		agentPod.Namespace = agentPodNamespace
@@ -220,6 +220,275 @@ func TestDeleterControllerSync(t *testing.T) {
 						kubeAPIClient.Actions(),
 					)
 				})
+
+				when("the agent pod is out of sync with the controller manager via volume mounts", func() {
+					it.Before(func() {
+						controllerManagerPod.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{
+							{
+								Name: "some-other-volume-mount",
+							},
+						}
+						r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
+						r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the controller manager via volumes", func() {
+					it.Before(func() {
+						controllerManagerPod.Spec.Volumes = []corev1.Volume{
+							{
+								Name: "some-other-volume",
+							},
+						}
+						r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
+						r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the controller manager via node selector", func() {
+					it.Before(func() {
+						controllerManagerPod.Spec.NodeSelector = map[string]string{
+							"some-other-node-selector-key": "some-other-node-selector-value",
+						}
+						r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
+						r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the controller manager via node name", func() {
+					it.Before(func() {
+						controllerManagerPod.Spec.NodeName = "some-other-node-name"
+						r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
+						r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the controller manager via tolerations", func() {
+					it.Before(func() {
+						controllerManagerPod.Spec.Tolerations = []corev1.Toleration{
+							{
+								Key: "some-other-toleration-key",
+							},
+						}
+						r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
+						r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync via restart policy", func() {
+					it.Before(func() {
+						updatedAgentPod := agentPod.DeepCopy()
+						updatedAgentPod.Spec.RestartPolicy = corev1.RestartPolicyAlways
+						r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+						r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+					})
+
+					it.Focus("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync via automount service account token", func() {
+					it.Before(func() {
+						updatedAgentPod := agentPod.DeepCopy()
+						updatedAgentPod.Spec.AutomountServiceAccountToken = boolPtr(true)
+						r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+						r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+					})
+
+					it.Focus("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the template via name", func() {
+					it.Before(func() {
+						updatedAgentPod := agentPod.DeepCopy()
+						updatedAgentPod.Spec.Containers[0].Name = "some-new-name"
+						r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+						r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the template via image", func() {
+					it.Before(func() {
+						updatedAgentPod := agentPod.DeepCopy()
+						updatedAgentPod.Spec.Containers[0].Image = "new-image"
+						r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+						r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
+
+				when("the agent pod is out of sync with the template via command", func() {
+					it.Before(func() {
+						updatedAgentPod := agentPod.DeepCopy()
+						updatedAgentPod.Spec.Containers[0].Command = []string{"some", "new", "command"}
+						r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+						r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
+					})
+
+					it("deletes the agent pod", func() {
+						startInformersAndController()
+						err := controllerlib.TestSync(t, subject, *syncContext)
+
+						r.NoError(err)
+						r.Equal(
+							[]coretesting.Action{
+								coretesting.NewDeleteAction(
+									podsGVR,
+									agentPodNamespace,
+									agentPod.Name,
+								),
+							},
+							kubeAPIClient.Actions(),
+						)
+					})
+				})
 			})
 
 			when("there is a non-matching controller manager pod via uid", func() {
@@ -272,221 +541,6 @@ func TestDeleterControllerSync(t *testing.T) {
 				})
 			})
 
-			when("the agent pod is out of sync with the controller manager via volume mounts", func() {
-				it.Before(func() {
-					controllerManagerPod.Spec.Containers[0].VolumeMounts = []corev1.VolumeMount{
-						{
-							Name: "some-other-volume-mount",
-						},
-					}
-					r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
-					r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync with the controller manager via volumes", func() {
-				it.Before(func() {
-					controllerManagerPod.Spec.Volumes = []corev1.Volume{
-						{
-							Name: "some-other-volume",
-						},
-					}
-					r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
-					r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync with the controller manager via node selector", func() {
-				it.Before(func() {
-					controllerManagerPod.Spec.NodeSelector = map[string]string{
-						"some-other-node-selector-key": "some-other-node-selector-value",
-					}
-					r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
-					r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync with the controller manager via node name", func() {
-				it.Before(func() {
-					controllerManagerPod.Spec.NodeName = "some-other-node-name"
-					r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
-					r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync with the controller manager via tolerations", func() {
-				it.Before(func() {
-					controllerManagerPod.Spec.Tolerations = []corev1.Toleration{
-						{
-							Key: "some-other-toleration-key",
-						},
-					}
-					r.NoError(kubeSystemInformerClient.Tracker().Add(controllerManagerPod))
-					r.NoError(kubeAPIClient.Tracker().Add(controllerManagerPod))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync via restart policy", func() {
-				it.Before(func() {
-					updatedAgentPod := agentPod.DeepCopy()
-					updatedAgentPod.Spec.RestartPolicy = corev1.RestartPolicyAlways
-					r.NoError(agentInformerClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
-					r.NoError(kubeAPIClient.Tracker().Update(podsGVR, updatedAgentPod, updatedAgentPod.Namespace))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync via automount service account tokem", func() {
-				it.Before(func() {
-					agentPod.Spec.AutomountServiceAccountToken = boolPtr(true)
-					r.NoError(agentInformerClient.Tracker().Update(podsGVR, agentPod, agentPod.Namespace))
-					r.NoError(kubeAPIClient.Tracker().Update(podsGVR, agentPod, agentPod.Namespace))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
-			when("the agent pod is out of sync with the template via image", func() {
-				it.Before(func() {
-					agentPod.Spec.Containers[0].Image = "new-image"
-					r.NoError(agentInformerClient.Tracker().Update(podsGVR, agentPod, agentPod.Namespace))
-					r.NoError(kubeAPIClient.Tracker().Update(podsGVR, agentPod, agentPod.Namespace))
-				})
-
-				it("deletes the agent pod", func() {
-					startInformersAndController()
-					err := controllerlib.TestSync(t, subject, *syncContext)
-
-					r.NoError(err)
-					r.Equal(
-						[]coretesting.Action{
-							coretesting.NewDeleteAction(
-								podsGVR,
-								agentPodNamespace,
-								agentPod.Name,
-							),
-						},
-						kubeAPIClient.Actions(),
-					)
-				})
-			})
-
 			when("there is no matching controller manager pod", func() {
 				it("deletes the agent pod", func() {
 					startInformersAndController()
diff --git a/internal/controller/kubecertagent/kubecertagent.go b/internal/controller/kubecertagent/kubecertagent.go
index 09f691d5..f4e0be91 100644
--- a/internal/controller/kubecertagent/kubecertagent.go
+++ b/internal/controller/kubecertagent/kubecertagent.go
@@ -37,6 +37,7 @@ type Info struct {
 	//   .Namespace: serves as the namespace of the agent pods
 	//   .Name: serves as the name prefix for each of the agent pods
 	//   .Labels: serves as a way to filter for agent pods
+	//   .Spec.Containers[0].Name: serves as the container name the agent pods
 	//   .Spec.Containers[0].Image: serves as the container image for the agent pods
 	//   .Spec.Containers[0].Command: serves as the container command for the agent pods
 	Template *corev1.Pod
@@ -118,6 +119,18 @@ func isAgentPodUpToDate(actualAgentPod, expectedAgentPod *corev1.Pod) bool {
 		actualAgentPod.Spec.Containers[0].VolumeMounts,
 		expectedAgentPod.Spec.Containers[0].VolumeMounts,
 	) &&
+		equality.Semantic.DeepEqual(
+			actualAgentPod.Spec.Containers[0].Name,
+			expectedAgentPod.Spec.Containers[0].Name,
+		) &&
+		equality.Semantic.DeepEqual(
+			actualAgentPod.Spec.Containers[0].Image,
+			expectedAgentPod.Spec.Containers[0].Image,
+		) &&
+		equality.Semantic.DeepEqual(
+			actualAgentPod.Spec.Containers[0].Command,
+			expectedAgentPod.Spec.Containers[0].Command,
+		) &&
 		equality.Semantic.DeepEqual(
 			actualAgentPod.Spec.Volumes,
 			expectedAgentPod.Spec.Volumes,