-
Notifications
You must be signed in to change notification settings - Fork 356
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
feat: determined_master_host and friends helm support, better defaults #10138
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10138 +/- ##
==========================================
+ Coverage 54.71% 54.73% +0.02%
==========================================
Files 1266 1266
Lines 159970 159989 +19
Branches 3662 3662
==========================================
+ Hits 87525 87572 +47
+ Misses 72312 72284 -28
Partials 133 133
Flags with carried forward coverage won't be shown. Click here to find out more.
|
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Same as the previous PR I approved other than the DNS param order. Bradley's gonna wait for the GKE tests to run to merge.
#10138) When `determined_master_ip` is unsettable via Helm and defaults to the service IP, life with proxies is hard. This change renames `determined_master_ip` to `determined_master_host` everywhere with some backwards compatibility, defaults `determined_master_host` to `<det_namespace>.<det_service_name>.svc.cluster.local`, and makes all of this overridable in Helm. (cherry picked from commit 4e374c3)
…rt, better defaults" (#10134)"
This reverts commit 782f7a0.
Ticket
DFR-529/530
Description
When
determined_master_ip
is unsettable via Helm and defaults to the service IP, life with proxies is hard. This change renamesdetermined_master_ip
todetermined_master_host
everywhere with some backwards compatibility, defaultsdetermined_master_host
to<det_namespace>.<det_service_name>.svc.cluster.local
, and makes all of this overridable in Helm.Reverted this, re-landing, had an embarrassing typo that broke everything.
Test Plan
Has automated tests, that's why I knew to revert.
Checklist
docs/release-notes/
See Release Note for details.