Kill rpcbind process when needed

src/modules/Nfs.rb

@@ -19,6 +19,7 @@
 
 require "yast"
 require "y2firewall/firewalld"
+require "yast2/execute"
 
 require "shellwords"
 
@@ -384,15 +385,10 @@ def Write
       # help text
       Wizard.RestoreHelp(_("Writing NFS client settings. Please wait..."))
 
-      if Ops.greater_than(Builtins.size(@nfs_entries), 0)
+      if @nfs_entries.any?
         Progress.NextStage
-        # portmap must not be started if it is running already (see bug # 9999)
-        Service.Start(@portmapper) unless Service.active?(@portmapper)
 
-        unless Service.active?(@portmapper)
-          Report.Error(Message.CannotStartService(@portmapper))
-          return false
-        end
+        return false unless start_portmapper
       end
 
       firewalld.reload
@@ -700,6 +696,28 @@ def check_and_install_required_packages
 
       true
     end
+
+    # Starts portmapper service
+    #
+    # @return [Boolean] true if the service was activated; false otherwise.
+    def start_portmapper
+      if !Service.active?(@portmapper)
+        # Before mounting NFS shares, libstorage-ng could execute rpcbind binary directly without using
+        # systemd. If so, the systemd service would look like stopped and systemd would fail when trying
+        # to start it. The following line ensures that the process is not running before systemd tries to
+        # starts the service (bsc#1161687).
+        Yast::Execute.locally.stdout("killall", @portmapper)
+
+        # portmap must not be started if it is already running (bsc#9999)
+        Service.Start(@portmapper)
+      end
+
+      return true if Service.active?(@portmapper)
+
+      Report.Error(Message.CannotStartService(@portmapper))
+
+      false
+    end
   end
 
   Nfs = NfsClass.new

test/nfs_test.rb

@@ -462,11 +462,14 @@ def find_mp(path)
       allow(Yast::Service).to receive(:Start)
       allow(Yast::Service).to receive(:Stop)
       allow(Yast::Service).to receive(:active?)
+      allow(Yast::Execute).to receive(:locally).and_return(execute_object)
 
       allow_read_side_effects
       mock_entries
     end
 
+    let(:execute_object) { instance_double(Yast::Execute, stdout: "") }
+
     let(:written) { false }
 
     it "writes the nfs configurations" do
@@ -477,26 +480,66 @@ def find_mp(path)
     context "when the configuration is written correctly" do
       let(:written) { true }
 
-      it "tries to start the portmapper service if it is not running" do
-        expect(Yast::Service).to receive(:active?).with("rpcbind").and_return(false)
-        expect(Yast::Service).to receive(:Start).with("rpcbind")
-        subject.Write()
+      before do
+        allow(Yast::Service).to receive(:active?).with("rpcbind")
+          .and_return(service_status1, service_status2)
       end
 
-      context "and the portmapper service was not activated" do
-        before do
-          allow(Yast::Service).to receive(:active?).with("rpcbind").twice.and_return(false)
-          allow(Yast::Message).to receive(:CannotStartService).and_return("cannot_start")
+      let(:service_status1) { nil }
+
+      let(:service_status2) { nil }
+
+      context "and the portmapper service is not active" do
+        let(:service_status1) { false }
+
+        it "tries to kill the portmapper process" do
+          expect(execute_object).to receive(:stdout).with("killall", "rpcbind")
+
+          subject.Write
+        end
+
+        it "tries to activate the portmapper service" do
+          expect(Yast::Service).to receive(:Start).with("rpcbind")
+
+          subject.Write
         end
 
-        it "reports an error" do
-          expect(Yast::Report).to receive(:Error).with("cannot_start")
+        context "and the portmapper service was activated" do
+          let(:service_status2) { true }
+
+          it "returns true" do
+            expect(subject.Write).to eql(true)
+          end
+        end
+
+        context "and the portmapper service was not activated" do
+          let(:service_status2) { false }
+
+          it "reports an error" do
+            expect(Yast::Report).to receive(:Error).with(/Cannot start/)
+
+            subject.Write
+          end
+
+          it "returns false" do
+            expect(subject.Write).to eql(false)
+          end
+        end
+      end
+
+      context "and the portmapper service is already active" do
+        let(:service_status1) { true }
+
+        let(:service_status2) { true }
+
+        it "does not try to activate the portmapper service again" do
+          expect(Yast::Service).to_not receive(:Start).with("rpcbind")
 
           subject.Write
         end
 
-        it "returns false" do
-          expect(subject.Write).to eql(false)
+        it "returns true" do
+          expect(subject.Write).to eql(true)
         end
       end
     end