Merge pull request #70 from yast/feature/service_widget

Use the new ServiceWidget

.gitignore

@@ -7,6 +7,7 @@ config.sub
 configure
 configure.ac
 configure.in
+coverage
 install-sh
 Makefile
 Makefile.in

package/yast2-iscsi-client.changes

@@ -1,3 +1,16 @@
+-------------------------------------------------------------------
+Tue Aug  7 11:52:21 UTC 2018 - dgonzalez@suse.com
+
+- Use Yast2::ServiceWidget to manage the service status
+  (part of fate#319428)
+- 4.1.0
+
+-------------------------------------------------------------------
+Fri Aug  3 11:56:56 UTC 2018 - dgonzalez@suse.com
+
+- Fix detection of service current status (bsc#1103681).
+- 4.0.2
+
 -------------------------------------------------------------------
 Wed Jun 27 12:40:11 CEST 2018 - schubi@suse.de
 

package/yast2-iscsi-client.spec

@@ -17,26 +17,25 @@
 
 
 Name:           yast2-iscsi-client
-Version:        4.0.1
+Version:        4.1.0
 Release:        0
 
 BuildRoot:      %{_tmppath}/%{name}-%{version}-build
 Source0:        %{name}-%{version}.tar.bz2
 
 Group:	        System/YaST
 License:        GPL-2.0
-# Service module switched to systemd
+# Yast2::SystemService
+BuildRequires:	yast2 >= 4.1.0
 BuildRequires:	yast2 >= 2.23.15
 BuildRequires:	docbook-xsl-stylesheets libxslt update-desktop-files
 BuildRequires:  yast2-devtools >= 3.1.10
 BuildRequires:  rubygem(rspec)
 
 Requires:	yast2-packager
 
-# network needs Wizard::OpenCancelOKDialog()
-#  function from yast2-2.18.2
-# Wizard::SetDesktopTitleAndIcon
-Requires:       yast2 >= 2.21.22
+# Yast2::SystemService
+Requires:	    yast2 >= 4.1.0
 
 BuildArchitectures:	noarch
 

src/include/iscsi-client/complex.rb

@@ -67,12 +67,18 @@ def ReadDialog
     end
 
     # Write settings dialog
-    # @return `abort if aborted and `next otherwise
+    #
+    # @return [Symbol] :abort if aborted, :next otherwise
     def WriteDialog
-      Wizard.RestoreHelp(Ops.get_string(@HELPS, "write", ""))
-      # IscsiClient::AbortFunction = PollAbort;
-      ret = IscsiClient.Write
-      ret ? :next : :abort
+      help = @HELPS.fetch("write") { "" }
+
+      Wizard.CreateDialog
+      Wizard.RestoreHelp(help)
+      result = IscsiClient.Write
+      Wizard.CloseDialog
+
+      return :next if result
+      :abort
     end
   end
 end

src/include/iscsi-client/dialogs.rb

@@ -29,6 +29,9 @@
 # $Id$
 #
 # Main file for iscsi-client configuration. Uses all other files.
+
+require "cwm/service_widget"
+
 module Yast
   module IscsiClientDialogsInclude
     def initialize_iscsi_client_dialogs(include_target)
@@ -50,34 +53,8 @@ def initialize_iscsi_client_dialogs(include_target)
       Yast.include include_target, "iscsi-client/widgets.rb"
 
       @widgets = {
-        "auto_start_up"    => CWMServiceStart.CreateAutoStartWidget(
-          {
-            "get_service_auto_start" => fun_ref(
-              IscsiClientLib.method(:GetStartService),
-              "boolean ()"
-            ),
-            "set_service_auto_start" => fun_ref(
-              IscsiClientLib.method(:SetStartService),
-              "void (boolean)"
-            ),
-            # radio button (starting SLP service - option 1)
-            "start_auto_button"      => _(
-              "When &Booting"
-            ),
-            # radio button (starting SLP service - option 2)
-            "start_manual_button"    => _(
-              "&Manually"
-            ),
-            "help"                   => Builtins.sformat(
-              CWMServiceStart.AutoStartHelpTemplate,
-              # part of help text, used to describe radiobuttons (matching starting SLP service but without "&")
-              _("When Booting"),
-              # part of help text, used to describe radiobuttons (matching starting SLP service but without "&")
-              _("Manually")
-            )
-          }
-        ),
-        "isns"             => {
+        "auto_start_up"       => service_widget.cwm_definition,
+        "isns"                => {
           "widget"            => :custom,
           "custom_widget"     => HBox(
             MinWidth(
@@ -384,6 +361,13 @@ def initialize_iscsi_client_dialogs(include_target)
       }
     end
 
+    # Widget to define state and start mode of the service
+    #
+    # @return [::CWM::ServiceWidget]
+    def service_widget
+      @service_widget ||= ::CWM::ServiceWidget.new(IscsiClient.services)
+    end
+
     # main tabbed dialog
     def GlobalDialog
       if Stage.initial

src/modules/IscsiClient.rb

@@ -30,7 +30,10 @@
 #
 # Representation of the configuration of iscsi-client.
 # Input and output routines.
+
 require "yast"
+require "yast2/system_service"
+require "yast2/compound_service"
 
 module Yast
   class IscsiClientClass < Module
@@ -57,9 +60,9 @@ def main
       # Data was modified?
       @modified = false
 
-
       @proposal_valid = false
 
+      # FIXME: write_only seems to be not in use
       # Write only, used during autoinstallation.
       # Don't run services and SuSEconfig, it's all done at one place.
       @write_only = false
@@ -69,6 +72,17 @@ def main
       @AbortFunction = fun_ref(method(:Modified), "boolean ()")
     end
 
+    # Returns iSCSI related services
+    #
+    # @return [Yast2::CompundService]
+    def services
+      @services ||= Yast2::CompoundService.new(
+        Yast2::SystemService.find("iscsi"),
+        Yast2::SystemService.find("iscsid"),
+        Yast2::SystemService.find("iscsiuio")
+      )
+    end
+
     # Abort function
     # @return [Boolean] return true if abort
     def Abort
@@ -205,9 +219,11 @@ def Read
       Builtins.sleep(sl)
 
       return false if Abort()
-      #    Progress::NextStep();
-      # read status of service
-      return false if !IscsiClientLib.getServiceStatus
+
+      if Mode.auto || Mode.commandline
+        return false unless IscsiClientLib.getServiceStatus
+      end
+
       # try auto login to target
       IscsiClientLib.autoLogOn
       Builtins.sleep(sl)
@@ -216,7 +232,6 @@ def Read
       #    if(!IscsiClientLib::autoLogOn()) return false;
       Progress.NextStage
 
-
       # read config file
       if IscsiClientLib.readSessions == false
         Report.Error(Message.CannotReadCurrentSettings)
@@ -263,7 +278,7 @@ def Write
       return false if Abort()
       Progress.NextStage
       # set open-iscsi service status
-      return false if !IscsiClientLib.setServiceStatus
+      return false unless save_status
       Builtins.sleep(sl)
 
       return false if Abort()
@@ -277,6 +292,20 @@ def Write
       true
     end
 
+    # Saves service status (start mode and starts/stops the service)
+    #
+    # @note For AutoYaST and for command line actions, it uses the old way for
+    # backward compatibility, see {IscsiClientLib#setServiceStatus}. When the
+    # service is configured by using the UI, it directly saves the service, see
+    # {Yast2::SystemService#save}.
+    def save_status
+      if Mode.auto || Mode.commandline
+        IscsiClientLib.setServiceStatus
+      else
+        services.save
+      end
+    end
+
     # Get all iscsi-client settings from the first parameter
     # (For use by autoinstallation.)
     # @param [Hash] settings The YCP structure to be imported.
@@ -315,6 +344,7 @@ def AutoPackages
     publish :function => :Modified, :type => "boolean ()"
     publish :variable => :modified, :type => "boolean"
     publish :variable => :proposal_valid, :type => "boolean"
+    # FIXME: write_only it is not used anymore
     publish :variable => :write_only, :type => "boolean"
     publish :variable => :AbortFunction, :type => "boolean ()"
     publish :function => :Abort, :type => "boolean ()"

src/modules/IscsiClientLib.rb

@@ -1076,6 +1076,11 @@ def loginIntoTarget(target)
     end
 
 
+    # FIXME: this method has too much responsibility and it is doing
+    # "unexpected" things according to its name. Ideally, it only must return
+    # the service status without changing the status of related services and
+    # sockets.
+    #
     # get status of iscsid
     def getServiceStatus
       ret = true
@@ -1096,7 +1101,7 @@ def getServiceStatus
         log.info "Status of iscsi.service: #{@iscsi_service_stat}, iscsid.socket: #{@iscsid_socket_stat}, iscsiuio.socket: #{@iscsiuio_socket_stat}"
 
         # if not running, start iscsi.service, iscsid.socket and iscsiuio.socket
-        if !@iscid_socket_stat
+        if !@iscsid_socket_stat
           Service.Stop("iscsid") if Service.active?("iscsid") 
           log.error "Cannot start iscsid.socket" if !socketStart(@iscsid_socket)
         end

test/iscsi_client_lib_test.rb

@@ -0,0 +1,76 @@
+#!/usr/bin/env rspec
+
+require_relative "test_helper"
+require_relative "../src/modules/IscsiClientLib"
+
+describe Yast::IscsiClientLibClass do
+  subject { described_class.new }
+
+  describe "#getServiceStatus" do
+    before do
+      allow(Yast::Service).to receive(:Stop).with(anything)
+      allow(Yast::Service).to receive(:active?).with(anything)
+      allow(Yast::SystemdSocket).to receive(:find!).with(anything)
+      allow(subject).to receive(:socketActive?)
+      allow(subject.log).to receive(:error)
+    end
+
+    context "for iscsid.socket" do
+      let(:iscsid_socket) { double("iscsid.socket", start: true) }
+
+      before do
+        allow(Yast::SystemdSocket).to receive(:find!).with("iscsid").and_return(iscsid_socket)
+        allow(subject).to receive(:socketActive?).with(iscsid_socket).and_return(socket_status)
+      end
+
+      context "when iscsid.socket and iscsid.service are active" do
+        let(:socket_status) { true }
+
+        before do
+          allow(Yast::Service).to receive(:active?).with("iscsid").and_return(true)
+        end
+
+        it "does not start the socket nor the service" do
+          expect(iscsid_socket).to_not receive(:start)
+          expect(Yast::Service).to_not receive(:Stop).with("iscsid")
+
+          subject.getServiceStatus
+        end
+      end
+
+      context "when iscsid.socket is not active" do
+        let(:socket_status) { false }
+
+        context "and socket is availbale" do
+          it "starts iscsid.socket" do
+            expect(iscsid_socket).to receive(:start)
+
+            subject.getServiceStatus
+          end
+
+          context "but iscsid.service is active" do
+            before do
+              allow(Yast::Service).to receive(:active?).with("iscsid").and_return(true)
+            end
+
+            it "stops iscsid.service" do
+              expect(Yast::Service).to receive(:Stop).with("iscsid")
+
+              subject.getServiceStatus
+            end
+          end
+        end
+
+        context "but socket is not availbale" do
+          let(:iscsid_socket) { nil }
+
+          it "logs an error" do
+            expect(subject.log).to receive(:error).with("Cannot start iscsid.socket")
+
+            subject.getServiceStatus
+          end
+        end
+      end
+    end
+  end
+end