Update protobuf and gRPC related dependencies #12767
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rolinh
added
release-note/misc
|
test-me-please |
aanm
approved these changes
Aug 4, 2020
sayboras
reviewed
Aug 4, 2020
|
All CI tests passed but somehow the GA tests didn't trigger. I'll mark this PR as draft and back to see if that works (I can't trigger them manually). EDIT: OK this didn't work. I'll try pushing something. |
This update requires non-trivial changes to the way we generate files
from `proto` definitions:
- Replace `github.com/golang/protobuf/protoc-gen-go` with
`google.golang.org/grpc/cmd/protoc-gen-go-grpc`. The former is
deprecated as it is an internal implementation of `protoc-gen-go`.
For more information, see [0].
Note that this package is still imported by the following plugin
that we use: `github.com/envoyproxy/protoc-gen-validate`. I have
bumped the version to the latest one (v0.4.0) but there is no
version nor PR yet that update the import path for this one so a
warning is still being issued when we generate the files from the
`proto` definitions.
- All `proto` files now need to specify `option go_package`.
- Update the Makefile to use the `paths=source_relative`. This allows
removing the `RAW_GO_MAPPINGS` definition and it works fine with the
new `go_package` option. Without this change, the files would be
generated to a folder named after the `go_package` option relative
to the `Makefile` (eg: `./github.com/cilium/cilium/api/v1/flow`).
- Use new `go-grpc_out` option with `protoc`. This generates the gRPC
realted Go code using the `protoc-gen-go-grpc` plugin. The `go_out`
option does not generate gRPC related Go code anymore.
[0]: golang/protobuf#1104 (comment)
Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
Changes from ba6565ff6bf66e200842c7e6ab7112caa845d98a require to re-generate protbuf related files. Note that to register services using the methods generated by `protoc-gen-go-grpc`, the service implementations must embed the corresponding `Unimplemented<ServiceName>Server` for future compatibility. This is a behavior change from the grpc code generator previously included with `protoc-gen-go`. One could optionally restore the old behavior by passing the option `requireUnimplementedServers=false` when generating files but this is not recommended. The 3 structs which implement the observer and peer services now embed the respective `Unimplemented<ServiceName>Server` structs. Signed-off-by: Robin Hahling <robin.hahling@gw-computing.net>
rolinh
force-pushed
the
pr/rolinh/update-protogen
branch
from
41ef486
to
421b8ce
Compare
|
test-me-please |
tklauser
approved these changes
Aug 5, 2020
kaworu
approved these changes
Aug 5, 2020
|
retest-net-next Edit: previous failure https://jenkins.cilium.io/job/Cilium-PR-K8s-1.12-net-next/129/ |
|
retest-runtime Edit: vm provisioning failure https://jenkins.cilium.io/job/Cilium-PR-Runtime-4.9/1457/ |
|
retest-gke |
|
retest-runtime |
maintainer-s-little-helper
bot
added
the
ready-to-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See commits for details.
Note: I wanted to bump
grpc-goto v1.31.0 but I can't due to a breaking change introduced in v1.30.0 ("Remove the deprecatednamingpackage.") that in turn breaksgo.etcd.io/etcd(see etcd-io/etcd#12124 for details).There's a PR opened for etcd to upgrade
grpc-go(etcd-io/etcd#12155) but it's stalled due to other breaking changes introduced in a minor version ofgrpc-gowhich breaks the API.