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

Use correct output format for CNI Add in networkPolicyOnly mode #2037

Conversation

antoninbas
Copy link
Contributor

As per the CNI specification for the Add command:

If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes #2002

As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes antrea-io#2002
@codecov-io
Copy link

Codecov Report

Merging #2037 (bab9152) into main (9a9804a) will increase coverage by 19.90%.
The diff coverage is 36.36%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2037       +/-   ##
===========================================
+ Coverage   41.69%   61.59%   +19.90%     
===========================================
  Files         127      265      +138     
  Lines       15844    19770     +3926     
===========================================
+ Hits         6606    12178     +5572     
+ Misses       8684     6331     -2353     
- Partials      554     1261      +707     
Flag Coverage Δ
kind-e2e-tests 52.87% <36.36%> (?)
unit-tests 41.71% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/cniserver/server.go 68.91% <36.36%> (+19.23%) ⬆️
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
pkg/legacyapis/core/v1alpha2/register.go 80.00% <0.00%> (ø)
pkg/agent/util/sysctl/sysctl_linux.go 0.00% <0.00%> (ø)
pkg/legacyapis/stats/v1alpha1/register.go 90.90% <0.00%> (ø)
pkg/agent/cniserver/ipam/ipam_service.go 63.15% <0.00%> (ø)
pkg/legacyapis/stats/register.go 81.81% <0.00%> (ø)
...agent/apiserver/handlers/appliedtogroup/handler.go 40.00% <0.00%> (ø)
pkg/ipfix/ipfix_set.go 100.00% <0.00%> (ø)
pkg/apis/controlplane/v1beta2/sets.go 66.66% <0.00%> (ø)
... and 226 more

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

@antoninbas
Copy link
Contributor Author

Thanks Quan. I will run the cloud tests and also validate the fix manually for containerd with AKS.

@antoninbas antoninbas added this to the Antrea v1.0 release milestone Apr 7, 2021
@antoninbas
Copy link
Contributor Author

/test-all

@antoninbas
Copy link
Contributor Author

I've verified this patch with our 3 cloud CI jobs (AKS / GKE / EKS)
I have also confirmed the fix manually in AKS, with K8s 1.20.2 (containerd used by default).
Without the patch:

abas-a01:antrea abas$ kubectl get pods -n kube-system -l k8s-app=kube-dns
NAME                       READY   STATUS              RESTARTS   AGE
coredns-78f6cf9c79-4ltp5   0/1     ContainerCreating   0          7m36s
coredns-78f6cf9c79-5q2mw   0/1     ContainerCreating   0          7m36s
abas-a01:antrea abas$ kubectl -n kube-system describe pods/coredns-78f6cf9c79-4ltp5
...
Events:
  Type     Reason                  Age                     From               Message
  ----     ------                  ----                    ----               -------
  Normal   Scheduled               7m51s                   default-scheduler  Successfully assigned kube-system/coredns-78f6cf9c79-4ltp5 to aks-nodepool1-11919205-vmss000001
  Warning  FailedCreatePodSandBox  7m50s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "2bbc79501ddb3a0fb5f9be962a1905cbc28a81f7ed73f04294f7a30b40b1c1f9": failed to find network info for sandbox "2bbc79501ddb3a0fb5f9be962a1905cbc28a81f7ed73f04294f7a30b40b1c1f9"
  Warning  FailedCreatePodSandBox  7m34s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "478c251b54f40e081137d8c95f86df16e60c3fb737d712cb89b9832982479aa4": failed to find network info for sandbox "478c251b54f40e081137d8c95f86df16e60c3fb737d712cb89b9832982479aa4"
  Warning  FailedCreatePodSandBox  7m20s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "84077d6f06e518086cedd8d6e1b43d50f4d9a5230d55f3f54f9a7afcc8b5e47d": failed to find network info for sandbox "84077d6f06e518086cedd8d6e1b43d50f4d9a5230d55f3f54f9a7afcc8b5e47d"
  Warning  FailedCreatePodSandBox  7m4s                    kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "e6771a57ce13ca574ce63a58b7ecfaa5c549ca3978a9340ff31ab671d4732a16": failed to find network info for sandbox "e6771a57ce13ca574ce63a58b7ecfaa5c549ca3978a9340ff31ab671d4732a16"
  Warning  FailedCreatePodSandBox  6m50s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "927d2456d8d8b0ce793a6a56965e8d06855f1d9674afb63d44f1ab9587335371": failed to find network info for sandbox "927d2456d8d8b0ce793a6a56965e8d06855f1d9674afb63d44f1ab9587335371"
  Warning  FailedCreatePodSandBox  6m36s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "2c044828e5b152bbc807a6c43857fd49a4f7614eadab82a4798feae78a9ae62a": failed to find network info for sandbox "2c044828e5b152bbc807a6c43857fd49a4f7614eadab82a4798feae78a9ae62a"
  Warning  FailedCreatePodSandBox  6m25s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "8c64ec21c3c7af28f983689fdd8d3744bf2d8662ca1ae893c6611cc61d467eaf": failed to find network info for sandbox "8c64ec21c3c7af28f983689fdd8d3744bf2d8662ca1ae893c6611cc61d467eaf"
  Warning  FailedCreatePodSandBox  6m13s                   kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "249513dc5d2caebdbebfa966d9aa05ec68f0764279375b8a642ef48b1732bfe7": failed to find network info for sandbox "249513dc5d2caebdbebfa966d9aa05ec68f0764279375b8a642ef48b1732bfe7"
  Warning  FailedCreatePodSandBox  6m1s                    kubelet            Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "f7c0ff62ce0908c67e5599b9879097d58af1c4c1569043e5f2ad415443242604": failed to find network info for sandbox "f7c0ff62ce0908c67e5599b9879097d58af1c4c1569043e5f2ad415443242604"
  Warning  FailedCreatePodSandBox  2m15s (x17 over 5m48s)  kubelet            (combined from similar events): Failed to create pod sandbox: rpc error: code = Unknown desc = failed to setup network for sandbox "6c6d80c7808c53a8216279374428c1185fd60903e6df1b1418b4ce83f5f784fe": failed to find network info for sandbox "6c6d80c7808c53a8216279374428c1185fd60903e6df1b1418b4ce83f5f784fe"

With the patch:

abas-a01:antrea abas$ kubectl get pods -n kube-system -l k8s-app=kube-dns
NAME                       READY   STATUS    RESTARTS   AGE
coredns-78f6cf9c79-68x6s   1/1     Running   0          12m
coredns-78f6cf9c79-8tdvc   1/1     Running   0          12m

NodeInfo:

    nodeInfo:
      architecture: amd64
      bootID: a56192a8-4c3a-4d40-9d72-4305a0745233
      containerRuntimeVersion: containerd://1.4.4+azure
      kernelVersion: 5.4.0-1043-azure
      kubeProxyVersion: v1.20.2
      kubeletVersion: v1.20.2
      machineID: 7afce25255cd4ef09794aeeb0685e1d0
      operatingSystem: linux
      osImage: Ubuntu 18.04.5 LTS
      systemUUID: 92d35183-006f-454e-abd5-ed598e4ec6a3

@antoninbas antoninbas merged commit 6f25769 into antrea-io:main Apr 7, 2021
@antoninbas antoninbas deleted the correct-cni-response-format-for-cni-add-in-networkPolicyOnly-mode branch April 7, 2021 22:01
antoninbas added a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
…ea-io#2037)

As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes antrea-io#2002
antoninbas added a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
…ea-io#2037)

As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes antrea-io#2002
antoninbas added a commit that referenced this pull request Apr 30, 2021
As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes #2002
antoninbas added a commit to antoninbas/antrea that referenced this pull request Apr 30, 2021
…ea-io#2037)

As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes antrea-io#2002
antoninbas added a commit that referenced this pull request May 1, 2021
As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes #2002
antoninbas added a commit to antoninbas/antrea that referenced this pull request May 1, 2021
…ea-io#2037)

As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes antrea-io#2002
antoninbas added a commit that referenced this pull request May 3, 2021
As per the CNI specification for the Add command:
> If the plugin was supplied a prevResult as part of its input
configuration, it MUST handle prevResult by either passing it through,
or modifying it appropriately.

In networkPolicyOnly mode, CNI chaining is used. Antrea receives a
prevResult from the container runtime and must output it in case of
success. We do not make any changes to the result, but we may convert it
to a different CNI version format (the default one for Antrea).

Previously, Antrea would output the entire input configuration (which
does include prevResult), which would cause containerd to return an
error.

The integration test was incorrect and had to be fixed.

Fixes #2002
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.

Antrea CNI returns wrong response format in networkPolicyOnly mode
4 participants