From f6aed93169148f4e2250860b6eedfefea1ca57f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ver=C3=B3nica=20L=C3=B3pez?= Date: Sat, 19 Sep 2026 16:29:02 -0600 Subject: [PATCH] fix(shard): advertise explicit multipooler pod DNS names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Pass each pooler's full pod DNS address through --hostname to avoid registering an unresolvable short name after discovery fails. - Keep the address aligned with the headless service and backup TLS SANs. - Cover namespace, pool, cell, replica, truncated-name, and rollout behavior. Signed-off-by: Verónica López --- .../controller/shard/pool_pod.go | 18 +++- .../controller/shard/pool_pod_test.go | 82 +++++++++++++++++-- 2 files changed, 90 insertions(+), 10 deletions(-) diff --git a/pkg/resource-handler/controller/shard/pool_pod.go b/pkg/resource-handler/controller/shard/pool_pod.go index 7d290607..f9f4c70e 100644 --- a/pkg/resource-handler/controller/shard/pool_pod.go +++ b/pkg/resource-handler/controller/shard/pool_pod.go @@ -84,6 +84,20 @@ func BuildPoolPod( }}, volumes...) serviceID := BuildPoolServiceID(podName) + headlessServiceName := buildHeadlessServiceName(shard, poolName, cellName) + multipooler := buildMultipoolerContainer(shard, poolSpec, poolName, cellName, serviceID) + // Supply the advertised address so failed hostname discovery cannot register + // a short pod name that other pods cannot resolve. The cluster.local suffix + // matches the operator's topology addresses and pgBackRest certificate SANs. + multipooler.Args = append( + multipooler.Args, + fmt.Sprintf( + "--hostname=%s.%s.%s.svc.cluster.local", + podName, + headlessServiceName, + shard.Namespace, + ), + ) annotations := map[string]string{ metadata.AnnotationSpecHash: "", // placeholder, computed below @@ -121,7 +135,7 @@ func BuildPoolPod( buildPgctldSidecar(shard, poolSpec), }, Containers: []corev1.Container{ - buildMultipoolerContainer(shard, poolSpec, poolName, cellName, serviceID), + multipooler, buildPostgresExporterContainer(shard), }, Volumes: volumes, @@ -130,7 +144,7 @@ func BuildPoolPod( NodeSelector: shard.Spec.CellTopologyLabels[multigresv1alpha1.CellName(cellName)], // Hostname is set to the pod name for DNS resolution via headless service. Hostname: podName, - Subdomain: buildHeadlessServiceName(shard, poolName, cellName), + Subdomain: headlessServiceName, }, } diff --git a/pkg/resource-handler/controller/shard/pool_pod_test.go b/pkg/resource-handler/controller/shard/pool_pod_test.go index 976a8970..48e13468 100644 --- a/pkg/resource-handler/controller/shard/pool_pod_test.go +++ b/pkg/resource-handler/controller/shard/pool_pod_test.go @@ -1,6 +1,9 @@ package shard import ( + "crypto/x509" + "fmt" + "slices" "strings" "testing" @@ -899,16 +902,79 @@ func TestBuildPoolPod_NodeSelector(t *testing.T) { } func TestBuildPoolPod_Hostname(t *testing.T) { - pod, err := BuildPoolPod(newTestShard(), "main", "z1", newTestPoolSpec(), 0, testScheme()) - if err != nil { - t.Fatalf("unexpected error: %v", err) + tests := map[string]struct { + clusterName string + namespace string + poolName string + cellName string + index int + }{ + "first replica": {"test-cluster", "default", "main", "z1", 0}, + "another pool and cell": {"test-cluster", "tenant-a", "extra", "zone-b", 1}, + "truncated names and surge index": { + strings.Repeat("cluster", 8), "tenant-b", "read-replicas", "us-east-1a", 10, + }, } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + shard := newTestShard() + shard.Labels["multigres.com/cluster"] = tt.clusterName + shard.Namespace = tt.namespace + pool := newTestPoolSpec() + pool.Cells = []multigresv1alpha1.CellName{multigresv1alpha1.CellName(tt.cellName)} + shard.Spec.Pools = map[multigresv1alpha1.PoolName]multigresv1alpha1.PoolSpec{ + multigresv1alpha1.PoolName(tt.poolName): pool, + } + pod, err := BuildPoolPod(shard, tt.poolName, tt.cellName, pool, tt.index, testScheme()) + require.NoError(t, err) + svc, err := BuildPoolHeadlessService( + shard, + tt.poolName, + tt.cellName, + pool, + testScheme(), + ) + require.NoError(t, err) - if pod.Spec.Hostname != pod.Name { - t.Errorf("hostname = %q, should match pod name %q", pod.Spec.Hostname, pod.Name) - } - if pod.Spec.Subdomain == "" { - t.Error("subdomain (headless service name) should not be empty") + assert.Equal(t, pod.Name, pod.Spec.Hostname) + assert.Equal(t, svc.Name, pod.Spec.Subdomain) + assert.True(t, svc.Spec.PublishNotReadyAddresses) + for key, value := range svc.Spec.Selector { + assert.Equal(t, value, pod.Labels[key], "headless service must select the pod") + } + hostname := fmt.Sprintf("%s.%s.%s.svc.cluster.local", pod.Name, svc.Name, tt.namespace) + var hostnameArgs []string + for _, container := range pod.Spec.Containers { + for _, arg := range container.Args { + if strings.HasPrefix(arg, "--hostname=") { + assert.Equal(t, "multipooler", container.Name) + hostnameArgs = append(hostnameArgs, arg) + } + } + } + assert.Equal(t, []string{"--hostname=" + hostname}, hostnameArgs) + cert := &x509.Certificate{DNSNames: pgBackRestPoolDNSNames(shard)} + assert.NoError( + t, + cert.VerifyHostname(hostname), + "advertised address must match backup TLS SANs", + ) + + // Existing pods without the explicit address must enter the normal drift rollout. + legacyPod := pod.DeepCopy() + for i := range legacyPod.Spec.Containers { + legacyPod.Spec.Containers[i].Args = slices.DeleteFunc( + legacyPod.Spec.Containers[i].Args, + func(arg string) bool { return strings.HasPrefix(arg, "--hostname=") }, + ) + } + assert.Equal(t, ComputeSpecHash(pod), pod.Annotations[metadata.AnnotationSpecHash]) + assert.NotEqual( + t, + ComputeSpecHash(legacyPod), + pod.Annotations[metadata.AnnotationSpecHash], + ) + }) } }