Merge fe405d9 into b6c8b24

package/yast2-dns-server.changes

@@ -1,3 +1,9 @@
+-------------------------------------------------------------------
+Tue Aug 14 07:46:55 UTC 2018 - dgonzalez@suse.com
+
+- Manage the start mode and service status directly from Ruby.
+- 4.1.1
+
 -------------------------------------------------------------------
 Tue Aug  7 12:14:35 UTC 2018 - dgonzalez@suse.com
 

package/yast2-dns-server.spec

@@ -17,7 +17,7 @@
 
 
 Name:           yast2-dns-server
-Version:        4.1.0
+Version:        4.1.1
 Release:        0
 Url:            https://github.com/yast/yast-dns-server
 

src/include/dns-server/dialog-main.rb

@@ -1898,12 +1898,36 @@ def SaveAndRestart(event)
 
     # Writes DNS server settings and saves the service
     #
-    # @note currently, the DnsServer is a Perl module, reason why the write of
-    # settings is being performed in two steps.
+    # As side effect, it also sets/resets the local forwarder
     #
     # @return [Boolean] true if settings are saved successfully; false otherwise
     def write_settings
-      DnsServer.Write && service.save
+      return false unless DnsServer.Write
+
+      performed_action = service.action
+      service_saved = service.save(keep_state: Mode.auto)
+
+      reset_local_forwarder(performed_action, service_saved)
+
+      service_saved
+    end
+
+    # Resets the local forwarder to "resolver"
+    #
+    # Local forwarder will be changed to "resolver" if
+    #
+    #   * its current value is not "bind"
+    #   * service was stopped
+    #   * service cannot be started, restarted or reloaded
+    #
+    # @param performed_action [Symbol] action executed
+    # @param service_saved [Boolean] result of peforms the service#save
+    def reset_local_forwarder(performed_action, service_saved)
+      return unless DnsServer.GetLocalForwarder == "bind"
+      return unless !service_saved || performed_action == :stop
+
+      DnsServer.SetLocalForwarder("resolver")
+      log.warn("Local forwarder set to: #{DnsServer.GetLocalForwarder}")
     end
 
     # Shows a popup asking to the user if wants to change settings

src/modules/DnsServer.pm

@@ -1526,49 +1526,6 @@ sub Write {
 
     $ret = {};
 
-    if (Mode->auto() || Mode->config())
-    {
-        # named has to be started
-        if ($start_service)
-        {
-        my $success = 1;
-        if (! $write_only)
-        {
-            # named is running
-            if (Service->Status("named") == 0) {
-            y2milestone("Reloading service 'named'");
-            $success = Service->Reload("named")
-            } else {
-            y2milestone("Restarting service 'named'");
-            $success = Service->Restart("named")
-            }
-        }
-        Service->Enable ("named");
-        if (! $success)
-        {
-            # Cannot start service 'named', because of error that follows Error:.  Do not translate named.
-            Report->Error (__("Error occurred while starting service named.\n\n"));
-            $ok = 0;
-            # There's no 'named' running -> prevent from blocking DNS queries
-            $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind";
-            y2warning("Local forwarder set to: ".GetLocalForwarder());
-        }
-        }
-        # named has to be stopped
-        else
-        {
-        if (! $write_only)
-        {
-            y2milestone("Stopping service 'named'");
-            Service->Stop("named");
-            # There's no 'named' running. Reset dns forwarder again
-            $self->SetLocalForwarder("resolver") if GetLocalForwarder() eq "bind";
-            y2warning("Local forwarder set to: ".GetLocalForwarder());
-        }
-        Service->Disable ("named");
-        }
-    }
-
     # First run finished
     if ($ok and first_run())
     {

test/main_test.rb

@@ -8,6 +8,8 @@
 Yast.import "DnsServerUI"
 
 describe "DnsServerDialogMainInclude" do
+  subject(:main_dialog) { CurrentDialogMain.new }
+
   class CurrentDialogMain
     include Yast::I18n
     include Yast::UIShortcuts
@@ -16,22 +18,31 @@ class CurrentDialogMain
     def initialize
       Yast.include self, "dns-server/dialog-main.rb"
     end
+
+    def fun_ref(*args)
+    end
   end
 
+  let(:auto) { false }
+
   before do
+    allow(Yast::Mode).to receive(:auto).and_return(auto)
     allow_any_instance_of(CurrentDialogMain).to receive(:fun_ref)
   end
 
   describe "#WriteDialog" do
-    subject(:main_dialog) { CurrentDialogMain.new }
-
     before do
       allow(Yast::DnsServer).to receive(:Write).and_return(dns_configuration_written)
       allow(Yast2::SystemService).to receive(:find).and_return(service)
+
+      allow(service).to receive(:save).and_return(save)
+      allow(service).to receive(:action).and_return(action)
     end
 
-    let(:service) { instance_double(Yast2::SystemService, save: true) }
+    let(:service) { instance_double(Yast2::SystemService) }
     let(:dns_configuration_written) { true }
+    let(:action) { :start }
+    let(:save) { true }
 
     context "when DNS configuration is written" do
       it "saves the system service" do
@@ -43,6 +54,59 @@ def initialize
       it "returns :next" do
         expect(main_dialog.WriteDialog).to eq(:next)
       end
+
+
+      context "and local forwarder is not \"bind\"" do
+        let(:save) { false }
+
+        before do
+          allow(Yast2::Popup).to receive(:show)
+          allow(Yast::DnsServer).to receive(:GetLocalForwarder).and_return("whatever")
+        end
+
+        it "resets DNS forwarder" do
+          expect(Yast::DnsServer).to_not receive(:SetLocalForwarder)
+
+          main_dialog.WriteDialog
+        end
+      end
+
+      context "and local forwarder is \"bind\"" do
+        before do
+          allow(Yast::DnsServer).to receive(:GetLocalForwarder).and_return("bind")
+        end
+
+        context "and service was stopped" do
+          let(:action) { :stop }
+
+          it "resets DNS forwarder" do
+            expect(Yast::DnsServer).to receive(:SetLocalForwarder).with("resolver")
+
+            main_dialog.WriteDialog
+          end
+        end
+
+        context "and start service fails" do
+          let(:save) { false }
+
+          it "resets DNS forwarder" do
+            allow(Yast2::Popup).to receive(:show)
+            expect(Yast::DnsServer).to receive(:SetLocalForwarder).with("resolver")
+
+            main_dialog.WriteDialog
+          end
+        end
+      end
+
+      context "in auto mode" do
+        let(:auto) { true }
+
+        it "keeps the server status" do
+          expect(service).to receive(:save).with(hash_including(keep_state: true))
+
+          main_dialog.WriteDialog
+        end
+      end
     end
 
     context "when the configuration is not written" do
@@ -53,7 +117,7 @@ def initialize
       let(:change_settings) { :yes }
       let(:dns_configuration_written) { false }
 
-      it "aks for changing the current settings" do
+      it "asks for changing the current settings" do
         expect(Yast2::Popup).to receive(:show)
           .with(instance_of(String), hash_including(buttons: :yes_no))