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

Remove use of legacy protobuf library #9531

Closed
gibmat opened this issue Nov 14, 2021 · 6 comments
Closed

Remove use of legacy protobuf library #9531

gibmat opened this issue Nov 14, 2021 · 6 comments
Labels
Incomplete Waiting on more information from reporter

Comments

@gibmat
Copy link
Contributor

gibmat commented Nov 14, 2021

Issue description

(I'll prefix this by saying I'm not very familiar with golang's protobuf libraries, although I've spent a good chunk of today learning about the legacy and new libraries....)

LXD is currently using both the legacy/deprecated (github.com/golang/protobuf) and new (github.com/protocolbuffers/protobuf-go) versions of the protobuf library. However, the legacy version is only used in one file (lxd/bgp/server.go), that's using the deprecated ptypes library methods. (The upstream pull request has comments in the diff indicating switching to the new methods should be very easy.)

Could you please remove LXD's use of the legacy protobuf library? That will help trim down its dependency tree, and prevent possible issues when mixing the old and new protobuf libraries.

(I initially found this issue in a library down in LXD's dependency tree while preparing to package it for Debian, but the same mixing of protobuf library versions exists in LXD.)

@stgraber
Copy link
Contributor

I tried to do so but couldn't as the gobgp package itself relies on the specific use of ptypes.

@stgraber stgraber added the Incomplete Waiting on more information from reporter label Nov 14, 2021
@stgraber
Copy link
Contributor

I suspect we may need https://github.com/osrg/gobgp to take the first step and move away from ptypes, then we can do the same on our side. The rest of our protobuf usage was already updated to the new packages.

@gibmat
Copy link
Contributor Author

gibmat commented Nov 14, 2021

Ah, got it. I'll open an issue with them to make that request.

@stgraber
Copy link
Contributor

Closing as not actionable on our side currently. Also Go is complaining every time I update our gomod, so I already have a constant reminder of this ;)

Once gobgp is fixed, our build will almost certainly start failing due to type mismatch, we'll then correct it and that will take care of it.

@gibmat
Copy link
Contributor Author

gibmat commented Dec 15, 2021

gobgp has tagged a couple 3.0.0-rc releases that moves to the new protobuf library (osrg/gobgp#2485). I've verified from a very simple standpoint that LXD 4.21 compiles with changes made to lxd/bgp/server.go, but as I don't have any BGP stuff setup locally, I can't test to ensure nothing's inadvertently broken LXD.

@stgraber
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Incomplete Waiting on more information from reporter
Projects
None yet
Development

No branches or pull requests

2 participants