Change the image type from an env variable to an annotation (#2063)

* Change the image type from an env variable to an annotation

api/v1beta2/foundationdb_env_variables.go

@@ -23,7 +23,4 @@ package v1beta2
 const (
 	// EnvNamePublicIP defines the FDB_PUBLIC_IP environment variable name.
 	EnvNamePublicIP = "FDB_PUBLIC_IP"
-
-	// EnvNameImageType defines the FDB_IMAGE_TYPE environment variable name.
-	EnvNameImageType = "FDB_IMAGE_TYPE"
 )

api/v1beta2/foundationdb_labels.go

@@ -52,6 +52,9 @@ const (
 	// The information is fetched from Pod.Spec.NodeName of the Pod resource.
 	NodeAnnotation = "foundationdb.org/current-node"
 
+	// ImageTypeAnnotation is an annotation key that specifies the image type of the Pod.
+	ImageTypeAnnotation = "foundationdb.org/image-type"
+
 	// FDBProcessGroupIDLabel represents the label that is used to represent a instance ID
 	FDBProcessGroupIDLabel = "foundationdb.org/fdb-process-group-id"
 

controllers/cluster_controller_test.go

@@ -1304,6 +1304,7 @@ var _ = Describe("cluster_controller", func() {
 							"foundationdb.org/existing-annotation": "test-value",
 							"fdb-annotation":                       "value1",
 							fdbv1beta2.NodeAnnotation:              item.Spec.NodeName,
+							fdbv1beta2.ImageTypeAnnotation:         string(fdbv1beta2.ImageTypeSplit),
 						}))
 					} else {
 						Expect(item.ObjectMeta.Annotations).To(Equal(map[string]string{
@@ -1312,6 +1313,7 @@ var _ = Describe("cluster_controller", func() {
 							fdbv1beta2.PublicIPSourceAnnotation: "pod",
 							"fdb-annotation":                    "value1",
 							fdbv1beta2.NodeAnnotation:           item.Spec.NodeName,
+							fdbv1beta2.ImageTypeAnnotation:      string(fdbv1beta2.ImageTypeSplit),
 						}))
 					}
 				}
@@ -1442,6 +1444,7 @@ var _ = Describe("cluster_controller", func() {
 							fdbv1beta2.LastSpecKey:              hash,
 							fdbv1beta2.PublicIPSourceAnnotation: "pod",
 							fdbv1beta2.NodeAnnotation:           item.Spec.NodeName,
+							fdbv1beta2.ImageTypeAnnotation:      string(fdbv1beta2.ImageTypeSplit),
 						}))
 					}
 
@@ -1551,6 +1554,7 @@ var _ = Describe("cluster_controller", func() {
 						fdbv1beta2.LastSpecKey:              hash,
 						fdbv1beta2.PublicIPSourceAnnotation: "pod",
 						fdbv1beta2.NodeAnnotation:           item.Spec.NodeName,
+						fdbv1beta2.ImageTypeAnnotation:      string(fdbv1beta2.ImageTypeSplit),
 					}))
 				}
 

controllers/update_metadata.go

@@ -22,7 +22,6 @@ package controllers
 
 import (
 	"context"
-
 	"github.com/go-logr/logr"
 
 	"github.com/FoundationDB/fdb-kubernetes-operator/internal"
@@ -45,33 +44,19 @@ func (updateMetadata) reconcile(ctx context.Context, r *FoundationDBClusterRecon
 	}
 	pvcMap := internal.CreatePVCMap(cluster, pvcs)
 
+	var shouldRequeue bool
 	for _, processGroup := range cluster.Status.ProcessGroups {
 		if processGroup.IsMarkedForRemoval() {
 			logger.V(1).Info("Ignore process group marked for removal",
 				"processGroupID", processGroup.ProcessGroupID)
 			continue
 		}
 
-		pod, err := r.PodLifecycleManager.GetPod(ctx, r, cluster, processGroup.GetPodName(cluster))
-		if err == nil {
-			metadata := internal.GetPodMetadata(cluster, processGroup.ProcessClass, processGroup.ProcessGroupID, "")
-			if metadata.Annotations == nil {
-				metadata.Annotations = make(map[string]string, 1)
-			}
-
-			if pod.Spec.NodeName != "" {
-				metadata.Annotations[fdbv1beta2.NodeAnnotation] = pod.Spec.NodeName
-			}
-
-			if !metadataCorrect(metadata, &pod.ObjectMeta) {
-				err := r.PodLifecycleManager.UpdateMetadata(ctx, r, cluster, pod)
-				if err != nil {
-					return &requeue{curError: err}
-				}
-			}
-		} else {
-			logger.V(1).Info("Could not find Pod for process group ID",
+		err = updatePodMetadata(ctx, r, cluster, processGroup)
+		if err != nil {
+			logger.Error(err, "Could not update Pod metadata",
 				"processGroupID", processGroup.ProcessGroupID)
+			shouldRequeue = true
 		}
 
 		// We can skip all stateless processes because they won't have a PVC attached.
@@ -94,16 +79,51 @@ func (updateMetadata) reconcile(ctx context.Context, r *FoundationDBClusterRecon
 		if !metadataCorrect(metadata, &pvc.ObjectMeta) {
 			err = r.Update(ctx, &pvc)
 			if err != nil {
-				return &requeue{curError: err}
+				logger.Error(err, "Could not update PVC metadata",
+					"processGroupID", processGroup.ProcessGroupID)
+				shouldRequeue = true
 			}
 		}
 	}
 
+	if shouldRequeue {
+		return &requeue{message: "could not update all metadata", delayedRequeue: true}
+	}
+
 	return nil
 }
 
+func updatePodMetadata(ctx context.Context, r *FoundationDBClusterReconciler, cluster *fdbv1beta2.FoundationDBCluster, processGroup *fdbv1beta2.ProcessGroupStatus) error {
+	pod, err := r.PodLifecycleManager.GetPod(ctx, r, cluster, processGroup.GetPodName(cluster))
+	if err != nil {
+		return err
+	}
+
+	desiredMetadata := internal.GetPodMetadata(cluster, processGroup.ProcessClass, processGroup.ProcessGroupID, "")
+	if !podMetadataCorrect(desiredMetadata, pod) {
+		return r.PodLifecycleManager.UpdateMetadata(ctx, r, cluster, pod)
+	}
+
+	return nil
+}
+
+func podMetadataCorrect(desiredMetadata metav1.ObjectMeta, pod *corev1.Pod) bool {
+	if desiredMetadata.Annotations == nil {
+		desiredMetadata.Annotations = make(map[string]string, 1)
+	}
+
+	if pod.Spec.NodeName != "" {
+		desiredMetadata.Annotations[fdbv1beta2.NodeAnnotation] = pod.Spec.NodeName
+	}
+
+	desiredMetadata.Annotations[fdbv1beta2.LastSpecKey] = pod.ObjectMeta.Annotations[fdbv1beta2.LastSpecKey]
+	// Don't change the annotation for the image type, this will require a pod update.
+	desiredMetadata.Annotations[fdbv1beta2.ImageTypeAnnotation] = string(internal.GetImageTypeFromAnnotation(pod.ObjectMeta.Annotations))
+
+	return metadataCorrect(desiredMetadata, &pod.ObjectMeta)
+}
+
 func metadataCorrect(desiredMetadata metav1.ObjectMeta, currentMetadata *metav1.ObjectMeta) bool {
-	desiredMetadata.Annotations[fdbv1beta2.LastSpecKey] = currentMetadata.Annotations[fdbv1beta2.LastSpecKey]
 	// If the annotations or labels have changed the metadata has to be updated.
 	return !mergeLabelsInMetadata(currentMetadata, desiredMetadata) && !mergeAnnotations(currentMetadata, desiredMetadata)
 }