be more relaxed when grub2-install failed to some devices (bsc#1125792)

src/lib/bootloader/grub2.rb

@@ -58,15 +58,16 @@ def write
       super
 
       device_map.write if Yast::Arch.x86_64 || Yast::Arch.i386
-      stage1.write
 
       # TODO: own class handling PBMR
       # set it only for gpt disk bsc#1008092
       pmbr_setup(*::Yast::BootStorage.gpt_disks(stage1.devices))
 
       # powernv must not call grub2-install (bnc#970582)
       unless Yast::Arch.board_powernv
-        @grub_install.execute(devices: stage1.devices, trusted_boot: trusted_boot)
+        failed = @grub_install.execute(devices: stage1.devices, trusted_boot: trusted_boot)
+        failed.each { |f| stage1.remove_device(f) }
+        stage1.write
       end
       # Do some mbr activations ( s390 do not have mbr nor boot flag on its disks )
       # powernv do not have prep partition, so we do not have any partition to activate (bnc#970582)

src/lib/bootloader/grub_install.rb

@@ -2,30 +2,74 @@
 require "yast2/execute"
 
 Yast.import "Arch"
+Yast.import "Report"
 
 module Bootloader
   # Wraps grub install script for easier usage.
   class GrubInstall
+    include Yast::Logger
+    include Yast::I18n
+
     def initialize(efi: false)
       @efi = efi
+      textdomain "bootloader"
     end
 
+    # Runs grub2 install command.
+    #
+    # @param devices[Array<String>] list to which grub2 should be installed.
+    #   Ignored when grub2 does not need device.
+    # @param secure_boot [Boolean] if secure boot variant should be used
+    # @param trusted_boot [Boolean] if trusted boot variant should be used
+    # @return [Array<String>] list for which install failed
     def execute(devices: [], secure_boot: false, trusted_boot: false)
       raise "cannot have secure boot without efi" if secure_boot && !efi
 
       cmd = basic_cmd(secure_boot, trusted_boot)
 
       if no_device_install?
         Yast::Execute.on_target(cmd)
+        []
       else
-        devices.each { |d| Yast::Execute.on_target(cmd + [d]) }
+        return [] if devices.empty?
+        last_failure = nil
+        res = devices.select do |device|
+          begin
+            Yast::Execute.on_target!(cmd + [device])
+            false
+          rescue Cheetah::ExecutionFailed => e
+            log.warn "Failed to install grub to device #{device}. #{e.inspect}"
+            last_failure = e
+            true
+          end
+        end
+
+        # Failed to install to all devices
+        if res.size == devices.size
+          report_failure(last_failure)
+        end
+
+        res
       end
     end
 
   private
 
     attr_reader :efi
 
+    def report_failure(exception)
+      Yast::Report.Error(
+        _(
+          "Installing GRUB2 to device failed.\n" \
+          "Command `%{command}`.\n" \
+          "Error output: %{stderr}"
+        ) % {
+          command:  exception.commands.inspect,
+          stderr:   exception.stderr
+        }
+      )
+    end
+
     # creates basic command for grub2 install without specifying any stage1
     # locations
     def basic_cmd(secure_boot, trusted_boot)

test/grub2_test.rb

@@ -73,7 +73,7 @@
 
       grub2_install = double(Bootloader::GrubInstall)
       expect(grub2_install).to receive(:execute)
-        .with(devices: ["/dev/sda", "/dev/sdb1"], trusted_boot: false)
+        .with(devices: ["/dev/sda", "/dev/sdb1"], trusted_boot: false).and_return([])
       expect(Bootloader::GrubInstall).to receive(:new).with(efi: false).and_return(grub2_install)
 
       subject.trusted_boot = false

test/grub_install_test.rb

@@ -27,8 +27,13 @@ def expect_grub2_install(target, device: nil, removable: false)
       params << "--no-nvram" << "--removable" if removable
       params << device if device
 
-      expect(Yast::Execute).to receive(:on_target)
-        .with(params)
+      if device
+        expect(Yast::Execute).to receive(:on_target!)
+          .with(params)
+      else
+        expect(Yast::Execute).to receive(:on_target)
+          .with(params)
+      end
     end
 
     context "initialized with efi: true" do
@@ -120,41 +125,59 @@ def expect_grub2_install(target, device: nil, removable: false)
 
       it "runs for each device passed in devices" do
         stub_arch("x86_64")
-        stub_efivars
         expect_grub2_install("i386-pc", device: "/dev/sda")
         expect_grub2_install("i386-pc", device: "/dev/sdb")
         expect_grub2_install("i386-pc", device: "/dev/sdc")
 
         subject.execute(devices: ["/dev/sda", "/dev/sdb", "/dev/sdc"])
       end
 
+      it "returns each device for which grub2-install failed" do
+        stub_arch("x86_64")
+        expect_grub2_install("i386-pc", device: "/dev/sdb")
+
+        allow(Yast::Execute).to receive(:on_target!) do |arg|
+          raise Cheetah::ExecutionFailed.new([], nil, nil, nil) if (arg & ["/dev/sda", "/dev/sdc"]).any?
+        end
+
+        expect(subject.execute(devices: ["/dev/sda", "/dev/sdb", "/dev/sdc"])).to contain_exactly("/dev/sda", "/dev/sdc")
+      end
+
+      it "opens a report if grub2-install failed for all devices" do
+        stub_arch("x86_64")
+
+        allow(Yast::Execute).to receive(:on_target!) do |arg|
+          raise Cheetah::ExecutionFailed.new([], nil, nil, nil)
+        end
+
+        expect(Yast::Report).to receive(:Error)
+
+        subject.execute(devices: ["/dev/sda", "/dev/sdb", "/dev/sdc"])
+      end
+
       it "runs with target i386-pc on i386" do
         stub_arch("i386")
-        stub_efivars
         expect_grub2_install("i386-pc", device: "/dev/sda")
 
         subject.execute(devices: ["/dev/sda"])
       end
 
       it "runs with target i386-pc on x86_64" do
         stub_arch("x86_64")
-        stub_efivars
         expect_grub2_install("i386-pc", device: "/dev/sda")
 
         subject.execute(devices: ["/dev/sda"])
       end
 
       it "runs with target powerpc-ieee1275 on ppc64" do
         stub_arch("ppc64")
-        stub_efivars
         expect_grub2_install("powerpc-ieee1275", device: "/dev/sda")
 
         subject.execute(devices: ["/dev/sda"])
       end
 
       it "runs with target s390x-emu on s390" do
         stub_arch("s390_64")
-        stub_efivars
 
         expect_grub2_install("s390x-emu")
 
@@ -164,7 +187,7 @@ def expect_grub2_install(target, device: nil, removable: false)
       it "pass directory argument when trusted boot is requested" do
         stub_arch("x86_64")
 
-        expect(Yast::Execute).to receive(:on_target) do |arg|
+        expect(Yast::Execute).to receive(:on_target!) do |arg|
           expect(arg).to include("--directory=/usr/lib/trustedgrub2/i386-pc")
         end