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

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

Open
kieron-dev opened this issue Sep 30, 2021 · 3 comments

Comments

@kieron-dev
Copy link
Contributor

kieron-dev commented Sep 30, 2021

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 kieron-dev created this issue from a note in Garden (Scheduled) Sep 30, 2021
@ameowlia
Copy link
Member

Hi @kieron-dev ,

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

Thanks!

@ameowlia ameowlia removed this from Issue Inbox in DEPRECATED App Platform - Diego Oct 28, 2021
@kieron-dev
Copy link
Contributor Author

kieron-dev commented Oct 28, 2021

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.

@ameowlia ameowlia moved this from Inbox to Issues - Triage Complete. Needs Fix. in DEPRECATED App Platform - Garden Containers Nov 11, 2021
@MarcPaquette
Copy link
Contributor

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

winkingturtle-vmw added a commit to cloudfoundry/cpu-entitlement-plugin that referenced this issue Apr 25, 2023
We are using go-log-cache now
Context: cloudfoundry/garden-runc-release#209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Garden
Scheduled
DEPRECATED App Platform - Garden Con...
Issues - Triage Complete. Needs Fix.
DEPRECATED - WG-Application-Runtime-P...
Old PRs and Issues (pre-project creat...
Development

No branches or pull requests

3 participants