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
commit f9adfd16b8d03b350b8dacf7bbb95fc1975dca14 Author: Alexey Serbin <[email protected]> AuthorDate: Wed Oct 2 16:21:32 2019 -0700 [mini_chronyd] introduce MiniChronyd::address() Introduced MiniChronyd::address() method to find IP address and port at which the underlying NTP server is listening for incoming requests. In case if the NTP server is bound to the wildcard address, use the loopback address for NTP communication with the server. Also, updated corresponding call sites to use the new method instead of accessing MiniChronyd's options. Prior to this patch, the ToolTest.TestRemoteReplicaCopy test scenario would fail if running with built-in NTP client enabled. Change-Id: I62a326a47160aefe9a5806162e6df7c4d6060ba5 Reviewed-on: http://gerrit.cloudera.org:8080/14355 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> --- src/kudu/clock/ntp-test.cc | 5 +++-- src/kudu/clock/test/mini_chronyd-test.cc | 20 ++++++++------------ src/kudu/clock/test/mini_chronyd.cc | 12 ++++++++++-- src/kudu/clock/test/mini_chronyd.h | 6 +++++- src/kudu/mini-cluster/external_mini_cluster.cc | 3 +-- 5 files changed, 27 insertions(+), 19 deletions(-) diff --git a/src/kudu/clock/ntp-test.cc b/src/kudu/clock/ntp-test.cc index 8cf7ee8..5985882 100644 --- a/src/kudu/clock/ntp-test.cc +++ b/src/kudu/clock/ntp-test.cc @@ -583,9 +583,10 @@ TEST_F(BuiltinNtpWithMiniChronydTest, SyncAndUnsyncReferenceServers) { options.index = 3; options.local = false; for (const auto& server : sync_servers) { + const auto addr = server->address(); MiniChronydServerOptions server_options; - server_options.address = server->options().bindaddress; - server_options.port = server->options().port; + server_options.address = addr.host(); + server_options.port = addr.port(); options.servers.emplace_back(std::move(server_options)); } diff --git a/src/kudu/clock/test/mini_chronyd-test.cc b/src/kudu/clock/test/mini_chronyd-test.cc index feba53f..a7776d5 100644 --- a/src/kudu/clock/test/mini_chronyd-test.cc +++ b/src/kudu/clock/test/mini_chronyd-test.cc @@ -59,8 +59,7 @@ TEST_F(MiniChronydTest, UnsynchronizedServer) { ASSERT_EQ(0, stats.ntp_packets_received); } - auto s = MiniChronyd::CheckNtpSource( - { HostPort(chrony->options().bindaddress, chrony->options().port) }); + auto s = MiniChronyd::CheckNtpSource({ chrony->address() }); ASSERT_TRUE(s.IsRuntimeError()) << s.ToString(); ASSERT_STR_CONTAINS(s.ToString(), "failed measure clock offset from reference NTP servers"); @@ -82,8 +81,7 @@ TEST_F(MiniChronydTest, BasicSingleServerInstance) { // A chronyd that uses the system clock as a reference lock should present // itself as reliable NTP server. - const HostPort ntp_endpoint(chrony->options().bindaddress, - chrony->options().port); + const HostPort ntp_endpoint(chrony->address()); { // Make sure the server opens ports to listen and serve requests // from NTP clients. @@ -129,8 +127,7 @@ TEST_F(MiniChronydTest, BasicMultipleServerInstances) { options.index = idx; unique_ptr<MiniChronyd> chrony; ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options)); - ntp_endpoints.emplace_back(chrony->options().bindaddress, - chrony->options().port); + ntp_endpoints.emplace_back(chrony->address()); servers.emplace_back(std::move(chrony)); } @@ -193,8 +190,7 @@ TEST_F(MiniChronydTest, MultiTierBasic) { options.index = idx; unique_ptr<MiniChronyd> chrony; ASSERT_OK(StartChronydAtAutoReservedPort(&chrony, &options)); - ntp_endpoints_0.emplace_back(chrony->options().bindaddress, - chrony->options().port); + ntp_endpoints_0.emplace_back(chrony->address()); servers_0.emplace_back(std::move(chrony)); } @@ -205,15 +201,15 @@ TEST_F(MiniChronydTest, MultiTierBasic) { options.index = idx; options.local = false; for (const auto& ref : servers_0) { + const auto addr = ref->address(); MiniChronydServerOptions server_options; - server_options.port = ref->options().port; - server_options.address = ref->options().bindaddress; + 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)); - ntp_endpoints_1.emplace_back(chrony->options().bindaddress, - chrony->options().port); + ntp_endpoints_1.emplace_back(chrony->address()); servers_1.emplace_back(std::move(chrony)); } diff --git a/src/kudu/clock/test/mini_chronyd.cc b/src/kudu/clock/test/mini_chronyd.cc index 183f2b2..ecb9624 100644 --- a/src/kudu/clock/test/mini_chronyd.cc +++ b/src/kudu/clock/test/mini_chronyd.cc @@ -161,15 +161,23 @@ MiniChronyd::~MiniChronyd() { } const MiniChronydOptions& MiniChronyd::options() const { - CHECK(process_) << "must start the chronyd process first"; return options_; } pid_t MiniChronyd::pid() const { - CHECK(process_) << "must start the chronyd process first"; + CHECK(process_) << "must start chronyd process first"; return process_->pid(); } +HostPort MiniChronyd::address() const { + CHECK(process_) << "must start chronyd process first"; + // If the test NTP server is bound to the wildcard IP address, + // use the loopback IP address to communicate with the server. + return HostPort(options_.bindaddress == kWildcardIpAddr ? kLoopbackIpAddr + : options_.bindaddress, + options_.port); +} + Status MiniChronyd::Start() { SCOPED_LOG_SLOW_EXECUTION(WARNING, 100, "starting chronyd"); CHECK(!process_); diff --git a/src/kudu/clock/test/mini_chronyd.h b/src/kudu/clock/test/mini_chronyd.h index 5cc5e4f..b7bd45e 100644 --- a/src/kudu/clock/test/mini_chronyd.h +++ b/src/kudu/clock/test/mini_chronyd.h @@ -188,12 +188,16 @@ class MiniChronyd { ~MiniChronyd(); - // Return the options the underlying chronyd has been started with. + // Return the options which the underlying chronyd is given to start with. const MiniChronydOptions& options() const; // Get the PID of the chronyd process. pid_t pid() const; + // Get the IP address and port at which the underlying NTP server is listening + // for incoming requests. Should be called only when NTP server is started. + HostPort address() const; + // Start the mini chronyd in server-only mode. Status Start() WARN_UNUSED_RESULT; diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc index cd33faf..23814be 100644 --- a/src/kudu/mini-cluster/external_mini_cluster.cc +++ b/src/kudu/mini-cluster/external_mini_cluster.cc @@ -165,8 +165,7 @@ Status ExternalMiniCluster::AddNtpFlags(std::vector<std::string>* flags) { vector<string> ntp_endpoints; CHECK_EQ(opts_.num_ntp_servers, ntp_servers_.size()); for (const auto& server : ntp_servers_) { - const auto& opt = server->options(); - ntp_endpoints.emplace_back(HostPort(opt.bindaddress, opt.port).ToString()); + ntp_endpoints.emplace_back(server->address().ToString()); } // Point the built-in NTP client to the test NTP server running as a part // of the cluster.
