Fix stuck tls migration (#2327)

* Fix stuck tls migration when pod spec is changed

Author: johscheuer

controllers/update_pods.go

@@ -339,7 +339,24 @@ func getPodsToUpdate(
 
 		// The Pod is updated, so we can continue.
 		if pod.ObjectMeta.Annotations[fdbv1beta2.LastSpecKey] == specHash {
-			continue
+			// Check here if the Pod spec matches the desired Pod spec.
+			var updated bool
+			updated, err = reconciler.PodLifecycleManager.PodIsUpdated(
+				ctx,
+				reconciler,
+				cluster,
+				pod,
+			)
+			if err != nil {
+				logger.Info("Skipping Pod due to error generating spec hash",
+					"processGroupID", processGroup.ProcessGroupID,
+					"error", err.Error())
+				continue
+			}
+
+			if updated {
+				continue
+			}
 		}
 
 		needsReplacement, err := replacements.ProcessGroupNeedsReplacements(

e2e/test_operator/operator_test.go

@@ -790,18 +790,49 @@ var _ = Describe("Operator", Label("e2e", "pr"), func() {
 			checkCoordinatorsTLSFlag(fdbCluster.GetCluster(), initialTLSSetting)
 		})
 
-		// Currently disabled until a new release of the operator is out
-		It("should update the TLS setting  and keep the cluster available", func() {
-			// Only change the TLS setting for the cluster and not for the sidecar otherwise we have to recreate
-			// all Pods which takes a long time since we recreate the Pods one by one.
-			Expect(
-				fdbCluster.SetTLS(
-					!initialTLSSetting,
-					fdbCluster.GetCluster().Spec.SidecarContainer.EnableTLS,
-				),
-			).NotTo(HaveOccurred())
-			Expect(fdbCluster.HasTLSEnabled()).To(Equal(!initialTLSSetting))
-			checkCoordinatorsTLSFlag(fdbCluster.GetCluster(), !initialTLSSetting)
+		When("the pod spec stays the same", func() {
+			It("should update the TLS setting  and keep the cluster available", func() {
+				// Only change the TLS setting for the cluster and not for the sidecar otherwise we have to recreate
+				// all Pods which takes a long time since we recreate the Pods one by one.
+				Expect(
+					fdbCluster.SetTLS(
+						!initialTLSSetting,
+						fdbCluster.GetCluster().Spec.SidecarContainer.EnableTLS,
+					),
+				).NotTo(HaveOccurred())
+				Expect(fdbCluster.HasTLSEnabled()).To(Equal(!initialTLSSetting))
+				checkCoordinatorsTLSFlag(fdbCluster.GetCluster(), !initialTLSSetting)
+			})
+		})
+
+		PWhen("the pod spec is changed", func() {
+			It("should update the TLS setting  and keep the cluster available", func() {
+				spec := fdbCluster.GetCluster().Spec.DeepCopy()
+				spec.MainContainer.EnableTLS = !initialTLSSetting
+
+				// Add a new env variable to ensure this will cause some additional replacements.
+				processSettings := spec.Processes[fdbv1beta2.ProcessClassGeneral]
+				for i, container := range processSettings.PodTemplate.Spec.Containers {
+					if container.Name != fdbv1beta2.MainContainerName {
+						continue
+					}
+
+					container.Env = append(container.Env, corev1.EnvVar{
+						Name:  "TESTING_TLS_CHANGE",
+						Value: "EMPTY",
+					})
+
+					processSettings.PodTemplate.Spec.Containers[i] = container
+					break
+				}
+
+				spec.Processes[fdbv1beta2.ProcessClassGeneral] = processSettings
+
+				fdbCluster.UpdateClusterSpecWithSpec(spec)
+				Expect(fdbCluster.WaitForReconciliation()).To(Succeed())
+				Expect(fdbCluster.HasTLSEnabled()).To(Equal(!initialTLSSetting))
+				checkCoordinatorsTLSFlag(fdbCluster.GetCluster(), !initialTLSSetting)
+			})
 		})
 	})
 

internal/restarts/restarts.go

@@ -36,7 +36,6 @@ func GetFilterConditions(
 		// not get any ConfigMap updates.
 		return map[fdbv1beta2.ProcessGroupConditionType]bool{
 			fdbv1beta2.IncorrectCommandLine: true,
-			fdbv1beta2.IncorrectPodSpec:     false,
 			fdbv1beta2.SidecarUnreachable:   false,
 			fdbv1beta2.IncorrectConfigMap:   false,
 		}

internal/restarts/restarts_test.go

@@ -43,7 +43,6 @@ var _ = Describe("restarts", func() {
 			},
 			map[fdbv1beta2.ProcessGroupConditionType]bool{
 				fdbv1beta2.IncorrectCommandLine: true,
-				fdbv1beta2.IncorrectPodSpec:     false,
 				fdbv1beta2.SidecarUnreachable:   false,
 				fdbv1beta2.IncorrectConfigMap:   false,
 			}),
@@ -55,7 +54,6 @@ var _ = Describe("restarts", func() {
 			},
 			map[fdbv1beta2.ProcessGroupConditionType]bool{
 				fdbv1beta2.IncorrectCommandLine: true,
-				fdbv1beta2.IncorrectPodSpec:     false,
 				fdbv1beta2.SidecarUnreachable:   false,
 				fdbv1beta2.IncorrectConfigMap:   false,
 			}),