Skip to content

Commit 29963bd

Browse files
fix(controller): resolve GlobalTopoServer from templates correctly
Update `getGlobalTopoRef` to use the TemplateResolver, ensuring child resources receive the correct topology address when a `CoreTemplate` is used. Previously, this only supported inline specs. Note: This commit includes updated test cases that cover both this fix and a separate name-length validation bug. The tests are expected to fail until the name-length validation logic is implemented in the next commit.
1 parent 736709a commit 29963bd

File tree

3 files changed

+64
-9
lines changed

3 files changed

+64
-9
lines changed

pkg/cluster-handler/controller/multigrescluster/multigrescluster_controller.go

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,12 @@ func (r *MultigresClusterReconciler) reconcileCells(ctx context.Context, cluster
227227
return err
228228
}
229229

230+
// Resolve Global Topo Reference ONCE for all cells
231+
globalTopoRef, err := r.getGlobalTopoRef(ctx, cluster, resolver)
232+
if err != nil {
233+
return err
234+
}
235+
230236
activeCellNames := make(map[string]bool)
231237

232238
allCellNames := []multigresv1alpha1.CellName{}
@@ -260,7 +266,7 @@ func (r *MultigresClusterReconciler) reconcileCells(ctx context.Context, cluster
260266
cellCR.Spec.MultiGateway = gatewaySpec
261267
cellCR.Spec.AllCells = allCellNames
262268

263-
cellCR.Spec.GlobalTopoServer = r.getGlobalTopoRef(cluster)
269+
cellCR.Spec.GlobalTopoServer = globalTopoRef
264270

265271
if localTopoSpec != nil {
266272
cellCR.Spec.TopoServer = *localTopoSpec
@@ -294,6 +300,12 @@ func (r *MultigresClusterReconciler) reconcileDatabases(ctx context.Context, clu
294300
return err
295301
}
296302

303+
// Resolve Global Topo Reference ONCE for all databases/shards
304+
globalTopoRef, err := r.getGlobalTopoRef(ctx, cluster, resolver)
305+
if err != nil {
306+
return err
307+
}
308+
297309
activeTGNames := make(map[string]bool)
298310

299311
for _, db := range cluster.Spec.Databases {
@@ -348,7 +360,7 @@ func (r *MultigresClusterReconciler) reconcileDatabases(ctx context.Context, clu
348360
MultiPooler: cluster.Spec.Images.MultiPooler,
349361
Postgres: cluster.Spec.Images.Postgres,
350362
}
351-
tgCR.Spec.GlobalTopoServer = r.getGlobalTopoRef(cluster)
363+
tgCR.Spec.GlobalTopoServer = globalTopoRef
352364
tgCR.Spec.Shards = resolvedShards
353365

354366
return controllerutil.SetControllerReference(cluster, tgCR, r.Scheme)
@@ -370,19 +382,35 @@ func (r *MultigresClusterReconciler) reconcileDatabases(ctx context.Context, clu
370382
return nil
371383
}
372384

373-
func (r *MultigresClusterReconciler) getGlobalTopoRef(cluster *multigresv1alpha1.MultigresCluster) multigresv1alpha1.GlobalTopoServerRef {
385+
func (r *MultigresClusterReconciler) getGlobalTopoRef(ctx context.Context, cluster *multigresv1alpha1.MultigresCluster, resolver *TemplateResolver) (multigresv1alpha1.GlobalTopoServerRef, error) {
386+
// 1. Determine Template Name
387+
topoTplName := cluster.Spec.TemplateDefaults.CoreTemplate
388+
if cluster.Spec.GlobalTopoServer.TemplateRef != "" {
389+
topoTplName = cluster.Spec.GlobalTopoServer.TemplateRef
390+
}
391+
392+
// 2. Resolve Template
393+
topoTpl, err := resolver.ResolveCoreTemplate(ctx, topoTplName)
394+
if err != nil {
395+
return multigresv1alpha1.GlobalTopoServerRef{}, err
396+
}
397+
398+
// 3. Resolve Final Spec (Inline vs Template)
399+
topoSpec := ResolveGlobalTopo(&cluster.Spec.GlobalTopoServer, topoTpl)
400+
401+
// 4. Construct Reference based on resolved spec
374402
address := ""
375-
if cluster.Spec.GlobalTopoServer.Etcd != nil {
403+
if topoSpec.Etcd != nil {
376404
address = fmt.Sprintf("%s-global-topo-client.%s.svc:2379", cluster.Name, cluster.Namespace)
377-
} else if cluster.Spec.GlobalTopoServer.External != nil && len(cluster.Spec.GlobalTopoServer.External.Endpoints) > 0 {
378-
address = string(cluster.Spec.GlobalTopoServer.External.Endpoints[0])
405+
} else if topoSpec.External != nil && len(topoSpec.External.Endpoints) > 0 {
406+
address = string(topoSpec.External.Endpoints[0])
379407
}
380408

381409
return multigresv1alpha1.GlobalTopoServerRef{
382410
Address: address,
383411
RootPath: "/multigres/global",
384412
Implementation: "etcd2",
385-
}
413+
}, nil
386414
}
387415

388416
func (r *MultigresClusterReconciler) updateStatus(ctx context.Context, cluster *multigresv1alpha1.MultigresCluster) error {

pkg/cluster-handler/controller/multigrescluster/multigrescluster_controller_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package multigrescluster
33
import (
44
"context"
55
"errors"
6+
"fmt"
67
"strings"
78
"testing"
89

@@ -210,6 +211,17 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
210211
if *deploy.Spec.Replicas != 5 {
211212
t.Errorf("MultiAdmin did not use admin-core template, got replicas: %d", *deploy.Spec.Replicas)
212213
}
214+
215+
// VERIFY FIX: Check that Child Cell received the correct Global Topo Address from the template
216+
// This confirms getGlobalTopoRef resolved the template correctly
217+
cell := &multigresv1alpha1.Cell{}
218+
if err := c.Get(ctx, types.NamespacedName{Name: clusterName + "-zone-a", Namespace: namespace}, cell); err != nil {
219+
t.Fatal(err)
220+
}
221+
expectedAddr := fmt.Sprintf("%s-global-topo-client.%s.svc:2379", clusterName, namespace)
222+
if cell.Spec.GlobalTopoServer.Address != expectedAddr {
223+
t.Errorf("Cell GlobalTopo address mismatch. Got %s, Want %s", cell.Spec.GlobalTopoServer.Address, expectedAddr)
224+
}
213225
},
214226
},
215227
{
@@ -231,6 +243,14 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
231243
if err := c.Get(ctx, types.NamespacedName{Name: clusterName + "-multiadmin", Namespace: namespace}, deploy); err != nil {
232244
t.Fatal("MultiAdmin not created")
233245
}
246+
// Verify External Topo Propagated
247+
cell := &multigresv1alpha1.Cell{}
248+
if err := c.Get(ctx, types.NamespacedName{Name: clusterName + "-zone-a", Namespace: namespace}, cell); err != nil {
249+
t.Fatal(err)
250+
}
251+
if cell.Spec.GlobalTopoServer.Address != "http://ext:2379" {
252+
t.Errorf("External address not propagated. Got %s", cell.Spec.GlobalTopoServer.Address)
253+
}
234254
},
235255
},
236256
{
@@ -331,6 +351,7 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
331351
name: "Create: External Topo with Empty Endpoints",
332352
cluster: func() *multigresv1alpha1.MultigresCluster {
333353
c := baseCluster.DeepCopy()
354+
c.Spec.TemplateDefaults.CoreTemplate = ""
334355
c.Spec.GlobalTopoServer = multigresv1alpha1.GlobalTopoServerSpec{
335356
External: &multigresv1alpha1.ExternalTopoServerSpec{
336357
Endpoints: []multigresv1alpha1.EndpointUrl{},
@@ -387,7 +408,7 @@ func TestMultigresClusterReconciler_Reconcile(t *testing.T) {
387408
return c
388409
}(),
389410
existingObjects: []client.Object{coreTpl, cellTpl, shardTpl},
390-
expectError: false,
411+
expectError: true, // FIXED: Now expects error
391412
validate: func(t *testing.T, c client.Client) {},
392413
},
393414
{

pkg/cluster-handler/controller/multigrescluster/template_logic.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,13 @@ type TemplateResolver struct {
2121
Defaults multigresv1alpha1.TemplateDefaults
2222
}
2323

24-
// ResolveCoreTemplate fetches and resolves a CoreTemplate by name, handling defaults.
24+
// ResolveCoreTemplate determines the target CoreTemplate name and fetches it.
25+
//
26+
// If templateName is empty, it uses the following precedence:
27+
// 1. The cluster-level default defined in TemplateDefaults.
28+
// 2. A CoreTemplate named "default" found in the same namespace where MultigresCluster is deployed.
29+
//
30+
// If the resolved template is not found, it returns an empty CoreTemplate object and nil error.
2531
func (r *TemplateResolver) ResolveCoreTemplate(ctx context.Context, templateName string) (*multigresv1alpha1.CoreTemplate, error) {
2632
name := templateName
2733
if name == "" {

0 commit comments

Comments
 (0)