Skip to content

Commit

Permalink
Avoid redundant wicked firmware interfaces calls
Browse files Browse the repository at this point in the history
  • Loading branch information
teclator committed May 4, 2023
1 parent 8f7397c commit 7e69365
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 31 deletions.
10 changes: 6 additions & 4 deletions src/lib/network/wicked.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def query_wicked(iface, query)
#
# @return [Array <String>] array of interface names
def ibft_interfaces
Yast::Execute.stdout.locally!(IBFT_CMD, "-l").split(/\s+/).uniq
@ibft_interfaces ||= Yast::Execute.stdout.locally!(IBFT_CMD, "-l").split(/\s+/).uniq
end

# Returns an array of interface names which are configured via firmware
Expand All @@ -100,17 +100,19 @@ def firmware_interfaces
#
# @return [Hash] configured by firmware interfaces indexed by the firmware extension
def firmware_interfaces_by_extension
return @firmware_interfaces_by_extension if @firmware_interfaces_by_extension

output = Yast::Execute.stdout.locally!(WICKED_PATH, "firmware", "interfaces")
output.split("\n").each_with_object({}) do |line, result|
@firmware_interfaces_by_extension = output.split("\n").each_with_object({}) do |line, result|
firmware, *interfaces = line.split(/\s+/)
result[firmware] = result.fetch(firmware, []) + interfaces if firmware
result[firmware.to_sym] = result.fetch(firmware.to_sym, []) + interfaces if firmware
end
end

# Returns the firmware extension used for configuring the given interface or nil when it is not
# configured by firmware
#
# @return [String, nil] Firmware extension used for configuring the interface or nil
# @return [Symbol, nil] Firmware extension used for configuring the interface or nil
def firmware_configured_by?(interface)
firmware_interfaces_by_extension.find { |_, v| v.include?(interface) }&.first
end
Expand Down
7 changes: 7 additions & 0 deletions src/lib/y2network/interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class Interface
attr_accessor :renaming_mechanism
# @return [String,nil]
attr_reader :old_name
# @return [Symbol,nil]
attr_accessor :firmware_configured_by

class << self
# Builds an interface based on a connection
Expand Down Expand Up @@ -157,5 +159,10 @@ def hotplug?

["usb", "pcmcia"].include?(hardware.hotplug)
end

# @return [Boolean] whether the interface is firmware configured or not
def firmware_configured?
!!firmware_configured_by
end
end
end
8 changes: 3 additions & 5 deletions src/lib/y2network/presenters/interface_summary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

require "yast"
require "y2network/presenters/interface_status"
require "network/wicked"

Yast.import "Summary"
Yast.import "HTML"
Expand All @@ -31,7 +30,6 @@ module Presenters
class InterfaceSummary
include Yast::I18n
include InterfaceStatus
include Yast::Wicked

# @return [String]
attr_reader :name
Expand Down Expand Up @@ -106,10 +104,10 @@ def text
rich << Yast::HTML.Bold(dev_name) << "<br>"

end
extension = firmware_configured_by?(hardware&.name)

if extension
rich << "<p><b>" << _("The device is configured by: ") << "</b>" << extension << "</p>"
if interface.firmware_configured?
rich << "<p><b>" << _("The device is configured by: ") << "</b>"
rich << interface.firmware_configured_by.to_s << "</p>"
else
rich << "<p>"
rich << _("The device is not configured. Press <b>Edit</b>\nto configure.\n")
Expand Down
3 changes: 3 additions & 0 deletions src/lib/y2network/wicked/interfaces_reader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# find current contact information at www.suse.com.

require "yast"
require "network/wicked"
require "y2network/interface"
require "y2network/interface_type"
require "y2network/virtual_interface"
Expand All @@ -40,6 +41,7 @@ module Wicked
#
# @see Y2Network::InterfacesCollection
class InterfacesReader
include Yast::Wicked
# Returns the collection of s390 group devices
#
# @return [Array<Y2Network::ConnectionConfig::Base>] Array of connection
Expand Down Expand Up @@ -97,6 +99,7 @@ def build_physical_interface(hwinfo)
iface.custom_driver = custom_driver_for(iface)
iface.type = InterfaceType.from_short_name(hwinfo.type) ||
TypeDetector.type_of(iface.name) || InterfaceType::UNKNOWN
iface.firmware_configured_by = firmware_configured_by?(iface.name)
end
end

Expand Down
7 changes: 7 additions & 0 deletions src/lib/y2network/widgets/delete_interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ def handle
:redraw
end

def disable?
return true unless @table.value
return true unless connection_config

false
end

def help
# TRANSLATORS: Help for 'Delete' interface configuration button.
_(
Expand Down
12 changes: 2 additions & 10 deletions src/lib/y2network/widgets/edit_interface.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,9 @@ def handle

def disable?
return true unless @table.value
return true if config.interfaces.by_name(@table.value)&.firmware_configured?

configured_by_firmware?
end

def configured_by_firmware?
return false if connection_config
return false unless config.backend?(:wicked)

require "network/wicked"
singleton_class.include Yast::Wicked
firmware_interfaces.include?(@table.value)
false
end

def help
Expand Down
3 changes: 1 addition & 2 deletions src/lib/y2network/widgets/interface_button.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ def selected_interface(config)
end

def disable?
return true unless @table.value
return true unless connection_config
false
end
end
end
Expand Down
7 changes: 2 additions & 5 deletions src/lib/y2network/widgets/interfaces_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

require "yast"
require "cwm/table"
require "network/wicked"
require "y2network/presenters/interface_summary"
require "y2network/presenters/s390_group_device_summary"

Expand All @@ -30,8 +29,6 @@
module Y2Network
module Widgets
class InterfacesTable < CWM::Table
include Yast::Wicked

def initialize(description)
textdomain "network"

Expand Down Expand Up @@ -140,7 +137,7 @@ def interface_item(interface)
end

def interface_protocol(connection)
return _("Not Configured") if connection.nil?
return _("Not configured") if connection.nil?

bootproto = connection.bootproto.name

Expand All @@ -155,7 +152,7 @@ def interface_protocol(connection)
def configuration_for(interface, connection)
return interface_protocol(connection) unless connection.nil?

firmware_configured?(interface) ? _("Configured by firmware") : _("Not Configured")
interface.firmware_configured? ? _("Configured by firmware") : _("Not configured")
end

def selected_item
Expand Down
14 changes: 14 additions & 0 deletions test/y2network/wicked/interfaces_reader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,20 @@
it "reads bonding interfaces"
it "reads interfaces configuration"

context "when the interface is configured by hardware" do
let(:firmware_interfaces ) { { :ibft => ["eth0"] } }

before do
allow(reader).to receive(:firmware_interfaces_by_extension).and_return(firmware_interfaces)
end

it "sets the extensions used for the configuring it" do
eth0 = reader.interfaces.by_name("eth0")
expect(eth0.firmware_configured_by).to eql(:ibft)
expect(eth0.firmware_configured?).to eql(true)
end
end

context "when a physical interface type is unknown" do
before do
allow(Yast::SCR).to receive(:Dir).with(Yast::Path.new(".network.section"))
Expand Down
13 changes: 12 additions & 1 deletion test/y2network/widgets/delete_interface_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@
let(:eth0) { Y2Network::Interface.new("eth0") }
let(:br0) { Y2Network::VirtualInterface.new("br0", type: Y2Network::InterfaceType::BRIDGE) }
let(:interfaces) { Y2Network::InterfacesCollection.new([eth0, br0]) }
let(:connections) { Y2Network::ConnectionConfigsCollection.new([eth0_conn, br0_conn]) }
let(:conn_collection) { [eth0_conn, br0_conn] }
let(:connections) { Y2Network::ConnectionConfigsCollection.new(conn_collection) }

let(:eth0_conn) do
Y2Network::ConnectionConfig::Ethernet.new.tap do |c|
c.name = "eth0"
Expand Down Expand Up @@ -65,6 +67,15 @@
expect(subject).to_not receive(:disable)
subject.init
end

context "but it is not configured" do
let(:conn_collection) { [] }

it "disables the widget" do
expect(subject).to receive(:disable)
subject.init
end
end
end

context "when no element is selected" do
Expand Down
32 changes: 28 additions & 4 deletions test/y2network/widgets/edit_interface_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
Y2Network::Config.new(interfaces: interfaces, connections: connections, source: :wicked)
end
let(:eth0) { Y2Network::PhysicalInterface.new("eth0") }
let(:eth1) { Y2Network::PhysicalInterface.new("eth1") }
let(:eth1) do
Y2Network::PhysicalInterface.new("eth1").tap do |i|
i.firmware_configured_by = :nbft
end
end
let(:interfaces) { Y2Network::InterfacesCollection.new([eth0, eth1]) }
let(:eth0_conn) do
Y2Network::ConnectionConfig::Ethernet.new.tap do |conn|
Expand All @@ -51,9 +55,20 @@

describe "#init" do
context "when an element is selected" do
it "does not disable the widget" do
expect(subject).to_not receive(:disable)
subject.init
context "and the selected interface is configured by firmware" do
let(:selected) { "eth1" }

it "disables the widget" do
expect(subject).to receive(:disable)
subject.init
end
end

context "and the selected interface is not configured by firmware" do
it "does not disable the widget" do
expect(subject).to_not receive(:disable)
subject.init
end
end
end

Expand All @@ -65,6 +80,15 @@
subject.init
end
end

context "when the selected interface is configured by firmware" do
let(:selected) { "eth1" }

it "disables the widget" do
expect(subject).to receive(:disable)
subject.init
end
end
end

describe "#handle" do
Expand Down

0 comments on commit 7e69365

Please sign in to comment.