Sharded multiple mongot: review fixes (#827)

Fixes to search with multiple-mongot
# Conflicts:
#	controllers/searchcontroller/mongodbsearch_reconcile_helper.go

Author: lsierant

api/v1/search/mongodbsearch_types.go

@@ -56,9 +56,9 @@ type MongoDBSearchSpec struct {
 	// +optional
 	Source *MongoDBSource `json:"source"`
 	// Replicas is the number of mongot pods to deploy.
-	// For ReplicaSet source: this many mongot pods total.
-	// For Sharded source: this many mongot pods per shard.
-	// When Replicas > 1, a load balancer configuration (lb.mode: Unmanaged with lb.endpoint)
+	// For ReplicaSet source: the number of mongot pods in total.
+	// For Sharded source: the number mongot pods per shard.
+	// When Replicas > 1, a load balancer configuration (spec.lb)
 	// is required to distribute traffic across mongot instances.
 	// +optional
 	// +kubebuilder:validation:Minimum=1
@@ -88,7 +88,7 @@ type MongoDBSearchSpec struct {
 	// `embedding` field of mongot config is generated using the values provided here.
 	// +optional
 	AutoEmbedding *EmbeddingConfig `json:"autoEmbedding,omitempty"`
-	// LoadBalancer configures how mongod/mongos connect to mongot (Managed vs Unmanaged/BYO LB).
+	// LoadBalancer configures how mongod/mongos connect to mongot (Managed vs Unmanaged/BYO Load Balancer).
 	// +optional
 	LoadBalancer *LoadBalancerConfig `json:"lb,omitempty"`
 }
@@ -98,19 +98,21 @@ type MongoDBSearchSpec struct {
 type LBMode string
 
 const (
-	// LBModeManaged indicates operator-managed Envoy load balancer
+	// LBModeManaged indicates operator-managed load balancer
 	LBModeManaged LBMode = "Managed"
 	// LBModeUnmanaged indicates user-provided L7 load balancer (BYO LB)
 	LBModeUnmanaged LBMode = "Unmanaged"
 )
 
 // LoadBalancerConfig configures how mongod/mongos connect to mongot
 type LoadBalancerConfig struct {
-	// Mode specifies the load balancer mode: Managed (operator-managed) or Unmanaged (BYO L7 LB)
+	// Mode specifies the load balancer mode: Managed (operator-managed) or Unmanaged (BYO L7 Load Balancer)
 	// +kubebuilder:validation:Required
 	Mode LBMode `json:"mode"`
 	// Endpoint is the LB endpoint for ReplicaSet, or a template for sharded clusters.
-	// For sharded clusters, use {shardName} as a placeholder substituted with the actual shard name.
+	// For sharded clusters, at least one {shardName} placeholder must be present and will be substituted with the actual shard name.
+	// Each shard must connect to their mongots using a shard-unique endpoint to allow LoadBalancer to
+	// route the traffic to the mongot instances for that shard.
 	// Example: "lb-{shardName}.example.com:27028"
 	// +optional
 	Endpoint string `json:"endpoint,omitempty"`
@@ -452,7 +454,7 @@ func (s *MongoDBSearch) IsLBModeUnmanaged() bool {
 	return s.Spec.LoadBalancer != nil && s.Spec.LoadBalancer.Mode == LBModeUnmanaged
 }
 
-// IsReplicaSetUnmanagedLB returns true if this is a ReplicaSet unmanaged LB configuration.
+// IsReplicaSetUnmanagedLB returns true if this is a ReplicaSet with unmanaged LB configuration.
 // An endpoint with a template placeholder ({shardName}) is NOT considered a ReplicaSet endpoint.
 func (s *MongoDBSearch) IsReplicaSetUnmanagedLB() bool {
 	return s.IsLBModeUnmanaged() &&

api/v1/search/mongodbsearch_validation.go

@@ -7,7 +7,6 @@ import (
 	v1 "github.com/mongodb/mongodb-kubernetes/api/v1"
 )
 
-// ValidateSpec validates the MongoDBSearch spec
 func (s *MongoDBSearch) ValidateSpec() error {
 	for _, res := range s.RunValidations() {
 		if res.Level == v1.ErrorLevel {
@@ -17,13 +16,11 @@ func (s *MongoDBSearch) ValidateSpec() error {
 	return nil
 }
 
-// RunValidations runs all validation rules and returns the results
 func (s *MongoDBSearch) RunValidations() []v1.ValidationResult {
 	validators := []func(*MongoDBSearch) v1.ValidationResult{
 		validateLBConfig,
 		validateUnmanagedLBConfig,
 		validateEndpointTemplate,
-		validateTLSConfig,
 	}
 
 	var results []v1.ValidationResult
@@ -36,7 +33,6 @@ func (s *MongoDBSearch) RunValidations() []v1.ValidationResult {
 	return results
 }
 
-// validateLBConfig validates the load balancer configuration
 func validateLBConfig(s *MongoDBSearch) v1.ValidationResult {
 	if s.Spec.LoadBalancer == nil {
 		// LB config is optional
@@ -77,31 +73,16 @@ func validateEndpointTemplate(s *MongoDBSearch) v1.ValidationResult {
 
 	endpoint := s.Spec.LoadBalancer.Endpoint
 
-	// Template must contain exactly one {shardName} placeholder
 	count := strings.Count(endpoint, ShardNamePlaceholder)
-	if count != 1 {
-		return v1.ValidationError("spec.lb.endpoint template must contain exactly one %s placeholder, found %d", ShardNamePlaceholder, count)
+	if count == 0 {
+		return v1.ValidationError("spec.lb.endpoint template must contain at least one %s placeholder to differentiate between shards, found %d", ShardNamePlaceholder, count)
 	}
 
-	// Template should have some content before or after the placeholder
-	if endpoint == ShardNamePlaceholder {
-		return v1.ValidationError("spec.lb.endpoint template must contain more than just the %s placeholder", ShardNamePlaceholder)
+	// Endpoint must contain more than just the placeholder(s)
+	stripped := strings.TrimSpace(strings.ReplaceAll(endpoint, ShardNamePlaceholder, ""))
+	if stripped == "" {
+		return v1.ValidationError("spec.lb.endpoint must contain more than just the %s placeholder", ShardNamePlaceholder)
 	}
 
 	return v1.ValidationSuccess()
 }
-
-// validateTLSConfig validates the TLS configuration
-func validateTLSConfig(s *MongoDBSearch) v1.ValidationResult {
-	if s.Spec.Security.TLS == nil {
-		return v1.ValidationSuccess()
-	}
-
-	// TLS is valid in all cases:
-	// 1. CertificateKeySecret.Name is specified - use explicit secret name
-	// 2. CertsSecretPrefix is specified - use {prefix}-{resourceName}-search-cert
-	// 3. Both are empty - use default {resourceName}-search-cert
-	// No validation error needed as we always have a valid fallback
-
-	return v1.ValidationSuccess()
-}

config/crd/bases/mongodb.com_mongodbsearch.yaml

@@ -80,12 +80,14 @@ spec:
                 type: object
               lb:
                 description: LoadBalancer configures how mongod/mongos connect to
-                  mongot (Managed vs Unmanaged/BYO LB).
+                  mongot (Managed vs Unmanaged/BYO Load Balancer).
                 properties:
                   endpoint:
                     description: |-
                       Endpoint is the LB endpoint for ReplicaSet, or a template for sharded clusters.
-                      For sharded clusters, use {shardName} as a placeholder substituted with the actual shard name.
+                      For sharded clusters, at least one {shardName} placeholder must be present and will be substituted with the actual shard name.
+                      Each shard must connect to their mongots using a shard-unique endpoint to allow LoadBalancer to
+                      route the traffic to the mongot instances for that shard.
                       Example: "lb-{shardName}.example.com:27028"
                     type: string
                   envoy:
@@ -94,7 +96,7 @@ spec:
                     type: object
                   mode:
                     description: 'Mode specifies the load balancer mode: Managed (operator-managed)
-                      or Unmanaged (BYO L7 LB)'
+                      or Unmanaged (BYO L7 Load Balancer)'
                     enum:
                     - Managed
                     - Unmanaged
@@ -176,9 +178,9 @@ spec:
                 default: 1
                 description: |-
                   Replicas is the number of mongot pods to deploy.
-                  For ReplicaSet source: this many mongot pods total.
-                  For Sharded source: this many mongot pods per shard.
-                  When Replicas > 1, a load balancer configuration (lb.mode: Unmanaged with lb.endpoint)
+                  For ReplicaSet source: the number of mongot pods in total.
+                  For Sharded source: the number mongot pods per shard.
+                  When Replicas > 1, a load balancer configuration (spec.lb)
                   is required to distribute traffic across mongot instances.
                 minimum: 1
                 type: integer

controllers/operator/mongodbshardedcluster_controller.go

@@ -1022,7 +1022,6 @@ func (r *ShardedClusterReconcileHelper) applySearchParametersForShards(ctx conte
 
 	log.Infof("Applying search parameters from MongoDBSearch %s", search.NamespacedName())
 
-	// Collect shard names for mongos configuration
 	shardNames := make([]string, sc.Spec.ShardCount)
 	for shardIdx := 0; shardIdx < sc.Spec.ShardCount; shardIdx++ {
 		shardNames[shardIdx] = sc.ShardRsName(shardIdx)

controllers/searchcontroller/enterprise_search_source_test.go

@@ -372,40 +372,3 @@ func TestShardedEnterpriseSearchSource_GetShardNames(t *testing.T) {
 		})
 	}
 }
-
-func TestShardedEnterpriseSearchSource_Validate(t *testing.T) {
-	tests := []struct {
-		name           string
-		shardCount     int
-		expectError    bool
-		expectedErrMsg string
-	}{
-		{
-			name:        "Valid - two shards",
-			shardCount:  2,
-			expectError: false,
-		},
-		{
-			name:        "Valid - single shard",
-			shardCount:  1,
-			expectError: false,
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			mdb := newShardedClusterMongoDB("my-cluster", "test-ns", tc.shardCount, "8.2.0")
-			search := newShardedUnmanagedLBSearch("test-search", "test-ns", "my-cluster", "")
-			src := NewShardedEnterpriseSearchSource(mdb, search)
-
-			err := src.Validate()
-
-			if tc.expectError {
-				assert.Error(t, err)
-				assert.Contains(t, err.Error(), tc.expectedErrMsg)
-			} else {
-				assert.NoError(t, err)
-			}
-		})
-	}
-}

controllers/searchcontroller/mongodbsearch_reconcile_helper.go

@@ -125,10 +125,6 @@ func (r *MongoDBSearchReconcileHelper) reconcile(ctx context.Context, log *zap.S
 		return workflow.Failed(err)
 	}
 
-	if err := r.ValidateAutoEmbeddingConfig(); err != nil {
-		return workflow.Failed(err)
-	}
-
 	if shardedSource, ok := r.db.(SearchSourceShardedDeployment); ok {
 		return r.reconcileSharded(ctx, log, shardedSource, version)
 	}
@@ -1057,7 +1053,7 @@ func (r *MongoDBSearchReconcileHelper) ValidateMultipleReplicasConfig() error {
 		if !r.mdbSearch.IsShardedUnmanagedLB() {
 			return xerrors.Errorf(
 				"multiple mongot replicas (%d) require unmanaged load balancer configuration; "+
-					"please configure spec.lb.mode=Unmanaged with spec.lb.endpoint for sharded clusters",
+					"please configure load balancing in spec.lb.",
 				r.mdbSearch.GetReplicas(),
 			)
 		}
@@ -1068,26 +1064,7 @@ func (r *MongoDBSearchReconcileHelper) ValidateMultipleReplicasConfig() error {
 	if !r.mdbSearch.IsReplicaSetUnmanagedLB() {
 		return xerrors.Errorf(
 			"multiple mongot replicas (%d) require unmanaged load balancer configuration; "+
-				"please configure spec.lb.mode=Unmanaged with spec.lb.endpoint",
-			r.mdbSearch.GetReplicas(),
-		)
-	}
-
-	return nil
-}
-
-// ValidateAutoEmbeddingConfig validates that auto embeddings are not configured with multiple mongot replicas.
-// Auto embeddings require the IsAutoEmbeddingViewWriter flag to be set to true for exactly one mongot instance.
-// When multiple replicas are configured, all mongot instances would have this flag set to true, causing conflicts.
-func (r *MongoDBSearchReconcileHelper) ValidateAutoEmbeddingConfig() error {
-	if r.mdbSearch.Spec.AutoEmbedding == nil {
-		return nil
-	}
-
-	if r.mdbSearch.HasMultipleReplicas() {
-		return xerrors.Errorf(
-			"auto embeddings are not supported with multiple mongot replicas (%d); "+
-				"please set spec.source.replicas to 1 or remove spec.autoEmbedding",
+				"please configure load balancing in spec.lb.",
 			r.mdbSearch.GetReplicas(),
 		)
 	}