This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 991ad54 [clock] clean up on chronyd test utility functions
991ad54 is described below
commit 991ad543ac75d6c38c052ef25cbe8fbf667dc3bc
Author: Alexey Serbin <[email protected]>
AuthorDate: Sun Oct 6 10:57:41 2019 -0700
[clock] clean up on chronyd test utility functions
This patch contains a minor clean-up on StartChronydAtAutoReservedPort
utility function and its usages throughout various tests.
This patch does not contain any functional changes.
Change-Id: I71505e3ff686da638920bd1e9c99ef84bc511452
Reviewed-on: http://gerrit.cloudera.org:8080/14378
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Adar Dembo <[email protected]>
---
src/kudu/clock/ntp-test.cc | 69 +++++++++++++++------------
src/kudu/clock/test/mini_chronyd-test.cc | 44 +++++++++--------
src/kudu/clock/test/mini_chronyd_test_util.cc | 19 +++-----
src/kudu/clock/test/mini_chronyd_test_util.h | 15 +++---
4 files changed, 77 insertions(+), 70 deletions(-)
diff --git a/src/kudu/clock/ntp-test.cc b/src/kudu/clock/ntp-test.cc
index 5985882..deb9b8e 100644
--- a/src/kudu/clock/ntp-test.cc
+++ b/src/kudu/clock/ntp-test.cc
@@ -461,11 +461,13 @@ TEST_F(BuiltinNtpWithMiniChronydTest, Basic) {
vector<unique_ptr<MiniChronyd>> servers;
vector<HostPort> servers_endpoints;
for (auto idx = 0; idx < 3; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
- servers_endpoints.emplace_back(options.bindaddress, options.port);
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
+ servers_endpoints.emplace_back(chrony->address());
servers.emplace_back(std::move(chrony));
}
@@ -504,11 +506,13 @@ TEST_F(BuiltinNtpWithMiniChronydTest,
NoIntersectionIntervalAtStartup) {
vector<HostPort> servers_endpoints;
vector<unique_ptr<MiniChronyd>> servers;
for (auto idx = 0; idx < 3; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
- servers_endpoints.emplace_back(options.bindaddress, options.port);
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
+ servers_endpoints.emplace_back(chrony->address());
servers.emplace_back(std::move(chrony));
}
@@ -539,11 +543,13 @@ TEST_F(BuiltinNtpWithMiniChronydTest,
SyncAndUnsyncReferenceServers) {
vector<unique_ptr<MiniChronyd>> sync_servers;
vector<HostPort> sync_servers_refs;
for (auto idx = 0; idx < 2; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
- sync_servers_refs.emplace_back(options.bindaddress, options.port);
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
+ sync_servers_refs.emplace_back(chrony->address());
sync_servers.emplace_back(std::move(chrony));
}
@@ -567,32 +573,35 @@ TEST_F(BuiltinNtpWithMiniChronydTest,
SyncAndUnsyncReferenceServers) {
// Start a server without any true time reference: it has nothing to
// synchronize with and stays unsynchronized.
{
- MiniChronydOptions options;
- options.index = 2;
- options.local = false;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
- unsync_servers_refs.emplace_back(options.bindaddress, options.port);
+ {
+ MiniChronydOptions options;
+ options.index = 2;
+ options.local = false;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
+ unsync_servers_refs.emplace_back(chrony->address());
unsync_servers.emplace_back(std::move(chrony));
}
// Another NTP server which uses those two synchronized, but far from each
// other NTP servers. As the result, the new NTP server runs unsynchonized.
{
- MiniChronydOptions options;
- options.index = 3;
- options.local = false;
- for (const auto& server : sync_servers) {
- const auto addr = server->address();
- MiniChronydServerOptions server_options;
- server_options.address = addr.host();
- server_options.port = addr.port();
- options.servers.emplace_back(std::move(server_options));
- }
-
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
- unsync_servers_refs.emplace_back(options.bindaddress, options.port);
+ {
+ MiniChronydOptions options;
+ options.index = 3;
+ options.local = false;
+ for (const auto& server : sync_servers) {
+ const auto addr = server->address();
+ MiniChronydServerOptions server_options;
+ server_options.address = addr.host();
+ server_options.port = addr.port();
+ options.servers.emplace_back(std::move(server_options));
+ }
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
+ unsync_servers_refs.emplace_back(chrony->address());
unsync_servers.emplace_back(std::move(chrony));
}
diff --git a/src/kudu/clock/test/mini_chronyd-test.cc
b/src/kudu/clock/test/mini_chronyd-test.cc
index a7776d5..86dc44b 100644
--- a/src/kudu/clock/test/mini_chronyd-test.cc
+++ b/src/kudu/clock/test/mini_chronyd-test.cc
@@ -49,7 +49,7 @@ TEST_F(MiniChronydTest, UnsynchronizedServer) {
{
MiniChronydOptions options;
options.local = false;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
}
// No client has talked to the NTP server yet.
@@ -77,7 +77,7 @@ TEST_F(MiniChronydTest, UnsynchronizedServer) {
TEST_F(MiniChronydTest, BasicSingleServerInstance) {
// Start chronyd at the specified port, making sure it's serving requests.
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony));
+ ASSERT_OK(StartChronydAtAutoReservedPort({}, &chrony));
// A chronyd that uses the system clock as a reference lock should present
// itself as reliable NTP server.
@@ -123,10 +123,12 @@ TEST_F(MiniChronydTest, BasicMultipleServerInstances) {
vector<unique_ptr<MiniChronyd>> servers;
vector<HostPort> ntp_endpoints;
for (int idx = 0; idx < 5; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
ntp_endpoints.emplace_back(chrony->address());
servers.emplace_back(std::move(chrony));
}
@@ -186,10 +188,12 @@ TEST_F(MiniChronydTest, MultiTierBasic) {
vector<unique_ptr<MiniChronyd>> servers_0;
vector<HostPort> ntp_endpoints_0;
for (auto idx = 0; idx < 3; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
ntp_endpoints_0.emplace_back(chrony->address());
servers_0.emplace_back(std::move(chrony));
}
@@ -197,18 +201,20 @@ TEST_F(MiniChronydTest, MultiTierBasic) {
vector<unique_ptr<MiniChronyd>> servers_1;
vector<HostPort> ntp_endpoints_1;
for (auto idx = 3; idx < 5; ++idx) {
- MiniChronydOptions options;
- options.index = idx;
- options.local = false;
- for (const auto& ref : servers_0) {
- const auto addr = ref->address();
- MiniChronydServerOptions server_options;
- server_options.address = addr.host();
- server_options.port = addr.port();
- options.servers.emplace_back(std::move(server_options));
- }
unique_ptr<MiniChronyd> chrony;
- ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options));
+ {
+ MiniChronydOptions options;
+ options.index = idx;
+ options.local = false;
+ for (const auto& ref : servers_0) {
+ const auto addr = ref->address();
+ MiniChronydServerOptions server_options;
+ server_options.address = addr.host();
+ server_options.port = addr.port();
+ options.servers.emplace_back(std::move(server_options));
+ }
+ ASSERT_OK(StartChronydAtAutoReservedPort(std::move(options), &chrony));
+ }
ntp_endpoints_1.emplace_back(chrony->address());
servers_1.emplace_back(std::move(chrony));
}
diff --git a/src/kudu/clock/test/mini_chronyd_test_util.cc
b/src/kudu/clock/test/mini_chronyd_test_util.cc
index 17d8ac5..9560182 100644
--- a/src/kudu/clock/test/mini_chronyd_test_util.cc
+++ b/src/kudu/clock/test/mini_chronyd_test_util.cc
@@ -54,20 +54,13 @@ Status ReservePort(int index, unique_ptr<Socket>* socket,
} // anonymous namespace
-Status StartChronydAtAutoReservedPort(unique_ptr<MiniChronyd>* chronyd,
- MiniChronydOptions* options) {
- MiniChronydOptions opts;
- if (options) {
- opts = *options;
- }
+Status StartChronydAtAutoReservedPort(MiniChronydOptions options,
+ unique_ptr<MiniChronyd>* chronyd) {
unique_ptr<Socket> socket;
- RETURN_NOT_OK(ReservePort(opts.index, &socket, &opts.bindaddress,
&opts.port));
- chronyd->reset(new MiniChronyd(opts));
- RETURN_NOT_OK((*chronyd)->Start());
- if (options) {
- *options = std::move(opts);
- }
- return Status::OK();
+ RETURN_NOT_OK(ReservePort(
+ options.index, &socket, &options.bindaddress, &options.port));
+ chronyd->reset(new MiniChronyd(std::move(options)));
+ return (*chronyd)->Start();
}
} // namespace clock
diff --git a/src/kudu/clock/test/mini_chronyd_test_util.h
b/src/kudu/clock/test/mini_chronyd_test_util.h
index 66d66f8..1903ef1 100644
--- a/src/kudu/clock/test/mini_chronyd_test_util.h
+++ b/src/kudu/clock/test/mini_chronyd_test_util.h
@@ -28,15 +28,14 @@ namespace clock {
class MiniChronyd;
struct MiniChronydOptions;
-// Reserve a port and start chronyd, outputting the result chronyd object
-// wrapped into std::unique_ptr smart pointer. The 'options' parameter is
-// in-out: the bound port and address are output into the 'port' and
-// 'bindaddress' fields correspondingly. All other fields of the 'options'
-// parameter are untouched, and the only field affective the port reservation
-// process is the 'index' field.
+// Reserve a port and start chronyd NTP server using MiniChronyd with
+// MiniChronydOptions options specified by 'options' parameter. The only field
+// affecting the port reservation process is 'MiniChronydOptions::index' field.
+// The result MiniChronyd object is wrapped into std::unique_ptr smart pointer
+// and output into the 'chronyd' out parameter.
Status StartChronydAtAutoReservedPort(
- std::unique_ptr<MiniChronyd>* chronyd,
- MiniChronydOptions* options = nullptr) WARN_UNUSED_RESULT;
+ MiniChronydOptions options,
+ std::unique_ptr<MiniChronyd>* chronyd) WARN_UNUSED_RESULT;
} // namespace clock
} // namespace kudu
