diff --git a/package/yast2-network.changes b/package/yast2-network.changes index c420855d8..3d8f5ee1a 100644 --- a/package/yast2-network.changes +++ b/package/yast2-network.changes @@ -1,3 +1,12 @@ +------------------------------------------------------------------- +Thu Apr 2 11:33:49 UTC 2020 - Imobach Gonzalez Sosa + +- AutoYaST: Do not try to activate network devices that are already + active in S390 systems (bsc#1163149). Related to jsc#SLE-7396. +- AutoYaST: Allow to use spaces or colons to separate channel IDs + in the "chanids" element. +- 4.2.65 + ------------------------------------------------------------------- Fri Mar 27 14:30:34 UTC 2020 - Knut Anderssen diff --git a/package/yast2-network.spec b/package/yast2-network.spec index d749f7ef0..caddc07b1 100644 --- a/package/yast2-network.spec +++ b/package/yast2-network.spec @@ -17,7 +17,7 @@ Name: yast2-network -Version: 4.2.64 +Version: 4.2.65 Release: 0 Summary: YaST2 - Network Configuration License: GPL-2.0-only diff --git a/src/lib/network/network_autoyast.rb b/src/lib/network/network_autoyast.rb index 5ecff83b3..fdb4bf7fc 100644 --- a/src/lib/network/network_autoyast.rb +++ b/src/lib/network/network_autoyast.rb @@ -125,15 +125,20 @@ def activate_s390_devices(section = nil) devices_section = Y2Network::AutoinstProfile::S390DevicesSection .new_from_hashes(profile_devices) connections = Y2Network::Autoinst::S390DevicesReader.new(devices_section).config - connections.each do |conn| + connections.each do |conn| builder = Y2Network::InterfaceConfigBuilder.for(conn.type, config: conn) activator = Y2Network::S390DeviceActivator.for(builder) + if !activator.configured_interface.empty? + log.info "Interface #{activator.configured_interface} is already active. " \ + "Skipping the activation." + next + end + log.info "Created interface #{activator.configured_interface}" if activator.configure rescue RuntimeError => e log.error("An error ocurred when trying to activate the s390 device: #{conn.inspect}") - log.error("Error: #{e.sinpect}") - + log.error("Error: #{e.inspect}") end true diff --git a/src/lib/y2network/autoinst/s390_devices_reader.rb b/src/lib/y2network/autoinst/s390_devices_reader.rb index 468485310..b449999cd 100644 --- a/src/lib/y2network/autoinst/s390_devices_reader.rb +++ b/src/lib/y2network/autoinst/s390_devices_reader.rb @@ -66,18 +66,29 @@ def create_config(device_section) end def load_qeth(config, device_section) - chanids = device_section.chanids - config.read_channel, config.write_channel, config.data_channel = chanids.split(" ") + config.read_channel, config.write_channel, config.data_channel = + chanids_from(device_section.chanids) config.layer2 = device_section.layer2 end def load_ctc(config, device_section) - config.read_channel, config.write_channel = device_section.chanids.split(" ") + config.read_channel, config.write_channel = chanids_from(device_section.chanids) config.protocol = device_section.protocol end def load_lcs(config, device_section) - config.read_channel, config.write_channel = device_section.chanids.split(" ") + config.read_channel, config.write_channel = chanids_from(device_section.chanids) + end + + # Separator used for the list of channel IDs + CHANIDS_SEPARATOR = ":".freeze + private_constant :CHANIDS_SEPARATOR + + # Returns the list of channel IDs from a string + # + # @return [Array] + def chanids_from(ids) + ids.to_s.split(CHANIDS_SEPARATOR) end end end diff --git a/src/lib/y2network/autoinst_profile/s390_device_section.rb b/src/lib/y2network/autoinst_profile/s390_device_section.rb index aaf3735fc..1908ec756 100644 --- a/src/lib/y2network/autoinst_profile/s390_device_section.rb +++ b/src/lib/y2network/autoinst_profile/s390_device_section.rb @@ -44,7 +44,7 @@ def self.attributes define_attr_accessors # @!attribute chanids - # @return [String] channel device id separated by spaces + # @return [String] channel device id separated by spaces or colons # @!attribute layer2 # @return [Boolean] Whether layer2 is enabler or not @@ -71,6 +71,17 @@ def self.new_from_network(connection_config) result end + # Creates an instance based on the profile representation used by the AutoYaST modules + # (array of hashes objects). + # + # @param hash [Hash] Networking section from an AutoYaST profile + # @return [S390DeviceSection] + def self.new_from_hashes(hash) + result = new + result.init_from_hashes(hash) + result + end + # Method used by {.new_from_network} to populate the attributes when cloning a network s390 # device # @@ -91,6 +102,25 @@ def init_from_config(config) true end + + # Method used by {.new_from_hashes} to populate the attributes when importing a profile + # + # @param hash [Hash] see {.new_from_hashes} + def init_from_hashes(hash) + super + self.chanids = normalized_chanids(hash["chanids"]) if hash["chanids"] + end + + private + + # Normalizes the list of channel IDs + # + # It replaces spaces with colons. + # + # @return [String] + def normalized_chanids(ids) + ids.gsub(/\ +/, ":") + end end end end diff --git a/src/lib/y2network/hwinfo.rb b/src/lib/y2network/hwinfo.rb index b7f888e75..c0d4c61d4 100644 --- a/src/lib/y2network/hwinfo.rb +++ b/src/lib/y2network/hwinfo.rb @@ -37,7 +37,7 @@ def netcards read_hardware @netcards = ReadHardware("netcard").map do |attrs| name = attrs["dev_name"] - extra_attrs = name ? extra_attrs_for(name) : {} + extra_attrs = name.to_s.empty? ? {} : extra_attrs_for(name) Hwinfo.new(attrs.merge(extra_attrs)) end end diff --git a/test/network_autoyast_test.rb b/test/network_autoyast_test.rb index 0c5fd0554..cc6d453ec 100755 --- a/test/network_autoyast_test.rb +++ b/test/network_autoyast_test.rb @@ -23,6 +23,8 @@ require "network/network_autoyast" require "y2network/sysconfig/config_reader" +require "y2network/s390_device_activators/qeth" + Yast.import "Profile" Yast.import "Lan" @@ -30,7 +32,7 @@ subject(:network_autoyast) { Yast::NetworkAutoYast.instance } let(:config) do Y2Network::Config.new( - interfaces: [], + interfaces: Y2Network::InterfacesCollection.new([]), routing: Y2Network::Routing.new(tables: []), dns: Y2Network::DNS.new, source: :sysconfig @@ -341,4 +343,54 @@ def keep_install_network_value(value) end end end + + describe "#activate_s390_devices" do + let(:section) do + [ + { + "chanids" => "0.0.0800 0.0.0801 0.0.0802", + "type" => "qeth" + } + ] + end + + let(:activator) do + instance_double( + Y2Network::S390DeviceActivators::Qeth, + configured_interface: configured_interface + ) + end + + let(:configured_interface) { "" } + + before do + allow(Y2Network::S390DeviceActivators::Qeth).to receive(:new) + .and_return(activator) + end + + it "activates the given device" do + expect(activator).to receive(:configure) + subject.activate_s390_devices(section) + end + + context "if the device is already active" do + let(:configured_interface) { "eth0" } + + it "does not activate the device" do + expect(activator).to_not receive(:configure) + subject.activate_s390_devices(section) + end + end + + context "if the activation fails" do + before do + allow(activator).to receive(:configure).and_raise(RuntimeError) + end + + it "logs the error" do + expect(subject.log).to receive(:error).at_least(1) + subject.activate_s390_devices(section) + end + end + end end diff --git a/test/y2network/autoinst/s390_devices_reader_test.rb b/test/y2network/autoinst/s390_devices_reader_test.rb index a4ba53352..afa4dacac 100755 --- a/test/y2network/autoinst/s390_devices_reader_test.rb +++ b/test/y2network/autoinst/s390_devices_reader_test.rb @@ -51,8 +51,19 @@ describe "#config" do it "builds a new Y2Network::ConnectionConfigsCollection" do - expect(subject.config).to be_a Y2Network::ConnectionConfigsCollection - expect(subject.config.size).to eq(2) + expect(subject.config).to contain_exactly( + an_object_having_attributes( + read_channel: "0.0.0700", + write_channel: "0.0.0701", + data_channel: "0.0.0702" + ), + an_object_having_attributes( + read_channel: "0.0.0800", + write_channel: "0.0.0801", + data_channel: "0.0.0802", + layer2: true + ) + ) end end end diff --git a/test/y2network/autoinst_profile/s390_device_section_test.rb b/test/y2network/autoinst_profile/s390_device_section_test.rb index 745a00106..5428b67d7 100644 --- a/test/y2network/autoinst_profile/s390_device_section_test.rb +++ b/test/y2network/autoinst_profile/s390_device_section_test.rb @@ -62,7 +62,23 @@ it "loads properly boot protocol" do section = described_class.new_from_hashes(hash) expect(section.type).to eq "ctc" + expect(section.chanids).to eq("0.0.0800:0.0.0801") expect(section.protocol).to eq "1" end + + context "using the old syntax for chanids" do + let(:hash) do + { + "type" => "ctc", + "chanids" => "0.0.0800 0.0.0801", + "protocol" => "1" + } + end + + it "loads properly boot protocol" do + section = described_class.new_from_hashes(hash) + expect(section.chanids).to eq("0.0.0800:0.0.0801") + end + end end end