garden-runc-release icon indicating copy to clipboard operation
garden-runc-release copied to clipboard

Get the protobuf duplicate fix registration warning/panic fixed in log-cache-release

Open kieron-dev opened this issue 2 years ago • 3 comments

We have provided a fix in go-cache-release in order to enable the CF CLI to stop pinning protobuf. The same fix will work for log-cache-release, and will enable us to remove the ldflag from the cpu entitlement plugin.

This also affects the cpu-entitlement-admin-plugin.

kieron-dev avatar Sep 30 '21 08:09 kieron-dev

Hi @kieron-dev ,

Your issue is very brief. Do you mind expanding on what you want? Or even better submitting a PR?

Thanks!

ameowlia avatar Oct 28 '21 16:10 ameowlia

Hi @ameowlia ,

I guess this is just a chore from the garden backlog. We're not actively picking up garden stories at the moment, so I won't be able to PR it, but I'll expand on the details here.

We recently fixed an issue in go-log-cache where the code had started to panic due to stricter checks in the gRPC code. We needed this so we could update the gRPC libraries in CF CLI to enable importing some kubernetes libraries. The issue is that when generating code from .proto files, all .proto files used in a binary must have distinct paths. go-log-cache imports go-loggregator, and both projects used ./ingress.proto and ./egress.proto. gRPC panicked due to having two ingress.protos and two egress.protos. The solution is simply to put those .proto files into a deeper directory structure and include those directories in the import path to make them distinct to the project. The difficulty is updating the code generation script to use current versions of the generation tools, bumping packages, and fixing the code.

log-cache suffers from exactly the same problem, and two garden projects cpu-entitlement-plugin and cpu-entitlement-admin-plugin use log-cache. We currently get around the panic by building the plugins with -X google.golang.org/protobuf/reflect/protoregistry.conflictPolicy=ignore set. We don't believe this is a good solution for the long term, and it would be better if the log-cache library were fixed.

So the suggested change here is to PR log-cache (contained in log-cache-release) reproducing the go-log-cache fix. Then update the plugins to use the new log-cache version.

However, go-log-cache might well be equivalent to log-cache. We just didn't know about it when we built these plugins, so perhaps the easiest fix is to change the plugins to use go-log-cache instead of log-cache.

kieron-dev avatar Oct 28 '21 17:10 kieron-dev

Runtime Tracker story has been created for this issue: https://www.pivotaltracker.com/story/show/180812108

MarcPaquette avatar Jan 05 '22 15:01 MarcPaquette