Skip to content

Commit 497f647

Browse files
committed
[memcached] Fix ServerListWithInet for IPv4
The brackets are required only for IPv6, with IPv4 it causes issues because cache doesn't get used as all the servers are marked as dead as not resolved. Resolves: https://issues.redhat.com/browse/OSPRH-20517 Related-Issue: https://issues.redhat.com/browse/OSPRH-12221
1 parent 63860ee commit 497f647

File tree

5 files changed

+101
-7
lines changed

5 files changed

+101
-7
lines changed

apis/test/helpers/memcached.go

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ func (tc *TestHelper) SimulateMemcachedReady(name types.NamespacedName) {
152152
serverListWithInet := []string{}
153153
for i := 0; i < int(*mc.Spec.Replicas); i++ {
154154
serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
155-
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
155+
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
156156
}
157157
mc.Status.ServerList = serverList
158158
mc.Status.ServerListWithInet = serverListWithInet
@@ -177,7 +177,7 @@ func (tc *TestHelper) SimulateTLSMemcachedReady(name types.NamespacedName) {
177177
serverListWithInet := []string{}
178178
for i := 0; i < int(*mc.Spec.Replicas); i++ {
179179
serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
180-
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
180+
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
181181
}
182182
mc.Status.ServerList = serverList
183183
mc.Status.ServerListWithInet = serverListWithInet
@@ -203,7 +203,7 @@ func (tc *TestHelper) SimulateMTLSMemcachedReady(name types.NamespacedName) {
203203
serverListWithInet := []string{}
204204
for i := 0; i < int(*mc.Spec.Replicas); i++ {
205205
serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
206-
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
206+
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
207207
}
208208
mc.Status.ServerList = serverList
209209
mc.Status.ServerListWithInet = serverListWithInet
@@ -226,3 +226,28 @@ func (tc *TestHelper) GetDefaultMemcachedSpec() memcachedv1.MemcachedSpec {
226226
},
227227
}
228228
}
229+
230+
// SimulateIPv6MemcachedReady simulates a ready state for a Memcached instance in a Kubernetes cluster with IPv6 server list formatting.
231+
func (tc *TestHelper) SimulateIPv6MemcachedReady(name types.NamespacedName) {
232+
t.Eventually(func(g t.Gomega) {
233+
mc := tc.GetMemcached(name)
234+
mc.Status.ObservedGeneration = mc.Generation
235+
mc.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
236+
mc.Status.ReadyCount = *mc.Spec.Replicas
237+
238+
serverList := []string{}
239+
serverListWithInet := []string{}
240+
for i := 0; i < int(*mc.Spec.Replicas); i++ {
241+
serverList = append(serverList, fmt.Sprintf("%s-%d.%s.%s.svc:11211", mc.Name, i, mc.Name, mc.Namespace))
242+
serverListWithInet = append(serverListWithInet, fmt.Sprintf("inet6:[%s-%d.%s.%s.svc]:11211", mc.Name, i, mc.Name, mc.Namespace))
243+
}
244+
mc.Status.ServerList = serverList
245+
mc.Status.ServerListWithInet = serverListWithInet
246+
247+
// This can return conflict so we have the t.Eventually block to retry
248+
g.Expect(tc.K8sClient.Status().Update(tc.Ctx, mc)).To(t.Succeed())
249+
250+
}, tc.Timeout, tc.Interval).Should(t.Succeed())
251+
252+
tc.Logger.Info("Simulated IPv6 memcached ready", "on", name)
253+
}

controllers/memcached/memcached_controller.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,8 +683,11 @@ func (r *Reconciler) GetServerLists(
683683
serverList = append(serverList, fmt.Sprintf("%s:%d", server, port))
684684

685685
// python-memcached requires inet(6) prefix according to the IP version
686-
// used by the memcached server.
687-
serverListWithInet = append(serverListWithInet, fmt.Sprintf("%s:[%s]:%d", prefix, server, memcached.MemcachedPort))
686+
// used by the memcached server. IPv6 addresses also need to be wrapped in brackets.
687+
if ipFamily == corev1.IPv6Protocol {
688+
server = fmt.Sprintf("[%s]", server)
689+
}
690+
serverListWithInet = append(serverListWithInet, fmt.Sprintf("%s:%s:%d", prefix, server, memcached.MemcachedPort))
688691
}
689692

690693
return serverList, serverListWithInet

tests/functional/memcached_controller_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,4 +290,70 @@ var _ = Describe("Memcached Controller", func() {
290290
)
291291
})
292292
})
293+
294+
When("testing server list formatting", func() {
295+
It("should format IPv4 server lists correctly (without brackets)", func() {
296+
// Create a memcached instance for testing
297+
memcachedSpec := GetDefaultMemcachedSpec()
298+
memcachedSpec["replicas"] = 3
299+
memcached := CreateMemcachedConfig(namespace, memcachedSpec)
300+
memcachedName := types.NamespacedName{
301+
Name: memcached.GetName(),
302+
Namespace: memcached.GetNamespace(),
303+
}
304+
DeferCleanup(th.DeleteInstance, memcached)
305+
306+
// Simulate IPv4 memcached ready (this uses the helper method which now correctly formats IPv4)
307+
th.SimulateMemcachedReady(memcachedName)
308+
309+
// Verify the server list formatting
310+
instance := GetMemcached(memcachedName)
311+
312+
// Verify serverList (should not have brackets)
313+
Expect(instance.Status.ServerList).To(HaveLen(3))
314+
for i := 0; i < 3; i++ {
315+
expectedServer := fmt.Sprintf("%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
316+
Expect(instance.Status.ServerList[i]).To(Equal(expectedServer))
317+
}
318+
319+
// Verify serverListWithInet for IPv4 (should not have brackets around hostname)
320+
Expect(instance.Status.ServerListWithInet).To(HaveLen(3))
321+
for i := 0; i < 3; i++ {
322+
expectedServerWithInet := fmt.Sprintf("inet:%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
323+
Expect(instance.Status.ServerListWithInet[i]).To(Equal(expectedServerWithInet))
324+
}
325+
})
326+
327+
It("should format IPv6 server lists correctly (with brackets)", func() {
328+
// Create a memcached instance for testing
329+
memcachedSpec := GetDefaultMemcachedSpec()
330+
memcachedSpec["replicas"] = 3
331+
memcached := CreateMemcachedConfig(namespace, memcachedSpec)
332+
memcachedName := types.NamespacedName{
333+
Name: memcached.GetName(),
334+
Namespace: memcached.GetNamespace(),
335+
}
336+
DeferCleanup(th.DeleteInstance, memcached)
337+
338+
// Simulate IPv6 memcached ready
339+
th.SimulateIPv6MemcachedReady(memcachedName)
340+
341+
// Verify the server list formatting
342+
instance := GetMemcached(memcachedName)
343+
344+
// Verify serverList (should not have brackets - same as IPv4)
345+
Expect(instance.Status.ServerList).To(HaveLen(3))
346+
for i := 0; i < 3; i++ {
347+
expectedServer := fmt.Sprintf("%s-%d.%s.%s.svc:11211", instance.Name, i, instance.Name, instance.Namespace)
348+
Expect(instance.Status.ServerList[i]).To(Equal(expectedServer))
349+
}
350+
351+
// Verify serverListWithInet for IPv6 (should have brackets around hostname)
352+
Expect(instance.Status.ServerListWithInet).To(HaveLen(3))
353+
for i := 0; i < 3; i++ {
354+
expectedServerWithInet := fmt.Sprintf("inet6:[%s-%d.%s.%s.svc]:11211", instance.Name, i, instance.Name, instance.Namespace)
355+
Expect(instance.Status.ServerListWithInet[i]).To(Equal(expectedServerWithInet))
356+
}
357+
})
358+
})
293359
})

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ commands:
1212
- script: |
1313
# get the first memcached from serverListWithInet and validate
1414
template='{{ (index .status.serverListWithInet 0) }}'
15-
regex="inet:\[memcached-0.memcached.$NAMESPACE.svc\]:11211"
15+
regex="inet:memcached-0.memcached.$NAMESPACE.svc:11211"
1616
memcached=$(oc get -n $NAMESPACE memcached memcached -o go-template="$template")
1717
matches=$(echo "$memcached" | sed -e "s?$regex??")
1818
if [ -z "$matches" ]; then

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ commands:
3434
# Whether or not TLS is configured, the serverListWithInit always exposes port 11211
3535
# get the first memcached from serverListWithInet and validate
3636
template='{{ (index .status.serverListWithInet 0) }}'
37-
regex="inet:\[memcached-0.memcached.$NAMESPACE.svc\]:11211"
37+
regex="inet:memcached-0.memcached.$NAMESPACE.svc:11211"
3838
memcached=$(oc get -n $NAMESPACE memcached memcached -o go-template="$template")
3939
matches=$(echo "$memcached" | sed -e "s?$regex??")
4040
if [ -z "$matches" ]; then

0 commit comments

Comments
 (0)