From c078f28722fe81d42a61e1bf1340cba94e13876e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Jul 2023 13:33:18 +0100 Subject: [PATCH 1/6] [web] Add info about deactivated allow LUN scan --- web/src/components/storage/ZFCPPage.jsx | 35 +++++++++++--------- web/src/components/storage/ZFCPPage.test.jsx | 6 ++-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/web/src/components/storage/ZFCPPage.jsx b/web/src/components/storage/ZFCPPage.jsx index c8cdf59075..179243fd99 100644 --- a/web/src/components/storage/ZFCPPage.jsx +++ b/web/src/components/storage/ZFCPPage.jsx @@ -422,26 +422,31 @@ const ControllersSection = ({ client, manager, load = noop, isLoading = false }) }; const Content = () => { - const AllowLUNScan = () => { + const LUNScanInfo = () => { return ( -

- The system is configured to perform automatic LUN scan. If a controller is running in - NPIV mode, then all its LUNs are automatically activated. -

+ + Automatic LUN scan is enabled. Activating a controller which is running in NPIV + mode will automatically configures all its LUNs. +

+ } + else={ +

+ Automatic LUN scan is disabled. LUNs have to be manually configured after + activating a controller. +

+ } + /> ); }; return ( - } - else={ - <> - } /> - - - } - /> + <> + + + ); }; diff --git a/web/src/components/storage/ZFCPPage.test.jsx b/web/src/components/storage/ZFCPPage.test.jsx index c96a701a5b..b42274aa52 100644 --- a/web/src/components/storage/ZFCPPage.test.jsx +++ b/web/src/components/storage/ZFCPPage.test.jsx @@ -98,7 +98,7 @@ describe("if allow-lun-scan is activated", () => { it("renders an explanation about allow-lun-scan", async () => { installerRender(); - await screen.findByText(/is configured to perform automatic LUN scan/); + await screen.findByText(/automatically configures all its LUNs/); }); }); @@ -107,10 +107,10 @@ describe("if allow-lun-scan is not activated", () => { client.getAllowLUNScan = jest.fn().mockResolvedValue(false); }); - it("does not render an explanation about allow-lun-scan", async () => { + it("renders an explanation about not using allow-lun-scan", async () => { installerRender(); - await waitFor(() => expect(screen.queryByText(/is configured to perform automatic LUN scan/)).toBeNull()); + await screen.findByText(/LUNs have to be manually configured/); }); }); From d4f0f962a08d1bf662a236ef03382ad572ceadfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Jul 2023 13:39:37 +0100 Subject: [PATCH 2/6] [web] Sort members in device selector --- web/src/components/storage/DeviceSelector.jsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/src/components/storage/DeviceSelector.jsx b/web/src/components/storage/DeviceSelector.jsx index 9c3ea1834c..8826405b50 100644 --- a/web/src/components/storage/DeviceSelector.jsx +++ b/web/src/components/storage/DeviceSelector.jsx @@ -122,7 +122,7 @@ const ItemContent = ({ device }) => { const members = device.members.map(m => m.split("/").at(-1)); - return
Members: {members.join(", ")}
; + return
Members: {members.sort().join(", ")}
; }; const RAIDInfo = () => { @@ -130,7 +130,7 @@ const ItemContent = ({ device }) => { const devices = device.devices.map(m => m.split("/").at(-1)); - return
Devices: {devices.join(", ")}
; + return
Devices: {devices.sort().join(", ")}
; }; const MultipathInfo = () => { @@ -138,7 +138,7 @@ const ItemContent = ({ device }) => { const wires = device.wires.map(m => m.split("/").at(-1)); - return
Wires: {wires.join(", ")}
; + return
Wires: {wires.sort().join(", ")}
; }; return ( From 02526a110602489d49358a0ce408b25e9c93f0b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 5 Jul 2023 13:54:25 +0100 Subject: [PATCH 3/6] [service] Wait for disks activation --- service/lib/agama/storage/zfcp/manager.rb | 13 +++++++++++-- service/test/agama/storage/zfcp/manager_test.rb | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/service/lib/agama/storage/zfcp/manager.rb b/service/lib/agama/storage/zfcp/manager.rb index 0fd8f8ff22..dfbbd1375d 100644 --- a/service/lib/agama/storage/zfcp/manager.rb +++ b/service/lib/agama/storage/zfcp/manager.rb @@ -91,13 +91,22 @@ def allow_lun_scan? # Activates the controller with the given channel id # - # @note: If "allow_lun_scan" is active, then all LUNs are automatically activated. + # @note: If "allow_lun_scan" is active, then all its LUNs are automatically activated. # # @param channel [String] # @return [Integer] Exit code of the chzdev command (0 on success) def activate_controller(channel) output = yast_zfcp.activate_controller(channel) - update_disks if output["exit"] == 0 + if output["exit"] == 0 + # LUNs activation could delay after activating the controller. This usually happens when + # activating a controller for first time because some SCSI initialization. Probing the + # disks should be done after all disks are activated. + # + # FIXME: waiting 2 seconds should be enough, but there is no guarantee that the disks + # are actually exported. + sleep(2) + update_disks + end output["exit"] end diff --git a/service/test/agama/storage/zfcp/manager_test.rb b/service/test/agama/storage/zfcp/manager_test.rb index 7ca8d6866a..054ca7171d 100644 --- a/service/test/agama/storage/zfcp/manager_test.rb +++ b/service/test/agama/storage/zfcp/manager_test.rb @@ -179,6 +179,7 @@ module Y2S390 describe "#activate_controller" do before do allow(yast_zfcp).to receive(:activate_controller).and_return(output) + allow(subject).to receive(:sleep) subject.on_disks_change(&callback) end From 6fa5a7741efa6b8e473f894addae11e22e3599e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Jul 2023 10:05:28 +0100 Subject: [PATCH 4/6] [service] Improve zFCP probing - zFCP disk changes are detected with each probing and callbacks are run if needed. --- service/lib/agama/storage/zfcp/manager.rb | 46 ++++-- .../test/agama/storage/zfcp/manager_test.rb | 146 +++++++----------- 2 files changed, 87 insertions(+), 105 deletions(-) diff --git a/service/lib/agama/storage/zfcp/manager.rb b/service/lib/agama/storage/zfcp/manager.rb index dfbbd1375d..354247f682 100644 --- a/service/lib/agama/storage/zfcp/manager.rb +++ b/service/lib/agama/storage/zfcp/manager.rb @@ -21,6 +21,7 @@ require "yast" require "y2s390/zfcp" +require "y2storage/storage_manager" require "agama/storage/zfcp/controller" require "agama/storage/zfcp/disk" @@ -54,14 +55,20 @@ def on_disks_change(&block) # Probes zFCP # - # Callbacks are called at the end, see {#on_probe}. + # Callbacks {#on_probe} are called after probing, and callbacks {#on_disks_change} are + # called if there is any change in the disks. def probe logger.info "Probing zFCP" + previous_disks = disks yast_zfcp.probe_controllers yast_zfcp.probe_disks @on_probe_callbacks&.each { |c| c.call(controllers, disks) } + + return unless disks_changed?(previous_disks) + + @on_disks_change_callbacks&.each { |c| c.call(disks) } end # zFCP controllers @@ -102,10 +109,10 @@ def activate_controller(channel) # activating a controller for first time because some SCSI initialization. Probing the # disks should be done after all disks are activated. # - # FIXME: waiting 2 seconds should be enough, but there is no guarantee that the disks - # are actually exported. + # FIXME: waiting 2 seconds should be enough, but there is no guarantee that all the + # disks are actually activated. sleep(2) - update_disks + probe end output["exit"] end @@ -119,7 +126,7 @@ def activate_controller(channel) # @return [Integer] Exit code of the chzdev command (0 on success) def activate_disk(channel, wwpn, lun) output = yast_zfcp.activate_disk(channel, wwpn, lun) - update_disks if output["exit"] == 0 + probe if output["exit"] == 0 output["exit"] end @@ -134,7 +141,7 @@ def activate_disk(channel, wwpn, lun) # @return [Integer] Exit code of the chzdev command (0 on success) def deactivate_disk(channel, wwpn, lun) output = yast_zfcp.deactivate_disk(channel, wwpn, lun) - update_disks if output["exit"] == 0 + probe if output["exit"] == 0 output["exit"] end @@ -163,15 +170,28 @@ def find_luns(channel, wwpn) # @return [Logger] attr_reader :logger - # Probes and runs the on_disk_change callbacsk if disks have changed - def update_disks - disk_records = yast_zfcp.disks.sort_by { |d| d["dev_name"] } - probe - new_disk_records = yast_zfcp.disks.sort_by { |d| d["dev_name"] } + # Whether threre is any change in the disks + # + # Checks whether any of the removed disks is still probed or whether any of the current + # disks is not probed yet. + # + # @param previous_disks [Array] Previously activated disks (before zFCP (re)probing) + # @return [Booelan] + def disks_changed?(previous_disks) + removed_disks = previous_disks - disks - return if disk_records == new_disk_records + return true if removed_disks.any? { |d| exist_disk?(d) } + return true if disks.any? { |d| !exist_disk?(d) } - @on_disks_change_callbacks&.each { |c| c.call(disks) } + false + end + + # Whether the given disk is probed + # + # @param disk [Disk] + # @return [Booelan] + def exist_disk?(disk) + !Y2Storage::StorageManager.instance.probed.find_by_name(disk.name).nil? end # Creates a zFCP controller from YaST data diff --git a/service/test/agama/storage/zfcp/manager_test.rb b/service/test/agama/storage/zfcp/manager_test.rb index 054ca7171d..fefae6a640 100644 --- a/service/test/agama/storage/zfcp/manager_test.rb +++ b/service/test/agama/storage/zfcp/manager_test.rb @@ -23,6 +23,9 @@ require "agama/storage/zfcp/manager" require "agama/storage/zfcp/controller" require "agama/storage/zfcp/disk" +require "y2storage/storage_manager" +require "y2storage/devicegraph" +require "y2storage/disk" # yast2-s390 is not available for all architectures. Defining Y2S390::ZFCP constant here in order to # make possible to mock that class independently on the architecture. Note that Y2S390 classes are @@ -38,6 +41,9 @@ module Y2S390 before do allow(Y2S390::ZFCP).to receive(:new).and_return(yast_zfcp) + allow(Y2Storage::StorageManager).to receive(:instance).and_return(storage_manager) + allow(storage_manager).to receive(:probed).and_return(devicegraph) + allow(devicegraph).to receive(:find_by_name).and_return(storage_disk) allow(yast_zfcp).to receive(:probe_controllers) allow(yast_zfcp).to receive(:probe_disks) allow(yast_zfcp).to receive(:controllers).and_return(*controller_records) @@ -46,6 +52,12 @@ module Y2S390 let(:yast_zfcp) { double(Y2S390::ZFCP) } + let(:storage_manager) { instance_double(Y2Storage::StorageManager) } + + let(:devicegraph) { instance_double(Y2Storage::Devicegraph) } + + let(:storage_disk) { instance_double(Y2Storage::Disk) } + let(:controller_records) { [[]] } let(:disk_records) { [[]] } @@ -80,15 +92,55 @@ module Y2S390 subject.probe end - let(:callback) { proc { |_, _| } } - it "runs the on_probe callbacks" do + callback = proc { |_, _| } subject.on_probe(&callback) expect(callback).to receive(:call).with([], []) subject.probe end + + context "if any zFCP disk was deactivated" do + let(:disk_records) { [[sda_record, sdb_record], [sda_record]] } + + it "runs the on_disks_change callbacks" do + callback = proc { |_| } + subject.on_disks_change(&callback) + + expect(callback).to receive(:call) + + subject.probe + end + end + + context "if any new zFCP disk was activated" do + let(:disk_records) { [[sda_record], [sda_record, sdb_record]] } + + let(:storage_disk) { nil } + + it "runs the on_disks_change callbacks" do + callback = proc { |_| } + subject.on_disks_change(&callback) + + expect(callback).to receive(:call) + + subject.probe + end + end + + context "if the zFCP disks have not changed" do + let(:disk_records) { [[sda_record, sdb_record], [sda_record, sdb_record]] } + + it "does not run the on_disks_change callbacks" do + callback = proc { |_| } + subject.on_disks_change(&callback) + + expect(callback).to_not receive(:call) + + subject.probe + end + end end describe "#controllers" do @@ -180,14 +232,10 @@ module Y2S390 before do allow(yast_zfcp).to receive(:activate_controller).and_return(output) allow(subject).to receive(:sleep) - - subject.on_disks_change(&callback) end let(:output) { { "exit" => 3 } } - let(:callback) { proc { |_| } } - it "tries to activate the controller with the given channel id" do expect(yast_zfcp).to receive(:activate_controller).with("0.0.fa00") @@ -208,26 +256,6 @@ module Y2S390 subject.activate_controller("0.0.fa00") end - - context "and the zFCP disks have changed" do - let(:disk_records) { [[sda_record], [sdb_record]] } - - it "runs the on_disks_change callbacks" do - expect(callback).to receive(:call) - - subject.activate_controller("0.0.fa00") - end - end - - context "and the zFCP disks have not changed" do - let(:disk_records) { [[sda_record], [sda_record]] } - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.activate_controller("0.0.fa00") - end - end end context "if the controller was not activated" do @@ -238,26 +266,16 @@ module Y2S390 subject.activate_controller("0.0.fa00") end - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.activate_controller("0.0.fa00") - end end end describe "#activate_disk" do before do allow(yast_zfcp).to receive(:activate_disk).and_return(output) - - subject.on_disks_change(&callback) end let(:output) { { "exit" => 3 } } - let(:callback) { proc { |_| } } - it "tries to activate the zFCP diks with the given channel id, WWPN and LUN" do expect(yast_zfcp) .to receive(:activate_disk).with("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") @@ -279,26 +297,6 @@ module Y2S390 subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") end - - context "and the zFCP disks have changed" do - let(:disk_records) { [[sda_record], [sdb_record]] } - - it "runs the on_disks_change callbacks" do - expect(callback).to receive(:call) - - subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end - end - - context "and the zFCP disks have not changed" do - let(:disk_records) { [[sda_record], [sda_record]] } - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end - end end context "if the zFCP disk was not activated" do @@ -309,26 +307,16 @@ module Y2S390 subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") end - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.activate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end end end describe "#deactivate_disk" do before do allow(yast_zfcp).to receive(:deactivate_disk).and_return(output) - - subject.on_disks_change(&callback) end let(:output) { { "exit" => 3 } } - let(:callback) { proc { |_| } } - it "tries to deactivate the zFCP diks with the given channel id, WWPN and LUN" do expect(yast_zfcp) .to receive(:deactivate_disk).with("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") @@ -350,26 +338,6 @@ module Y2S390 subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") end - - context "and the zFCP disks have changed" do - let(:disk_records) { [[sda_record], [sdb_record]] } - - it "runs the on_disks_change callbacks" do - expect(callback).to receive(:call) - - subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end - end - - context "and the zFCP disks have not changed" do - let(:disk_records) { [[sda_record], [sda_record]] } - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end - end end context "if the zFCP disk was not deactivated" do @@ -380,12 +348,6 @@ module Y2S390 subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") end - - it "does not run the on_disks_change callbacks" do - expect(callback).to_not receive(:call) - - subject.deactivate_disk("0.0.fa00", "0x500507630708d3b3", "0x0013000000000000") - end end end From 16cfac60ad72b004e3bc709d2fa8bfb43cd9fa88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Jul 2023 10:07:53 +0100 Subject: [PATCH 5/6] [web] Changelog --- web/package/cockpit-agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index ef18ecdcb8..75f7c248e6 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Jul 5 13:59:58 UTC 2023 - José Iván López González + +- Add info about deactivated zFCP auto_lun_scan and sort members in + device selector (gh#openSUSE/agama#650). + ------------------------------------------------------------------- Mon Jul 3 10:18:32 UTC 2023 - José Iván López González From 967e495f8b679fbbc02a081b4fc83d9742a79d91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 6 Jul 2023 10:08:14 +0100 Subject: [PATCH 6/6] [service] Changelog --- service/package/rubygem-agama.changes | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index 1269fd80a1..fa17a0775c 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Wed Jul 5 14:02:23 UTC 2023 - José Iván López González + +- Delay zFCP probing after activating a controller and ensure the + system is marked as deprecated if needed after probing zFCP + (gh#openSUSE/agama#650). + ------------------------------------------------------------------- Wed Jun 14 15:11:56 UTC 2023 - José Iván López González