-
Notifications
You must be signed in to change notification settings - Fork 96
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
Improve external access implementaton and documentation via NodePort #121
Conversation
README.md
Outdated
[Install minikube](https://k8s-docs.netlify.app/en/docs/tasks/tools/install-minikube/) (if needed), then start a 4-node cluster: | ||
|
||
```sh | ||
minikube start | ||
minikube start --nodes 4 --extra-config=apiserver.service-node-port-range=8081-65535 | ||
``` | ||
|
||
This command starts minikube with 4 nodes (1 control plane, 3 worker nodes) and will extend the NodePort range to include default ports for Redpanda services. |
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 changed this to install a 4-node cluster rather than a single node, since the default external access method is via NodePort. So each node where Redpanda runs will open the same node, allowing access directly to each broker for external clients. But this only works if we have at least the same number of nodes as replicas in the statefulset, which is 3 by default.
- [External access via LoadBalancer](./with-external-via-loadbalancer/README.md) | ||
- [External access via Ingress](./with-external-via-ingress/README.md) |
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.
These files are what will be added in later PRs (one per file). Right now they don't exist.
# Optional external section | ||
external: | ||
enabled: false | ||
# enabled: true | ||
# Type of external access (options are NodePort and LoadBalancer) | ||
# type: NodePort | ||
# External port | ||
# This listener port will be used for the external port if this is not included | ||
port: 31644 |
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 section is optional for each listener. If it doesn't exist and external is enabled for all listeners, then the NodePort service will attempt to use the port (in this case on line 94) as the NodePort. By default this will result in the service failing to start with an error if you haven't configured Kubernetes to expand the NodePort range. I included details on how to do this for minikube in the startup section of the main README. I wasn't able to find details on how to do this in kind though, and this is entirely optional.
redpanda/values.yaml
Outdated
name: internal | ||
port: 9093 | ||
# TODO make tls section optional | ||
tls: | ||
enabled: false |
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.
A kafka listener with the name internal
is required, as this name is how the code identifies this listener as always being advertised internally rather than externally. This is because the external section is optional, so if global external access is enabled by not explicitly disabled in this listener then it would not be an internal listener without this additional logic.
redpanda/values.yaml
Outdated
@@ -180,7 +227,7 @@ storage: | |||
# used to store Redpanda's data, otherwise `hostPath` is used. | |||
persistentVolume: | |||
# TODO possibly replace with storage.enabled | |||
enabled: true | |||
enabled: false |
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 is a weird line to include in this PR, but it was required to make sure minikube would work. More investigation into why is needed, and there is a ticket in the v2 project related to this: #79
external: | ||
# Default external access value for all listeners except RPC | ||
# External config doesn't apply to RPC listeners as they are never externally accessible | ||
# These values can be overridden by each listener if needed | ||
enabled: true | ||
# Default external access type (options are NodePort and LoadBalancer) | ||
# TODO include IP range for load balancer that support it: https://github.com/redpanda-data/helm-charts/issues/106 | ||
type: NodePort | ||
domain: local | ||
# annotations: | ||
# For example: | ||
# cloud.google.com/load-balancer-type: "Internal" | ||
# service.beta.kubernetes.io/aws-load-balancer-type: nlb |
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 is the most important change to values.yaml
as far as this PR is concerned. This is the top-level external section.
70b09ff
to
c1ade48
Compare
Latest update resolves merge conflicts, updates Redpanda to v22.1.6, and ensures minikube starts with enough memory given to the worker nodes to satisfy default memory settings for Redpanda. |
c1ade48
to
9f7d685
Compare
- update advertised listeners so each listener has expected values - NodePort uses top-level external section - NodePort uses each listener - kafka listener named internal is now required (doesn't have to be first in list) - remove unneeded cluster service
9f7d685
to
267680f
Compare
While verifying merge conflict resolutions, I noticed the NodePort documentation section order was off, so this is also resolved. |
This PR updates how the external access configuration is handled in
values.yaml
, and then focused on NodePort service as the default method for providing external access:first in list)
I started down the path of doing the same level of work for LoadBalancer, but I decided to break the PR into separate NodePort and LoadBalancer PRs... expect a LoadBalancer PR soon with similar details.
NodePort was used prior to this PR. The issue with the previous version related to advertised listeners, and handling listeners properly (kafka and the other listener groups), the values.yaml layout, and the documentation.
I'm surprised people had as much success as they did with the previous version especially with how advertised listeners worked before. There was this template variable $advertiseAddress that was used for every kafka listener, so there was no way to use a different host name for different listeners (internal, external, etc.): https://github.com/redpanda-data/helm-charts/pull/121/files#diff-f74d1889126f524c4b7e7e7836a376c58b0ca5aabe3720e03732b0e25d6ee380
Also each listener can take a list of listeners, but the helm chart did not use any of them but the first entry for most listener groups (this is still a problem in other areas that will be resolved in subsequent PRs): https://github.com/redpanda-data/helm-charts/pull/121/files#diff-69ec53469d658295718b28d4286a5ca368657dd3179a72c4ef50e928e88568cbL59
A big change is to the values.yaml layout to include a top-level external section along with the ability for each listener to have its own external section to customize behavior per listener if needed. This isn't too noticeable in terms of added functionality... it's benefit is more around ease-of-use due to and simpler values.yaml: https://github.com/redpanda-data/helm-charts/pull/121/files#diff-b46fb23874338d25278532c8599e01bd3f82a6045c00316c889e5747f7acee47
And then also the documentation, which hopefully covers most of what needs to be considered in order to ensure NodePort external access will work in whatever scenario and/or environment. There's a balance between just a TLDR and explaining everything, and then I'm also sure I didn't cover all details as much as could be. Some things may also be left out that should be included (let me know if you think that's the case): https://github.com/redpanda-data/helm-charts/pull/121/files#diff-79616ba1246321de4561899f03fca4d6b0fdf535391a81a1ab55349c88705385