Merge adc298a into 835a431

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 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.1.5
+
 -------------------------------------------------------------------
 Thu Nov  8 15:20:26 UTC 2018 - knut.andersse@suse.com
 

package/yast2-ntp-client.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-ntp-client
-Version:        4.1.4
+Version:        4.1.5
 Release:        0
 Summary:        YaST2 - NTP Client Configuration
 License:        GPL-2.0-or-later
@@ -40,7 +40,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.1.17
 Requires:       yast2-ruby-bindings >= 1.0.0
 Requires:       rubygem(%rb_default_ruby_abi:cfa) >= 0.6.0
 BuildArch:      noarch

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)
@@ -184,24 +189,16 @@ 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
-        # 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)
-        NtpClient.config_has_been_read = true
+        ntp_items = configured_ntp_items
       end
-      ntp_items = Builtins.add(ntp_items, "")
-      Builtins.y2milestone("ntp_items :%1", ntp_items)
+
+      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
+
+      log.info "ntp_items :#{ntp_items}"
       UI.ChangeWidget(Id(:ntp_address), :Items, ntp_items)
 
       nil
@@ -363,7 +360,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
@@ -477,6 +474,46 @@ def add_or_install_required_package
         )
       end
     end
+
+    # 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 ID and
+    # label
+    #
+    # @return [Array<Yast::Term>] ntp address Item
+    def timezone_ntp_items
+      timezone_country = Timezone.GetCountryForTimezone(Timezone.timezone)
+      NtpClient.GetNtpServersByCountry(timezone_country, true)
+    end
+
+    # List of dhcp ntp servers Yast::Term items with the ntp address ID and
+    # label
+    #
+    # @return [Array<Yast::Term>] 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 ID and label
+    #
+    # @return [Array<Yast::Term>] ntp address table Item
+    def fallback_ntp_items
+      dhcp_items = dhcp_ntp_items
+
+      log.info("Nothing found in /etc/chrony.conf")
+      if 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
 

src/lib/y2ntp_client/dialog/add_pool.rb

@@ -0,0 +1,96 @@
+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
+        # TRANSLATORS: title for choosing a ntp server dialog
+        _("Available NTP servers")
+      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 available_pools
+        { local:  Widgets::LocalList, public: Widgets::PublicList }
+      end
+
+      def pool_for(type)
+        available_pools.fetch(type, Widgets::LocalList).new(@address_widget.value)
+      end
+
+      def min_height
+        8
+      end
+
+      def buttons
+        [ok_button, cancel_button]
+      end
+    end
+  end
+end

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)

src/lib/y2ntp_client/widgets/main_widgets.rb

@@ -24,13 +24,7 @@ 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. " \
-          "<b>Configuration Source</b> 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'."
-        )
+        "<p>#{help_text}</p>"
       end
 
       def opt
@@ -39,9 +33,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
@@ -74,6 +68,18 @@ def store
         Yast::NtpClient.modified = true
         Yast::NtpClient.ntp_policy = value
       end
+
+    private
+
+      def help_text
+        # TRANSLATORS: 'man 8 netconfig' is a command, do not translate that
+        _(
+          "The NTP configuration may be provided by the local network over DHCP. " \
+          "<b>Configuration Source</b> 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
     end
 
     # Widget to configure how ntp will be started