Merge pull request #104 from numtide/feat/granular-resolver-logic

feat(resolver): implement granular resolution and default shard injection

Author: fernando-villalba

pkg/resolver/cell.go

@@ -8,40 +8,74 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	multigresv1alpha1 "github.com/numtide/multigres-operator/api/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
 )
 
-// ResolveCellTemplate fetches and resolves a CellTemplate by name, handling defaults.
+// ResolveCell determines the final configuration for a specific Cell.
+// It orchestrates: Template Lookup -> Fetch -> Merge -> Defaulting.
+func (r *Resolver) ResolveCell(
+	ctx context.Context,
+	cellSpec *multigresv1alpha1.CellConfig,
+) (*multigresv1alpha1.StatelessSpec, *multigresv1alpha1.LocalTopoServerSpec, error) {
+	// 1. Fetch Template (Logic handles defaults)
+	templateName := cellSpec.CellTemplate
+	tpl, err := r.ResolveCellTemplate(ctx, templateName)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	// 2. Merge Logic
+	gateway, localTopo := mergeCellConfig(tpl, cellSpec.Overrides, cellSpec.Spec)
+
+	// 3. Apply Deep Defaults (Level 4)
+	// We use empty resources for Gateway default, as the specific values are often deployment-dependent,
+	// but we must ensure Replicas is at least 1.
+	defaultStatelessSpec(gateway, corev1.ResourceRequirements{}, 1)
+
+	// Note: We do NOT default LocalTopo here because it is optional.
+	// If it is nil, it remains nil (meaning the cell uses Global Topo).
+	// If it is non-nil (e.g. from template), we apply Etcd defaults.
+	if localTopo != nil && localTopo.Etcd != nil {
+		defaultEtcdSpec(localTopo.Etcd)
+	}
+
+	return gateway, localTopo, nil
+}
+
+// ResolveCellTemplate fetches and resolves a CellTemplate by name.
+// If name is empty, it resolves using the Cluster Defaults, then the Namespace Default.
 func (r *Resolver) ResolveCellTemplate(
 	ctx context.Context,
-	templateName string,
+	name string,
 ) (*multigresv1alpha1.CellTemplate, error) {
-	name := templateName
+	resolvedName := name
 	isImplicitFallback := false
 
-	if name == "" {
-		name = r.TemplateDefaults.CellTemplate
+	if resolvedName == "" {
+		resolvedName = r.TemplateDefaults.CellTemplate
 	}
-	if name == "" {
-		name = FallbackCellTemplate
+	if resolvedName == "" {
+		resolvedName = FallbackCellTemplate
 		isImplicitFallback = true
 	}
 
 	tpl := &multigresv1alpha1.CellTemplate{}
-	err := r.Client.Get(ctx, types.NamespacedName{Name: name, Namespace: r.Namespace}, tpl)
+	err := r.Client.Get(ctx, types.NamespacedName{Name: resolvedName, Namespace: r.Namespace}, tpl)
 	if err != nil {
 		if errors.IsNotFound(err) {
 			if isImplicitFallback {
+				// We return an empty struct instead of nil to satisfy tests expecting non-nil structure.
 				return &multigresv1alpha1.CellTemplate{}, nil
 			}
-			return nil, fmt.Errorf("referenced CellTemplate '%s' not found: %w", name, err)
+			return nil, fmt.Errorf("referenced CellTemplate '%s' not found: %w", resolvedName, err)
 		}
 		return nil, fmt.Errorf("failed to get CellTemplate: %w", err)
 	}
 	return tpl, nil
 }
 
-// MergeCellConfig merges a template spec with overrides and an inline spec to produce the final configuration.
-func MergeCellConfig(
+// mergeCellConfig merges a template spec with overrides and an inline spec.
+func mergeCellConfig(
 	template *multigresv1alpha1.CellTemplate,
 	overrides *multigresv1alpha1.CellOverrides,
 	inline *multigresv1alpha1.CellInlineSpec,
@@ -65,6 +99,10 @@ func MergeCellConfig(
 	}
 
 	if inline != nil {
+		// Inline spec completely replaces the template for the components it defines
+		// However, for Multigres 'Spec' blocks, usually 'Spec' is exclusive to 'TemplateRef'.
+		// The design allows "Inline Spec" OR "Template + Overrides".
+		// If Inline Spec is present, we generally prefer it entirely.
 		gw := inline.MultiGateway.DeepCopy()
 		var topo *multigresv1alpha1.LocalTopoServerSpec
 		if inline.LocalTopoServer != nil {

pkg/resolver/cell_test.go

@@ -16,6 +16,100 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
+func TestResolver_ResolveCell(t *testing.T) {
+	t.Parallel()
+
+	scheme := runtime.NewScheme()
+	_ = multigresv1alpha1.AddToScheme(scheme)
+	_, cellTpl, _, ns := setupFixtures(t)
+
+	tests := map[string]struct {
+		config   *multigresv1alpha1.CellConfig
+		objects  []client.Object
+		wantGw   *multigresv1alpha1.StatelessSpec
+		wantTopo *multigresv1alpha1.LocalTopoServerSpec
+		wantErr  bool
+	}{
+		"Template Found": {
+			config: &multigresv1alpha1.CellConfig{
+				CellTemplate: "default",
+			},
+			objects: []client.Object{cellTpl},
+			wantGw: &multigresv1alpha1.StatelessSpec{
+				Replicas:  ptr.To(int32(1)),
+				Resources: corev1.ResourceRequirements{},
+			},
+			wantTopo: &multigresv1alpha1.LocalTopoServerSpec{
+				Etcd: &multigresv1alpha1.EtcdSpec{
+					Image:     "local-etcd-default",
+					Replicas:  ptr.To(DefaultEtcdReplicas),
+					Resources: DefaultResourcesEtcd(),
+					Storage:   multigresv1alpha1.StorageSpec{Size: DefaultEtcdStorageSize},
+				},
+			},
+		},
+		"Template Not Found (Error)": {
+			config: &multigresv1alpha1.CellConfig{
+				CellTemplate: "missing",
+			},
+			wantErr: true,
+		},
+		"Inline Overrides": {
+			config: &multigresv1alpha1.CellConfig{
+				Spec: &multigresv1alpha1.CellInlineSpec{
+					MultiGateway: multigresv1alpha1.StatelessSpec{
+						Replicas: ptr.To(int32(3)),
+					},
+				},
+			},
+			wantGw: &multigresv1alpha1.StatelessSpec{
+				Replicas:  ptr.To(int32(3)),
+				Resources: corev1.ResourceRequirements{},
+			},
+			wantTopo: nil, // Inline spec didn't provide one
+		},
+		"Client Error": {
+			config: &multigresv1alpha1.CellConfig{CellTemplate: "any"},
+			// Will use mock client logic inside test runner
+			wantErr: true,
+		},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			var c client.Client
+			if name == "Client Error" {
+				c = &mockClient{failGet: true, err: errors.New("fail")}
+			} else {
+				c = fake.NewClientBuilder().
+					WithScheme(scheme).
+					WithObjects(tc.objects...).
+					Build()
+			}
+			r := NewResolver(c, ns, multigresv1alpha1.TemplateDefaults{})
+
+			gw, topo, err := r.ResolveCell(t.Context(), tc.config)
+			if tc.wantErr {
+				if err == nil {
+					t.Error("Expected error")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("Unexpected error: %v", err)
+			}
+
+			if diff := cmp.Diff(tc.wantGw, gw, cmpopts.IgnoreUnexported(resource.Quantity{}), cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("Gateway Diff (-want +got):\n%s", diff)
+			}
+			if diff := cmp.Diff(tc.wantTopo, topo, cmpopts.IgnoreUnexported(resource.Quantity{}), cmpopts.EquateEmpty()); diff != "" {
+				t.Errorf("Topo Diff (-want +got):\n%s", diff)
+			}
+		})
+	}
+}
+
 func TestResolver_ResolveCellTemplate(t *testing.T) {
 	t.Parallel()
 
@@ -237,7 +331,7 @@ func TestMergeCellConfig(t *testing.T) {
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
-			gw, topo := MergeCellConfig(tc.tpl, tc.overrides, tc.inline)
+			gw, topo := mergeCellConfig(tc.tpl, tc.overrides, tc.inline)
 
 			if diff := cmp.Diff(tc.wantGw, gw, cmpopts.IgnoreUnexported(resource.Quantity{}), cmpopts.EquateEmpty()); diff != "" {
 				t.Errorf("Gateway mismatch (-want +got):\n%s", diff)