Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions service/lib/agama/storage/iscsi/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
72 changes: 58 additions & 14 deletions service/lib/agama/storage/iscsi/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ def probed?

# Probes iSCSI.
def probe
@probed = true
probe_initiator
probe_nodes
@probed = true
end

# Performs an iSCSI discovery.
Expand Down Expand Up @@ -155,11 +155,11 @@ def probe_initiator
# @return [Array<ISCSI::Node>]
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

Expand Down Expand Up @@ -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? &&
Expand Down Expand Up @@ -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

Expand All @@ -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?
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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<Node>]
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<Node>]
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
Expand Down
6 changes: 6 additions & 0 deletions service/package/rubygem-agama-yast.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Wed Feb 4 14:01:15 UTC 2026 - Ancor Gonzalez Sosa <ancor@suse.com>

- 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 <jreidinger@suse.com>

Expand Down
92 changes: 85 additions & 7 deletions service/test/agama/storage/iscsi/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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