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

connectivity: Skip IPv6 requests in north-south-loadbalancing-with-l7-policy when running on < 1.14.0 Cilium #1629

Merged
merged 1 commit into from
May 19, 2023

Conversation

jschwinger233
Copy link
Member

cilium/cilium#21954 for the IPv6 path was resolved only for v1.14, but not for v1.13. In order to be able to run the latest connectivity tests on v1.13, we need to skip curl requests to the IPv6 addresses in that particular test.

Fixes: #1627

Signed-off-by: Zhichuan Liang <gray.isovalent.com>

@jschwinger233 jschwinger233 temporarily deployed to ci May 15, 2023 08:39 — with GitHub Actions Inactive
@jschwinger233 jschwinger233 temporarily deployed to ci May 15, 2023 09:02 — with GitHub Actions Inactive
@jschwinger233 jschwinger233 marked this pull request as ready for review May 15, 2023 09:33
@jschwinger233 jschwinger233 requested a review from a team as a code owner May 15, 2023 09:33
@jschwinger233
Copy link
Member Author

Ran ./cilium connectivity test -v --test north-south-loadbalancing --datapath locally for v.1.13 and v.1.14, looks good.

// Skip IPv6 requests when running on <1.14.0 Cilium with CNPs
if check.GetIPFamily(addr.Address) == check.IPFamilyV6 &&
versioncheck.MustCompile("<1.14.0")(t.Context().CiliumVersion) &&
len(t.CiliumNetworkPolicies()) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Please don't do my mistakes - please check t.knps too (#1587) 😅

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -705,6 +705,7 @@ func (ct *ConnectivityTest) CurlCommand(peer TestPeer, ipFam IPFamily, opts ...s
peer.Scheme(),
net.JoinHostPort(peer.Address(ipFam), fmt.Sprint(peer.Port())),
peer.Path()))
println(strings.Join(cmd, " "))
Copy link
Member

Choose a reason for hiding this comment

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

Nit: do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry - - I should have checked my PR

…-policy when running on < 1.14.0 Cilium

cilium/cilium#21954 for the IPv6 path was
resolved only for v1.14, but not for v1.13. In order to be able to run
the latest connectivity tests on v1.13, we need to skip curl requests to
the IPv6 addresses in that particular test.

Fixes: cilium#1627

Signed-off-by: Zhichuan Liang <gray.liang@isovalent.com>
@jschwinger233 jschwinger233 temporarily deployed to ci May 18, 2023 07:30 — with GitHub Actions Inactive
@brb brb merged commit c3e950c into cilium:main May 19, 2023
15 of 17 checks passed
julianwiedmann added a commit that referenced this pull request Mar 19, 2024
#1629 initially limited the IPv6
test to v1.14, as v1.13 was missing some functionality.

But now that v1.13 has cilium/cilium#31161, the
IPv6 path should actually work. So let's extend the test coverage.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
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.

connectivity: Skip IPv6 requests in north-south-loadbalancing-with-l7-policy when running on < 1.14.0 Cilium
2 participants