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

Fail in Agent initialization if GRE tunnel type is used with IPv6 #3156

Conversation

antoninbas
Copy link
Contributor

The 'gre' tunnel type does not work for IPv6 overlays. The correct
tunnel type for OVS would be 'ip6gre'. For dual-stack clusters with both
an IPv4 and an IPv6 overlay, we would need 2 default tunnel ports: one
for IPv4 (with type 'gre') and one for IPv6 (with type 'ip6gre'). This
would add complexity (and require testing) as the code currently assumes
a single default tunnel port. Rather than trying to support IPv6 with
the added complexity that it entails, we choose to fail during Agent
initialization for now if the user-provided tunnel type is 'gre' and the
cluster supports IPv6.

Note that this means that the default manifest for IPsec
(antrea-ipsec.yml) cannot be used in an IPv6 cluster as the tunnel type
defaults to GRE in that case. However, this is not new, and it is better
to fail explicitly rather than have a cluster where the Agent appears to
be running fine but there is no connectivity.

See #3150

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

@antoninbas antoninbas added area/transit/encapsulation Issues or PRs related to encapsulation. area/transit/ipv6 Issues or PRs related to IPv6. labels Dec 21, 2021
@antoninbas antoninbas requested review from tnqn and jianjuns December 21, 2021 21:07
@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2021

Codecov Report

Merging #3156 (584782b) into main (1d27432) will decrease coverage by 19.52%.
The diff coverage is 31.25%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3156       +/-   ##
===========================================
- Coverage   65.34%   45.81%   -19.53%     
===========================================
  Files         268      345       +77     
  Lines       26901    38720    +11819     
===========================================
+ Hits        17578    17740      +162     
- Misses       7415    18873    +11458     
- Partials     1908     2107      +199     
Flag Coverage Δ
integration-tests 35.82% <ø> (?)
kind-e2e-tests 51.83% <31.25%> (-3.94%) ⬇️
unit-tests ?

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

Impacted Files Coverage Δ
pkg/agent/agent.go 45.63% <31.25%> (-7.50%) ⬇️
pkg/controller/networkpolicy/endpoint_querier.go 2.85% <0.00%> (-88.58%) ⬇️
pkg/controller/egress/controller.go 0.00% <0.00%> (-88.45%) ⬇️
...g/agent/apiserver/handlers/featuregates/handler.go 0.00% <0.00%> (-82.36%) ⬇️
pkg/controller/ipam/validate.go 0.00% <0.00%> (-82.26%) ⬇️
pkg/agent/util/iptables/lock.go 0.00% <0.00%> (-81.82%) ⬇️
pkg/controller/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-80.29%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam_controller.go 0.00% <0.00%> (-79.52%) ⬇️
pkg/agent/cniserver/ipam/antrea_ipam.go 1.04% <0.00%> (-78.13%) ⬇️
.../registry/networkpolicy/clustergroupmember/rest.go 7.84% <0.00%> (-76.48%) ⬇️
... and 252 more

Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

The approach works for me.

pkg/agent/agent.go Outdated Show resolved Hide resolved
jianjuns
jianjuns previously approved these changes Dec 21, 2021
tnqn
tnqn previously approved these changes Dec 23, 2021
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 antoninbas added this to the Antrea v1.6 release milestone Jan 26, 2022
@antoninbas antoninbas force-pushed the fail-in-agent-initialization-if-using-gre-with-ipv6 branch from ce50ae7 to 23d8be1 Compare March 17, 2022 01:15
@antoninbas antoninbas requested review from tnqn and jianjuns March 17, 2022 01:15
@antoninbas
Copy link
Contributor Author

@jianjuns @tnqn I rebased and resolved conflicts, could I get another review so I can merge this for 1.6?

@antoninbas
Copy link
Contributor Author

/test-all

jianjuns
jianjuns previously approved these changes Mar 17, 2022
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

We can merge to 1.6.

@tnqn tnqn added the action/release-note Indicates a PR that should be included in release notes. label Mar 17, 2022
}
continue
}
if i.nodeConfig.PodIPv6CIDR != nil {
klog.Warningf("One IPv6 PodCIDR is already configured on this Node, ignore the IPv6 subnet CIDR %s", localSubnet.String())
klog.InfoS("One IPv6 PodCIDR is already configured on this Node, ignore the IPv4 Subnet CIDR", "subnet", localSubnet)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
klog.InfoS("One IPv6 PodCIDR is already configured on this Node, ignore the IPv4 Subnet CIDR", "subnet", localSubnet)
klog.InfoS("One IPv6 PodCIDR is already configured on this Node, ignore the IPv6 Subnet CIDR", "subnet", localSubnet)

otherwise it would mean dual-stack is not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, copy-paste error

The 'gre' tunnel type does not work for IPv6 overlays. The correct
tunnel type for OVS would be 'ip6gre'. For dual-stack clusters with both
an IPv4 and an IPv6 overlay, we would need 2 default tunnel ports: one
for IPv4 (with type 'gre') and one for IPv6 (with type 'ip6gre'). This
would add complexity (and require testing) as the code currently assumes
a single default tunnel port. Rather than trying to support IPv6 with
the added complexity that it entails, we choose to fail during Agent
initialization for now if the user-provided tunnel type is 'gre' and the
cluster supports IPv6.

Note that this means that the default manifest for IPsec
(antrea-ipsec.yml) cannot be used in an IPv6 cluster as the tunnel type
defaults to GRE in that case. However, this is not new, and it is better
to fail explicitly rather than have a cluster where the Agent appears to
be running fine but there is no connectivity.

See antrea-io#3150

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas force-pushed the fail-in-agent-initialization-if-using-gre-with-ipv6 branch from 23d8be1 to 584782b Compare March 17, 2022 16:53
@antoninbas antoninbas requested a review from tnqn March 17, 2022 16:54
@antoninbas
Copy link
Contributor Author

/test-all

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 antoninbas merged commit 4db3c18 into antrea-io:main Mar 17, 2022
@antoninbas antoninbas deleted the fail-in-agent-initialization-if-using-gre-with-ipv6 branch March 17, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes. area/transit/encapsulation Issues or PRs related to encapsulation. area/transit/ipv6 Issues or PRs related to IPv6.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants