Move well-known env vars and config map keys into constants (#2101)

fixes #1412 and #2029
---------

Co-authored-by: Nicole Morales <nicole_morales@apple.com>
Co-authored-by: Johannes Scheuermann <johscheuer@users.noreply.github.com>

api/v1beta1/foundationdbcluster_types_test.go

@@ -26,6 +26,8 @@ import (
 	"path/filepath"
 	"time"
 
+	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
+
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 
 	corev1 "k8s.io/api/core/v1"
@@ -2411,11 +2413,11 @@ var _ = Describe("[api] FoundationDBCluster", func() {
 			Entry("With a placeholder",
 				testCase{
 					processAddr: ProcessAddress{
-						StringAddress: "$POD_IP",
+						StringAddress: fmt.Sprintf("$%s", fdbv1beta2.EnvNamePodIP),
 						Port:          4500,
 						Flags:         map[string]bool{},
 					},
-					expected: "$POD_IP:4500",
+					expected: fmt.Sprintf("$%s:4500", fdbv1beta2.EnvNamePodIP),
 				}),
 		)
 	})

api/v1beta2/foundationdb_constants.go

@@ -32,4 +32,20 @@ const (
 
 	// NoneFaultDomainKey represents the none fault domain, where every Pod is a fault domain.
 	NoneFaultDomainKey = "foundationdb.org/none"
+
+	/*
+		Config map constants
+	*/
+
+	// ClusterFileKey defines the key name in the ConfigMap whose value is the content of the cluster file, the connection string.
+	ClusterFileKey = "cluster-file"
+
+	// CaFileKey defines the key name in the ConfigMap whose value contains the trusted certificate authority PEM
+	CaFileKey = "ca-file"
+
+	// SidecarConfKey defines the key name in the ConfigMap whose value contains the configuration for the sidecar
+	SidecarConfKey = "sidecar-conf"
+
+	// RunningVersionKey defines the key name in the ConfigMap whose value is the FDB version that the cluster is currently running.
+	RunningVersionKey = "running-version"
 )

api/v1beta2/foundationdb_env_variables.go

@@ -21,6 +21,75 @@
 package v1beta2
 
 const (
-	// EnvNamePublicIP defines the FDB_PUBLIC_IP environment variable name.
+	// EnvNameDNSName specifies the DNS locality (identifies the pod when using DNS)
+	EnvNameDNSName = "FDB_DNS_NAME"
+
+	// EnvNameMachineID specifies the Machine ID locality.
+	EnvNameMachineID = "FDB_MACHINE_ID"
+
+	// EnvNameZoneID specifies Zone ID locality.
+	// The current default value of EnvNameZoneID is the hostname
+	EnvNameZoneID = "FDB_ZONE_ID"
+
+	// EnvNameClusterFile specifies the path to the cluster file.
+	EnvNameClusterFile = "FDB_CLUSTER_FILE"
+
+	// EnvNameBinaryDir specifies the path of the FDB binary's directory
+	EnvNameBinaryDir = "BINARY_DIR"
+
+	// EnvNameAdditionalEnvFile if specified for the `foundationdb-kubernetes-sidecar` and `foundationdb-kubernetes-init`
+	// containers, its content will be sourced before any container command runs, and you can override or define there
+	// any other environment variable; this can be used for example to inject environment variables using a shared volume
+	EnvNameAdditionalEnvFile = "ADDITIONAL_ENV_FILE"
+
+	// EnvNameTLSCaFile specifies the path to the certificate authority file for TLS connections
+	EnvNameTLSCaFile = "FDB_TLS_CA_FILE"
+
+	// EnvNameTLSCert specifies the path to the certificate file for TLS connections
+	EnvNameTLSCert = "FDB_TLS_CERTIFICATE_FILE"
+
+	// EnvNameTLSKeyFile specifies the path to the key file for TLS connections
+	EnvNameTLSKeyFile = "FDB_TLS_KEY_FILE"
+
+	// EnvNameTLSVerifyPeers specifies the peer verification rules for incoming TLS connections to the split-image sidecar.
+	// See https://apple.github.io/foundationdb/tls.html#peer-verification for the format
+	EnvNameTLSVerifyPeers = "FDB_TLS_VERIFY_PEERS"
+
+	// EnvNameFDBIgnoreExternalClientFailures specifies whether to ignore the failure to initialize some of the external clients
+	// TODO FDB 7.3 adds a check for loading external client library, which doesn't work with 6.3.
+	//  Consider remove this option once 6.3 is no longer being used.
+	EnvNameFDBIgnoreExternalClientFailures = "FDB_NETWORK_OPTION_IGNORE_EXTERNAL_CLIENT_FAILURES"
+
+	// EnvNameFDBTraceLogGroup sets the 'LogGroup' attribute with the specified value for all events in the trace output files; default value is 'default'
+	EnvNameFDBTraceLogGroup = "FDB_NETWORK_OPTION_TRACE_LOG_GROUP"
+
+	// EnvNameFDBTraceLogDirPath enables trace logs output to a file in the given directory
+	EnvNameFDBTraceLogDirPath = "FDB_NETWORK_OPTION_TRACE_ENABLE"
+
+	// EnvNameFDBExternalClientDir specifies path to search for dynamic libraries and adds them to the list of client
+	// libraries for use by the multi-version client API. Must be set before setting up the network.
+	EnvNameFDBExternalClientDir = "FDB_NETWORK_OPTION_EXTERNAL_CLIENT_DIRECTORY"
+
+	// EnvNameClientThreadsPerVersion specifies the number of client threads to be spawned.  Each cluster will be
+	// serviced by a single client thread. Spawns multiple worker threads for each version of the client that is loaded.
+	// Setting this to a number greater than one implies disable_local_client.
+	EnvNameClientThreadsPerVersion = "FDB_NETWORK_OPTION_CLIENT_THREADS_PER_VERSION"
+
+	// EnvNamePublicIP will be used to set the public address of the started fdbserver process
 	EnvNamePublicIP = "FDB_PUBLIC_IP"
+
+	// EnvNamePodIP will be used to set the listen address of the started fdbserver process
+	EnvNamePodIP = "FDB_POD_IP"
+
+	// EnvNamePodName tells the unified FDB kubernetes monitor the name of its pod
+	EnvNamePodName = "FDB_POD_NAME"
+
+	// EnvNamePodNamespace tells the unified FDB kubernetes monitor the K8s namespace it is running in
+	EnvNamePodNamespace = "FDB_POD_NAMESPACE"
+
+	// EnvNameNodeName tells the unified FDB kubernetes monitor the K8s node it is running on
+	EnvNameNodeName = "FDB_NODE_NAME"
+
+	// EnvNameInstanceID specifies the instance ID to the split-image-sidecar or unified FDB kubernetes monitor
+	EnvNameInstanceID = "FDB_INSTANCE_ID"
 )

controllers/change_coordinators_test.go

@@ -101,7 +101,7 @@ var _ = Describe("Change coordinators", func() {
 
 					for _, pod := range pods.Items {
 						container := pod.Spec.Containers[1]
-						container.Env = append(container.Env, corev1.EnvVar{Name: "FDB_DNS_NAME", Value: internal.GetPodDNSName(cluster, pod.Name)})
+						container.Env = append(container.Env, corev1.EnvVar{Name: fdbv1beta2.EnvNameDNSName, Value: internal.GetPodDNSName(cluster, pod.Name)})
 						pod.Spec.Containers[1] = container
 						Expect(k8sClient.Update(context.TODO(), &pod)).NotTo(HaveOccurred())
 					}

controllers/cluster_controller_test.go

@@ -338,7 +338,7 @@ var _ = Describe("cluster_controller", func() {
 					for _, envVar := range pod.Spec.InitContainers[0].Env {
 						env[envVar.Name] = envVar.Value
 					}
-					Expect(env["FDB_DNS_NAME"]).To(Equal(internal.GetPodDNSName(cluster, pod.Name)))
+					Expect(env[fdbv1beta2.EnvNameDNSName]).To(Equal(internal.GetPodDNSName(cluster, pod.Name)))
 				}
 			})
 		})
@@ -1548,7 +1548,7 @@ var _ = Describe("cluster_controller", func() {
 
 					for _, pod := range pods.Items {
 						Expect(len(pod.Spec.Containers[0].Env)).To(Equal(1))
-						Expect(pod.Spec.Containers[0].Env[0].Name).To(Equal("FDB_CLUSTER_FILE"))
+						Expect(pod.Spec.Containers[0].Env[0].Name).To(Equal(fdbv1beta2.EnvNameClusterFile))
 					}
 				})
 			})
@@ -1580,7 +1580,7 @@ var _ = Describe("cluster_controller", func() {
 
 					for _, pod := range pods.Items {
 						Expect(len(pod.Spec.Containers[0].Env)).To(Equal(1))
-						Expect(pod.Spec.Containers[0].Env[0].Name).To(Equal("FDB_CLUSTER_FILE"))
+						Expect(pod.Spec.Containers[0].Env[0].Name).To(Equal(fdbv1beta2.EnvNameClusterFile))
 					}
 				})
 			})
@@ -1679,11 +1679,11 @@ var _ = Describe("cluster_controller", func() {
 					Expect(container.Name).To(Equal(fdbv1beta2.SidecarContainerName))
 					var podIPEnv corev1.EnvVar
 					for _, env := range container.Env {
-						if env.Name == "FDB_POD_IP" {
+						if env.Name == fdbv1beta2.EnvNamePodIP {
 							podIPEnv = env
 						}
 					}
-					Expect(podIPEnv.Name).To(Equal("FDB_POD_IP"))
+					Expect(podIPEnv.Name).To(Equal(fdbv1beta2.EnvNamePodIP))
 					Expect(podIPEnv.ValueFrom).NotTo(BeNil())
 					Expect(podIPEnv.ValueFrom.FieldRef.FieldPath).To(Equal("status.podIP"))
 				}

controllers/update_pods.go

@@ -262,7 +262,7 @@ func getPodsToUpdate(ctx context.Context, logger logr.Logger, reconciler *Founda
 			continue
 		}
 
-		zone := substitutions["FDB_ZONE_ID"]
+		zone := substitutions[fdbv1beta2.EnvNameZoneID]
 		if reconciler.InSimulation {
 			zone = "simulation"
 		}

controllers/update_status.go

@@ -162,7 +162,7 @@ func (updateStatus) reconcile(ctx context.Context, r *FoundationDBClusterReconci
 	clusterStatus.RunningVersion = cluster.Status.RunningVersion
 
 	if clusterStatus.RunningVersion == "" {
-		version, present := existingConfigMap.Data["running-version"]
+		version, present := existingConfigMap.Data[fdbv1beta2.RunningVersionKey]
 		if present {
 			clusterStatus.RunningVersion = version
 		}
@@ -174,7 +174,7 @@ func (updateStatus) reconcile(ctx context.Context, r *FoundationDBClusterReconci
 
 	clusterStatus.ConnectionString = cluster.Status.ConnectionString
 	if clusterStatus.ConnectionString == "" {
-		clusterStatus.ConnectionString = existingConfigMap.Data[internal.ClusterFileKey]
+		clusterStatus.ConnectionString = existingConfigMap.Data[fdbv1beta2.ClusterFileKey]
 	}
 
 	if clusterStatus.ConnectionString == "" {
@@ -536,7 +536,7 @@ func validateProcessGroups(ctx context.Context, r *FoundationDBClusterReconciler
 			for _, container := range pod.Spec.Containers {
 				if container.Name == fdbv1beta2.SidecarContainerName || container.Name == fdbv1beta2.MainContainerName {
 					for _, env := range container.Env {
-						if env.Name == "FDB_POD_IP" {
+						if env.Name == fdbv1beta2.EnvNamePodIP {
 							hasPodIP = true
 						}
 					}

e2e/fixtures/fdb_cluster_specs.go

@@ -193,23 +193,23 @@ func (factory *Factory) createPodTemplate(
 					},
 					Env: []corev1.EnvVar{
 						{
-							Name:  "FDB_TLS_CERTIFICATE_FILE",
+							Name:  fdbv1beta2.EnvNameTLSCert,
 							Value: "/tmp/fdb-certs/tls.crt",
 						},
 						{
-							Name:  "FDB_TLS_CA_FILE",
+							Name:  fdbv1beta2.EnvNameTLSCaFile,
 							Value: "/tmp/fdb-certs/ca.pem",
 						},
 						{
-							Name:  "FDB_TLS_KEY_FILE",
+							Name:  fdbv1beta2.EnvNameTLSKeyFile,
 							Value: "/tmp/fdb-certs/tls.key",
 						},
 						{
-							Name:  "FDB_TLS_VERIFY_PEERS",
+							Name:  fdbv1beta2.EnvNameTLSVerifyPeers,
 							Value: "I.CN=localhost,I.O=Example Inc.,S.CN=localhost,S.O=Example Inc.",
 						},
 						{
-							Name:  "FDB_NETWORK_OPTION_TRACE_ENABLE",
+							Name:  fdbv1beta2.EnvNameFDBTraceLogDirPath,
 							Value: "/var/log/fdb-trace-logs",
 						},
 					},
@@ -236,19 +236,19 @@ func (factory *Factory) createPodTemplate(
 					},
 					Env: []corev1.EnvVar{
 						{
-							Name:  "FDB_TLS_CERTIFICATE_FILE",
+							Name:  fdbv1beta2.EnvNameTLSCert,
 							Value: "/tmp/fdb-certs/tls.crt",
 						},
 						{
-							Name:  "FDB_TLS_CA_FILE",
+							Name:  fdbv1beta2.EnvNameTLSCaFile,
 							Value: "/tmp/fdb-certs/ca.pem",
 						},
 						{
-							Name:  "FDB_TLS_KEY_FILE",
+							Name:  fdbv1beta2.EnvNameTLSKeyFile,
 							Value: "/tmp/fdb-certs/tls.key",
 						},
 						{
-							Name:  "FDB_TLS_VERIFY_PEERS",
+							Name:  fdbv1beta2.EnvNameTLSVerifyPeers,
 							Value: "I.CN=localhost,I.O=Example Inc.,S.CN=localhost,S.O=Example Inc.",
 						},
 					},

fdbclient/admin_client.go

@@ -213,7 +213,7 @@ func (client *cliAdminClient) getArgsAndTimeout(command cliCommand) ([]string, t
 		args = append(args, client.knobs...)
 	}
 
-	traceDir := os.Getenv("FDB_NETWORK_OPTION_TRACE_ENABLE")
+	traceDir := os.Getenv(fdbv1beta2.EnvNameFDBTraceLogDirPath)
 	if traceDir != "" {
 		args = append(args, "--log")
 

fdbclient/admin_client_test.go

@@ -196,7 +196,7 @@ var _ = Describe("admin_client_test", func() {
 	DescribeTable("getting the args for the command", func(command cliCommand, client *cliAdminClient, traceOption string, traceFormat string, expectedArgs []string, expectedTimeout time.Duration) {
 		Expect(client).NotTo(BeNil())
 		if traceOption != "" {
-			GinkgoT().Setenv("FDB_NETWORK_OPTION_TRACE_ENABLE", traceOption)
+			GinkgoT().Setenv(fdbv1beta2.EnvNameFDBTraceLogDirPath, traceOption)
 		}
 		if traceFormat != "" {
 			GinkgoT().Setenv("FDB_NETWORK_OPTION_TRACE_FORMAT", traceFormat)

fdbclient/command_runner.go

@@ -46,9 +46,9 @@ type realCommandRunner struct {
 // the other fdb tools.
 func getEnvironmentVariablesWithoutExcludedFdbEnv() []string {
 	excludedEnvironmentVariables := map[string]fdbv1beta2.None{
-		"FDB_NETWORK_OPTION_EXTERNAL_CLIENT_DIRECTORY":       {},
-		"FDB_NETWORK_OPTION_IGNORE_EXTERNAL_CLIENT_FAILURES": {},
-		"FDB_NETWORK_OPTION_CLIENT_THREADS_PER_VERSION":      {},
+		fdbv1beta2.EnvNameFDBExternalClientDir:            {},
+		fdbv1beta2.EnvNameFDBIgnoreExternalClientFailures: {},
+		fdbv1beta2.EnvNameClientThreadsPerVersion:         {},
 	}
 
 	osVariables := os.Environ()

fdbclient/command_runner_test.go

@@ -24,6 +24,8 @@ import (
 	"context"
 	"strings"
 
+	fdbv1beta2 "github.com/FoundationDB/fdb-kubernetes-operator/api/v1beta2"
+
 	"github.com/go-logr/logr"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -52,22 +54,22 @@ var _ = Describe("command_runner", func() {
 		var envVariablesKeys []string
 
 		BeforeEach(func() {
-			GinkgoT().Setenv("FDB_NETWORK_OPTION_EXTERNAL_CLIENT_DIRECTORY", "")
-			GinkgoT().Setenv("FDB_NETWORK_OPTION_IGNORE_EXTERNAL_CLIENT_FAILURES", "")
-			GinkgoT().Setenv("FDB_NETWORK_OPTION_CLIENT_THREADS_PER_VERSION", "")
+			GinkgoT().Setenv(fdbv1beta2.EnvNameFDBExternalClientDir, "")
+			GinkgoT().Setenv(fdbv1beta2.EnvNameFDBIgnoreExternalClientFailures, "")
+			GinkgoT().Setenv(fdbv1beta2.EnvNameClientThreadsPerVersion, "")
 
-			GinkgoT().Setenv("FDB_TLS_CERTIFICATE_FILE", "")
+			GinkgoT().Setenv(fdbv1beta2.EnvNameTLSCert, "")
 
 			for _, env := range getEnvironmentVariablesWithoutExcludedFdbEnv() {
 				envVariablesKeys = append(envVariablesKeys, strings.Split(env, "=")[0])
 			}
 		})
 
 		It("should exclude the listed FDB variables but include all others", func() {
-			Expect(envVariablesKeys).NotTo(ContainElement("FDB_NETWORK_OPTION_EXTERNAL_CLIENT_DIRECTORY"))
-			Expect(envVariablesKeys).NotTo(ContainElement("FDB_NETWORK_OPTION_IGNORE_EXTERNAL_CLIENT_FAILURES"))
-			Expect(envVariablesKeys).NotTo(ContainElement("FDB_NETWORK_OPTION_CLIENT_THREADS_PER_VERSION"))
-			Expect(envVariablesKeys).To(ContainElement("FDB_TLS_CERTIFICATE_FILE"))
+			Expect(envVariablesKeys).NotTo(ContainElement(fdbv1beta2.EnvNameFDBExternalClientDir))
+			Expect(envVariablesKeys).NotTo(ContainElement(fdbv1beta2.EnvNameFDBIgnoreExternalClientFailures))
+			Expect(envVariablesKeys).NotTo(ContainElement(fdbv1beta2.EnvNameClientThreadsPerVersion))
+			Expect(envVariablesKeys).To(ContainElement(fdbv1beta2.EnvNameTLSCert))
 		})
 	})
 })

internal/configmap_helper.go

@@ -30,18 +30,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-const (
-	// ClusterFileKey defines the key name in the ConfigMap
-	ClusterFileKey = "cluster-file"
-)
-
 // GetConfigMap builds a config map for a cluster's dynamic config
 func GetConfigMap(cluster *fdbv1beta2.FoundationDBCluster) (*corev1.ConfigMap, error) {
 	data := make(map[string]string)
 
 	connectionString := cluster.Status.ConnectionString
-	data[ClusterFileKey] = connectionString
-	data["running-version"] = cluster.Status.RunningVersion
+	data[fdbv1beta2.ClusterFileKey] = connectionString
+	data[fdbv1beta2.RunningVersionKey] = cluster.Status.RunningVersion
 
 	var caFile strings.Builder
 	for _, ca := range cluster.Spec.TrustedCAs {
@@ -52,7 +47,7 @@ func GetConfigMap(cluster *fdbv1beta2.FoundationDBCluster) (*corev1.ConfigMap, e
 	}
 
 	if caFile.Len() > 0 {
-		data["ca-file"] = caFile.String()
+		data[fdbv1beta2.CaFileKey] = caFile.String()
 	}
 
 	desiredCountStruct, err := cluster.GetProcessCountsWithDefaults()
@@ -185,11 +180,11 @@ func GetConfigMapMonitorConfEntry(pClass fdbv1beta2.ProcessClass, imageType fdbv
 // This will omit keys that we do not expect the Pods to reference e.g. for storage Pods only include the storage config.
 func GetDynamicConfHash(configMap *corev1.ConfigMap, pClass fdbv1beta2.ProcessClass, imageType fdbv1beta2.ImageType, serversPerPod int) (string, error) {
 	fields := []string{
-		ClusterFileKey,
+		fdbv1beta2.ClusterFileKey,
 		GetConfigMapMonitorConfEntry(pClass, imageType, serversPerPod),
-		"running-version",
-		"ca-file",
-		"sidecar-conf",
+		fdbv1beta2.RunningVersionKey,
+		fdbv1beta2.CaFileKey,
+		fdbv1beta2.SidecarConfKey,
 	}
 	var data = make(map[string]string, len(fields))