Skip to content
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

Upgrade to dataplaneapi v2 #14

Merged
merged 1 commit into from
Sep 30, 2020
Merged

Upgrade to dataplaneapi v2 #14

merged 1 commit into from
Sep 30, 2020

Conversation

ShimmerGlass
Copy link
Collaborator

@ShimmerGlass ShimmerGlass commented Mar 10, 2020

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

Fixes: #6

Copy link
Collaborator

@pierresouchay pierresouchay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@dcorbett-haproxy What do you think?

@aiharos
Copy link
Collaborator

aiharos commented Apr 15, 2020

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
Copy link
Collaborator

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

@gdubicki
Copy link
Contributor

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

@ShimmerGlass
Copy link
Collaborator Author

@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
Copy link
Collaborator

aiharos commented Sep 11, 2020

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
Copy link
Collaborator Author

@aiharos yes, running the dataplane from consul-connect

@ShimmerGlass
Copy link
Collaborator Author

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

@aiharos
Copy link
Collaborator

aiharos commented Sep 30, 2020

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.

@aiharos aiharos merged commit 8b553af into master Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

changes to dataplane-api causing issues
4 participants