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

Update Protobuf dependencies for code generation #6130

Merged
merged 1 commit into from
Mar 21, 2024

Conversation

antoninbas
Copy link
Contributor

This removes our direct dependency on github.com/golang/protobuf, which has been superseded by google.golang.org/protobuf.

Some notable changes since the last time we updated:

  • The versioning scheme for Protobuf has changed (see https://protobuf.dev/support/version-support/)
  • The Golang gRPC code generator is now a standalone tool (protoc-gen-go-grpc) and is no longer included as a plugin in protoc-gen-go. With protoc-gen-go-grpc, the service implementations must embed the corresponding Unimplemented<ServiceName>Server for future compatibility (which we are now doing for CNIServer).

A new codegen image is released: antrea/codegen:kubernetes-1.29.2-build.0

New versions:

  • protoc: v26.0
  • protoc-gen-go: v1.33.0
  • protoc-gen-go-grpc: v1.3.0

In our case, we don't have to worry about client-server compatibility, given that antrea-cni (client) is always reinstalled by the antrea-agent Pod. Even if it was a concern for us, there has been no change in the Protobuf wire format.

This removes our direct dependency on github.com/golang/protobuf, which
has been superseded by google.golang.org/protobuf.

Some notable changes since the last time we updated:
* The versioning scheme for Protobuf has changed (see
  https://protobuf.dev/support/version-support/)
* The Golang gRPC code generator is now a standalone tool
  (protoc-gen-go-grpc) and is no longer included as a plugin in
  protoc-gen-go. With protoc-gen-go-grpc, the service implementations
  must embed the corresponding `Unimplemented<ServiceName>Server` for
  future compatibility (which we are now doing for `CNIServer`).

A new codegen image is released: antrea/codegen:kubernetes-1.29.2-build.0

New versions:
* protoc: v26.0
* protoc-gen-go: v1.33.0
* protoc-gen-go-grpc: v1.3.0

In our case, we don't have to worry about client-server compatibility,
given that antrea-cni (client) is always reinstalled by the antrea-agent
Pod. Even if it was a concern for us, there has been no change in the
Protobuf wire format.

Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
Copy link
Contributor

@luolanzone luolanzone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two nits

@@ -16,10 +16,13 @@ docker push antrea/codegen:<TAG>
The `docker push` command will fail if you do not have permission to push to the
`antrea` Dockerhub repository.

The image can only be built on an x86_64 machine (no arm support).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The image can only be built on an x86_64 machine (no arm support).
The image can only be built on a x86_64 machine (no arm support).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this is correct. x86's pronunciation starts with a vowel.

@@ -0,0 +1,197 @@
// Copyright 2019 Antrea Authors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why the year info is still 2019? is this generated manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess is that protoc automatically uses the information from the .proto source file, but I didn't look into it.

Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -16,10 +16,13 @@ docker push antrea/codegen:<TAG>
The `docker push` command will fail if you do not have permission to push to the
`antrea` Dockerhub repository.

The image can only be built on an x86_64 machine (no arm support).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guess this is correct. x86's pronunciation starts with a vowel.

@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas antoninbas merged commit df82b76 into antrea-io:main Mar 21, 2024
50 of 55 checks passed
@antoninbas antoninbas deleted the update-proto-deps-for-codegen branch March 21, 2024 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants