Fix restoring SCR root (bsc#1207968)

[jreidinger]

Problem

The issue is that bootloader ( yeah, not network ) crash with calling method on nil. Reason why it gets nil is that it failed to find /etc/sysconfig/bootloader. In logs I see several other failed reads e.g. to /etc/sysconfig/kdump. When manually reproduce it I notice that files are there. So only idea is switched SCR. So I check when it was last changed and it was in save_network to local one, but not back. So problem is that network switch SCR to '/' and do not restore it back.

• https://bugzilla.suse.com/show_bug.cgi?id=1207968
• https://openqa.suse.de/tests/10441806

Solution

Quickly from logs it is obvious that issue is at return from block at https://github.com/yast/yast-network/blob/master/src/lib/network/clients/save_network.rb#L98 where I though that return is from method and replace with break will fix it. But to be sure I write also test program. Another part is issue is that on_local method is not robust enough and e.g. if exception is returned it also do not get restore SCR chroot at https://github.com/yast/yast-network/blob/master/src/lib/network/clients/save_network.rb#L74 .

So to verify this ruby behavior I create testing program:

def test(&block)
  puts "pre block"

  block.call

  puts "post block"
ensure
  puts "ensured block"
end

def a
  test do
    puts "in block"
    return
  end
end

def b
  test do
    puts "in block"
    break
  end
end

def c
  test do
    puts "in block"
  end
end

puts "method a"
a
puts
puts "method b"
b
puts
puts "method c"
c

and get following output ( on both ruby 2.5 and ruby 3.1.2 ):

method a
pre block
in block
ensured block

method b
pre block
in block
ensured block

method c
pre block
in block
post block
ensured block

So conclusion for me is that replacing return with break does not help with skipping calls after block.call which really surprise me ( feel free to enlighten me why ). So only correct solution ( that is correct also to handle properly exceptions ) is to enclose restore of SCR to ensure block.

Testing

• Added a new unit test sadly save_network unit test is so messy and overmocked that I failed to test new behavior
• Tested manually (thanks lada for yupdate, test was easy with using this branch )

[coveralls]

Coverage: 80.438%. Remained the same when pulling b2d8292 on fix_closing_scr into 6c667c3 on master.

[ancorgs]

LGTM. ensure is definitely the right thing to do.

[yast-bot]

✔️ Public Jenkins job #321 successfully finished
✔️ Created OBS submit request #1064238

[yast-bot]

✔️ Internal Jenkins job #216 successfully finished
✔️ Created IBS submit request #289752