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 grpc/protobuf dependencies #1091

Closed
alexmt opened this issue Apr 16, 2021 · 7 comments
Closed

Upgrade grpc/protobuf dependencies #1091

alexmt opened this issue Apr 16, 2021 · 7 comments
Labels
enhancement New feature or request no-issue-activity tech debt Technical debt

Comments

@alexmt
Copy link
Contributor

alexmt commented Apr 16, 2021

Summary

Argo Rollouts API server uses old grcp libraries that have to be upgraded:

@alexmt alexmt added enhancement New feature or request tech debt Technical debt labels Apr 16, 2021
@alexmt
Copy link
Contributor Author

alexmt commented Apr 16, 2021

Attempted to upgrade and failed. Using this ticket to document what I've tried and which issues I've faced:

  • github.com/grpc-ecosystem/grpc-gateway v2.3.0 no longer focused on using protoc and trying to replace it with buf. Migration to buf was not straightforward: could not find how to point buf to protos from k8s/apimachinery, could not find a way to use buf without changing our proto files.

  • github.com/grpc-ecosystem/grpc-gateway v2 generates code that relies on google.golang.org/protobuf (go protobuf v2). So we have to upgrade to google.golang.org/protobuf first

  • google.golang.org/protobuf does not work with github.com/gogo/protobuf. So we have to switch back to official go plugin google.golang.org/protobuf/cmd/protoc-gen-go.

  • google.golang.org/protobuf/cmd/protoc-gen-go - has changed and now requires to have full package name in go_package . Protos from https://github.com/kubernetes/apimachinery still don't have full package name. E.g. https://github.com/kubernetes/apimachinery/blob/master/pkg/apis/meta/v1/generated.proto . That was a dead-end, could not find a way to workaround.

@jessesuen
Copy link
Member

@alexmt I've managed to split out most toolchain binaries and remove them as a go.mod library dependency in PR #1095. Do you think your findings still hold true?

@jessesuen
Copy link
Member

google.golang.org/protobuf/cmd/protoc-gen-go - has changed and now requires to have full package name in go_package . Protos from https://github.com/kubernetes/apimachinery still don't have full package name. E.g. https://github.com/kubernetes/apimachinery/blob/master/pkg/apis/meta/v1/generated.proto . That was a dead-end, could not find a way to workaround.

I spent a few hours on this and hit the same dead end as you.

# protoc pkg/apiclient/rollout/rollout.proto
PATH=/Users/jsuen/go/src/github.com/argoproj/argo-rollouts/dist:$PATH protoc -I . -I ./vendor -I /Users/jsuen/go/src -I ./vendor/github.com/googleapis/googleapis --go_out=plugins=grpc:/Users/jsuen/go/src --grpc-gateway_out=logtostderr=true:/Users/jsuen/go/src --swagger_out=logtostderr=true,fqn_for_swagger_name=true:. pkg/apiclient/rollout/rollout.proto
protoc-gen-go: invalid Go import path "resource" for "k8s.io/apimachinery/pkg/api/resource/generated.proto"

The import path must contain at least one forward slash ('/') character.

See https://developers.google.com/protocol-buffers/docs/reference/go-generated#package for more information.

--go_out: protoc-gen-go: Plugin failed with status code 1.
make: *** [api-proto] Error 1

I agree we cannot get past this upstream k8s generated.proto have this problem and its not feasible to change upstream.

Looks like we are stuck with v1 protobuf and grpc-gateway until kubernetes moves up.

@blkperl
Copy link
Contributor

blkperl commented Dec 15, 2021

There is an upstream issue that will hopefully unblock us kubernetes/kubernetes#96564

@leoluz
Copy link
Contributor

leoluz commented Apr 2, 2022

FYI, I'm giving it another try in argoproj/argo-cd#8929

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

This issue is stale because it has been open 60 days with no activity.

@leoluz
Copy link
Contributor

leoluz commented Dec 6, 2022

grpc runtime library updated in argoproj/argo-cd#8929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request no-issue-activity tech debt Technical debt
Projects
None yet
Development

No branches or pull requests

4 participants