Replace the custom pod IP patterns with a more explicit selector for IP families.

Author: brownleej

.github/workflows/pull_request.yml

@@ -54,6 +54,7 @@ jobs:
       - name: Run Python lint
         run: |
           pycodestyle --max-line-length=120 ./**/*.py
+          black --check --diff ./
   build:
     name: Build
     runs-on: ubuntu-latest

api/v1beta1/foundationdbcluster_types.go

@@ -2421,12 +2421,11 @@ type RoutingConfig struct {
 	// This supports the values `pod` and `service`.
 	PublicIPSource *PublicIPSource `json:"publicIPSource,omitempty"`
 
-	// PodIPIndex tells the pod to use a specific entry from the podIPs list
-	// instead of the default podIP. The entry be the first one that matches the
-	// pattern that is provided in this field.
+	// PodIPFamily tells the pod which family of IP addresses to use.
+	// You can use 4 to represent IPv4, and 6 to represent IPv6.
 	// This feature is only supported in FDB 7.0 or later, and requires
 	// dual-stack support in your Kubernetes environment.
-	PodIPPattern *string `json:"podIPPattern,omitempty"`
+	PodIPFamily *int `json:"podIPFamily,omitempty"`
 }
 
 // RequiredAddressSet provides settings for which addresses we need to listen

api/v1beta1/zz_generated.deepcopy.go

@@ -7570,8 +7570,8 @@ spec:
                   properties:
                     headlessService:
                       type: boolean
-                    podIPPattern:
-                      type: string
+                    podIPFamily:
+                      type: integer
                     publicIPSource:
                       type: string
                   type: object

config/crd/bases/apps.foundationdb.org_foundationdbclusters.yaml

@@ -28,6 +28,7 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	"net"
 	"regexp"
 	"sort"
 	"strconv"
@@ -891,7 +892,7 @@ func (instance FdbInstance) GetPublicIPs() []string {
 }
 
 func getPublicIpsForPod(pod *corev1.Pod) []string {
-	var podIPPattern *regexp.Regexp
+	var podIPFamily *int
 
 	if pod == nil {
 		return []string{}
@@ -900,27 +901,41 @@ func getPublicIpsForPod(pod *corev1.Pod) []string {
 	for _, container := range pod.Spec.Containers {
 		if container.Name == "foundationdb-kubernetes-sidecar" {
 			for indexOfArgument, argument := range container.Args {
-				if argument == "--public-ip-pattern" && indexOfArgument < len(container.Args)-1 {
-					patternString := container.Args[indexOfArgument+1]
-					pattern, err := regexp.Compile(patternString)
+				if argument == "--public-ip-family" && indexOfArgument < len(container.Args)-1 {
+					familyString := container.Args[indexOfArgument+1]
+					family, err := strconv.Atoi(familyString)
 					if err != nil {
-						log.Error(err, "Error parsing public IP pattern", "pattern", pattern)
+						log.Error(err, "Error parsing public IP family", "family", familyString)
 						return nil
 					}
-					podIPPattern = pattern
+					podIPFamily = &family
 					break
 				}
 			}
 		}
 	}
 
-	if podIPPattern != nil {
+	if podIPFamily != nil {
 		podIPs := pod.Status.PodIPs
 		matchingIPs := make([]string, 0, len(podIPs))
 
-		for _, ip := range podIPs {
-			if podIPPattern.Match([]byte(ip.IP)) {
-				matchingIPs = append(matchingIPs, ip.IP)
+		for _, podIP := range podIPs {
+			ip := net.ParseIP(podIP.IP)
+			if ip == nil {
+				log.Error(nil, "Failed to parse IP from pod", "ip", podIP)
+				continue
+			}
+			matches := false
+			switch *podIPFamily {
+			case 4:
+				matches = ip.To4() != nil
+			case 6:
+				matches = ip.To4() == nil
+			default:
+				log.Error(nil, "Could not match IP address against IP family", "family", *podIPFamily)
+			}
+			if matches {
+				matchingIPs = append(matchingIPs, podIP.IP)
 			}
 		}
 		return matchingIPs

controllers/cluster_controller.go

@@ -3903,10 +3903,10 @@ var _ = Describe(string(fdbtypes.ProcessClassClusterController), func() {
 			})
 		})
 
-		Context("with a pod IP pattern configured", func() {
+		Context("with a v6 pod IP family configured", func() {
 			BeforeEach(func() {
-				pattern := ":"
-				cluster.Spec.Routing.PodIPPattern = &pattern
+				family := 6
+				cluster.Spec.Routing.PodIPFamily = &family
 				pod, err := GetPod(cluster, "storage", 1)
 				Expect(err).NotTo(HaveOccurred())
 				pod.Status.PodIP = "1.1.1.1"
@@ -3937,6 +3937,27 @@ var _ = Describe(string(fdbtypes.ProcessClassClusterController), func() {
 			})
 		})
 
+		Context("with a v4 pod IP family configured", func() {
+			BeforeEach(func() {
+				family := 4
+				cluster.Spec.Routing.PodIPFamily = &family
+				pod, err := GetPod(cluster, "storage", 1)
+				Expect(err).NotTo(HaveOccurred())
+				pod.Status.PodIP = "1.1.1.1"
+				pod.Status.PodIPs = []corev1.PodIP{
+					{IP: "1.1.1.2"},
+					{IP: "2001:db8::ff00:42:8329"},
+				}
+				instance.Pod = pod
+				instance.Metadata = &pod.ObjectMeta
+			})
+
+			It("should select the address based on the spec", func() {
+				result := instance.GetPublicIPs()
+				Expect(result).To(Equal([]string{"1.1.1.2"}))
+			})
+		})
+
 		Context("with no pod", func() {
 			BeforeEach(func() {
 				pod, err := GetPod(cluster, "storage", 1)

controllers/cluster_controller_test.go

@@ -461,13 +461,13 @@ func configureSidecarContainer(container *corev1.Container, initMode bool, insta
 		if usePublicIPFromService {
 			publicIPKey = fmt.Sprintf("metadata.annotations['%s']", fdbtypes.PublicIPAnnotation)
 		} else {
-			pattern := cluster.Spec.Routing.PodIPPattern
-			if pattern == nil {
+			family := cluster.Spec.Routing.PodIPFamily
+			if family == nil {
 				publicIPKey = "status.podIP"
 			} else {
 				publicIPKey = "status.podIPs"
-				sidecarArgs = append(sidecarArgs, "--public-ip-pattern")
-				sidecarArgs = append(sidecarArgs, fmt.Sprint(*pattern))
+				sidecarArgs = append(sidecarArgs, "--public-ip-family")
+				sidecarArgs = append(sidecarArgs, fmt.Sprint(*family))
 			}
 		}
 		sidecarEnv = append(sidecarEnv, corev1.EnvVar{Name: "FDB_PUBLIC_IP", ValueFrom: &corev1.EnvVarSource{

controllers/pod_models.go

@@ -383,8 +383,8 @@ var _ = Describe("pod_models", func() {
 
 		Context("with a pod IP pattern defined", func() {
 			BeforeEach(func() {
-				pattern := ":"
-				cluster.Spec.Routing.PodIPPattern = &pattern
+				family := 6
+				cluster.Spec.Routing.PodIPFamily = &family
 				spec, err = GetPodSpec(cluster, fdbtypes.ProcessClassStorage, 1)
 			})
 
@@ -403,8 +403,8 @@ var _ = Describe("pod_models", func() {
 					"fdbcli",
 					"--main-container-version",
 					"6.2.20",
-					"--public-ip-pattern",
-					":",
+					"--public-ip-family",
+					"6",
 					"--init-mode",
 				}))
 				Expect(initContainer.Env).To(Equal([]corev1.EnvVar{
@@ -435,8 +435,8 @@ var _ = Describe("pod_models", func() {
 					"fdbcli",
 					"--main-container-version",
 					"6.2.20",
-					"--public-ip-pattern",
-					":",
+					"--public-ip-family",
+					"6",
 				}))
 				Expect(sidecarContainer.Env).To(Equal([]corev1.EnvVar{
 					{Name: "FDB_PUBLIC_IP", ValueFrom: &corev1.EnvVarSource{

controllers/pod_models_test.go

@@ -462,7 +462,7 @@ RoutingConfig allows configuring routing to our pods, and services that sit in f
 | ----- | ----------- | ------ | -------- |
 | headlessService | Headless determines whether we want to run a headless service for the cluster. | *bool | false |
 | publicIPSource | PublicIPSource specifies what source a process should use to get its public IPs.  This supports the values `pod` and `service`. | *PublicIPSource | false |
-| podIPPattern | PodIPIndex tells the pod to use a specific entry from the podIPs list instead of the default podIP. The entry be the first one that matches the pattern that is provided in this field. This feature is only supported in FDB 7.0 or later, and requires dual-stack support in your Kubernetes environment. | *string | false |
+| podIPFamily | PodIPFamily tells the pod which family of IP addresses to use. You can use 4 to represent IPv4, and 6 to represent IPv6. This feature is only supported in FDB 7.0 or later, and requires dual-stack support in your Kubernetes environment. | *int | false |
 
 [Back to TOC](#table-of-contents)
 

docs/cluster_spec.md

@@ -21,6 +21,7 @@
 
 import argparse
 import hashlib
+import ipaddress
 import logging
 import json
 import os
@@ -56,9 +57,7 @@ def __init__(self):
             ),
             action="store_true",
         )
-        parser.add_argument(
-            "--bind-address", help="IP and port to bind on"
-        )
+        parser.add_argument("--bind-address", help="IP and port to bind on")
         parser.add_argument(
             "--tls",
             help=("This flag enables TLS for incoming connections"),
@@ -150,10 +149,10 @@ def __init__(self):
             help=("The version of the main foundationdb container in the pod"),
         )
         parser.add_argument(
-            "--public-ip-pattern",
+            "--public-ip-family",
             help=(
                 "Tells the sidecar to treat the public IP as a comma-separated "
-                "list, and use the first entry matching this pattern"
+                "list, and use the first entry in the specified IP family"
             ),
         )
         parser.add_argument(
@@ -224,7 +223,7 @@ def __init__(self):
             "FDB_MACHINE_ID",
             "FDB_ZONE_ID",
             "FDB_INSTANCE_ID",
-            "FDB_POD_IP"
+            "FDB_POD_IP",
         ]:
             self.substitutions[key] = os.getenv(key, "")
 
@@ -298,11 +297,14 @@ def __init__(self):
         if os.getenv("COPY_ONCE", "0") == "1":
             self.init_mode = True
 
-        if args.public_ip_pattern:
-            regex = re.compile(args.public_ip_pattern)
-
-            self.substitutions["FDB_PUBLIC_IP"] = Config.extract_desired_ip(regex, self.substitutions["FDB_PUBLIC_IP"])
-            self.substitutions["FDB_POD_IP"] = Config.extract_desired_ip(regex, self.substitutions["FDB_POD_IP"])
+        if args.public_ip_family:
+            version = int(args.public_ip_family)
+            self.substitutions["FDB_PUBLIC_IP"] = Config.extract_desired_ip(
+                version, self.substitutions["FDB_PUBLIC_IP"]
+            )
+            self.substitutions["FDB_POD_IP"] = Config.extract_desired_ip(
+                version, self.substitutions["FDB_POD_IP"]
+            )
 
         if not self.bind_address:
             if self.substitutions["FDB_POD_IP"] != "":
@@ -326,16 +328,16 @@ def is_at_least(self, target_version):
         )
 
     @classmethod
-    def extract_desired_ip(cls, pattern, string):
+    def extract_desired_ip(cls, version, string):
         if string == "":
             return string
 
         ips = string.split(",")
-        matching_ips = [ip for ip in ips if pattern.search(ip)]
+        matching_ips = [ip for ip in ips if ipaddress.ip_address(ip).version == version]
         if len(matching_ips) == 0:
-            raise Exception("Failed to find IP matching %s in %s" % (pattern, ips))
+            raise Exception(f"Failed to find IPv{version} entry in {ips}")
         ip = matching_ips[0]
-        if ":" in ip:
+        if version == 6:
             ip = "[%s]" % ip
         return ip
 
@@ -353,12 +355,13 @@ def start(cls):
         This method starts the server.
         """
         config = Config.shared()
-        port_index = config.bind_address.rindex(":")
-        address = config.bind_address[:port_index]
-        port = config.bind_address[port_index+1:]
+        colon_index = config.bind_address.rindex(":")
+        port_index = colon_index + 1
+        address = config.bind_address[:colon_index]
+        port = config.bind_address[port_index:]
         log.info(f"Listening on {address}:{port}")
 
-        if ":" in address:
+        if address.startswith("[") and address.endswith("]"):
             server = ThreadingHTTPServerV6((address[1:-1], int(port)), cls)
         else:
             server = ThreadingHTTPServer((address, int(port)), cls)