Merge SLE-12-SP4 into SLE-15-GA

package/yast2-ntp-client.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Thu Nov  8 15:20:26 UTC 2018 - knut.andersse@suse.com
+
+- Save the service status according to the user preferences
+  (bsc#1075039)
+- Only write the configuration once, and do not save changes when
+  we are only synchronizing the date. (bsc#1108497)
+- 4.0.14
+
 -------------------------------------------------------------------
 Wed Oct  3 14:11:34 UTC 2018 - knut.anderssen@suse.com
 

package/yast2-ntp-client.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-ntp-client
-Version:        4.0.13
+Version:        4.0.14
 Release:        0
 Summary:        YaST2 - NTP Client Configuration
 License:        GPL-2.0+

src/clients/ntp-client_proposal.rb

@@ -326,57 +326,47 @@ def WriteNtpSettings(ntp_servers, ntp_server, run_service)
       true
     end
 
-    # params:
-    #   server (taken from UI if empty)
-    #   servers (intended to use all of opensuse.pool.ntp.org,
-    # 	   but I did not have time to make it work)
-    #   run_service (set to true if empty)
-    #   write_only (bnc#589296)
-    #   ntpdate_only (TODO rename to onetime)
-    # return:
-    #   `success, `invalid_hostname or `ntpdate_failed
-    def Write(param)
-      log.info "ntp client proposal Write with #{param.inspect}"
-      ntp_servers = param["servers"] || []
-      ntp_server = param["server"] || ""
-      run_service = param.fetch("run_service", true)
-      if ntp_server == ""
-        # get the value from UI only when it wasn't given as a parameter
-        ntp_server = UI.QueryWidget(Id(:ntp_address), :Value)
-      end
-      return :invalid_hostname if !ValidateSingleServer(ntp_server)
+    # Writes the NTP settings
+    #
+    # @param [Hash] params
+    # @option params [String] "server" The NTP server address, taken from the UI if empty
+    # @option params [Array<String>] "servers" A collection of NTP servers
+    # @option params [Boolean] "run_service" Whether service should be active and enable
+    # @option params [Boolean] "write_only" If only is needed to write the settings, (bnc#589296)
+    # @option params [Boolean] "ntpdate_only" ? TODO: rename to onetime
+    #
+    # @return [Symbol] :invalid_hostname, when a not valid ntp_server is given
+    #                  :ntpdate_failed, when the ntp sychronization fails
+    #                  :success, when settings (and sync if proceed) were performed successfully
+    def Write(params)
+      log.info "ntp client proposal Write with #{params.inspect}"
 
-      WriteNtpSettings(ntp_servers, ntp_server, run_service)
-      return :success if param["write_only"]
+      # clean params
+      params.compact!
 
-      # In 1st stage, schedule packages for installation
-      if Stage.initial
-        Yast.import "Packages"
-        Packages.addAdditionalPackage(NtpClientClass::REQUIRED_PACKAGE)
-      # Otherwise, prompt user for confirming pkg installation
-      elsif !PackageSystem.CheckAndInstallPackages([NtpClientClass::REQUIRED_PACKAGE])
-        Report.Error(
-          Builtins.sformat(
-            _(
-              "Synchronization with NTP server is not possible\nwithout package %1 installed."
-            ),
-            NtpClientClass::REQUIRED_PACKAGE
-          )
-        )
-      end
+      ntp_server  = params.fetch("server", "")
+      ntp_servers = params.fetch("servers", [])
+      run_service = params.fetch("run_service", NtpClient.run_service)
+
+      # Get the ntp_server value from UI only if isn't present (probably wasn't given as parameter)
+      ntp_server = UI.QueryWidget(Id(:ntp_address), :Value) if ntp_server.strip.empty?
+
+      return :invalid_hostname unless ValidateSingleServer(ntp_server)
+
+      add_or_install_required_package unless params["write_only"]
+
+      WriteNtpSettings(ntp_servers, ntp_server, run_service) unless params["ntpdate_only"]
 
-      ret = 0
+      return :success if params["write_only"]
+
+      # Only if network is running try to synchronize the ntp server
       if NetworkService.isNetworkRunning
-        # Only if network is running try to synchronize the ntp server
         Popup.ShowFeedback("", _("Synchronizing with NTP server..."))
-        ret = NtpClient.sync_once(ntp_server)
+        exit_code = NtpClient.sync_once(ntp_server)
         Popup.ClearFeedback
-      end
-
-      return :ntpdate_failed if ret != 0
 
-      # User wants more than running one time sync (synchronize on boot)
-      WriteNtpSettings(ntp_servers, ntp_server, run_service) if !param["ntpdate_only"]
+        return :ntpdate_failed unless exit_code.zero?
+      end
 
       :success
     end
@@ -466,6 +456,24 @@ def ui_try_save
       # success, exit
       true
     end
+
+  private
+
+    def add_or_install_required_package
+      # In 1st stage, schedule packages for installation
+      if Stage.initial
+        Yast.import "Packages"
+        Packages.addAdditionalPackage(NtpClientClass::REQUIRED_PACKAGE)
+      # Otherwise, prompt user for confirming pkg installation
+      elsif !PackageSystem.CheckAndInstallPackages([NtpClientClass::REQUIRED_PACKAGE])
+        Report.Error(
+          Builtins.sformat(
+            _("Synchronization with NTP server is not possible\nwithout package %1 installed."),
+            NtpClientClass::REQUIRED_PACKAGE
+          )
+        )
+      end
+    end
   end
 end
 

src/modules/NtpClient.rb

@@ -128,7 +128,7 @@ def main
       @ntp_selected = false
     end
 
-    # CFA instance for reading/writing /etc/ntp.conf
+    # CFA instance for reading/writing /etc/chrony.conf
     def ntp_conf
       @chrony_conf ||= CFA::ChronyConf.new
     end
@@ -154,7 +154,7 @@ def progress?
     # @param server [String] to sync against
     # @return [Integer] exit code of sync command
     def sync_once(server)
-      log.info "Running ont time sync with #{server}"
+      log.info "Running one time sync with #{server}"
 
       # -q: set system time and quit
       # -t: timeout in seconds

test/ntp_client_proposal_test.rb

@@ -0,0 +1,169 @@
+require_relative "test_helper"
+
+require_relative "../src/clients/ntp-client_proposal.rb"
+
+Yast.import "Packages"
+
+describe Yast::NtpClientProposalClient do
+  subject do
+    client = described_class.new
+    client.main
+    client
+  end
+
+  describe "#Write" do
+    let(:ntp_server) { "fake.pool.ntp.org" }
+    let(:write_only) { false }
+    let(:ntpdate_only) { false }
+    let(:params) do
+      {
+        "server"       => ntp_server,
+        "write_only"   => write_only,
+        "ntpdate_only" => ntpdate_only
+      }
+    end
+    let(:ntp_client) { Yast::NtpClient }
+    let(:initial_stage) { false }
+    let(:network_running) { false }
+
+    before do
+      allow(subject).to receive(:WriteNtpSettings)
+
+      allow(Yast::Stage).to receive(:initial).and_return(initial_stage)
+      allow(Yast::PackageSystem).to receive(:CheckAndInstallPackages)
+      allow(Yast::Report).to receive(:Error)
+      allow(Yast::NetworkService).to receive(:isNetworkRunning).and_return(network_running)
+    end
+
+    context "with a not valid hostname" do
+      let(:ntp_server) { nil }
+
+      it "does not write settings" do
+        expect(subject).to_not receive(:WriteNtpSettings)
+
+        subject.Write(params)
+      end
+
+      it "returns :invalid_hostname" do
+        expect(subject.Write(params)).to eq(:invalid_hostname)
+      end
+    end
+
+    context "with valid hostname" do
+      before do
+        allow(subject).to receive(:ValidateSingleServer).and_return(true)
+      end
+
+      context "but in 'write_only' mode" do
+        let(:write_only) { true }
+
+        it "only writes settings" do
+          expect(subject).to receive(:WriteNtpSettings).once
+          expect(Yast::Stage).to_not receive(:initial)
+          expect(Yast::NetworkService).to_not receive(:isNetworkRunning)
+
+          subject.Write(params)
+        end
+
+        it "returns :success" do
+          expect(subject.Write(params)).to eq(:success)
+        end
+      end
+
+      context "but 'run_service' param is not given" do
+        it "uses the current value of NtpClient.run_service" do
+          ntp_client.run_service = true
+          expect(subject).to receive(:WriteNtpSettings).with(anything, anything, true)
+          subject.Write({})
+
+          ntp_client.run_service = false
+          expect(subject).to receive(:WriteNtpSettings).with(anything, anything, false)
+          subject.Write({})
+        end
+      end
+
+      context "and is in the initial stage" do
+        let(:initial_stage) { true }
+
+        it "imports Yast::Packages" do
+          allow(Yast).to receive(:import).and_call_original
+          expect(Yast).to receive(:import).with("Packages")
+
+          subject.Write(params)
+        end
+
+        it "adds the additional package" do
+          expect(Yast::Packages).to receive(:addAdditionalPackage)
+
+          subject.Write(params)
+        end
+      end
+
+      context "and is not in the  initial stage" do
+        it "asks user to confirm the package installation" do
+          expect(Yast::PackageSystem).to receive(:CheckAndInstallPackages)
+
+          subject.Write(params)
+        end
+
+        context "but user rejects the package installation" do
+          before do
+            allow(Yast::PackageSystem).to receive(:CheckAndInstallPackages).and_return(false)
+          end
+
+          it "reports an error" do
+            expect(Yast::Report).to receive(:Error).with(/Synchronization with NTP server is not/)
+
+            subject.Write(params)
+          end
+        end
+      end
+
+      context "and network is not available" do
+        it "does not performs the ntp syncronization" do
+          expect(Yast::NtpClient).to_not receive(:sync_once)
+
+          subject.Write(params)
+        end
+
+        it "returns :success" do
+          expect(subject.Write(params)).to eq(:success)
+        end
+      end
+
+      context "and network is available" do
+        let(:network_running) { true }
+
+        it "returns :ntpdate_failed if synchronization fails" do
+          allow(Yast::NtpClient).to receive(:sync_once).with(ntp_server).and_return(1)
+
+          expect(subject.Write(params)).to eq(:ntpdate_failed)
+        end
+
+        it "returns :success if syncronization was successfully" do
+          allow(Yast::NtpClient).to receive(:sync_once).with(ntp_server).and_return(0)
+
+          expect(subject.Write(params)).to eq(:success)
+        end
+      end
+
+      context "and user only wants to syncronize date" do
+        let(:ntpdate_only) { true }
+
+        it "does not write settings" do
+          expect(subject).to_not receive(:WriteNtpSettings)
+
+          subject.Write(params)
+        end
+      end
+
+      context "and user wants to syncronize on boot" do
+        it "writes settings only once" do
+          expect(subject).to receive(:WriteNtpSettings).once
+
+          subject.Write(params)
+        end
+      end
+    end
+  end
+end