From ec7c5bd0064426ce4748478b718d32652ca628d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ladislav=20Slez=C3=A1k?= Date: Wed, 22 Mar 2023 10:07:39 +0100 Subject: [PATCH] Invalid storage device handling (#488) * Handle undefined/invalid target device in the web UI * Set the DBus service status back to idle even when it raises an exception * yupdate improvement --- .yupdate.pre | 4 ++ Rakefile | 68 ++++++++++--------- doc/yupdate.md | 7 ++ .../dinstaller/dbus/with_service_status.rb | 5 +- service/package/rubygem-d-installer.changes | 6 ++ .../dbus/with_service_status_test.rb | 12 ++++ web/package/cockpit-d-installer.changes | 6 ++ .../components/storage/ProposalSummary.jsx | 8 +++ 8 files changed, 81 insertions(+), 35 deletions(-) diff --git a/.yupdate.pre b/.yupdate.pre index 695ebb7532..35daf7e11f 100755 --- a/.yupdate.pre +++ b/.yupdate.pre @@ -17,6 +17,10 @@ # run the yupdate script several times # +if [ "$YUPDATE_SKIP_FRONTEND" == "1" ]; then + exit 0 +fi + # the needed packages for compiling the d-installer cockpit module PACKAGES=(appstream-glib-devel make npm) diff --git a/Rakefile b/Rakefile index 046736f96e..0658a015c7 100644 --- a/Rakefile +++ b/Rakefile @@ -93,41 +93,45 @@ SERVICES_DIR = "/usr/share/dbus-1/d-installer-services" if File.exist?("/.packages.initrd") || `mount`.match?(/^[\w]+ on \/ type overlay/) Rake::Task["install"].clear task :install do - destdir = ENV["DESTDIR"] || "/" - - puts "Installing the DBus service..." - Dir.chdir("service") do - sh "gem build d-installer.gemspec" - sh "gem install --local --force --no-format-exec --no-doc --build-root #{destdir.shellescape} d-installer-*.gem" - - # update the DBus configuration files - FileUtils.mkdir_p(SERVICES_DIR) - sh "cp share/org.opensuse.DInstaller*.service #{SERVICES_DIR}" - sh "cp share/dbus.conf /usr/share/dbus-1/d-installer.conf" - - # update the systemd service file - source_file = "share/systemd.service" - target_file = "/usr/lib/systemd/system/d-installer.service" - - unless FileUtils.identical?(source_file, target_file) - FileUtils.cp(source_file, target_file) - sh "systemctl daemon-reload" + if ENV["YUPDATE_SKIP_BACKEND"] != "1" + destdir = ENV["DESTDIR"] || "/" + + puts "Installing the DBus service..." + Dir.chdir("service") do + sh "gem build d-installer.gemspec" + sh "gem install --local --force --no-format-exec --no-doc --build-root #{destdir.shellescape} d-installer-*.gem" + + # update the DBus configuration files + FileUtils.mkdir_p(SERVICES_DIR) + sh "cp share/org.opensuse.DInstaller*.service #{SERVICES_DIR}" + sh "cp share/dbus.conf /usr/share/dbus-1/d-installer.conf" + + # update the systemd service file + source_file = "share/systemd.service" + target_file = "/usr/lib/systemd/system/d-installer.service" + + unless FileUtils.identical?(source_file, target_file) + FileUtils.cp(source_file, target_file) + sh "systemctl daemon-reload" + end end end - puts "Installing the Web frontend..." - Dir.chdir("web") do - node_env = ENV["NODE_ENV"] || "production" - sh "NODE_ENV=#{node_env.shellescape} make install" - - # clean up the extra files when switching the development/production mode - if node_env == "production" - # remove the uncompressed and development files - FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/index.{css,html,js}")) - FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.map")) - else - # remove the compressed files - FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.gz")) + if ENV["YUPDATE_SKIP_FRONTEND"] != "1" + puts "Installing the Web frontend..." + Dir.chdir("web") do + node_env = ENV["NODE_ENV"] || "production" + sh "NODE_ENV=#{node_env.shellescape} make install" + + # clean up the extra files when switching the development/production mode + if node_env == "production" + # remove the uncompressed and development files + FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/index.{css,html,js}")) + FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.map")) + else + # remove the compressed files + FileUtils.rm_f(Dir.glob("/usr/share/cockpit/d-installer/*.gz")) + end end end end diff --git a/doc/yupdate.md b/doc/yupdate.md index f7ab533aa4..d20cf820e5 100644 --- a/doc/yupdate.md +++ b/doc/yupdate.md @@ -48,6 +48,13 @@ You can modify the update process with these environment variables: mode. The files will not be minimized and additional `*.map` files will be generated. This helps with debugging in the browser, you can get the locations in the original source files. +- `YUPDATE_SKIP_FRONTEND=1` - Skip updating the web frontend. Use this option + when you use the webpack development server for running the web frontend. + In that case updating the web frontend does not make sense because it is + running in a different server. This saves some time and disk/RAM space. +- `YUPDATE_SKIP_BACKEND=1` - Skip updating the D-Bus service backend. This is + similar to the previous option, use it when you do want to keep the D-Bus + service unchanged. ## Notes diff --git a/service/lib/dinstaller/dbus/with_service_status.rb b/service/lib/dinstaller/dbus/with_service_status.rb index 9597dc9b42..7e2cebbcfc 100644 --- a/service/lib/dinstaller/dbus/with_service_status.rb +++ b/service/lib/dinstaller/dbus/with_service_status.rb @@ -38,10 +38,9 @@ def service_status # @return [Object] the result of the given block def busy_while(&block) service_status.busy - result = block.call + block.call + ensure service_status.idle - - result end end end diff --git a/service/package/rubygem-d-installer.changes b/service/package/rubygem-d-installer.changes index 28f2b78c9e..b0091cd194 100644 --- a/service/package/rubygem-d-installer.changes +++ b/service/package/rubygem-d-installer.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Mar 21 16:44:27 UTC 2023 - Ladislav Slezák + +- Fixed exception handling so service always goes back to the + "idle" state when finishing a block (related to bsc#1209523) + ------------------------------------------------------------------- Tue Mar 21 16:28:26 UTC 2023 - Ancor Gonzalez Sosa diff --git a/service/test/dinstaller/dbus/with_service_status_test.rb b/service/test/dinstaller/dbus/with_service_status_test.rb index dac7902618..2486a75f76 100644 --- a/service/test/dinstaller/dbus/with_service_status_test.rb +++ b/service/test/dinstaller/dbus/with_service_status_test.rb @@ -48,5 +48,17 @@ class WithServiceStatusTest result = subject.busy_while { "test" } expect(result).to eq("test") end + + context "the passed block raises an exception" do + it "sets the idle status and passes the exception up" do + expect(subject.service_status).to receive(:busy) + expect(subject.service_status).to receive(:idle) + + class TestException < RuntimeError; end + + expect { subject.busy_while { raise TestException } }.to raise_error(TestException) + end + end + end end diff --git a/web/package/cockpit-d-installer.changes b/web/package/cockpit-d-installer.changes index 84076a1e6b..4e29031961 100644 --- a/web/package/cockpit-d-installer.changes +++ b/web/package/cockpit-d-installer.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Tue Mar 21 16:41:06 UTC 2023 - Ladislav Slezák + +- Do not crash when setting an invalid target device using the + command line interface (bsc#1209523) + ------------------------------------------------------------------- Mon Mar 20 15:13:28 UTC 2023 - Imobach Gonzalez Sosa diff --git a/web/src/components/storage/ProposalSummary.jsx b/web/src/components/storage/ProposalSummary.jsx index f3a454237f..f4d83fb760 100644 --- a/web/src/components/storage/ProposalSummary.jsx +++ b/web/src/components/storage/ProposalSummary.jsx @@ -35,6 +35,14 @@ export default function ProposalSummary({ proposal }) { const [candidateDevice] = result.candidateDevices; const device = proposal.availableDevices.find(d => d.id === candidateDevice); + if (device === undefined) { + return ( + + Required device {candidateDevice} not found + + ); + } + return ( Install using device {device.label} and deleting all its content