-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Allow k3s to customize apiServerPort on helm-controller #7834
Conversation
go.mod
Outdated
@@ -16,6 +16,8 @@ replace ( | |||
github.com/golang/protobuf => github.com/golang/protobuf v1.5.3 | |||
github.com/googleapis/gax-go/v2 => github.com/googleapis/gax-go/v2 v2.1.1 | |||
github.com/juju/errors => github.com/k3s-io/nocode v0.0.0-20200630202308-cb097102c09f | |||
|
|||
github.com/k3s-io/helm-controller => github.com/acorn-io/helm-controller v0.0.0-20230626182853-bb03cf954008 |
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.
Why are we now replacing a k3s-io org repo with an external one?
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 believe this is just to test the change before k3s-io/helm-controller#198 is merged.
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.
This will be changed once the dependency PR is merged
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.
Ready for update: https://github.com/k3s-io/helm-controller/releases/tag/v0.15.2
830a7e2
to
527cea0
Compare
LGTM, looks like you need to rebase and resolve a conflict though. |
Signed-off-by: Daishan Peng <daishan@acorn.io>
527cea0
to
803b217
Compare
@brandond thanks for the quick approval, rebased 👍 |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #7834 +/- ##
===========================================
+ Coverage 19.85% 51.52% +31.67%
===========================================
Files 83 143 +60
Lines 7686 14519 +6833
===========================================
+ Hits 1526 7481 +5955
+ Misses 5930 5852 -78
- Partials 230 1186 +956
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
Proposed Changes
When using helm-controller with bootstrap option, the apiServer port is hard-coded to 6443. This means that if you are using a port other than 6443 for api-server, it will be broken. Instead, it should be parameterized and pass down apiServerPort config to helm controller
Helm-controller PR: k3s-io/helm-controller#198
Types of Changes
Verification
Testing
Linked Issues
#7835
User-Facing Change
Further Comments