diff --git a/service/lib/agama/storage/iscsi/config.rb b/service/lib/agama/storage/iscsi/config.rb index 4e085bb321..3d5092cb6e 100644 --- a/service/lib/agama/storage/iscsi/config.rb +++ b/service/lib/agama/storage/iscsi/config.rb @@ -38,20 +38,22 @@ def initialize @targets = [] end - # Whether the config includes a target with the given name. + # Whether the config includes a target with the given name and portal. # # @param name [String] + # @param portal [String] # @return [Boolean] - def include_target?(name) - !find_target(name).nil? + def include_target?(name, portal) + !find_target(name, portal).nil? end - # Searchs for a target with the given name. + # Searchs for a target with the given name and portal. # # @param name [String] + # @param portal [String] # @return [Configs::Target, nil] - def find_target(name) - targets.find { |t| t.name == name } + def find_target(name, portal) + targets.find { |t| t.name == name && t.portal?(portal) } end # All portals. diff --git a/service/lib/agama/storage/iscsi/manager.rb b/service/lib/agama/storage/iscsi/manager.rb index 2ce7c2e95d..20aa639309 100644 --- a/service/lib/agama/storage/iscsi/manager.rb +++ b/service/lib/agama/storage/iscsi/manager.rb @@ -62,9 +62,9 @@ def probed? # Probes iSCSI. def probe - @probed = true probe_initiator probe_nodes + @probed = true end # Performs an iSCSI discovery. @@ -155,11 +155,11 @@ def probe_initiator # @return [Array] def probe_nodes @nodes = adapter.read_nodes - # Targets are set as locked if they are already connected at the time of probing for first + # Nodes are set as locked if they are already connected at the time of probing for first # time. This usually happens when there is iBFT activation or the targets are manually # connected (e.g., by using a script). - @locked_targets ||= @nodes.select(&:connected).map(&:target) - @locked_targets.each { |t| find_node(t)&.locked = true } + init_locked_nodes(@nodes.select(&:connected)) unless probed? + locked_nodes.each { |n| n.locked = true } @nodes end @@ -188,7 +188,7 @@ def nodes_configured?(config) # # @return [Boolean] def node_configured?(node, config) - target_config = config.find_target(node.target) + target_config = config.find_target(node.target, node.portal) if target_config node.connected? && @@ -234,7 +234,7 @@ def disconnect_missing_targets(config) nodes .select(&:connected?) .reject(&:locked?) - .reject { |n| config.include_target?(n.target) } + .reject { |n| config.include_target?(n.target, n.portal) } .each { |n| disconnect_node(n) } end @@ -249,7 +249,7 @@ def configure_targets(config) # # @param target_config [ISCSI::Configs::Target] def configure_target(target_config) - node = find_node(target_config.name) + node = find_node(target_config) return unless node if node.connected? @@ -294,7 +294,7 @@ def disconnect_node(node) logger.info("Disconnecting iSCSI node: #{node.inspect}") adapter.logout(node).tap do |success| # Unlock the node if it was correctly disconnected. - @locked_targets&.delete(node.target) if success + unregister_locked_node(node) if success end end @@ -324,7 +324,7 @@ def update_node(node, target_config) # @param target_config [ISCSI::Configs::Target] # @return [Boolean] def credentials_changed?(target_config) - previous_credentials = previous_config&.find_target(target_config.name)&.credentials + previous_credentials = find_previous_target(target_config)&.credentials previous_credentials != target_config.credentials end @@ -333,16 +333,60 @@ def credentials_changed?(target_config) # @param target_config [ISCSI::Configs::Target] # @return [Boolean] def startup_changed?(target_config) - previous_startup = previous_config&.find_target(target_config.name)&.startup + previous_startup = find_previous_target(target_config)&.startup previous_startup != target_config.startup end - # Finds a node with the given name. + # Finds the equivalent target in the previous configuration, if any. + # + # @param target_config [ISCSI::Configs::Target] + # @return [ISCSI::Configs::Target, nil] + def find_previous_target(target_config) + previous_config&.find_target(target_config.name, target_config.portal) + end + + # Finds the node corresponding to the given target configuration. # - # @param name [String] + # @param target_config [ISCSI::Configs::Target] # @return [Node, nil] - def find_node(name) - nodes.find { |n| n.target == name } + def find_node(target_config) + nodes.find { |n| n.target == target_config.name && n.portal == target_config.portal } + end + + # Nodes that should be marked as locked according to the status of the system in the first + # probe, no matter the value of their Node#locked attribute + # + # @return [Array] + def locked_nodes + @locked_node_ids.map do |i| + nodes.find { |n| n.target == i[:target] && n.portal == i[:portal] } + end.compact + end + + # Method to be called during the initial probing in order to identify the nodes that + # should be marked as locked. + # + # @param nodes [Array] + def init_locked_nodes(nodes) + @locked_node_ids = [] + nodes.each { |n| register_locked_node(n) } + end + + # @see #locked_nodes + # + # @param node [Node] + def register_locked_node(node) + @locked_node_ids ||= [] + @locked_node_ids << { target: node.target, portal: node.portal } + end + + # @see #locked_nodes + # + # @param node [Node] + def unregister_locked_node(node) + return unless @locked_node_ids + + @locked_node_ids.delete({ target: node.target, portal: node.portal }) end end end diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 54c5e1616c..dec4da714b 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Feb 4 14:01:15 UTC 2026 - Ancor Gonzalez Sosa + +- Identify iSCSI nodes using the combination of name and portal + (gh#agama-project/agama#3123). + ------------------------------------------------------------------- Mon Feb 2 08:13:48 UTC 2026 - Josef Reidinger diff --git a/service/test/agama/storage/iscsi/manager_test.rb b/service/test/agama/storage/iscsi/manager_test.rb index be3a069090..8772e52c73 100644 --- a/service/test/agama/storage/iscsi/manager_test.rb +++ b/service/test/agama/storage/iscsi/manager_test.rb @@ -43,7 +43,7 @@ Agama::Storage::ISCSI::Node.new.tap do |target| target.target = "iqn.2023-01.com.example:12ac588" target.address = "192.168.100.102" - target.port = 3264 + target.port = 3260 target.interface = "default" target.ibft = true target.startup = "onboot" @@ -56,7 +56,7 @@ Agama::Storage::ISCSI::Node.new.tap do |target| target.target = "iqn.2023-01.com.example:12aca" target.address = "192.168.100.110" - target.port = 3264 + target.port = 3260 target.interface = "default" target.ibft = false target.startup = "manual" @@ -69,7 +69,7 @@ Agama::Storage::ISCSI::Node.new.tap do |target| target.target = "iqn.2023-01.com.example:12123" target.address = "192.168.100.110" - target.port = 3264 + target.port = 3260 target.interface = "default" target.ibft = false target.startup = "manual" @@ -147,11 +147,11 @@ it "performs iSCSI discovery without credentials" do expect(adapter).to receive(:discover) do |host, port, options| expect(host).to eq("192.168.100.101") - expect(port).to eq(3264) + expect(port).to eq(3260) expect(options).to eq({ credentials: {} }) end expect(subject).to receive(:probe) - subject.discover("192.168.100.101", 3264) + subject.discover("192.168.100.101", 3260) end it "performs iSCSI discovery with credentials" do @@ -163,11 +163,11 @@ } expect(adapter).to receive(:discover) do |host, port, options| expect(host).to eq("192.168.100.101") - expect(port).to eq(3264) + expect(port).to eq(3260) expect(options).to eq({ credentials: credentials }) end expect(subject).to receive(:probe) - subject.discover("192.168.100.101", 3264, credentials: credentials) + subject.discover("192.168.100.101", 3260, credentials: credentials) end end @@ -354,6 +354,65 @@ end end end + + context "if several targets share the name but in different portals" do + before do + allow(adapter).to receive(:read_nodes).and_return([node1, node2]) + end + + let(:node1) do + Agama::Storage::ISCSI::Node.new.tap do |target| + target.target = "iqn.2023-01.com.example:12ac588" + target.address = "192.168.100.102" + target.port = 3260 + target.interface = "default" + target.ibft = false + target.startup = "onboot" + target.connected = true + target.locked = true + end + end + + let(:node2) do + Agama::Storage::ISCSI::Node.new.tap do |target| + target.target = "iqn.2023-01.com.example:12ac588" + target.address = "192.168.100.102" + target.port = 3265 + target.interface = "default" + target.ibft = false + target.startup = "manual" + target.connected = false + target.locked = false + end + end + + let(:targets) do + [ + { + name: "iqn.2023-01.com.example:12ac588", + address: "192.168.100.102", + port: 3260, + interface: "default", + startup: "onboot" + }, + { + name: "iqn.2023-01.com.example:12ac588", + address: "192.168.100.102", + port: 3265, + interface: "default", + startup: "onboot" + } + ] + end + + it "performs the correct operation for each node" do + expect(adapter).to_not receive(:logout).with(node1) + expect(adapter).to_not receive(:login).with(node1) + expect(adapter).to_not receive(:logout).with(node2) + expect(adapter).to receive(:login).with(node2, anything) + subject.configure(config_json) + end + end end end @@ -547,5 +606,24 @@ end end end + + context "if the given config contains a target with the same name but different portal" do + let(:config_json) do + { + initiator: previous_config_json[:initiator], + targets: [target] + } + end + + let(:target) do + new_target = previous_config_json[:targets].first.clone + new_target[:address] = "192.168.100.1" + new_target + end + + it "returns false" do + expect(subject.configured?(config_json)).to eq(false) + end + end end end