-
Notifications
You must be signed in to change notification settings - Fork 21
Fix: Use CLUSTER_DOMAIN environment variable instead of hardcoded cluster.local #481
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
base: master
Are you sure you want to change the base?
Conversation
…alue `cluster.local`
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
… field - Introduced `ClusterDomain` field in `MongoDBCommunitySpec`. - Updated functions to use `ClusterDomain` from spec instead of environment variables. - Adjusted unit tests for new `ClusterDomain` behavior.
…d functions - Updated `GetMongodConfigParameters` to accept `ClusterDomain` as a parameter. - Modified calls to `mongotHostAndPort` to include the `ClusterDomain` argument.
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.
Most of the changes are really good!
We just need to clarify spec.GetClusterDomain
issue from the comment.
@MaciejKaras, one question: my changes require updating the custom resource CRD. I didn't include the changes for Helm charts in this PR because I believe they will be released automatically in a new repository version. |
changelog/20250925_fix_use_cluster_domain_environment_variable_instead_of.md
Outdated
Show resolved
Hide resolved
changelog/20250925_fix_use_cluster_domain_environment_variable_instead_of.md
Outdated
Show resolved
Hide resolved
@edimarlnx Change request should contain also changes to helm charts. Please generate CRD changes by running |
…_instead_of.md Co-authored-by: Maciej Karaś <[email protected]>
…_instead_of.md Co-authored-by: Maciej Karaś <[email protected]>
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.
Nice work @edimarlnx and thanks for the fix!
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.
Really thanks for your collaboration @edimarlnx, we really appreciate it.
LGTM from my side.
changelog/20250925_feature_use_cluster_domain_environment_variable.md
Outdated
Show resolved
Hide resolved
@edimarlnx
Also please run |
- Updated calls to `mdb.MongoURI()` and `mdb.MongoSRVURI()` to exclude unnecessary string argument.
…ementation - Expanded description of changes for `ClusterDomain` field in `CRD` and related methods - Added references to updated methods, tests, and configurations
changelog/20250925_feature_use_cluster_domain_environment_variable.md
Outdated
Show resolved
Hide resolved
- Refined description of `ClusterDomain` support in the changelog - Streamlined explanation of the fallback hierarchy and related CRD changes
@edimarlnx still some unit tests are failing + linting as well. |
@edimarlnx last linting issue ;)
|
Would you have any documentation to run all these tests locally? |
@edimarlnx Unfortunately not :( additionally outside contributors cannot access CI logs, which is not helping either. We should definitely provide some guidance on how to run at least:
This could be added in CONTRIBUTING.md. I will bring this up to the team. |
I tried, but it shows an error and shows an internal link for instructions on how to resolve it. ![]() ![]() If it's okay with you, I'll continue with trial and error. I will fix the ones you mentioned and push them to the tests again. Okay? |
Hey @edimarlnx, after running linter locally here are the changes needed to fix the PR: Subject: [PATCH] Fix linter issues
---
Index: mongodb-community-operator/api/v1/mongodbcommunity_types.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/mongodb-community-operator/api/v1/mongodbcommunity_types.go b/mongodb-community-operator/api/v1/mongodbcommunity_types.go
--- a/mongodb-community-operator/api/v1/mongodbcommunity_types.go (revision 47adda14bdb1cac747f5925ffe70a0f45b0cba0b)
+++ b/mongodb-community-operator/api/v1/mongodbcommunity_types.go (date 1760102034848)
@@ -6,7 +6,6 @@
"regexp"
"strings"
- "github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/envvar"
"github.com/stretchr/objx"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -22,6 +21,7 @@
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/automationconfig"
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/kube/annotations"
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/constants"
+ "github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/envvar"
"github.com/mongodb/mongodb-kubernetes/mongodb-community-operator/pkg/util/scale"
)
@@ -1146,11 +1146,12 @@
func (m MongoDBCommunity) GetAgentMaxLogFileDurationHours() int {
return m.Spec.AgentConfiguration.MaxLogFileDurationHours
}
+
func (m MongoDBCommunitySpec) GetClusterDomain() string {
if m.ClusterDomain != "" {
return m.ClusterDomain
}
- return envvar.GetEnvOrDefault(constants.ClusterDomainEnv, defaultClusterDomain)
+ return envvar.GetEnvOrDefault(constants.ClusterDomainEnv, defaultClusterDomain) // nolint:forbidigo
}
type automationConfigReplicasScaler struct {
Index: public/crds.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/public/crds.yaml b/public/crds.yaml
--- a/public/crds.yaml (revision 47adda14bdb1cac747f5925ffe70a0f45b0cba0b)
+++ b/public/crds.yaml (date 1760102037475)
@@ -6844,6 +6844,9 @@
x-kubernetes-preserve-unknown-fields: true
type: object
type: object
+ clusterDomain:
+ format: hostname
+ type: string
featureCompatibilityVersion:
description: |-
FeatureCompatibilityVersion configures the feature compatibility version that will
|
Summary
This PR adds support for custom cluster domains in Kubernetes deployments by introducing a configurable
ClusterDomain
field in the MongoDB resource spec and utilizing theCLUSTER_DOMAIN
environment variable. This change improves flexibility for deployments in Kubernetes clusters that use custom DNS suffixes instead of the defaultcluster.local
.Key Changes:
CRD & API Changes
ClusterDomain
field toMongoDBCommunitySpec
with hostname format validationconfig/crd/bases/mongodbcommunity.mongodb.com_mongodbcommunity.yaml
andhelm_chart/crds/mongodbcommunity.mongodb.com_mongodbcommunity.yaml
GetClusterDomain()
method inmongodbcommunity_types.go:1149
with fallback hierarchy: spec field →CLUSTER_DOMAIN
environment variable → defaultcluster.local
ClusterDomainEnv
constant inconstants.go:14
with value"CLUSTER_DOMAIN"
Connection URI Refactoring
Removed
clusterDomain
parameter from all public URI methods - now retrieve domain directly from spec:MongoURI()
inmongodbcommunity_types.go:927
MongoSRVURI()
inmongodbcommunity_types.go:934
MongoAuthUserURI()
inmongodbcommunity_types.go:941
MongoAuthUserSRVURI()
inmongodbcommunity_types.go:955
Hosts()
inmongodbcommunity_types.go:967
Controller Updates
buildAutomationConfig()
inreplica_set_controller.go:533
to usemdb.Spec.GetClusterDomain()
getMongodConfigSearchModification()
inreplica_set_controller.go:837
to accept and passclusterDomain
parameterReconcile()
method inreplica_set_controller.go:237,250
updateConnectionStringSecrets()
inmongodb_users.go:44
to removeclusterDomain
parameterapplySearchOverrides()
inmongodbreplicaset_controller.go:659
to pass cluster domain toGetMongodConfigParameters()
MongoDB Search Integration
GetMongodConfigParameters()
inmongodbsearch_reconcile_helper.go:391
to acceptclusterDomain
parametermongotHostAndPort()
inmongodbsearch_reconcile_helper.go:406
to acceptclusterDomain
parameterCommunitySearchSource.HostSeeds()
(community_search_source.go:27
)EnterpriseResourceSearchSource.HostSeeds()
(enterprise_search_source.go:25
)Testing
mongodbcommunity_types_test.go
to use spec field instead of parameterProof of Work
cluster.local
references replaced with configurable cluster domaincluster.local
Files modified:
changelog/20250925_feature_use_cluster_domain_environment_variable.md
(new)config/crd/bases/mongodbcommunity.mongodb.com_mongodbcommunity.yaml
(addedclusterDomain
field)helm_chart/crds/mongodbcommunity.mongodb.com_mongodbcommunity.yaml
(addedclusterDomain
field)controllers/operator/mongodbreplicaset_controller.go:659
controllers/searchcontroller/community_search_source.go:29
controllers/searchcontroller/enterprise_search_source.go:27
controllers/searchcontroller/mongodbsearch_reconcile_helper.go:391-407
mongodb-community-operator/api/v1/mongodbcommunity_types.go:75,927-1152
mongodb-community-operator/api/v1/mongodbcommunity_types_test.go
(multiple test updates)mongodb-community-operator/controllers/mongodb_users.go:44-82
mongodb-community-operator/controllers/replica_set_controller.go:237,250,533-837
mongodb-community-operator/pkg/util/constants/constants.go:14
Checklist