kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [kudu] branch master updated: [clock] clean up on chronyd test utility functions
Date Mon, 07 Oct 2019 06:20:48 GMT
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 <alexey@apache.org>
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 <aserbin@cloudera.com>
    Reviewed-by: Adar Dembo <adar@cloudera.com>
---
 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


Mime
View raw message