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 antrea cni to work with 0.4.0 version #784

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

moshe010
Copy link
Contributor

@moshe010 moshe010 commented Jun 3, 2020

This patch update the antrea cni to get the supported version
for the cni pkg. It also update the host-local plugin in the Ubunut
image with version that support cni spec 0.4.0

@antrea-bot
Copy link
Collaborator

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

  • /test-e2e: to trigger e2e tests.
  • /skip-e2e: to skip e2e tests.
  • /test-conformance: to trigger conformance tests.
  • /skip-conformance: to skip conformance tests.
  • /test-networkpolicy: to trigger networkpolicy tests.
  • /skip-networkpolicy: to skip networkpolicy tests.
  • /test-windows-conformance: to trigger windows conformance tests.
  • /skip-windows-conformance: to skip windows conformance tests.
  • /test-all: to trigger all tests.
  • /skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

@@ -29,7 +29,7 @@ func main() {
cni.ActionAdd.Request,
cni.ActionCheck.Request,
cni.ActionDel.Request,
cni_version.PluginSupports("0.1.0", "0.2.0", "0.3.0", "0.3.1"),
cni_version.All,
Copy link
Contributor

Choose a reason for hiding this comment

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

should we actually switch to All or just add 0.4.0 explicitly to the list of supported CNI versions? What is a new CNI version is released?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this more, I think my point was mostly moot. If a new CNI version is released, the value of All would not change until we decide to explicitly update the CNI library version we use for Antrea (https://github.com/vmware-tanzu/antrea/blob/master/go.mod#L15). And it stands to reason that we would only do that if we were ready to support the new CNI spec version...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so in the go.mod you have cni 0.7.1 https://github.com/vmware-tanzu/antrea/blob/master/go.mod#L15 which support cni spec 0.4.0 see https://github.com/containernetworking/cni/blob/v0.7.1/pkg/version/version.go#L39 .
So IMO the "All" should be good.

Also I think you also need to change this https://github.com/vmware-tanzu/antrea/blob/master/pkg/agent/cniserver/server.go#L107 to be taken from the containernetworking/cni/. The support version should be defined by cni version (/containernetworking/cni) in the go.mod.

In the current implementation you need to change it in sevral places

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 mind making the change to https://github.com/vmware-tanzu/antrea/blob/master/pkg/agent/cniserver/server.go#L107 as part of this PR? And any other related change you deem necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure will do

antoninbas
antoninbas previously approved these changes Jun 3, 2020
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd like to have a +1 from @tnqn as well.

tnqn
tnqn previously approved these changes Jun 4, 2020
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, just to confirm, the code is to make antrea work when user override cniVersion to 0.4.0 in cniConf: https://github.com/vmware-tanzu/antrea/blob/master/build/yamls/base/conf/antrea-cni.conflist, right?

@moshe010
Copy link
Contributor Author

moshe010 commented Jun 4, 2020

LGTM, just to confirm, the code is to make antrea work when user override cniVersion to 0.4.0 in cniConf: https://github.com/vmware-tanzu/antrea/blob/master/build/yamls/base/conf/antrea-cni.conflist, right?

yes, don't merged it yet I have another change to add

This patch update the antrea cni to get the supported version
for the cni pkg (both client and server). It also update the host-local plugin in the Ubunut
image with version that support cni spec 0.4.0
@tnqn
Copy link
Member

tnqn commented Jun 4, 2020

/test-all

@tnqn
Copy link
Member

tnqn commented Jun 4, 2020

Thanks @moshe010 for the update.

@tnqn tnqn merged commit 97bac64 into antrea-io:master Jun 4, 2020
GraysonWu pushed a commit to GraysonWu/antrea that referenced this pull request Sep 22, 2020
This patch update the antrea cni to get the supported version
for the cni pkg (both client and server). It also update the host-local plugin in the Ubunut
image with version that support cni spec 0.4.0
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.

5 participants