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

Fix Antrea Octant plugin build #4107

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

antoninbas
Copy link
Contributor

The plugin had 2 conflicting indirect Go dependencies:

  • github.com/googleapis/gnostic
  • github.com/google/gnostic

Having these 2 dependencies caused the same .proto file to be registered
twice, and caused the antrea-octant-plugin binary to panic on start.

github.com/googleapis/gnostic is the old module name, while
github.com/google/gnostic is the new one.

New K8s versions (e.g., v1.24 which is used by Antrea) depend on
github.com/google/gnostic, while older versions (e.g., v1.21 which is
used by Octant) depend on github.com/googleapis/gnostic. To resolve this
issue (at least temporarily), we ensure that only K8s v1.21 libraries
are used to build the plugin, and we eliminate the
github.com/google/gnostic dependency. This is achieved by adding a few
missing replace directives.

Fixes #4083

Signed-off-by: Antonin Bas abas@vmware.com

The plugin had 2 conflicting indirect Go dependencies:
- github.com/googleapis/gnostic
- github.com/google/gnostic

Having these 2 dependencies caused the same .proto file to be registered
twice, and caused the antrea-octant-plugin binary to panic on start.

github.com/googleapis/gnostic is the old module name, while
github.com/google/gnostic is the new one.

New K8s versions (e.g., v1.24 which is used by Antrea) depend on
github.com/google/gnostic, while older versions (e.g., v1.21 which is
used by Octant) depend on github.com/googleapis/gnostic. To resolve this
issue (at least temporarily), we ensure that only K8s v1.21 libraries
are used to build the plugin, and we eliminate the
github.com/google/gnostic dependency. This is achieved by adding a few
missing replace directives.

Fixes antrea-io#4083

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas marked this pull request as ready for review August 11, 2022 22:44
@antoninbas antoninbas added action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels Aug 11, 2022
@antoninbas antoninbas added this to the Antrea v1.8 release milestone Aug 11, 2022
@antoninbas antoninbas added the kind/bug Categorizes issue or PR as related to a bug. label Aug 11, 2022
@antoninbas
Copy link
Contributor Author

@tnqn this should be merged for the v1.8 release (and also backported to v1.7).

Comment on lines -192 to -195

// Newer version of github.com/googleapis/gnostic make use of newer gopkg.in/yaml(v3), which conflicts with
// explicit imports of gopkg.in/yaml.v2.
replace github.com/googleapis/gnostic v0.5.5 => github.com/googleapis/gnostic v0.4.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this as it doesn't seem required anymore

@codecov
Copy link

codecov bot commented Aug 11, 2022

Codecov Report

Merging #4107 (941af97) into main (39b0eb5) will increase coverage by 0.31%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4107      +/-   ##
==========================================
+ Coverage   67.10%   67.42%   +0.31%     
==========================================
  Files         299      299              
  Lines       45443    45443              
==========================================
+ Hits        30494    30638     +144     
+ Misses      12564    12409     -155     
- Partials     2385     2396      +11     
Flag Coverage Δ
integration-tests 35.44% <ø> (-0.07%) ⬇️
kind-e2e-tests 50.38% <ø> (+0.58%) ⬆️
unit-tests 44.29% <ø> (-0.04%) ⬇️
Impacted Files Coverage Δ
...nt/apiserver/handlers/serviceexternalip/handler.go 37.03% <0.00%> (-14.82%) ⬇️
pkg/apiserver/certificate/certificate.go 70.37% <0.00%> (-6.49%) ⬇️
pkg/controller/networkpolicy/tier.go 50.00% <0.00%> (-5.00%) ⬇️
...trollers/multicluster/resourceexport_controller.go 75.13% <0.00%> (-3.56%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 75.25% <0.00%> (-3.02%) ⬇️
pkg/agent/util/ipset/ipset.go 67.56% <0.00%> (-2.71%) ⬇️
pkg/agent/util/iptables/iptables.go 45.07% <0.00%> (-1.56%) ⬇️
pkg/agent/agent.go 53.21% <0.00%> (-0.38%) ⬇️
pkg/agent/memberlist/cluster.go 74.12% <0.00%> (-0.32%) ⬇️
pkg/agent/route/route_linux.go 53.44% <0.00%> (+0.25%) ⬆️
... and 21 more

Copy link
Contributor

@mengdie-song mengdie-song left a comment

Choose a reason for hiding this comment

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

Thanks for the change, LGTM.

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

@tnqn
Copy link
Member

tnqn commented Aug 15, 2022

/skip-all

@tnqn tnqn merged commit 98f24bc into antrea-io:main Aug 15, 2022
@antoninbas antoninbas deleted the fix-antrea-octant-plugin-build branch August 15, 2022 17:24
heanlan pushed a commit to heanlan/antrea that referenced this pull request Mar 29, 2023
The plugin had 2 conflicting indirect Go dependencies:
- github.com/googleapis/gnostic
- github.com/google/gnostic

Having these 2 dependencies caused the same .proto file to be registered
twice, and caused the antrea-octant-plugin binary to panic on start.

github.com/googleapis/gnostic is the old module name, while
github.com/google/gnostic is the new one.

New K8s versions (e.g., v1.24 which is used by Antrea) depend on
github.com/google/gnostic, while older versions (e.g., v1.21 which is
used by Octant) depend on github.com/googleapis/gnostic. To resolve this
issue (at least temporarily), we ensure that only K8s v1.21 libraries
are used to build the plugin, and we eliminate the
github.com/google/gnostic dependency. This is achieved by adding a few
missing replace directives.

Fixes antrea-io#4083

Signed-off-by: Antonin Bas <abas@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

antrea-octant-plugin can't be load
3 participants