v1 operator: support configuring additional listeners with different ports #21
Conversation
|
|
paulzhang97
changed the title
paulzhang97
force-pushed
the
paulz/additional-listeners-different-ports
branch
from
f1035fc
to
b44461d
Compare
…e additional listeners to configurator through env var
paulzhang97
force-pushed
the
paulz/additional-listeners-different-ports
branch
3 times, most recently
from
ef51332
to
7a3df19
Compare
paulzhang97
changed the title
paulzhang97
requested review from
RafalKorepta,
alejandroEsc,
0x5d,
c4milo,
ncole and
frenchfrywpepper
| // - pandaproxy.pandaproxy_api | ||
| // - pandaproxy.advertised_pandaproxy_api | ||
| // example: redpanda.kafka_api: "[{'name':'private-link','address':'0.0.0.0','port':39002}]" | ||
| // example: redpanda.advertised_kafka_api: "[{'name':'private-link','address':'{{ .Index }}-f415bda0-{{ .HostIP | sha256sum | substr 0 7 }}.clb2jkpb06k16j807u20.byoc.ign.cloud.redpanda.com','port': 'port': {{30092 | add .Index}}}]" |
There was a problem hiding this comment.
NIT: If possible please redact or change our cloud DNS names in public repository.
| } | ||
| } | ||
|
|
||
| // We will reply on the webhook to validate and reject invalid configurations, e.g. port conflict. |
There was a problem hiding this comment.
Where that validation is done?
There was a problem hiding this comment.
I was thinking to add it later in separate PR. Now depending on the lifespan of v1 operator, is it still worth to change the webhook of cluster CRD?
There was a problem hiding this comment.
It is not worth it. Just ignore. Maybe remove that comment.
There was a problem hiding this comment.
Rremoved the comment.
| @@ -50,6 +52,7 @@ const ( | |||
| validateMountedVolumeEnvVar = "VALIDATE_MOUNTED_VOLUME" | |||
| redpandaRPCPortEnvVar = "REDPANDA_RPC_PORT" | |||
| svcFQDNEnvVar = "SERVICE_FQDN" | |||
| additionalListenersEnvVar = "ADDITIONAL_LISTENERS" | |||
There was a problem hiding this comment.
Putting structured data in an unstructured environment variable feels icky. Can a configmap be used instead?
There was a problem hiding this comment.
Is this a comma-separated list of values?
There was a problem hiding this comment.
uh, I see, it is a JSON document. Is this the only way to pass it? if so, I would recommend encoding it in base64 or doing what @ncole is suggesting.
There was a problem hiding this comment.
I thought about passing the config through configmap. I gave it up because of broader changes.
- Reading configmap has more code than reading env in Configurator. If we don't like the name of configmap to be hardcoded, we will need to either change
clusterCRD or pass the configmap name in new env var. - When there is change to additional listeners, if using configmap, we will need to change v1 operator to watch the configmap and restart the broker pods. If using env, no need to watch, restarting the broker pods is triggered by the changes to statefulset spec.
- Regarding structured vs unstructured, I feel that the data in configmap is not formally structured, it is still string and needs to be parsed as well.
Currently there is no special char in the additional listener config, base64 encoding is not necessary. Without base64 encoding, it is a little convenient to examine the configurations since no base64 decoding.
There was a problem hiding this comment.
Agree with @paulzhang97 the cloud v1 shouldn't be developed. More effort should be developed into Redpanda helm chart.
paulzhang97
force-pushed
the
paulz/additional-listeners-different-ports
branch
from
5d14963
to
9782616
Compare
…N, Address, TLS config from the default external listener if not set
paulzhang97
force-pushed
the
paulz/additional-listeners-different-ports
branch
from
9782616
to
5803756
Compare
Support the additional listeners that can be configured with different ports.
Support the following additional listener configurations,
Additional listeners are passed to Configurator through the new env variable named
ADDITIONAL_LISTENERS.The env var can take string template containing
.IndexandHostIPvariables, e.g."'address': '{{.Index}}-{{.HostIP | sha256sum | substr 0 8}}.redpanda.com', 'port': {{39002 | add .Index}}".If
addressis not set inredpanda.advertised_kafka_apiorpandaproxy.advertised_pandaproxy_api, it will be set to the address in the advertised listeners namedkafka-externalorproxy-externalcorrespondingly.If
authentication_metholdis not set inredpanda.kafka_apiorpandaproxy.pandaproxy_api, it will be set toauthentication_metholdin the listeners namedkafka-externalorproxy-externalcorrespondingly.Change RP statefulset set to expose the container ports for additional listeners.
Change RP statefulset set to set the env
ADDITIONAL_LISTENERSfor the init container Configurator if the additional listeners are set throughadditionalConfiguration.