haproxytech / haproxy-consul-connect

HaProxy Connector for Consul Connect. Enables Service Mesh with Consul and HaProxy using TLS and Consul Discovery
Apache License 2.0
95 stars 20 forks source link

Upgrade to dataplaneapi v2 #14

Closed ShimmerGlass closed 4 years ago

ShimmerGlass commented 4 years ago

The API path for dataplane api now use the /v2 prefix.

Fixes: #6

aiharos commented 4 years ago

There will be some (not many) incompatible API changes between dataplanes v1 and v2. We should postpone merging until after v2 is released and we can review any potential other changes that need to be made - this would have an impact on supporting multiple dataplane versions too.

Having said that, the dataplaneapi v2 should be a direct upgrade with only additional features, so there might be no need to keep using v1.

pierresouchay commented 4 years ago

@aiharos Yes, but it supposes to have the new binary for dataplaneapi

gdubicki commented 4 years ago

Dataplane API is already in v. 2.0.2, can we merge this please?

ShimmerGlass commented 4 years ago

@gdubicki @aiharos rebased and upgraded the models. I added proper logging for the dataplane as well.

@pierresouchay handling multiple versions of the dataplane would be pretty complex (see the size of this patch). However, we could bundle the dataplane with haproxy-connect so people don't have to deploy it and manage its version themselves, simplifying things greatly. We could package it in the binary and extract it to the fs, but I would be in favor of actually running the server from haproxy-connect's code, easier to maintain and debug IMO. @aiharos any thoughts on that?

aiharos commented 4 years ago

We could package it in the binary and extract it to the fs, but I would be in favor of actually running the server from haproxy-connect's code, easier to maintain and debug IMO. @aiharos any thoughts on that?

By "server" you mean running the dataplaneapi from the consul-connect code, or something else?

ShimmerGlass commented 4 years ago

@aiharos yes, running the dataplane from consul-connect

ShimmerGlass commented 4 years ago

Rebased. To be clear, I think we can merge this and answer the packaging question in an other PR

aiharos commented 4 years ago

What do you think about handling several versions ? what about a flag --dataplaneapi-version=v2 (for now), once the v2 API is the default, make it the default?

Dataplaneapi v2 is the default now, and since it is a direct superset of features over v1 (and no changes in the API endpoints used by haproxy-consul-connect, @ShimmerGlass agrees that we can merge this.