K8SPSMDB-1537: connection string secret fixes#2442
Draft
pooknull wants to merge 2 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates how the operator builds MongoDB connection strings and manages the lifecycle of the generated connection-string Secrets, primarily improving correctness for LoadBalancer mongos services and avoiding references to terminating pods.
Changes:
- Use mongos Service FQDNs for internal connection strings when mongos is exposed via
LoadBalancer(instead of using Service ClusterIP). - Skip terminating (
DeletionTimestamp != nil) replset and mongos pods when generating host lists for connection strings. - Set connection-string Secret owner references to the
PerconaServerMongoDBCR (instead of the system users Secret), so they are removed with cluster deletion; update tests accordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/psmdb/service.go | Adjust mongos internal host resolution for LoadBalancer; skip terminating pods when building replset/mongos address lists. |
| pkg/psmdb/service_test.go | Update MongosHost unit tests to expect FQDN for internal LB addressing. |
| pkg/psmdb/client_test.go | Add tests ensuring terminating pods are skipped in generated replset/mongos URIs; update LB-internal expectation to FQDN. |
| pkg/controller/perconaservermongodb/users.go | Update call-site for connection-string Secret creation after signature/ownership change. |
| pkg/controller/perconaservermongodb/secrets.go | Change connection-string Secret owner reference to the cluster CR. |
| pkg/controller/perconaservermongodb/secrets_test.go | Update expected mongos connection string values and owner-reference assertions; minor formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
239
to
+243
| for _, pod := range pods { | ||
| if pod.DeletionTimestamp != nil { | ||
| continue | ||
| } | ||
|
|
Comment on lines
270
to
+274
| for _, pod := range list.Items { | ||
| if pod.DeletionTimestamp != nil { | ||
| continue | ||
| } | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://perconadev.atlassian.net/browse/K8SPSMDB-1537
DESCRIPTION
This PR fixes internal mongos connection strings when
LoadBalancerservice type is used.Previously, internal connection strings used the service ClusterIP for LoadBalancer services:
With this change, internal mongos connection strings now use FQDNs instead:
This PR also changes connection string secret owner references to point to the cluster instead of the user secret, so the secrets are removed when the cluster is deleted. It makes more sense because connection string secret after the cluster was deleted could remain with outdated connection strings.
Also, connection string generation now skips terminating pods, preventing generated connection strings from containing pods that are being removed.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability