Initial support for multi-networking #451

dotnwat · 2021-01-22T05:20:52Z

This PR adds the plumbing for supporting multi-network configuration. The basic idea is that listeners and advertised apis are associated using common naming. Given the following configuration:

  kafka_api:
    - name: public
      address: "0.0.0.0"
      port: 9092
    - name: private
      address: "0.0.0.0"
      port: 9093

  advertised_kafka_api:
    - name: public
      address: public.asdf.com
      port: 9092
    - name: private
      address: internal.network
      port: 9093

kafka will report public.asdf.com as an advertised listener if the client is connected on the public listener connection, and similar behavior for the private network.

src/v/config/property.h

src/v/rpc/server.cc

src/v/rpc/types.h

src/v/kafka/requests/metadata_request.cc

src/v/config/tests/advertised_kafka_api_test.cc

emaxerrno

Really quite fantastic!!!

WHat are your thoughts on defaulting the name to "default"

dotnwat · 2021-01-22T06:14:49Z

Really quite fantastic!!!

WHat are your thoughts on defaulting the name to "default"

We could display empty stings in logs as explicitly defaulted? But I don't really care either way.

mmaslankaprv · 2021-01-22T06:44:13Z

This looks really great, the only thing I miss is the backward compatibility for raft::configuration it also stores model::broker

BenPope

I was trying to test it with the helm chart, but had no luck.

src/v/redpanda/application.cc

The RPC server configuration now supports associating a name with a listener (via struct server_endpoint). This name is made available via the connection context when a connection is accepted for a listener. Signed-off-by: Noah Watkins <noah@vectorized.io>

This feature is available now in c++20 and works when all the members are also comparable. Signed-off-by: Noah Watkins <noah@vectorized.io>

dotnwat · 2021-01-22T23:35:45Z

I was trying to test it with the helm chart, but had no luck.

offline chatting looks like this is an issue in the helm chart with constructing the proper config file

dotnwat · 2021-01-22T23:35:54Z

This looks really great, the only thing I miss is the backward compatibility for raft::configuration it also stores model::broker

done

This patch introduces a wire protocol and storage format breaking change. Extends types to support associating multiple named listeners per broker. The main type introduced is struct broker_endpoint which is a (name, address) pair. model::broker is extended to store a set of broker endpoints. existing tests have been built assuming a single listener, and these tests are updated to take the first listener in the set which should provide compatability until the tests need to be generalized. Signed-off-by: Noah Watkins <noah@vectorized.io>

- Backwards compatibility tests - Multiple endpoints test Signed-off-by: Noah Watkins <noah@vectorized.io>

When advertised listeners are announced by a broker they are filtered such that the endpoint name matches the connection name. Signed-off-by: Noah Watkins <noah@vectorized.io>

Useful for backwards compat or simplifying schema common cases. Signed-off-by: Noah Watkins <noah@vectorized.io>

Both kafka api and advertised api endpoint configuration options now support multiple endpoints. This maintains backwards compatability. That is: kafka_api: address: xxx port: 333 kafka_api: - address: xxx port: 333 will both parse into equivalent configurations. Signed-off-by: Noah Watkins <noah@vectorized.io>

Signed-off-by: Noah Watkins <noah@vectorized.io>

dotnwat · 2021-01-23T02:56:48Z

tests for multiple endpoints in broker object
support backwards compatibility for raft::group_configuration (bumped to v3)
use constructor delegation

https://github.com/vectorizedio/redpanda/compare/631febd7ac7c4ebed82091bbdd32912e70cb34fc..1a7e1d5

@mmaslankaprv if you have time it'd be great to look at (1) the backward compat i added and(2) the test i added for that feature.

emaxerrno · 2021-01-23T03:37:53Z

src/v/model/metadata.h

+ * Old version for use in backwards compatibility serialization /
+ * deserialization helpers.
+ */
+struct broker_v0 {


thanks for doing this.

emaxerrno

lgtm.

The kafka api and advertised kafka api configuration can now be set to list of listeners. Remove old configuration from configmap resource. Reorganize the init container to set only seed server list. Due to rpk start deprecation the pod now uses rpk redpanda start subcommand. Reference PRs: #413 #525 #451

The kafka api and advertised kafka api configuration can now be set to list of listeners. Remove old configuration from configmap resource. Reorganize the init container to set only seed server list. Due to rpk start deprecation the pod now uses rpk redpanda start subcommand. Reference PRs: redpanda-data#413 redpanda-data#525 redpanda-data#451

dotnwat requested review from Lazin, emaxerrno and mmaslankaprv January 22, 2021 05:20

dotnwat requested a review from BenPope as a code owner January 22, 2021 05:20