From ccb2709c7897cd539c1a7990fb539647b1dc16ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Fri, 9 Nov 2018 08:36:40 +0000 Subject: [PATCH 1/8] Bring back the 'Select from' widget for choosing a ntp server --- src/clients/ntp-client_proposal.rb | 43 +++- src/lib/y2ntp_client/dialog/add_pool.rb | 88 +++++++ src/lib/y2ntp_client/dialog/pool.rb | 11 +- src/lib/y2ntp_client/dynamic_servers.rb | 22 ++ src/lib/y2ntp_client/widgets/main_widgets.rb | 6 +- src/lib/y2ntp_client/widgets/pool_widgets.rb | 238 +++++++++++++++++- src/modules/NtpClient.rb | 8 +- test/y2ntp_client/dynamic_servers_test.rb | 39 +++ .../y2ntp_client/widgets/pool_widgets_test.rb | 13 + 9 files changed, 446 insertions(+), 22 deletions(-) create mode 100644 src/lib/y2ntp_client/dialog/add_pool.rb create mode 100644 src/lib/y2ntp_client/dynamic_servers.rb create mode 100755 test/y2ntp_client/dynamic_servers_test.rb mode change 100644 => 100755 test/y2ntp_client/widgets/pool_widgets_test.rb diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index 9dbf12be..911d8d7b 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -184,20 +184,18 @@ def MakeProposal end if NtpClient.config_has_been_read || NtpClient.ntp_selected - Builtins.y2milestone("ntp_items will be filled from /etc/chrony.conf") + log.info("ntp_items will be filled from /etc/chrony.conf") # grr, GUNS means all of them are used and here we just pick one - ntp_items = Builtins.maplist(NtpClient.GetUsedNtpServers) do |server| - Item(Id(server), server) - end + ntp_items = configured_ntp_items + # avoid calling Read again (bnc #427712) NtpClient.config_has_been_read = true end - if ntp_items == [] - Builtins.y2milestone( - "Nothing found in /etc/chrony.conf, proposing current timezone-based NTP server list" - ) - time_zone_country = Timezone.GetCountryForTimezone(Timezone.timezone) - ntp_items = NtpClient.GetNtpServersByCountry(time_zone_country, true) + + if ntp_items.empty? + log.info("Nothing found in /etc/chrony.conf, " \ + "proposing current timezone-based NTP server list") + ntp_items = dhcp_ntp_items.empty? ? timezone_ntp_items : dhcp_ntp_items NtpClient.config_has_been_read = true end ntp_items = Builtins.add(ntp_items, "") @@ -474,6 +472,31 @@ def add_or_install_required_package ) end end + + # Configured ntp servers Yast::Term items with the ntp address as the ID + # and label + # + # @return [Yast::Term] ntp address table Item + def configured_ntp_items + NtpClient.GetUsedNtpServers + end + + # Public list of ntp servers Yast::Term items with the ntp address as the + # ID and label + # + # @return [Array] ntp address Item + def timezone_ntp_items + timezone_country = Timezone.GetCountryForTimezone(Timezone.timezone) + NtpClient.GetNtpServersByCountry(timezone_country, true) + end + + # List of ntp servers Yast::Term items with the ntp address as the ID and + # label + # + # @return [Array] ntp address table Item + def dhcp_ntp_items + NtpClient.dhcp_ntp_servers + end end end diff --git a/src/lib/y2ntp_client/dialog/add_pool.rb b/src/lib/y2ntp_client/dialog/add_pool.rb new file mode 100644 index 00000000..ee3f46c6 --- /dev/null +++ b/src/lib/y2ntp_client/dialog/add_pool.rb @@ -0,0 +1,88 @@ +require "yast" + +require "cwm/popup" + +Yast.import "Label" +Yast.import "NtpClient" +Yast.import "Stage" +Yast.import "Popup" + +# Work around YARD inability to link across repos/gems: + +# @!macro [new] seeAbstractWidget +# @see http://www.rubydoc.info/github/yast/yast-yast2/CWM/AbstractWidget:${0} +# @!macro [new] seeCustomWidget +# @see http://www.rubydoc.info/github/yast/yast-yast2/CWM/CustomWidget:${0} +# @!macro [new] seeItemsSelection +# @see http://www.rubydoc.info/github/yast/yast-yast2/CWM/ItemsSelection:${0} +# @!macro [new] seeDialog +# @see http://www.rubydoc.info/github/yast/yast-yast2/CWM/Dialog:${0} +# @!macro [new] seePopup +# @see http://www.rubydoc.info/github/yast/yast-yast2/CWM/Popup:${0} + +module Y2NtpClient + module Dialog + # Dialog to add/edit ntp pool server + class AddPool < ::CWM::Popup + # Constructor + # + # @param address_widget [CWM::InputField] + # @param pool_type [Symbol] + def initialize(address_widget, pool_type) + textdomain "ntp-client" + @address_widget = address_widget + @address = @address_widget.value + @pool_chooser = pool_for(pool_type) + end + + # @macro seeDialog + def title + _("Local ntp servers discovered") + end + + # @macro seeDialog + def contents + VBox( + @pool_chooser + ) + end + + def next_handler + return :cancel if @pool_chooser.value.to_s.empty? + @address = @pool_chooser.value + + :next + end + + # @macro seeDialog + def next_button + ok_button_label + end + + # @macro seeDialog + def run + result = super + @address_widget.value = @address if result == :next + + result + end + + private + + # @macro seeDialog + def ok_button + PushButton(Id(:next), Opt(:default), ok_button_label) + end + + def pool_for(type) + { local: Widgets::LocalList, + public: Widgets::PublicList }.fetch(type, Widgets::LocalList).new(@address_widget.value) + end + + # @macro see + def min_height + 8 + end + end + end +end diff --git a/src/lib/y2ntp_client/dialog/pool.rb b/src/lib/y2ntp_client/dialog/pool.rb index d2f2ecc4..abca673a 100644 --- a/src/lib/y2ntp_client/dialog/pool.rb +++ b/src/lib/y2ntp_client/dialog/pool.rb @@ -32,10 +32,19 @@ def contents HBox( @address_widget, HSpacing(), - Widgets::TestButton.new(@address_widget) + VBox( + VSpacing(1), + Widgets::SelectFrom.new(@address_widget) + ), + HSpacing(), + VBox( + VSpacing(1), + Widgets::TestButton.new(@address_widget) + ) ), VSpacing(), HBox( + HSpacing(), Widgets::Iburst.new(@options), HSpacing(), Widgets::Offline.new(@options) diff --git a/src/lib/y2ntp_client/dynamic_servers.rb b/src/lib/y2ntp_client/dynamic_servers.rb new file mode 100644 index 00000000..e01b54ed --- /dev/null +++ b/src/lib/y2ntp_client/dynamic_servers.rb @@ -0,0 +1,22 @@ +require "yast" + +module Y2NtpClient + # Bunch of methods for retrieving ntp servers dinamically, i.e. by dhcp or + # slp. + module DynamicServers + def dhcp_ntp_servers + Yast.import "Lan" + Yast.import "LanItems" + Yast.import "NetworkService" + + # When proposing NTP servers we need to know + # 1) list of (dhcp) interfaces + # 2) network service in use + # We can either use networking submodule for network service handling and get list of + # interfaces e.g. using a bash command or initialize whole networking module. + Yast::Lan.ReadWithCacheNoGUI + + Yast::LanItems.dhcp_ntp_servers.values.reduce(&:concat) || [] + end + end +end diff --git a/src/lib/y2ntp_client/widgets/main_widgets.rb b/src/lib/y2ntp_client/widgets/main_widgets.rb index 70f926a5..3cc4f793 100644 --- a/src/lib/y2ntp_client/widgets/main_widgets.rb +++ b/src/lib/y2ntp_client/widgets/main_widgets.rb @@ -26,10 +26,10 @@ def label def help # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that _( - "The NTP configuration may be provided by the local network over DHCP. " \ + "

The NTP configuration may be provided by the local network over DHCP. " \ "Configuration Source can simply enable or disable using that configuration. " \ "In cases where there may be multiple DHCP sources, it can prioritize them: " \ - "see 'man 8 netconfig'." + "see 'man 8 netconfig'.

" ) end @@ -127,7 +127,7 @@ def help "Select whether to start the NTP daemon now and on every system boot. \n" \ "Selecting Synchronize without Daemon the NTP daemon will not be activated\n" \ "and the system time will be set periodically by a cron script. \n" \ - "The interval is configurable, by default it is %d minutes." + "The interval is configurable, by default it is %d minutes.

" ) % Yast::NtpClientClass::DEFAULT_SYNC_INTERVAL end diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index aa5d1289..f0875015 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -1,10 +1,13 @@ require "yast" require "cwm/widget" +require "y2ntp_client/dialog/add_pool" +require "y2ntp_client/dynamic_servers" Yast.import "Address" Yast.import "NtpClient" Yast.import "Popup" +Yast.import "NetworkService" module Y2NtpClient module Widgets @@ -12,19 +15,23 @@ module Widgets class PoolAddress < CWM::InputField attr_reader :address + # @macro seeAbstractWidget def initialize(initial_value) textdomain "ntp-client" @address = initial_value end + # @macro seeAbstractWidget def label _("A&ddress") end + # @macro seeAbstractWidget def init self.value = @address end + # @macro seeAbstractWidget def validate return true if Yast::Address.Check(value) @@ -33,6 +40,7 @@ def validate false end + # @macro seeAbstractWidget def store @address = value end @@ -80,9 +88,11 @@ def store end def help - _("Quick Initial Sync specifies whether the 'iburst' option is used. This option " \ - "sends 4 poll requests in 2 second intervals during the initialization. It is useful for " \ - "a quick synchronization during the start of the machine.") + "

" + + _("Quick Initial Sync specifies whether the 'iburst' option is used. This " \ + "option sends 4 poll requests in 2 second intervals during the initialization. It is " \ + "useful for a quick synchronization during the start of the machine.") + + "

" end end @@ -110,10 +120,228 @@ def store end def help - _("Start Offline specifies whether the 'offline' option is used. This option " \ + _("

Start Offline specifies whether the 'offline' option is used. This option " \ "skips this server during the start-up. It is useful for a machine which starts " \ "without the network, because it speeds up the boot, and synchronizes when the machine " \ - "gets connected to the network.") + "gets connected to the network.

") + end + end + + # Menu Button for choosing which type of server should be added to the + # address input field. + class SelectFrom < CWM::MenuButton + def initialize(address_widget) + Yast.import "Popup" + @address_widget = address_widget + end + + # @macro seeAbstractWidget + def label + _("Select") + end + + # @macro seeAbstractWidget + def opt + [:notify] + end + + # @macro seeItemsSelection + def items + [[:local, _("Local Server")], [:public, _("Public Server")]] + end + + # @macro seeAbstractWidget + def handle(event) + case event["ID"] + when :local, :public + Dialog::AddPool.new(@address_widget, event["ID"]).run + end + + nil + end + + # @macro seeAbstractWidget + def cwm_definition + additional = {} + # handle_events are by default widget_id, but in radio buttons, events are + # in fact single RadioButton + if !handle_all_events + event_ids = items.map(&:first) + additional["handle_events"] = event_ids + end + + super.merge(additional) + end + end + + # List of ntp servers obtained from DHCP + class LocalList < CWM::SelectionBox + include DynamicServers + # Constructor + # + # @param address [String] current ntp pool address + def initialize(address) + textdomain "ntp-client" + @address = address + + Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do + @servers = ntp_servers + end + end + + # @macro seeAbstractWidget + def init + self.value = @address + end + + # @macro seeAbstractWidget + def opt + [:hstretch] + end + + # @macro seeAbstractWidget + def label + _("Syncronization server") + end + + # @macro seeItemsSelection + def items + @servers.map { |s| [s, s] } + end + + # @macro seeAbstractWidget + def help + _("

List of available ntp servers provided by DHCP. " \ + "Servers already in use are discarded.

") + end + + private + + # List of available ntp servers provided by DHCP. Servers already in use + # are discarded. + # + # @return [Array] list of ntp servers provided by dhcp + def ntp_servers + dhcp_ntp_servers.reject { |s| Yast::NtpClient.ntp_conf.pools.keys.include?(s) } + end + end + + # List of public ntp servers filtered by country + class PublicList < CWM::CustomWidget + # Constructor + # + # @param address [String] current ntp pool address + def initialize(address) + textdomain "ntp-client" + @country_pools = CountryPools.new + @country = Country.new(country_for(address), @country_pools) + end + + # @macro seeAbstractWidget + def label + _("Public Servers") + end + + # @macro seeCustomWidget + def contents + VBox( + @country, + VSpacing(), + @country_pools + ) + end + + def value + @country_pools.value + end + + private + + def country_for(address) + Yast::NtpClient.GetNtpServers.fetch(address, {})["country"] + end + end + + # Country chooser + class Country < CWM::ComboBox + def initialize(country, country_pools) + textdomain "ntp-client" + + @country = country + @country_pools = country_pools + end + + # @macro seeAbstractWidget + def init + self.value = @country + refresh_country_pools + end + + # @macro seeAbstractWidget + def opt + [:notify, :hstretch] + end + + # @macro seeAbstractWidget + def label + _("Country") + end + + # @macro seeItemsSelection + def items + country_names.map { |c, l| [c, l] } + end + + # @macro seeAbstractWidget + def handle + @country = value.to_s + refresh_country_pools + nil + end + + private + + def refresh_country_pools + @country_pools.refresh(@country) + end + + def country_names + { "" => _("ALL") }.merge(Yast::NtpClient.GetCountryNames) + end + end + + # Ntp sources selector filtered by country + class CountryPools < CWM::ComboBox + def initialize + textdomain "ntp-client" + end + + # macro seeAbstractWidget + def opt + [:hstretch] + end + + # macro seeAbstractWidget + def label + _("Ntp Servers") + end + + # macro seeItemsSelection + def items + ntp_servers.map { |s, v| [s, "#{s} (#{v["country"]})"] } + end + + def refresh(country) + @country = country + change_items(items) + end + + private + + def ntp_servers + servers = Yast::NtpClient.GetNtpServers + return servers if @country.to_s.empty? + servers.find_all { |_s, v| v["country"] == @country } end end end diff --git a/src/modules/NtpClient.rb b/src/modules/NtpClient.rb index 8aa3f0ee..0bd2d859 100644 --- a/src/modules/NtpClient.rb +++ b/src/modules/NtpClient.rb @@ -12,6 +12,7 @@ require "yast" require "yaml" require "cfa/chrony_conf" +require "y2ntp_client/dynamic_servers" require "yast2/target_file" # required to cfa work on changed scr require "ui/text_helpers" @@ -19,6 +20,7 @@ module Yast class NtpClientClass < Module include Logger include ::UI::TextHelpers + include ::Y2NtpClient::DynamicServers # the default synchronization interval in minutes when running in the manual # sync mode ("Synchronize without Daemon" option, ntp started from cron) @@ -95,8 +97,8 @@ def main @service_name = "chronyd" # Netconfig policy: for merging and prioritizing static and DHCP config. - # FIXME: get a public URL - # https://svn.suse.de/svn/sysconfig/branches/mt/dhcp6-netconfig/netconfig/doc/README + # https://github.com/openSUSE/sysconfig/blob/master/doc/README.netconfig + # https://github.com/openSUSE/sysconfig/blob/master/config/sysconfig.config-network @ntp_policy = DEFAULT_NTP_POLICY # Active Directory controller @@ -192,7 +194,7 @@ def GetNtpServers deep_copy(@ntp_servers) end - # Get the mapping between country codea and names ("CZ" -> "Czech Republic") + # Get the mapping between country codes and names ("CZ" -> "Czech Republic") # @return a map the country codes and names mapping def GetCountryNames if @country_names.nil? diff --git a/test/y2ntp_client/dynamic_servers_test.rb b/test/y2ntp_client/dynamic_servers_test.rb new file mode 100755 index 00000000..3a59ea5c --- /dev/null +++ b/test/y2ntp_client/dynamic_servers_test.rb @@ -0,0 +1,39 @@ +#! /usr/bin/env rspec +require_relative "../test_helper" + +require "cwm/rspec" +require "y2ntp_client/dynamic_servers" + +Yast.import "Lan" +Yast.import "LanItems" + +class Dummy + include Y2NtpClient::DynamicServers +end + +describe Y2NtpClient::DynamicServers do + let(:subject) { Dummy.new } + let(:servers) do + { + "eth0" => ["0.pool.ntp.org", "1.pool.ntp.org"], + "eth1" => ["2.pool.ntp.org"] + } + end + + before do + allow(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return(servers) + end + + describe "#dhcp_ntp_servers" do + it "reads the current network configuration" do + expect(Yast::Lan).to receive(:ReadWithCacheNoGUI) + subject.dhcp_ntp_servers + end + + it "returns a list of the ntp_servers provided by dhcp " do + expect(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return(servers) + expect(subject.dhcp_ntp_servers) + .to eql(["0.pool.ntp.org", "1.pool.ntp.org", "2.pool.ntp.org"]) + end + end +end diff --git a/test/y2ntp_client/widgets/pool_widgets_test.rb b/test/y2ntp_client/widgets/pool_widgets_test.rb old mode 100644 new mode 100755 index 89441ec3..6a19d9e2 --- a/test/y2ntp_client/widgets/pool_widgets_test.rb +++ b/test/y2ntp_client/widgets/pool_widgets_test.rb @@ -1,3 +1,4 @@ +#! /usr/bin/env rspec require_relative "../../test_helper" require "cwm/rspec" @@ -26,3 +27,15 @@ include_examples "CWM::CheckBox" end + +describe Y2NtpClient::Widgets::LocalList do + subject { described_class.new({}) } + + include_examples "CWM::ComboBox" +end + +describe Y2NtpClient::Widgets::PublicList do + subject { described_class.new({}) } + + include_examples "CWM::CustomWidget" +end From 90bd489a551f5cca002cfe0a242855c9eda22845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Fri, 9 Nov 2018 08:37:09 +0000 Subject: [PATCH 2/8] Do not synchronize if the service is active --- src/clients/ntp-client_proposal.rb | 49 +++++++++++++------- src/lib/y2ntp_client/dialog/add_pool.rb | 1 - src/lib/y2ntp_client/widgets/pool_widgets.rb | 6 +++ test/ntp_client_proposal_test.rb | 10 ++++ 4 files changed, 49 insertions(+), 17 deletions(-) mode change 100644 => 100755 test/ntp_client_proposal_test.rb diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index 911d8d7b..c671baca 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -61,6 +61,8 @@ def main when "SetUseNTP" NtpClient.ntp_selected = Ops.get_boolean(@param, "ntp_used", false) @ret = true + when "dhcp_ntp_servers" + @ret = NtpClient.dhcp_ntp_servers when "MakeProposal" @ret = MakeProposal() when "Write" @@ -128,7 +130,10 @@ def ui_help_text def ui_enable_disable_widgets(enabled) UI.ChangeWidget(Id(:ntp_address), :Enabled, enabled) UI.ChangeWidget(Id(:run_service), :Enabled, enabled) - if !NetworkService.isNetworkRunning + # FIXME: With chronyd, we cannot synchronize if the service is already + # running, we could force a makestep in this case, but then the button + # should be reworded and maybe the user should confirm it (bsc#1087048) + if !NetworkService.isNetworkRunning || Service.Active(NtpClient.service_name) UI.ChangeWidget(Id(:ntp_now), :Enabled, false) else UI.ChangeWidget(Id(:ntp_now), :Enabled, enabled) @@ -187,17 +192,12 @@ def MakeProposal log.info("ntp_items will be filled from /etc/chrony.conf") # grr, GUNS means all of them are used and here we just pick one ntp_items = configured_ntp_items - - # avoid calling Read again (bnc #427712) - NtpClient.config_has_been_read = true end - if ntp_items.empty? - log.info("Nothing found in /etc/chrony.conf, " \ - "proposing current timezone-based NTP server list") - ntp_items = dhcp_ntp_items.empty? ? timezone_ntp_items : dhcp_ntp_items - NtpClient.config_has_been_read = true - end + ntp_items = fallback_ntp_items if ntp_items.empty? + # Once read or proposed any config we consider it as read (bnc#427712) + NtpClient.config_has_been_read = true + ntp_items = Builtins.add(ntp_items, "") Builtins.y2milestone("ntp_items :%1", ntp_items) UI.ChangeWidget(Id(:ntp_address), :Items, ntp_items) @@ -358,7 +358,7 @@ def Write(params) return :success if params["write_only"] # Only if network is running try to synchronize the ntp server - if NetworkService.isNetworkRunning + if NetworkService.isNetworkRunning && !Service.Active(NtpClient.service_name) Popup.ShowFeedback("", _("Synchronizing with NTP server...")) exit_code = NtpClient.sync_once(ntp_server) Popup.ClearFeedback @@ -474,15 +474,15 @@ def add_or_install_required_package end # Configured ntp servers Yast::Term items with the ntp address as the ID - # and label + # and as the label # # @return [Yast::Term] ntp address table Item def configured_ntp_items - NtpClient.GetUsedNtpServers + NtpClient.GetUsedNtpServers.map { |s| Item(Id(s), s) } end # Public list of ntp servers Yast::Term items with the ntp address as the - # ID and label + # ID and as the label # # @return [Array] ntp address Item def timezone_ntp_items @@ -490,12 +490,29 @@ def timezone_ntp_items NtpClient.GetNtpServersByCountry(timezone_country, true) end + # List of dhcp ntp servers Yast::Term items with the ntp address as the + # ID and as the label + # + # @return [Array] ntp address table Item + def dhcp_ntp_items + Yast::Lan.dhcp_ntp_servers.map { |s| Item(Id(s), s) } + end + # List of ntp servers Yast::Term items with the ntp address as the ID and # label # # @return [Array] ntp address table Item - def dhcp_ntp_items - NtpClient.dhcp_ntp_servers + def fallback_ntp_items + dhcp_items = dhcp_ntp_items + + log.info("Nothing found in /etc/chrony.conf") + unless dhcp_items.empty? + log.info("Proposing current timezone-based NTP server list") + return timezone_ntp_items + end + + log.info("Proposing NTP server list provided by DHCP") + dhcp_items end end end diff --git a/src/lib/y2ntp_client/dialog/add_pool.rb b/src/lib/y2ntp_client/dialog/add_pool.rb index ee3f46c6..9aef8a54 100644 --- a/src/lib/y2ntp_client/dialog/add_pool.rb +++ b/src/lib/y2ntp_client/dialog/add_pool.rb @@ -79,7 +79,6 @@ def pool_for(type) public: Widgets::PublicList }.fetch(type, Widgets::LocalList).new(@address_widget.value) end - # @macro see def min_height 8 end diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index f0875015..4d23b5c4 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -251,10 +251,16 @@ def contents ) end + # The value of this widget is the current country pool entry selected def value @country_pools.value end + # @macro seeAbstractWidget + def help + _("

List of public ntp servers filtered by Country.") + end + private def country_for(address) diff --git a/test/ntp_client_proposal_test.rb b/test/ntp_client_proposal_test.rb old mode 100644 new mode 100755 index afc82f83..d829bf0d --- a/test/ntp_client_proposal_test.rb +++ b/test/ntp_client_proposal_test.rb @@ -1,3 +1,5 @@ +#! /usr/bin/env rspec + require_relative "test_helper" require_relative "../src/clients/ntp-client_proposal.rb" @@ -33,6 +35,7 @@ allow(Yast::PackageSystem).to receive(:CheckAndInstallPackages) allow(Yast::Report).to receive(:Error) allow(Yast::NetworkService).to receive(:isNetworkRunning).and_return(network_running) + allow(Yast::Service).to receive(:Active).with(ntp_client.service_name).and_return(false) end context "with a not valid hostname" do @@ -155,6 +158,13 @@ subject.Write(params) end + + it "does not try to synchronize if the service is running" do + allow(Yast::Service).to receive(:Active).with(ntp_client.service_name).and_return(true) + expect(Yast::NtpClient).to_not receive(:sync_once) + + subject.Write(params) + end end context "and user wants to syncronize on boot" do From e3bfea4c2ddd4411523e298ff24e895300660972 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 21:45:23 +0000 Subject: [PATCH 3/8] Removed dynamic servers once the method was moved to Yast::Lan --- src/clients/ntp-client_proposal.rb | 5 ++- src/lib/y2ntp_client/dialog/add_pool.rb | 4 ++ src/lib/y2ntp_client/dynamic_servers.rb | 22 ----------- src/lib/y2ntp_client/widgets/pool_widgets.rb | 19 +++++---- src/modules/NtpClient.rb | 2 - test/y2ntp_client/dynamic_servers_test.rb | 39 ------------------- .../y2ntp_client/widgets/pool_widgets_test.rb | 8 +++- 7 files changed, 25 insertions(+), 74 deletions(-) delete mode 100644 src/lib/y2ntp_client/dynamic_servers.rb delete mode 100755 test/y2ntp_client/dynamic_servers_test.rb diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index c671baca..2d11e3e1 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -13,6 +13,7 @@ def main textdomain "ntp-client" Yast.import "Address" + Yast.import "Lan" Yast.import "NetworkService" Yast.import "NtpClient" Yast.import "Service" @@ -62,7 +63,7 @@ def main NtpClient.ntp_selected = Ops.get_boolean(@param, "ntp_used", false) @ret = true when "dhcp_ntp_servers" - @ret = NtpClient.dhcp_ntp_servers + @ret = Yast::Lan.dhcp_ntp_servers when "MakeProposal" @ret = MakeProposal() when "Write" @@ -506,7 +507,7 @@ def fallback_ntp_items dhcp_items = dhcp_ntp_items log.info("Nothing found in /etc/chrony.conf") - unless dhcp_items.empty? + if dhcp_items.empty? log.info("Proposing current timezone-based NTP server list") return timezone_ntp_items end diff --git a/src/lib/y2ntp_client/dialog/add_pool.rb b/src/lib/y2ntp_client/dialog/add_pool.rb index 9aef8a54..549f905d 100644 --- a/src/lib/y2ntp_client/dialog/add_pool.rb +++ b/src/lib/y2ntp_client/dialog/add_pool.rb @@ -82,6 +82,10 @@ def pool_for(type) def min_height 8 end + + def buttons + [ok_button, cancel_button] + end end end end diff --git a/src/lib/y2ntp_client/dynamic_servers.rb b/src/lib/y2ntp_client/dynamic_servers.rb deleted file mode 100644 index e01b54ed..00000000 --- a/src/lib/y2ntp_client/dynamic_servers.rb +++ /dev/null @@ -1,22 +0,0 @@ -require "yast" - -module Y2NtpClient - # Bunch of methods for retrieving ntp servers dinamically, i.e. by dhcp or - # slp. - module DynamicServers - def dhcp_ntp_servers - Yast.import "Lan" - Yast.import "LanItems" - Yast.import "NetworkService" - - # When proposing NTP servers we need to know - # 1) list of (dhcp) interfaces - # 2) network service in use - # We can either use networking submodule for network service handling and get list of - # interfaces e.g. using a bash command or initialize whole networking module. - Yast::Lan.ReadWithCacheNoGUI - - Yast::LanItems.dhcp_ntp_servers.values.reduce(&:concat) || [] - end - end -end diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index 4d23b5c4..361e1962 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -2,12 +2,11 @@ require "cwm/widget" require "y2ntp_client/dialog/add_pool" -require "y2ntp_client/dynamic_servers" Yast.import "Address" Yast.import "NtpClient" Yast.import "Popup" -Yast.import "NetworkService" +Yast.import "Lan" module Y2NtpClient module Widgets @@ -172,25 +171,29 @@ def cwm_definition super.merge(additional) end + + def help + _("

Select permits to choose a server from the list of servers" \ + "offered by DHCP or from a public list filtered by country.

") \ + end end # List of ntp servers obtained from DHCP class LocalList < CWM::SelectionBox - include DynamicServers # Constructor # # @param address [String] current ntp pool address def initialize(address) textdomain "ntp-client" @address = address - - Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do - @servers = ntp_servers - end + @servers = [] end # @macro seeAbstractWidget def init + Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do + @servers = ntp_servers + end self.value = @address end @@ -222,7 +225,7 @@ def help # # @return [Array] list of ntp servers provided by dhcp def ntp_servers - dhcp_ntp_servers.reject { |s| Yast::NtpClient.ntp_conf.pools.keys.include?(s) } + Yast::Lan.dhcp_ntp_servers.reject { |s| Yast::NtpClient.ntp_conf.pools.keys.include?(s) } end end diff --git a/src/modules/NtpClient.rb b/src/modules/NtpClient.rb index 0bd2d859..f05f8b71 100644 --- a/src/modules/NtpClient.rb +++ b/src/modules/NtpClient.rb @@ -12,7 +12,6 @@ require "yast" require "yaml" require "cfa/chrony_conf" -require "y2ntp_client/dynamic_servers" require "yast2/target_file" # required to cfa work on changed scr require "ui/text_helpers" @@ -20,7 +19,6 @@ module Yast class NtpClientClass < Module include Logger include ::UI::TextHelpers - include ::Y2NtpClient::DynamicServers # the default synchronization interval in minutes when running in the manual # sync mode ("Synchronize without Daemon" option, ntp started from cron) diff --git a/test/y2ntp_client/dynamic_servers_test.rb b/test/y2ntp_client/dynamic_servers_test.rb deleted file mode 100755 index 3a59ea5c..00000000 --- a/test/y2ntp_client/dynamic_servers_test.rb +++ /dev/null @@ -1,39 +0,0 @@ -#! /usr/bin/env rspec -require_relative "../test_helper" - -require "cwm/rspec" -require "y2ntp_client/dynamic_servers" - -Yast.import "Lan" -Yast.import "LanItems" - -class Dummy - include Y2NtpClient::DynamicServers -end - -describe Y2NtpClient::DynamicServers do - let(:subject) { Dummy.new } - let(:servers) do - { - "eth0" => ["0.pool.ntp.org", "1.pool.ntp.org"], - "eth1" => ["2.pool.ntp.org"] - } - end - - before do - allow(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return(servers) - end - - describe "#dhcp_ntp_servers" do - it "reads the current network configuration" do - expect(Yast::Lan).to receive(:ReadWithCacheNoGUI) - subject.dhcp_ntp_servers - end - - it "returns a list of the ntp_servers provided by dhcp " do - expect(Yast::LanItems).to receive(:dhcp_ntp_servers).and_return(servers) - expect(subject.dhcp_ntp_servers) - .to eql(["0.pool.ntp.org", "1.pool.ntp.org", "2.pool.ntp.org"]) - end - end -end diff --git a/test/y2ntp_client/widgets/pool_widgets_test.rb b/test/y2ntp_client/widgets/pool_widgets_test.rb index 6a19d9e2..0796792c 100755 --- a/test/y2ntp_client/widgets/pool_widgets_test.rb +++ b/test/y2ntp_client/widgets/pool_widgets_test.rb @@ -22,7 +22,7 @@ include_examples "CWM::CheckBox" end -describe Y2NtpClient::Widgets::Iburst do +describe Y2NtpClient::Widgets::Offline do subject { described_class.new({}) } include_examples "CWM::CheckBox" @@ -39,3 +39,9 @@ include_examples "CWM::CustomWidget" end + +describe Y2NtpClient::Widgets::SelectFrom do + subject { described_class.new({}) } + + include_examples "CWM::AbstractWidget" +end From 0bea7e6688370c38bc637f940fa67221a8bad06c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 21:49:19 +0000 Subject: [PATCH 4/8] Changelog & version bump. --- package/yast2-ntp-client.changes | 11 +++++++++++ package/yast2-ntp-client.spec | 6 ++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/package/yast2-ntp-client.changes b/package/yast2-ntp-client.changes index 2dd1b394..9a48d1f2 100644 --- a/package/yast2-ntp-client.changes +++ b/package/yast2-ntp-client.changes @@ -1,3 +1,14 @@ +------------------------------------------------------------------- +Thu Nov 8 19:20:26 UTC 2018 - knut.andersse@suse.com + +- fate#323454 + - Bring back the menu button for choosing a ntp address from a + public ntp servers list or from the ones returned by dhcp. + - When no configuration is proposed by default, then use the dhcp + offered servers as fallback. +- Do not synchronize if chronyd service is running (bsc#1087048) +- 4.0.15 + ------------------------------------------------------------------- Thu Nov 8 15:20:26 UTC 2018 - knut.andersse@suse.com diff --git a/package/yast2-ntp-client.spec b/package/yast2-ntp-client.spec index 7809e82e..3588e43b 100644 --- a/package/yast2-ntp-client.spec +++ b/package/yast2-ntp-client.spec @@ -17,7 +17,7 @@ Name: yast2-ntp-client -Version: 4.0.14 +Version: 4.0.15 Release: 0 Summary: YaST2 - NTP Client Configuration License: GPL-2.0+ @@ -31,6 +31,7 @@ BuildRequires: update-desktop-files BuildRequires: yast2 >= 3.2.21 BuildRequires: yast2-country-data BuildRequires: yast2-devtools >= 3.1.10 +BuildRequires: yast2-network >= 4.0.44 BuildRequires: rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0 BuildRequires: rubygem(%rb_default_ruby_abi:rspec) BuildRequires: rubygem(%rb_default_ruby_abi:yast-rake) @@ -40,7 +41,8 @@ Requires: augeas-lenses Requires: yast2 >= 3.2.21 Requires: yast2-country-data # needed for network/config agent -Requires: yast2-network +# Yast::Lan.dhcp_ntp_servers +Requires: yast2-network >= 4.0.44 Requires: yast2-ruby-bindings >= 1.0.0 Requires: rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0 BuildArch: noarch From 2f45c7db69548b2aadd95409757d645d7c1339b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Tue, 13 Nov 2018 11:28:38 +0000 Subject: [PATCH 5/8] Added convenience method to obtain dhcp_ntp_servers --- src/clients/ntp-client_proposal.rb | 5 ++-- src/lib/y2ntp_client/widgets/pool_widgets.rb | 24 ++++++++++++++------ src/modules/NtpClient.rb | 7 ++++++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index 2d11e3e1..06af1687 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -13,7 +13,6 @@ def main textdomain "ntp-client" Yast.import "Address" - Yast.import "Lan" Yast.import "NetworkService" Yast.import "NtpClient" Yast.import "Service" @@ -63,7 +62,7 @@ def main NtpClient.ntp_selected = Ops.get_boolean(@param, "ntp_used", false) @ret = true when "dhcp_ntp_servers" - @ret = Yast::Lan.dhcp_ntp_servers + @ret = NtpClient.dhcp_ntp_servers when "MakeProposal" @ret = MakeProposal() when "Write" @@ -496,7 +495,7 @@ def timezone_ntp_items # # @return [Array] ntp address table Item def dhcp_ntp_items - Yast::Lan.dhcp_ntp_servers.map { |s| Item(Id(s), s) } + NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s), s) } end # List of ntp servers Yast::Term items with the ntp address as the ID and diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index 361e1962..7779dbab 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -6,7 +6,6 @@ Yast.import "Address" Yast.import "NtpClient" Yast.import "Popup" -Yast.import "Lan" module Y2NtpClient module Widgets @@ -130,7 +129,6 @@ def help # address input field. class SelectFrom < CWM::MenuButton def initialize(address_widget) - Yast.import "Popup" @address_widget = address_widget end @@ -191,9 +189,7 @@ def initialize(address) # @macro seeAbstractWidget def init - Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do - @servers = ntp_servers - end + read_available_servers self.value = @address end @@ -220,12 +216,26 @@ def help private + # Convenience method to read and initialize the list of available servers + def read_available_servers + Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do + @servers = available_servers + end + end + # List of available ntp servers provided by DHCP. Servers already in use # are discarded. # # @return [Array] list of ntp servers provided by dhcp - def ntp_servers - Yast::Lan.dhcp_ntp_servers.reject { |s| Yast::NtpClient.ntp_conf.pools.keys.include?(s) } + def available_servers + Yast::NtpClient.dhcp_ntp_servers.reject { |s| configured_servers.include?(s) } + end + + # List of ntp servers in use. + # + # @return [Array] list of already configured ntp servers + def configured_servers + Yast::NtpClient.ntp_conf.pools.keys end end diff --git a/src/modules/NtpClient.rb b/src/modules/NtpClient.rb index f05f8b71..a0be0faf 100644 --- a/src/modules/NtpClient.rb +++ b/src/modules/NtpClient.rb @@ -56,6 +56,7 @@ def main Yast.import "Directory" Yast.import "FileUtils" + Yast.import "Lan" Yast.import "Language" Yast.import "Message" Yast.import "Mode" @@ -580,6 +581,12 @@ def AutoPackages { "install" => @required_packages, "remove" => [] } end + # Convenience method to obtain the list of ntp servers proposed by DHCP + # @see https://www.rubydoc.info/github/yast/yast-network/Yast/LanClass:${0} + def dhcp_ntp_servers + Yast::Lan.dhcp_ntp_servers + end + publish variable: :AbortFunction, type: "boolean ()" publish variable: :modified, type: "boolean" publish variable: :write_only, type: "boolean" From 5eed304f2158b8fb2e897601c7fd5780bbc860e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Tue, 13 Nov 2018 13:09:30 +0000 Subject: [PATCH 6/8] Changes based on code review. --- src/clients/ntp-client_proposal.rb | 18 ++++++------ src/lib/y2ntp_client/dialog/add_pool.rb | 8 ++++-- src/lib/y2ntp_client/widgets/main_widgets.rb | 15 +++++----- src/lib/y2ntp_client/widgets/pool_widgets.rb | 29 ++++++++++++++------ 4 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index 06af1687..8f6c7119 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -198,8 +198,8 @@ def MakeProposal # Once read or proposed any config we consider it as read (bnc#427712) NtpClient.config_has_been_read = true - ntp_items = Builtins.add(ntp_items, "") - Builtins.y2milestone("ntp_items :%1", ntp_items) + ntp_items << "" + log.info "ntp_items :#{ntp_items}" UI.ChangeWidget(Id(:ntp_address), :Items, ntp_items) nil @@ -473,16 +473,15 @@ def add_or_install_required_package end end - # Configured ntp servers Yast::Term items with the ntp address as the ID - # and as the label + # Configured ntp servers Yast::Term items with the ntp address ID and label # # @return [Yast::Term] ntp address table Item def configured_ntp_items NtpClient.GetUsedNtpServers.map { |s| Item(Id(s), s) } end - # Public list of ntp servers Yast::Term items with the ntp address as the - # ID and as the label + # Public list of ntp servers Yast::Term items with the ntp address ID and + # label # # @return [Array] ntp address Item def timezone_ntp_items @@ -490,16 +489,15 @@ def timezone_ntp_items NtpClient.GetNtpServersByCountry(timezone_country, true) end - # List of dhcp ntp servers Yast::Term items with the ntp address as the - # ID and as the label + # List of dhcp ntp servers Yast::Term items with the ntp address ID and + # label # # @return [Array] ntp address table Item def dhcp_ntp_items NtpClient.dhcp_ntp_servers.map { |s| Item(Id(s), s) } end - # List of ntp servers Yast::Term items with the ntp address as the ID and - # label + # List of ntp servers Yast::Term items with the ntp address ID and label # # @return [Array] ntp address table Item def fallback_ntp_items diff --git a/src/lib/y2ntp_client/dialog/add_pool.rb b/src/lib/y2ntp_client/dialog/add_pool.rb index 549f905d..ef7ba095 100644 --- a/src/lib/y2ntp_client/dialog/add_pool.rb +++ b/src/lib/y2ntp_client/dialog/add_pool.rb @@ -37,6 +37,7 @@ def initialize(address_widget, pool_type) # @macro seeDialog def title + # TRANSLATORS: dialog title _("Local ntp servers discovered") end @@ -74,9 +75,12 @@ def ok_button PushButton(Id(:next), Opt(:default), ok_button_label) end + def available_pools + { local: Widgets::LocalList, public: Widgets::PublicList } + end + def pool_for(type) - { local: Widgets::LocalList, - public: Widgets::PublicList }.fetch(type, Widgets::LocalList).new(@address_widget.value) + available_pools.fetch(type, Widgets::LocalList).new(@address_widget.value) end def min_height diff --git a/src/lib/y2ntp_client/widgets/main_widgets.rb b/src/lib/y2ntp_client/widgets/main_widgets.rb index 3cc4f793..28f3e8dc 100644 --- a/src/lib/y2ntp_client/widgets/main_widgets.rb +++ b/src/lib/y2ntp_client/widgets/main_widgets.rb @@ -24,13 +24,14 @@ def label end def help - # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that - _( - "

The NTP configuration may be provided by the local network over DHCP. " \ - "Configuration Source can simply enable or disable using that configuration. " \ - "In cases where there may be multiple DHCP sources, it can prioritize them: " \ - "see 'man 8 netconfig'.

" - ) + "

" + + # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that + _( + "

The NTP configuration may be provided by the local network over DHCP. " \ + "Configuration Source can simply enable or disable using that configuration. " \ + "In cases where there may be multiple DHCP sources, it can prioritize them: " \ + "see 'man 8 netconfig'.

" + ) + "

" end def opt diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index 7779dbab..fce745a6 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -21,6 +21,7 @@ def initialize(initial_value) # @macro seeAbstractWidget def label + # TRANSLATORS: widget label _("A&ddress") end @@ -52,6 +53,7 @@ def initialize(address_widget) end def label + # TRANSLATORS: widget label _("Test") end @@ -70,6 +72,7 @@ def initialize(options) end def label + # TRANSLATORS: widget label _("Quick Initial Sync") end @@ -89,8 +92,7 @@ def help "

" + _("Quick Initial Sync specifies whether the 'iburst' option is used. This " \ "option sends 4 poll requests in 2 second intervals during the initialization. It is " \ - "useful for a quick synchronization during the start of the machine.") + - "

" + "useful for a quick synchronization during the start of the machine.") + "

" end end @@ -102,6 +104,7 @@ def initialize(options) end def label + # TRANSLATORS: widget label _("Start Offline") end @@ -118,10 +121,11 @@ def store end def help - _("

Start Offline specifies whether the 'offline' option is used. This option " \ - "skips this server during the start-up. It is useful for a machine which starts " \ - "without the network, because it speeds up the boot, and synchronizes when the machine " \ - "gets connected to the network.

") + "

" + + _("Start Offline specifies whether the 'offline' option is used. This option " \ + "skips this server during the start-up. It is useful for a machine which starts " \ + "without the network, because it speeds up the boot, and synchronizes when the " \ + "machine gets connected to the network.") + "

" end end @@ -134,6 +138,7 @@ def initialize(address_widget) # @macro seeAbstractWidget def label + # TRANSLATORS: widget label _("Select") end @@ -160,8 +165,7 @@ def handle(event) # @macro seeAbstractWidget def cwm_definition additional = {} - # handle_events are by default widget_id, but in radio buttons, events are - # in fact single RadioButton + # handle also the items id events if !handle_all_events event_ids = items.map(&:first) additional["handle_events"] = event_ids @@ -170,6 +174,7 @@ def cwm_definition super.merge(additional) end + # @macro seeAbstractWidget def help _("

Select permits to choose a server from the list of servers" \ "offered by DHCP or from a public list filtered by country.

") \ @@ -183,6 +188,7 @@ class LocalList < CWM::SelectionBox # @param address [String] current ntp pool address def initialize(address) textdomain "ntp-client" + @address = address @servers = [] end @@ -200,6 +206,7 @@ def opt # @macro seeAbstractWidget def label + # TRANSLATORS: widget label _("Syncronization server") end @@ -246,12 +253,14 @@ class PublicList < CWM::CustomWidget # @param address [String] current ntp pool address def initialize(address) textdomain "ntp-client" + @country_pools = CountryPools.new @country = Country.new(country_for(address), @country_pools) end # @macro seeAbstractWidget def label + # TRANSLATORS: widget label _("Public Servers") end @@ -271,7 +280,7 @@ def value # @macro seeAbstractWidget def help - _("

List of public ntp servers filtered by Country.") + _("

List of public ntp servers filtered by country.

") end private @@ -303,6 +312,7 @@ def opt # @macro seeAbstractWidget def label + # TRANSLATORS: widget label _("Country") end @@ -342,6 +352,7 @@ def opt # macro seeAbstractWidget def label + # TRANSLATORS: widget label _("Ntp Servers") end From 407a2dafd0c769b3c8505a3bc217fe5de0990105 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Wed, 14 Nov 2018 09:17:52 +0000 Subject: [PATCH 7/8] Changes based on CR. --- package/yast2-ntp-client.changes | 6 +- package/yast2-ntp-client.spec | 1 - src/clients/ntp-client_proposal.rb | 1 - src/lib/y2ntp_client/dialog/add_pool.rb | 5 +- src/lib/y2ntp_client/widgets/main_widgets.rb | 17 ++-- src/lib/y2ntp_client/widgets/pool_widgets.rb | 85 ++++++++++++-------- test/test_helper.rb | 9 +++ 7 files changed, 72 insertions(+), 52 deletions(-) diff --git a/package/yast2-ntp-client.changes b/package/yast2-ntp-client.changes index 9a48d1f2..4054465c 100644 --- a/package/yast2-ntp-client.changes +++ b/package/yast2-ntp-client.changes @@ -2,9 +2,9 @@ Thu Nov 8 19:20:26 UTC 2018 - knut.andersse@suse.com - fate#323454 - - Bring back the menu button for choosing a ntp address from a - public ntp servers list or from the ones returned by dhcp. - - When no configuration is proposed by default, then use the dhcp + - Bring back the menu button for choosing an NTP address from a + public NTP servers list or from the ones returned by DHCP. + - When no configuration is proposed by default, then use the DHCP offered servers as fallback. - Do not synchronize if chronyd service is running (bsc#1087048) - 4.0.15 diff --git a/package/yast2-ntp-client.spec b/package/yast2-ntp-client.spec index 3588e43b..a10a1ba6 100644 --- a/package/yast2-ntp-client.spec +++ b/package/yast2-ntp-client.spec @@ -31,7 +31,6 @@ BuildRequires: update-desktop-files BuildRequires: yast2 >= 3.2.21 BuildRequires: yast2-country-data BuildRequires: yast2-devtools >= 3.1.10 -BuildRequires: yast2-network >= 4.0.44 BuildRequires: rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0 BuildRequires: rubygem(%rb_default_ruby_abi:rspec) BuildRequires: rubygem(%rb_default_ruby_abi:yast-rake) diff --git a/src/clients/ntp-client_proposal.rb b/src/clients/ntp-client_proposal.rb index 8f6c7119..2b8f14bf 100644 --- a/src/clients/ntp-client_proposal.rb +++ b/src/clients/ntp-client_proposal.rb @@ -198,7 +198,6 @@ def MakeProposal # Once read or proposed any config we consider it as read (bnc#427712) NtpClient.config_has_been_read = true - ntp_items << "" log.info "ntp_items :#{ntp_items}" UI.ChangeWidget(Id(:ntp_address), :Items, ntp_items) diff --git a/src/lib/y2ntp_client/dialog/add_pool.rb b/src/lib/y2ntp_client/dialog/add_pool.rb index ef7ba095..baaf9729 100644 --- a/src/lib/y2ntp_client/dialog/add_pool.rb +++ b/src/lib/y2ntp_client/dialog/add_pool.rb @@ -30,6 +30,7 @@ class AddPool < ::CWM::Popup # @param pool_type [Symbol] def initialize(address_widget, pool_type) textdomain "ntp-client" + @address_widget = address_widget @address = @address_widget.value @pool_chooser = pool_for(pool_type) @@ -37,8 +38,8 @@ def initialize(address_widget, pool_type) # @macro seeDialog def title - # TRANSLATORS: dialog title - _("Local ntp servers discovered") + # TRANSLATORS: title for choosing a ntp server dialog + _("Available NTP servers") end # @macro seeDialog diff --git a/src/lib/y2ntp_client/widgets/main_widgets.rb b/src/lib/y2ntp_client/widgets/main_widgets.rb index 28f3e8dc..1f0b1b81 100644 --- a/src/lib/y2ntp_client/widgets/main_widgets.rb +++ b/src/lib/y2ntp_client/widgets/main_widgets.rb @@ -24,14 +24,11 @@ def label end def help - "

" + - # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that - _( - "

The NTP configuration may be provided by the local network over DHCP. " \ - "Configuration Source can simply enable or disable using that configuration. " \ - "In cases where there may be multiple DHCP sources, it can prioritize them: " \ - "see 'man 8 netconfig'.

" - ) + "

" + # TRANSLATORS: configuration source combo box help + _("

The NTP configuration may be provided by the local network over DHCP. " \ + "Configuration Source can simply enable or disable using that configuration. " \ + "In cases where there may be multiple DHCP sources, it can prioritize them: " \ + "see '%{manual}'.

") % { manual: "man 8 netconfig" } end def opt @@ -40,9 +37,9 @@ def opt def items items = [ - # combo box item + # TRANSLATORS: combo box item ["", _("Static")], - # combo box item + # TRANSLATORS: combo box item ["auto", _("Dynamic")] ] current_policy = Yast::NtpClient.ntp_policy diff --git a/src/lib/y2ntp_client/widgets/pool_widgets.rb b/src/lib/y2ntp_client/widgets/pool_widgets.rb index fce745a6..68474595 100644 --- a/src/lib/y2ntp_client/widgets/pool_widgets.rb +++ b/src/lib/y2ntp_client/widgets/pool_widgets.rb @@ -16,12 +16,13 @@ class PoolAddress < CWM::InputField # @macro seeAbstractWidget def initialize(initial_value) textdomain "ntp-client" + @address = initial_value end # @macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: input field label _("A&ddress") end @@ -49,12 +50,13 @@ def store class TestButton < CWM::PushButton def initialize(address_widget) textdomain "ntp-client" + @address_widget = address_widget end def label - # TRANSLATORS: widget label - _("Test") + # TRANSLATORS: push button label + _("&Test") end def handle @@ -68,11 +70,12 @@ def handle class Iburst < CWM::CheckBox def initialize(options) textdomain "ntp-client" + @options = options end def label - # TRANSLATORS: widget label + # TRANSLATORS: checkbox label _("Quick Initial Sync") end @@ -89,29 +92,34 @@ def store end def help - "

" + - _("Quick Initial Sync specifies whether the 'iburst' option is used. This " \ - "option sends 4 poll requests in 2 second intervals during the initialization. It is " \ - "useful for a quick synchronization during the start of the machine.") + "

" + # TRANSLATORS: checkbox help for enabling quick synchronization + _("

Quick Initial Sync is useful for a quick synchronization" \ + "during the start of the machine.

") end end # Enable offline option class Offline < CWM::CheckBox + # Constructor + # + # @param options [Hash] current ntp server address options def initialize(options) textdomain "ntp-client" @options = options end + # @macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: check box label _("Start Offline") end + # @macro seeAbstractWidget def init self.value = @options.key?("offline") end + # @macro seeAbstractWidget def store if value @options["offline"] = nil @@ -120,25 +128,27 @@ def store end end + # @macro seeAbstractWidget def help - "

" + - _("Start Offline specifies whether the 'offline' option is used. This option " \ - "skips this server during the start-up. It is useful for a machine which starts " \ - "without the network, because it speeds up the boot, and synchronizes when the " \ - "machine gets connected to the network.") + "

" + # TRANSLATORS: help text for the offline check box + _("

Start Offline marks the server to be skiped during the start-up, but" \ + "will synchronize once the machine gets connected to the network.

") end end # Menu Button for choosing which type of server should be added to the # address input field. class SelectFrom < CWM::MenuButton + # Constructor + # + # @param address_widget [PoolAddress] the dialog pool address widget def initialize(address_widget) @address_widget = address_widget end # @macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: menu button label _("Select") end @@ -149,15 +159,15 @@ def opt # @macro seeItemsSelection def items + # TRANSLATORS: Menu button entries for choosing an address from a local + # servers list or from a public one [[:local, _("Local Server")], [:public, _("Public Server")]] end # @macro seeAbstractWidget def handle(event) - case event["ID"] - when :local, :public - Dialog::AddPool.new(@address_widget, event["ID"]).run - end + id = event["ID"] + Dialog::AddPool.new(@address_widget, id).run if [:local, :public].include?(id) nil end @@ -176,16 +186,17 @@ def cwm_definition # @macro seeAbstractWidget def help + # TRANSLATORS: Help for the select menu button _("

Select permits to choose a server from the list of servers" \ "offered by DHCP or from a public list filtered by country.

") \ end end - # List of ntp servers obtained from DHCP + # List of NTP servers obtained from DHCP class LocalList < CWM::SelectionBox # Constructor # - # @param address [String] current ntp pool address + # @param address [String] current NTP pool address def initialize(address) textdomain "ntp-client" @@ -206,8 +217,8 @@ def opt # @macro seeAbstractWidget def label - # TRANSLATORS: widget label - _("Syncronization server") + # TRANSLATORS: selection box label + _("Synchronization Server") end # @macro seeItemsSelection @@ -217,7 +228,8 @@ def items # @macro seeAbstractWidget def help - _("

List of available ntp servers provided by DHCP. " \ + # TRANSLATORS: help text for the local servers selection box + _("

List of available NTP servers provided by DHCP. " \ "Servers already in use are discarded.

") end @@ -225,32 +237,32 @@ def help # Convenience method to read and initialize the list of available servers def read_available_servers - Yast::Popup.Feedback(_("Getting ntp sources from DHCP"), Yast::Message.takes_a_while) do + Yast::Popup.Feedback(_("Getting NTP sources from DHCP"), Yast::Message.takes_a_while) do @servers = available_servers end end - # List of available ntp servers provided by DHCP. Servers already in use + # List of available NTP servers provided by DHCP. Servers already in use # are discarded. # - # @return [Array] list of ntp servers provided by dhcp + # @return [Array] list of NTP servers provided by DHCP def available_servers Yast::NtpClient.dhcp_ntp_servers.reject { |s| configured_servers.include?(s) } end - # List of ntp servers in use. + # List of NTP servers in use. # - # @return [Array] list of already configured ntp servers + # @return [Array] list of already configured NTP servers def configured_servers Yast::NtpClient.ntp_conf.pools.keys end end - # List of public ntp servers filtered by country + # List of public NTP servers filtered by country class PublicList < CWM::CustomWidget # Constructor # - # @param address [String] current ntp pool address + # @param address [String] current NTP pool address def initialize(address) textdomain "ntp-client" @@ -260,7 +272,8 @@ def initialize(address) # @macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: custom widget label, the widget permits to select a + # public server from a selection box, filtering the list by country _("Public Servers") end @@ -280,7 +293,8 @@ def value # @macro seeAbstractWidget def help - _("

List of public ntp servers filtered by country.

") + # TRANSLATORS: help text for the public servers custom widget + _("

List of public NTP servers filtered by country.

") end private @@ -312,7 +326,7 @@ def opt # @macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: combo box label _("Country") end @@ -335,6 +349,7 @@ def refresh_country_pools end def country_names + # TRANSLATORS: Combo box entry for not filtering entries { "" => _("ALL") }.merge(Yast::NtpClient.GetCountryNames) end end @@ -352,7 +367,7 @@ def opt # macro seeAbstractWidget def label - # TRANSLATORS: widget label + # TRANSLATORS: combo box label _("Ntp Servers") end diff --git a/test/test_helper.rb b/test/test_helper.rb index ea82523f..c6d747d8 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -17,6 +17,15 @@ end end +# stub module to prevent its Import +# Useful for modules from different yast packages, to avoid build dependencies +def stub_module(name) + Yast.const_set name.to_sym, Class.new { def self.fake_method; end } +end + +# stub classes from other modules to speed up a build +stub_module("Lan") + if ENV["COVERAGE"] require "simplecov" SimpleCov.start do From 4966191e73af85311daafbd9a30b6fde46c6f308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Wed, 14 Nov 2018 11:55:14 +0000 Subject: [PATCH 8/8] Described placeholder for translations --- src/lib/y2ntp_client/widgets/main_widgets.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lib/y2ntp_client/widgets/main_widgets.rb b/src/lib/y2ntp_client/widgets/main_widgets.rb index 1f0b1b81..81710941 100644 --- a/src/lib/y2ntp_client/widgets/main_widgets.rb +++ b/src/lib/y2ntp_client/widgets/main_widgets.rb @@ -18,13 +18,15 @@ def initialize end def label - # TRANSLATORS: label for widget that allows to define if ntp configiration is only - # from its source or dynamically extended e.g. via DHCP + # TRANSLATORS: label for widget that allows to define if ntp + # configuration is only from its source or dynamically extended + # e.g. via DHP _("Configuration Source") end def help - # TRANSLATORS: configuration source combo box help + # TRANSLATORS: configuration source combo box help, %{manual} is a + # manual page reference, e.g. "man 8 netconfig" _("

The NTP configuration may be provided by the local network over DHCP. " \ "Configuration Source can simply enable or disable using that configuration. " \ "In cases where there may be multiple DHCP sources, it can prioritize them: " \