You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2025/11/04 18:39:44 UTC
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Ashwani Raina has uploaded this change for review. ( http://gerrit.cloudera.org:8080/23630
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
[tests] Add IPv6 coverage to multiple tests
This patch is mainly to bring IPv6 coverage to different tests that make
use of local addresses to test out different scenarios.
- A new macro SKIP_IF_HOSTNAME_RESOLUTION_FAILURE_EXPECTED() is added
to ensure tests that require fully qualified name resolution do not
fail due to absence of DNS record on a specific node.
- Coverage is mainly provided by running same tests with different
ip_config_mode value i.e. 'ipv4', 'ipv6', 'dual'.
- For client-test, the coverage is not added for 'dual' mode because of
the additional time it takes to run those tests. However, the tests
have been verified to complete successfully for 'dual' mode.
- Some minor and miscellaneous changes to rename tests to get rid of
CLANG tidy warnings.
Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.h
M src/kudu/client/session-internal.h
M src/kudu/common/partial_row.h
M src/kudu/master/catalog_manager.h
M src/kudu/master/mini_master-test.cc
M src/kudu/rpc/proxy-test.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/tablet/tablet.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/mini_oidc.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
16 files changed, 364 insertions(+), 257 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/23630/1
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@16
PS2, Line 16: For client-test, the coverage is not added for 'dual' mode because of
: the additional time it takes to run those tests.
> How much additional time does it take? ie would it be linear in terms of ju
IIRC, it was around 250 seconds for dual mode on my laptop. However, this could reduce way down on dist-test slaves (as per dist-test test logs).
The increase in time taken is more or less linear with addition of each mode.
Yeah, skipping tests for dual mode conditionally (based on whether slow tests are allowed or not) sounds good to me.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@537
PS2, Line 537: *out = Substitute("[$0]", kWildcardIpV6Addr);
> In dual mode, why bind to wildcard [::] instead of loopback [::1]?
Binding to loopback for dual mode won't accept IPv4 requests. The idea is to allow both IPv4 and IPv6 connections to succeed when the server is configured with 'dual' mode i.e. dual stack.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@850
PS2, Line 850: string GetBindIpForDaemon(int index, BindMode bind_mode) {
> Should GetBindIpForDaemon() return with brackets for consistency? (ToIpInte
Not required as the return type is a string and is passed to network functions like getaddrinfo for DNS resolution and such functions expect node names that can be resolved and Ip enclosed in brackets would fail with unknown name or service.
The calls that require bracket enclosed IP go through either some transformative objects like HostPort that keep track of both host and port and both of them are likely to get used in those functions. Or explicit transformation invoked via ToIpInterface.
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Nov 2025 11:11:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
Okay, that seems not a significant bump in runtime.
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Nov 2025 21:43:54 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2106/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 07 Nov 2025 08:44:25 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 1:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2098/
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 04 Nov 2025 18:39:53 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
> Curios if you got a chance to run the updated tests with and
> without the changes to see the time difference?
>
> Compilation failure reported in the build:
> /home/jenkins-slave/workspace/build_and_test/src/kudu/rpc/rpc-test-base.h:39:10:
> fatal error: kudu/rpc/rtest.pb.h: No such file or directory
> #include "kudu/rpc/rtest.pb.h"
Yes, but on my local machine. On dist-test cluster the timing varies by a lot.
Without this change, client-test took 807 seconds
With this change, client-test took 1307 seconds.
Thanks for the heads up on compilation error.
I moved SKIP_IF_HOSTNAME_RESOLUTION_FAILURE_EXPECTED() definition to src/kudu/util/test_util.h.
That is where SKIP_IF_SLOW_NOT_ALLOWED() is defined as well.
That fixed the error.
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 13 Nov 2025 09:34:55 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
Curios if you got a chance to run the updated tests with and without the changes to see the time difference?
Compilation failure reported in the build:
/home/jenkins-slave/workspace/build_and_test/src/kudu/rpc/rpc-test-base.h:39:10: fatal error: kudu/rpc/rtest.pb.h: No such file or directory
#include "kudu/rpc/rtest.pb.h"
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Nov 2025 21:44:34 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG
Commit Message:
PS2:
> At this point it's time to double check, I guess: what is ipv6-only mode, a
ipv6-only mode is when sockets are restricted to receive and send ipv6 packets only.
Yes, it is possible to disallow packets from IPv4-on-IPv6 address if server socket is configured with IPV6_V6ONLY socket option.
There is an existing gerrit patch that was posted before this one that takes care of setting socket option to IPV6_V6ONLY, when "ipv6" config mode is set.
https://gerrit.cloudera.org/#/c/23651/
Ideally, 23651 should be reviewed first and then this one.
'dual' mode, in this context, means rpc server running both protocol stacks, and capable to serve request from both IPv4 as well IPv6 sockets. It is a default behaviour for an interface configured with both IPv4 and IPv6 addresses.
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@16
PS2, Line 16: For client-test, the coverage is not added for 'dual' mode because of
: the additional time it takes to run those tests.
> +1 for adding SKIP_IF_SLOW_NOT_ALLOWED into tests if they run for more than
Done.
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@19
PS2, Line 19: - Some minor and miscellaneous changes to rename tests to get rid of
: CLANG tidy warnings.
> Please separate these updates into its own changelist, and add the actual I
Somehow, I am not able to locate the misc changes in this patch but I distinctly remember renaming tests reported by CLANG. If I find those changes, I will make sure to post a separate patch for those.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc@125
PS2, Line 125: "ipv4", "ipv6", "dual"
> Here and elsewhere: I guess it's time to introduce constants for these toke
Done.
Please take a look: https://gerrit.cloudera.org/#/c/23707/
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@524
PS2, Line 524: ToIpInterface(string* out)
> Given the code in this method, I think this name is very misleading. 'ToIp
Changed to ToLoopbackIpInterface.
Since this is needed only by test scaffolding, moved to right place.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@537
PS2, Line 537: *out = Substitute("[$0]", kWildcardIpV6Addr);
> This smells fishy. Is this for test testing harness? I'm not sure we shou
Actually, it is not. The third case i.e. DUAL is there for completeness purpose, just in case it is required to be used from non-test locations. Currently, ToIpInterface is not used for dual mode anywhere from the tests.
I am fine with removing this from here and probably return error for DUAL case.
Btw, we do use IPv4 wildcard address in some existing unit tests like kudu-tool-test.cc, webserver-test.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@859
PS2, Line 859: if (fam == AF_INET6 || fam == AF_UNSPEC) {
: return kLoopbackIpV6Addr;
: }
> I don't think this provides the functionality required by UNIQUE_LOOPBACK.
We can use IPv4-mapped-IPv6 address range with 127.0.0.0/8 to make it unique but that is not really using a IPv6 stack in true sense when it comes to kernel processing requests. A server listening on ::FFFF:127.0.0.1 (with disabled IPV6-V6ONLY) accepts ipv4 and ipv6 (per socket abstraction) client connections. However, with same config, the same connect call might fail if the client tries to connect to the same server after explicitly binding to a IPv6 loopback address i.e. ::1.
IPv4-mapped IPv6 address is not really a fair analogy when it comes to covering IPv6 scenario for a unique loopback. A strict counterpart for a UNIQUE_LOOPBACK in IPv6 doesn't exist.
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 25 Nov 2025 13:58:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/23630
to look at the new patch set (#2).
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
[tests] Add IPv6 coverage to multiple tests
This patch is mainly to bring IPv6 coverage to different tests that make
use of local addresses to test out different scenarios.
- A new macro SKIP_IF_HOSTNAME_RESOLUTION_FAILURE_EXPECTED() is added
to ensure tests that require fully qualified name resolution do not
fail due to absence of DNS record on a specific node.
- Coverage is mainly provided by running same tests with different
ip_config_mode value i.e. 'ipv4', 'ipv6', 'dual'.
- For client-test, the coverage is not added for 'dual' mode because of
the additional time it takes to run those tests. However, the tests
have been verified to complete successfully for 'dual' mode.
- Some minor and miscellaneous changes to rename tests to get rid of
CLANG tidy warnings.
Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
---
M src/kudu/client/client-test.cc
M src/kudu/client/client.h
M src/kudu/client/meta_cache.h
M src/kudu/client/session-internal.h
M src/kudu/common/partial_row.h
M src/kudu/master/catalog_manager.h
M src/kudu/rpc/proxy-test.cc
M src/kudu/tablet/tablet.h
M src/kudu/tserver/mini_tablet_server.cc
M src/kudu/util/jwt-util-test.cc
M src/kudu/util/mini_oidc.cc
M src/kudu/util/net/net_util-test.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
14 files changed, 337 insertions(+), 252 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/30/23630/2
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@16
PS2, Line 16: For client-test, the coverage is not added for 'dual' mode because of
: the additional time it takes to run those tests.
How much additional time does it take? ie would it be linear in terms of just one other ip_config?
Maybe add SKIP_IF_SLOW_NOT_ALLOWED in ClientParamTest's SetUp if GetParam is dual? What do you think about this?
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@537
PS2, Line 537: *out = Substitute("[$0]", kWildcardIpV6Addr);
In dual mode, why bind to wildcard [::] instead of loopback [::1]?
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@850
PS2, Line 850: string GetBindIpForDaemon(int index, BindMode bind_mode) {
Should GetBindIpForDaemon() return with brackets for consistency? (ToIpInterface adds brackets)
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 10 Nov 2025 14:12:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 1: Verified-1
Build Failed
http://jenkins.kudu.apache.org/job/pre_commit/2098/ : FAILURE
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 1
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 04 Nov 2025 19:14:49 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
Build Started http://jenkins.kudu.apache.org/job/pre_commit/2106/
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 07 Nov 2025 07:19:55 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] Add IPv6 coverage to multiple tests
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/23630 )
Change subject: [tests] Add IPv6 coverage to multiple tests
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG
Commit Message:
PS2:
At this point it's time to double check, I guess: what is ipv6-only mode, actually?
Is it even possible to configure IP stack on Linux to disallow IPv4-on-IPv6 address mapping if there is an incoming connection from a IPv4 address? I'm not sure about that.
From the other side, I don't see setting IPV6_V6ONLY for sockets. Then how IPv6-only mode is achieved then?
One more question to clarify: what is 'dual' mode? Is that something about the default behavior of the IP stack on Linux where the IPv6 is enabled?
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@16
PS2, Line 16: For client-test, the coverage is not added for 'dual' mode because of
: the additional time it takes to run those tests.
> IIRC, it was around 250 seconds for dual mode on my laptop. However, this c
+1 for adding SKIP_IF_SLOW_NOT_ALLOWED into tests if they run for more than 3 seconds in DEBUG build on a relatively contemporary hardware.
http://gerrit.cloudera.org:8080/#/c/23630/2//COMMIT_MSG@19
PS2, Line 19: - Some minor and miscellaneous changes to rename tests to get rid of
: CLANG tidy warnings.
Please separate these updates into its own changelist, and add the actual IPv6 updates on top of that changelist.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc
File src/kudu/util/jwt-util-test.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/jwt-util-test.cc@125
PS2, Line 125: "ipv4", "ipv6", "dual"
Here and elsewhere: I guess it's time to introduce constants for these tokens and start using the new constants instead at least in tests?
Probably, it makes sense doing so in its own changelist, maybe updating already existing tests that use so-called IP mode as well. After that, it could be possible to use the new constants in this patch as well?
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@524
PS2, Line 524: ToIpInterface(string* out)
Given the code in this method, I think this name is very misleading. 'ToIpInterface' what?
First: is this functionality is needed by only by the test scaffolding? If so, then I don't think this is the right place to place it.
Second: please name methods to reflect their actual semantics.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@537
PS2, Line 537: *out = Substitute("[$0]", kWildcardIpV6Addr);
> Binding to loopback for dual mode won't accept IPv4 requests. The idea is t
This smells fishy. Is this for test testing harness? I'm not sure we should use non-loopback addresses for testing. It seems I didn't pay enough attention to this in http://gerrit.cloudera.org:8080/23411, but now I realize it's necessary to address that.
http://gerrit.cloudera.org:8080/#/c/23630/2/src/kudu/util/net/net_util.cc@859
PS2, Line 859: if (fam == AF_INET6 || fam == AF_UNSPEC) {
: return kLoopbackIpV6Addr;
: }
I don't think this provides the functionality required by UNIQUE_LOOPBACK. The idea behind UNIQUE_LOOPBACK is to have a unique loopback IP address so it would be necessary to bother about port conflicts.
Doesn't IPv6 stack on Linux provide similar functionality for the range of IPv4-mapped-on-IPv6 addresses, i.e. addresses in the form ::FFFF:<IPv4 127.0.0.0/8 address>?
--
To view, visit http://gerrit.cloudera.org:8080/23630
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idc0df890c71e101964e7c7c84b5847963fab1451
Gerrit-Change-Number: 23630
Gerrit-PatchSet: 2
Gerrit-Owner: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Nov 2025 06:11:11 +0000
Gerrit-HasComments: Yes