- Notifications
You must be signed in to change notification settings - Fork 24
CLOUDP-338399: External mongod for Search #308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLOUDP-338399: External mongod for Search #308
Conversation
MCK 1.2.1 Release NotesOther Changes
|
… anandsyncs/external-mongod
fealebenpae left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I just recommend explicitly using distinct names for the mdbc and mdbs resources to make sure the implicit name matching isn't kicking in at all.
Please resolve the lint issue and merge.
lsierant left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left some comments
| sample_movies_helper.assert_search_query(retry_timeout=60) | ||
| | ||
| | ||
| def get_connection_string(mdbc: MongoDBCommunity, user_name: str, user_password: str) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably make this an utility function as we have few copies of it already
docker/mongodb-kubernetes-tests/tests/search/search_community_external_mongod_tls.py Show resolved Hide resolved
docker/mongodb-kubernetes-tests/tests/search/search_community_external_mongod_tls.py Outdated Show resolved Hide resolved
mongodb-community-operator/controllers/replica_set_controller.go Outdated Show resolved Hide resolved
| Enabled bool `json:"enabled"` | ||
| // +optional | ||
| CASecretRef *userv1.SecretKeyRef `json:"caSecretRef,omitempty"` | ||
| CA *corev1.LocalObjectReference `json:"ca,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need that object instead of just a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used this to keep it kind of consistent with rest of the codebase.
Summary
This pull request introduces support for using external MongoDB deployments as sources for
MongoDBSearchresources, in addition to the previously supported operator-managed deployments. The changes include updates to the CRD API, controller logic, and supporting code to handle configuration, validation, and reconciliation for external sources. Additionally, new end-to-end (e2e) test tasks are added to ensure this functionality is covered.External MongoDB Source Support
MongoDBSourcespec (ExternalMongoDBSourceand supporting types) to allow specifying external MongoDB deployments, including host/port, credentials, and TLS settings.IsExternalMongoDBSourcemethod and updates to resource reference logic.Controller and Reconciliation Logic Updates
SearchSourceDBResourceinterface and its implementations to support both internal and external MongoDB sources, including connection details and version validation.Testing and CI
e2e_search_external_basicande2e_search_external_tls) and included them in the appropriate task groups to ensure coverage in CI.These changes collectively enable
MongoDBSearchresources to work with both operator-managed and external MongoDB deployments, increasing flexibility for users and improving the robustness of the operator.Proof of Work
Tests pass
Checklist
skip-changeloglabel if not needed