Review fixes

Author: lsierant

api/v1/search/mongodbsearch_types.go

@@ -38,6 +38,15 @@ type Prometheus struct {
 	Port int `json:"port,omitempty"`
 }
 
+func (p *Prometheus) GetPort() int32 {
+	if p.Port == 0 {
+		return MongotDefaultPrometheusPort
+	}
+
+	//nolint:gosec
+	return int32(p.Port)
+}
+
 type MongoDBSearchSpec struct {
 	// Optional version of MongoDB Search component (mongot). If not set, then the operator will set the most appropriate version of MongoDB Search.
 	// +optional
@@ -229,15 +238,6 @@ func (s *MongoDBSearch) GetMongotGrpcPort() int32 {
 	return MongotDefaultGrpcPort
 }
 
-func (s *MongoDBSearch) GetMongotMetricsPort() int32 {
-	if s.Spec.Prometheus == nil || s.Spec.Prometheus.Port == 0 {
-		return MongotDefaultPrometheusPort
-	}
-
-	//nolint:gosec
-	return int32(s.Spec.Prometheus.Port)
-}
-
 // TLSSecretNamespacedName will get the namespaced name of the Secret containing the server certificate and key
 func (s *MongoDBSearch) TLSSecretNamespacedName() types.NamespacedName {
 	return types.NamespacedName{Name: s.Spec.Security.TLS.CertificateKeySecret.Name, Namespace: s.Namespace}

api/v1/search/zz_generated.deepcopy.go

@@ -112,7 +112,7 @@ spec:
                 properties:
                   port:
                     description: Port where metrics endpoint will be exposed on. Defaults
-                      to 9216.
+                      to 9946.
                     maximum: 65535
                     minimum: 0
                     type: integer

config/crd/bases/mongodb.com_mongodbsearch.yaml

@@ -3,10 +3,10 @@ package operator
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -54,15 +54,14 @@ func init() {
 
 // getReleaseJsonPath searches for a specified target directory by traversing the directory tree backwards from the current working directory
 func getReleaseJsonPath() (string, error) {
-	repositoryRootDirName := "mongodb-kubernetes"
 	releaseFileName := "release.json"
 
 	currentDir, err := os.Getwd()
 	if err != nil {
 		return "", err
 	}
 	for currentDir != "/" {
-		if strings.HasSuffix(currentDir, repositoryRootDirName) {
+		if _, err := os.Stat(filepath.Join(currentDir, releaseFileName)); !errors.Is(err, os.ErrNotExist) {
 			return filepath.Join(currentDir, releaseFileName), nil
 		}
 		currentDir = filepath.Dir(currentDir)

controllers/operator/appdbreplicaset_controller_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/ghodss/yaml"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -22,12 +23,12 @@ import (
 	"github.com/mongodb/mongodb-kubernetes/api/v1/status"
 	userv1 "github.com/mongodb/mongodb-kubernetes/api/v1/user"
 	"github.com/mongodb/mongodb-kubernetes/controllers/operator/mock"
-	"github.com/mongodb/mongodb-kubernetes/controllers/operator/workflow"
 	"github.com/mongodb/mongodb-kubernetes/controllers/searchcontroller"
 	mdbcv1 "github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/api/v1"
 	"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/api/v1/common"
 	"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/mongot"
 	"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/constants"
+	"github.com/mongodb/mongodb-kubernetes/pkg/util"
 )
 
 func newMongoDBCommunity(name, namespace string) *mdbcv1.MongoDBCommunity {
@@ -135,10 +136,6 @@ func buildExpectedMongotConfig(search *searchv1.MongoDBSearch, mdbc *mdbcv1.Mong
 			},
 			Wireproto: wireprotoServer,
 		},
-		Metrics: mongot.ConfigMetrics{
-			Enabled: true,
-			Address: fmt.Sprintf("0.0.0.0:%d", search.GetMongotMetricsPort()),
-		},
 		HealthCheck: mongot.ConfigHealthCheck{
 			Address: fmt.Sprintf("0.0.0.0:%d", search.GetMongotHealthCheckPort()),
 		},
@@ -205,22 +202,16 @@ func TestMongoDBSearchReconcile_Success(t *testing.T) {
 			mdbc := newMongoDBCommunity("mdb", mock.TestNamespace)
 			reconciler, c := newSearchReconciler(mdbc, search)
 
-			res, err := reconciler.Reconcile(
-				ctx,
-				reconcile.Request{NamespacedName: types.NamespacedName{Name: search.Name, Namespace: search.Namespace}},
-			)
-			expected, _ := workflow.OK().ReconcileResult()
-			assert.NoError(t, err)
-			assert.Equal(t, expected, res)
+			checkSearchReconcileSuccessful(ctx, t, reconciler, c, search)
 
 			svc := &corev1.Service{}
-			err = c.Get(ctx, search.SearchServiceNamespacedName(), svc)
+			err := c.Get(ctx, search.SearchServiceNamespacedName(), svc)
 			assert.NoError(t, err)
 			servicePortNames := []string{}
 			for _, port := range svc.Spec.Ports {
 				servicePortNames = append(servicePortNames, port.Name)
 			}
-			expectedPortNames := []string{"mongot-grpc", "metrics", "healthcheck"}
+			expectedPortNames := []string{"mongot-grpc", "healthcheck"}
 			if tc.withWireproto {
 				expectedPortNames = append(expectedPortNames, "mongot-wireproto")
 			}
@@ -254,14 +245,42 @@ func checkSearchReconcileFailed(
 		reconcile.Request{NamespacedName: types.NamespacedName{Name: search.Name, Namespace: search.Namespace}},
 	)
 	assert.NoError(t, err)
-	assert.True(t, res.RequeueAfter > 0)
+	assert.Less(t, res.RequeueAfter, util.TWENTY_FOUR_HOURS)
 
 	updated := &searchv1.MongoDBSearch{}
 	assert.NoError(t, c.Get(ctx, types.NamespacedName{Name: search.Name, Namespace: search.Namespace}, updated))
 	assert.Equal(t, status.PhaseFailed, updated.Status.Phase)
 	assert.Contains(t, updated.Status.Message, expectedMsg)
 }
 
+// checkSearchReconcileSuccessful performs reconcile to check if it gets to a Running state.
+// In case it's a first reconcile and still Pending it's retried with mocked sts simulated as ready.
+func checkSearchReconcileSuccessful(
+	ctx context.Context,
+	t *testing.T,
+	reconciler *MongoDBSearchReconciler,
+	c client.Client,
+	search *searchv1.MongoDBSearch,
+) {
+	namespacedName := types.NamespacedName{Name: search.Name, Namespace: search.Namespace}
+	res, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+	require.NoError(t, err)
+	mdbs := &searchv1.MongoDBSearch{}
+	require.NoError(t, c.Get(ctx, namespacedName, mdbs))
+	if mdbs.Status.Phase == status.PhasePending {
+		// mark mocked search statefulset as ready to not return Pending this time
+		require.NoError(t, mock.MarkAllStatefulSetsAsReady(ctx, search.Namespace, c))
+
+		res, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: namespacedName})
+		require.NoError(t, err)
+		mdbs = &searchv1.MongoDBSearch{}
+		require.NoError(t, c.Get(ctx, namespacedName, mdbs))
+	}
+
+	require.Equal(t, util.TWENTY_FOUR_HOURS, res.RequeueAfter)
+	require.Equal(t, status.PhaseRunning, mdbs.Status.Phase)
+}
+
 func TestMongoDBSearchReconcile_InvalidVersion(t *testing.T) {
 	ctx := context.Background()
 	search := newMongoDBSearch("search", mock.TestNamespace, "mdb")

controllers/operator/mongodbsearch_controller_test.go

@@ -331,12 +331,12 @@ func buildSearchHeadlessService(search *searchv1.MongoDBSearch) corev1.Service {
 		TargetPort: intstr.FromInt32(search.GetMongotGrpcPort()),
 	})
 
-	if search.GetPrometheus() != nil {
+	if prometheus := search.GetPrometheus(); prometheus != nil {
 		serviceBuilder.AddPort(&corev1.ServicePort{
 			Name:       "prometheus",
 			Protocol:   corev1.ProtocolTCP,
-			Port:       search.GetMongotMetricsPort(),
-			TargetPort: intstr.FromInt32(search.GetMongotMetricsPort()),
+			Port:       prometheus.GetPort(),
+			TargetPort: intstr.FromInt32(prometheus.GetPort()),
 		})
 	}
 
@@ -389,14 +389,9 @@ func createMongotConfig(search *searchv1.MongoDBSearch, db SearchSourceDBResourc
 		}
 
 		if prometheus := search.GetPrometheus(); prometheus != nil {
-			port := search.GetMongotMetricsPort()
-			if prometheus.Port != 0 {
-				//nolint:gosec
-				port = int32(prometheus.Port)
-			}
 			config.Metrics = mongot.ConfigMetrics{
 				Enabled: true,
-				Address: fmt.Sprintf("0.0.0.0:%d", port),
+				Address: fmt.Sprintf("0.0.0.0:%d", prometheus.GetPort()),
 			}
 		}
 
@@ -484,9 +479,9 @@ func (r *MongoDBSearchReconcileHelper) getMongotImage() string {
 		return ""
 	}
 
-	for _, container := range r.mdbSearch.Spec.StatefulSetConfiguration.SpecWrapper.Spec.Template.Spec.Containers {
-		if container.Name == MongotContainerName {
-			return container.Image
+	for _, c := range r.mdbSearch.Spec.StatefulSetConfiguration.SpecWrapper.Spec.Template.Spec.Containers {
+		if c.Name == MongotContainerName {
+			return c.Image
 		}
 	}
 

controllers/searchcontroller/mongodbsearch_reconcile_helper.go

@@ -295,7 +295,6 @@ def get_user_sample_movies_helper(mdb):
 def assert_search_service_prometheus_port(mdbs: MongoDBSearch, should_exist: bool, expected_port: int = 9946):
     service_name = f"{mdbs.name}-search-svc"
     service = get_service(mdbs.namespace, service_name)
-
     assert service is not None
 
     ports = {p.name: p.port for p in service.spec.ports}
@@ -308,6 +307,10 @@ def assert_search_service_prometheus_port(mdbs: MongoDBSearch, should_exist: boo
 
 
 def deploy_mongodb_tools_pod(namespace: str):
+    """
+    Deploys a bastion pod to perform connectivty checks using pod exec. It's similar to how we
+    run connectivity checks in snippets.
+    """
     from kubetester import get_pod_when_ready
 
     pod_body = {
@@ -345,6 +348,8 @@ def assert_search_pod_prometheus_endpoint(mdbs: MongoDBSearch, should_be_accessi
     url = f"http://{service_fqdn}:{port}/metrics"
 
     if should_be_accessible:
+        # We don't necessarily need the connectivity test to run via a bastion pod as we could connect to it directly when running test in pod.
+        # But it's not requiring forwarding when running locally.
         result = KubernetesTester.run_command_in_pod_container(
             "mongodb-tools-pod", mdbs.namespace, ["curl", "-f", "-s", url], container="mongodb-tools"
         )

docker/mongodb-kubernetes-tests/tests/search/search_enterprise_tls.py

@@ -112,7 +112,7 @@ spec:
                 properties:
                   port:
                     description: Port where metrics endpoint will be exposed on. Defaults
-                      to 9216.
+                      to 9946.
                     maximum: 65535
                     minimum: 0
                     type: integer

helm_chart/crds/mongodb.com_mongodbsearch.yaml

@@ -4134,7 +4134,7 @@ spec:
                 properties:
                   port:
                     description: Port where metrics endpoint will be exposed on. Defaults
-                      to 9216.
+                      to 9946.
                     maximum: 65535
                     minimum: 0
                     type: integer