Merge pull request #477 from karelyatin/OSPRH-20517

[memcached] Fix ServerListWithInet for IPv4

Author: openshift-merge-bot[bot]

apis/test/helpers/memcached.go

@@ -152,7 +152,7 @@ func (tc *TestHelper) SimulateMemcachedReady(name types.NamespacedName) {
 		serverListWithInet := []string{}
 		for i := 0; i < int(*mc.Spec.Replicas); i++ {
 			serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
-			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
+			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
 		}
 		mc.Status.ServerList = serverList
 		mc.Status.ServerListWithInet = serverListWithInet
@@ -177,7 +177,7 @@ func (tc *TestHelper) SimulateTLSMemcachedReady(name types.NamespacedName) {
 		serverListWithInet := []string{}
 		for i := 0; i < int(*mc.Spec.Replicas); i++ {
 			serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
-			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
+			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
 		}
 		mc.Status.ServerList = serverList
 		mc.Status.ServerListWithInet = serverListWithInet
@@ -203,7 +203,7 @@ func (tc *TestHelper) SimulateMTLSMemcachedReady(name types.NamespacedName) {
 		serverListWithInet := []string{}
 		for i := 0; i < int(*mc.Spec.Replicas); i++ {
 			serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
-			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
+			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
 		}
 		mc.Status.ServerList = serverList
 		mc.Status.ServerListWithInet = serverListWithInet
@@ -226,3 +226,28 @@ func (tc *TestHelper) GetDefaultMemcachedSpec() memcachedv1.MemcachedSpec {
 		},
 	}
 }
+
+// SimulateIPv6MemcachedReady simulates a ready state for a Memcached instance in a Kubernetes cluster with IPv6 server list formatting.
+func (tc *TestHelper) SimulateIPv6MemcachedReady(name types.NamespacedName) {
+	t.Eventually(func(g t.Gomega) {
+		mc := tc.GetMemcached(name)
+		mc.Status.ObservedGeneration = mc.Generation
+		mc.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
+		mc.Status.ReadyCount = *mc.Spec.Replicas
+
+		serverList := []string{}
+		serverListWithInet := []string{}
+		for i := 0; i < int(*mc.Spec.Replicas); i++ {
+			serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
+			serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet6:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
+		}
+		mc.Status.ServerList = serverList
+		mc.Status.ServerListWithInet = serverListWithInet
+
+		// This can return conflict so we have the t.Eventually block to retry
+		g.Expect(tc.K8sClient.Status().Update(tc.Ctx, mc)).To(t.Succeed())
+
+	}, tc.Timeout, tc.Interval).Should(t.Succeed())
+
+	tc.Logger.Info("Simulated IPv6 memcached ready", "on", name)
+}

controllers/memcached/memcached_controller.go

@@ -683,8 +683,11 @@ func (r *Reconciler) GetServerLists(
 		serverList = append(serverList, fmt.Sprintf("%s:%d", server, port))
 
 		// python-memcached requires inet(6) prefix according to the IP version
-		// used by the memcached server.
-		serverListWithInet = append(serverListWithInet, fmt.Sprintf("%s:[%s]:%d", prefix, server, memcached.MemcachedPort))
+		// used by the memcached server. IPv6 addresses also need to be wrapped in brackets.
+		if ipFamily == corev1.IPv6Protocol {
+			server = fmt.Sprintf("[%s]", server)
+		}
+		serverListWithInet = append(serverListWithInet, fmt.Sprintf("%s:%s:%d", prefix, server, memcached.MemcachedPort))
 	}
 
 	return serverList, serverListWithInet

tests/functional/memcached_controller_test.go

@@ -290,4 +290,70 @@ var _ = Describe("Memcached Controller", func() {
 			)
 		})
 	})
+
+	When("testing server list formatting", func() {
+		It("should format IPv4 server lists correctly (without brackets)", func() {
+			// Create a memcached instance for testing
+			memcachedSpec := GetDefaultMemcachedSpec()
+			memcachedSpec["replicas"] = 3
+			memcached := CreateMemcachedConfig(namespace, memcachedSpec)
+			memcachedName := types.NamespacedName{
+				Name:      memcached.GetName(),
+				Namespace: memcached.GetNamespace(),
+			}
+			DeferCleanup(th.DeleteInstance, memcached)
+
+			// Simulate IPv4 memcached ready (this uses the helper method which now correctly formats IPv4)
+			th.SimulateMemcachedReady(memcachedName)
+
+			// Verify the server list formatting
+			instance := GetMemcached(memcachedName)
+
+			// Verify serverList (should not have brackets)
+			Expect(instance.Status.ServerList).To(HaveLen(3))
+			for i := 0; i < 3; i++ {
+				expectedServer := fmt.Sprintf("%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
+				Expect(instance.Status.ServerList[i]).To(Equal(expectedServer))
+			}
+
+			// Verify serverListWithInet for IPv4 (should not have brackets around hostname)
+			Expect(instance.Status.ServerListWithInet).To(HaveLen(3))
+			for i := 0; i < 3; i++ {
+				expectedServerWithInet := fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
+				Expect(instance.Status.ServerListWithInet[i]).To(Equal(expectedServerWithInet))
+			}
+		})
+
+		It("should format IPv6 server lists correctly (with brackets)", func() {
+			// Create a memcached instance for testing
+			memcachedSpec := GetDefaultMemcachedSpec()
+			memcachedSpec["replicas"] = 3
+			memcached := CreateMemcachedConfig(namespace, memcachedSpec)
+			memcachedName := types.NamespacedName{
+				Name:      memcached.GetName(),
+				Namespace: memcached.GetNamespace(),
+			}
+			DeferCleanup(th.DeleteInstance, memcached)
+
+			// Simulate IPv6 memcached ready
+			th.SimulateIPv6MemcachedReady(memcachedName)
+
+			// Verify the server list formatting
+			instance := GetMemcached(memcachedName)
+
+			// Verify serverList (should not have brackets - same as IPv4)
+			Expect(instance.Status.ServerList).To(HaveLen(3))
+			for i := 0; i < 3; i++ {
+				expectedServer := fmt.Sprintf("%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
+				Expect(instance.Status.ServerList[i]).To(Equal(expectedServer))
+			}
+
+			// Verify serverListWithInet for IPv6 (should have brackets around hostname)
+			Expect(instance.Status.ServerListWithInet).To(HaveLen(3))
+			for i := 0; i < 3; i++ {
+				expectedServerWithInet := fmt.Sprintf("inet6:[%s-%d.%s.%s.svc]:11211", instance.Name, i, instance.Name, instance.Namespace)
+				Expect(instance.Status.ServerListWithInet[i]).To(Equal(expectedServerWithInet))
+			}
+		})
+	})
 })

tests/kuttl/tests/memcached/02-assert.yaml

@@ -12,7 +12,7 @@ commands:
   - script: |
       # get the first memcached from serverListWithInet and validate
       template='{{ (index .status.serverListWithInet 0) }}'
-      regex="inet:\[memcached-0.memcached.$NAMESPACE.svc\]:11211"
+      regex="inet:memcached-0.memcached.$NAMESPACE.svc:11211"
       memcached=$(oc get -n $NAMESPACE memcached memcached -o go-template="$template")
       matches=$(echo "$memcached" | sed -e "s?$regex??")
       if [ -z "$matches" ]; then

tests/kuttl/tests/memcached/04-assert.yaml

@@ -34,7 +34,7 @@ commands:
       # Whether or not TLS is configured, the serverListWithInit always exposes port 11211
       # get the first memcached from serverListWithInet and validate
       template='{{ (index .status.serverListWithInet 0) }}'
-      regex="inet:\[memcached-0.memcached.$NAMESPACE.svc\]:11211"
+      regex="inet:memcached-0.memcached.$NAMESPACE.svc:11211"
       memcached=$(oc get -n $NAMESPACE memcached memcached -o go-template="$template")
       matches=$(echo "$memcached" | sed -e "s?$regex??")
       if [ -z "$matches" ]; then