Merge c883a35 into 113ed20

package/yast2-network.changes

@@ -1,3 +1,12 @@
+-------------------------------------------------------------------
+Thu Apr  2 11:33:49 UTC 2020 - Imobach Gonzalez Sosa <igonzalezsosa@suse.com>
+
+- 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 <kanderssen@suse.com>
 

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

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

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<String>]
+      def chanids_from(ids)
+        ids.to_s.split(CHANIDS_SEPARATOR)
       end
     end
   end

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

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

test/network_autoyast_test.rb

@@ -23,14 +23,16 @@
 
 require "network/network_autoyast"
 require "y2network/sysconfig/config_reader"
+require "y2network/s390_device_activators/qeth"
+
 Yast.import "Profile"
 Yast.import "Lan"
 
 describe "NetworkAutoYast" do
   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

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

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