From 3b5b11d64a99b17980ce97cebdefe9c4595f40d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Mon, 18 May 2020 10:03:45 +0100 Subject: [PATCH 01/10] Added Autoinst::Config class for storing AY specific config --- src/lib/y2network/autoinst/config.rb | 59 +++++++++++++++++++ .../autoinst_profile/networking_section.rb | 17 +++++- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 src/lib/y2network/autoinst/config.rb diff --git a/src/lib/y2network/autoinst/config.rb b/src/lib/y2network/autoinst/config.rb new file mode 100644 index 000000000..ed751b6d3 --- /dev/null +++ b/src/lib/y2network/autoinst/config.rb @@ -0,0 +1,59 @@ +# Copyright (c) [2019] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +module Y2Network + module Autoinst + # This class is responsible of storing network settings that are only + # relevant to the autoinstallation proccess. + class Config + # @return [Boolean] controls whether the network configuration should be + # run before the proposal + attr_accessor :before_proposal + # @return [Boolean] controls whether the network should be restarted + # immediately after write or not + attr_accessor :start_immediately + # @return [Boolean] controls whether the configuration done during the + # installation should be copied to the target system at the end + attr_accessor :keep_install_network + # @return [Integer] + attr_accessor :ip_check_timeout + + # Constructor + # + # @param opts [Hash] + # @option opts [Boolean] :before_proposal + # @option opts [Boolean] :start_immediately + # @option opts [Boolean] :keep_install_network + # @option opts [Integer] :ip_check_timetout + def initialize(opts = {}) + @before_proposal = opts.fetch(:before_proposal, false) + @start_immediately = opts.fetch(:start_immediately, false) + @keep_install_network = opts.fetch(:keep_install_network, true) + @ip_check_timeout = opts.fetch(:ip_check_timeout, -1) + end + + # Return whether the network should be copied at the end + # + # @return [Boolean] + def copy_network? + keep_install_network || before_proposal + end + end + end +end diff --git a/src/lib/y2network/autoinst_profile/networking_section.rb b/src/lib/y2network/autoinst_profile/networking_section.rb index cbe15993e..ca30be0fa 100644 --- a/src/lib/y2network/autoinst_profile/networking_section.rb +++ b/src/lib/y2network/autoinst_profile/networking_section.rb @@ -35,6 +35,17 @@ module AutoinstProfile # # @see RoutingSection class NetworkingSection + + # @return [Boolean] + attr_accessor :setup_before_proposal + # @return [Boolean] + attr_accessor :start_immediately + # @return [Boolean] + attr_accessor :keep_install_network + # @return [Integer] + attr_accessor :strict_ip_check_timeout + + # @return [RoutingSection] attr_accessor :routing # @return [DNSSection] @@ -53,6 +64,10 @@ class NetworkingSection # @return [NetworkingSection] def self.new_from_hashes(hash) result = new + result.setup_before_proposal = hash.fetch("setup_before_proposal", false) + result.start_immediately = hash.fetch("start_immediately", false) + result.keep_install_network = hash.fetch("keep_install_network", true) + result.strict_ip_check_timeout = hash.fetch("strict_ip_check_timeout", -1) result.routing = RoutingSection.new_from_hashes(hash["routing"]) if hash["routing"] result.dns = DNSSection.new_from_hashes(hash["dns"]) if hash["dns"] if hash["interfaces"] @@ -92,7 +107,7 @@ def to_hashes "dns" => dns.to_hashes, "interfaces" => interfaces.to_hashes, "net-udev" => udev_rules.to_hashes, - "s390-devices" => s390_devices.to_hashes + "s390-devices" => s390_devices.to_hashes, } end end From 18dedb8fa0b78d37c70d517260fe2c15e83c0e93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Mon, 18 May 2020 10:08:46 +0100 Subject: [PATCH 02/10] Use the new Autoinst::Config instead of old LanItems references --- src/clients/lan_auto.rb | 23 ++++------- src/lib/network/network_autoyast.rb | 4 +- src/modules/Lan.rb | 62 +++++++++++++++++++---------- src/modules/LanItems.rb | 15 ------- 4 files changed, 51 insertions(+), 53 deletions(-) diff --git a/src/clients/lan_auto.rb b/src/clients/lan_auto.rb index 3fdbd90d8..e70b25794 100644 --- a/src/clients/lan_auto.rb +++ b/src/clients/lan_auto.rb @@ -25,6 +25,8 @@ module Yast # Client providing autoyast functionality class LanAutoClient < Client + include Yast::Logger + def main Yast.import "UI" @@ -96,23 +98,14 @@ def main result = Lan.WriteOnly Builtins.y2error("Writing lan config failed") if !result @ret &&= result + timeout = Lan.autoinst.ip_check_timeout || -1 - if Ops.get(LanItems.autoinstall_settings, "strict_IP_check_timeout") - if Lan.isAnyInterfaceDown - @timeout = Ops.get_integer( - LanItems.autoinstall_settings, - "strict_IP_check_timeout", - 0 - ) - Builtins.y2debug("timeout %1", @timeout) - @error_text = _("Configuration Error: uninitialized interface.") - if @timeout == 0 - Popup.Error(@error_text) - else - Popup.TimedError(@error_text, @timeout) - end - end + if (timeout >= 0) && Lan.isAnyInterfaceDown + Builtins.y2debug("timeout %1", timeout) + error_text = _("Configuration Error: uninitialized interface.") + timeout == 0 ? Popup.Error(error_text) : Popup.TimedError(error_text, timeout) end + Progress.set(@progress_orig) else Builtins.y2error("unknown function: %1", @func) diff --git a/src/lib/network/network_autoyast.rb b/src/lib/network/network_autoyast.rb index fdb4bf7fc..fc6f83048 100644 --- a/src/lib/network/network_autoyast.rb +++ b/src/lib/network/network_autoyast.rb @@ -42,10 +42,10 @@ def initialize # import has to be done here, there are some collisions otherwise Yast.import "Arch" Yast.import "Lan" - Yast.import "LanItems" Yast.import "Linuxrc" Yast.import "Host" Yast.import "AutoInstall" + Yast.import "Stage" end # Merges existing config from system into given configuration map @@ -165,7 +165,7 @@ def configure_hosts(write: false) # Checks if the profile asks for keeping installation network configuration def keep_net_config? - ret = ay_networking_section.fetch("keep_install_network", true) + ret = Lan.autoinst.keep_install_network log.info("NetworkAutoYast: keep installation network: #{ret}") diff --git a/src/modules/Lan.rb b/src/modules/Lan.rb index 96bbfcdf8..819e72736 100644 --- a/src/modules/Lan.rb +++ b/src/modules/Lan.rb @@ -34,6 +34,7 @@ require "ui/text_helpers" require "y2firewall/firewalld" require "y2network/autoinst_profile/networking_section" +require "y2network/autoinst/config" require "y2network/config" require "y2network/virtualization_config" require "y2network/interface_config_builder" @@ -106,6 +107,11 @@ def main @configs = {} end + # @return [Boolean] + attr_accessor :write_only + # @return [Autoinst::Config] + attr_accessor :autoinst + #------------------ # GLOBAL FUNCTIONS #------------------ @@ -466,7 +472,7 @@ def Write(gui: true) step_labels << _("Writing firewall configuration") if firewalld.installed? # Progress stage 9 - step_labels = Builtins.add(step_labels, _("Activate network services")) if !@write_only + step_labels = Builtins.add(step_labels, _("Activate network services")) if !write_only # Progress stage 10 step_labels = Builtins.add(step_labels, _("Update configuration")) @@ -539,7 +545,7 @@ def Write(gui: true) Builtins.sleep(sl) end - if !@write_only + if !write_only return false if Abort() # Progress step 9 @@ -554,7 +560,7 @@ def Write(gui: true) # Progress step 10 ProgressNextStage(_("Updating configuration...")) - update_mta_config if !@write_only + update_mta_config if !write_only Builtins.sleep(sl) if NetworkService.is_network_manager @@ -587,14 +593,14 @@ def Write(gui: true) # Only write configuration without starting any init scripts and SuSEconfig # @return true on success def WriteOnly - @write_only = !Ops.get_boolean( - LanItems.autoinstall_settings, - "start_immediately", - false - ) + @write_only = !autoinst.start_immediately Write() end + def autoinst + @autoinst ||= Y2Network::Autoinst::Config.new + end + # If there's key in modify, upcase key and assign the value to ret # @return ret def UpcaseCondSet(ret, modify, key) @@ -649,12 +655,15 @@ def FromAY(input) def Import(settings) settings = {} if settings.nil? - Lan.Read(:cache) + Read(:cache) profile = Y2Network::AutoinstProfile::NetworkingSection.new_from_hashes(settings) config = Y2Network::Config.from(:autoinst, profile, system_config) add_config(:yast, config) + @autoinst = autoinst_config(profile) + if Arch.s390 + NetworkAutoYast.instance.activate_s390_devices(settings.fetch("s390-devices", {})) + end - LanItems.Import(settings) NetworkConfig.Import(settings["config"] || {}) # Ensure that the /etc/hosts has been read to no blank out it in case of # not defined section (bsc#1058396) @@ -682,18 +691,10 @@ def Export "ipv6" => @ipv6, "routing" => profile.routing&.to_hashes || {}, "managed" => NetworkService.is_network_manager, - "start_immediately" => Ops.get_boolean( - LanItems.autoinstall_settings, - "start_immediately", - false - ), # start_immediately, - "keep_install_network" => Ops.get_boolean( - LanItems.autoinstall_settings, - "keep_install_network", - true - ) + "start_immediately" => autoinst.start_immediately, + "keep_install_network" => autoinst.keep_install_network } - Builtins.y2milestone("Exported map: %1", ay) + log.info("Exported map: #{ay}") deep_copy(ay) end @@ -859,6 +860,25 @@ def yast_config # @return [Array] attr_reader :configs + def autoinst_config(section) + return unless autoinst_settings?(section) + + Y2Network::Autoinst::Config.new( + before_proposal: section.setup_before_proposal, + start_immediately: section.start_immediately, + keep_install_network: section.keep_install_network, + ip_check_timeout: section.strict_ip_check_timeout + ) + end + + def autoinst_settings?(section) + return false unless section + + !section.start_immediately.nil? || + !section.keep_install_network.nil? || + !section.setup_before_proposal.nil? + end + def activate_network_service # If the second installation stage has been called by yast.ssh via # ssh, we should not restart network because systemctl diff --git a/src/modules/LanItems.rb b/src/modules/LanItems.rb index c4df26e60..f58232081 100644 --- a/src/modules/LanItems.rb +++ b/src/modules/LanItems.rb @@ -276,21 +276,6 @@ def reset_cache # @return [Boolean] on success def Import(settings) reset_cache - - @autoinstall_settings["start_immediately"] = settings.fetch("start_immediately", false) - @autoinstall_settings["strict_IP_check_timeout"] = settings.fetch("strict_IP_check_timeout", - -1) - @autoinstall_settings["keep_install_network"] = settings.fetch("keep_install_network", true) - - # FIXME: createS390Device does two things, it - # - updates internal structures - # - creates s390 device eth emulation - # So, it belongs partly into Import and partly into Write. Note, that - # the code is currently unable to revert already created emulated device. - if Arch.s390 - NetworkAutoYast.instance.activate_s390_devices(settings.fetch("s390-devices", {})) - end - # settings == {} has special meaning 'Reset' used by AY SetModified() if !settings.empty? From 4d81c17565826e8d912f73e8fe7c1ecfaaab9649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Mon, 18 May 2020 13:01:38 +0100 Subject: [PATCH 03/10] Added a parameter which indicates if the udev rules should be applied --- src/lib/y2network/sysconfig/interfaces_writer.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/lib/y2network/sysconfig/interfaces_writer.rb b/src/lib/y2network/sysconfig/interfaces_writer.rb index ed90fad2c..e8584be01 100644 --- a/src/lib/y2network/sysconfig/interfaces_writer.rb +++ b/src/lib/y2network/sysconfig/interfaces_writer.rb @@ -34,6 +34,10 @@ module Sysconfig # # @see Y2Network::InterfacesCollection class InterfacesWriter + def initialize(reload: true) + @reload = reload + end + # Writes interfaces hardware configuration and refreshes udev # # @param interfaces [Y2Network::InterfacesCollection] Interfaces collection @@ -44,6 +48,10 @@ def write(interfaces) private + def reload? + @reload + end + # Creates an udev rule to set the driver for the given interface # # @param iface [Interface] Interface to generate the udev rule for @@ -60,7 +68,7 @@ def driver_udev_rule_for(iface) def update_udevd(interfaces) update_renaming_udev_rules(interfaces) update_drivers_udev_rules(interfaces) - reload_udev_rules + reload_udev_rules if reload? end # Writes down the current interfaces udev rules and the custom rules that @@ -105,7 +113,7 @@ def shut_down_old_interfaces(interfaces) # # @param iface_name [String] Interface's name def shut_down_interface(iface_name) - Yast::Execute.on_target("/sbin/ifdown", iface_name) unless Yast::Mode.autoinst + Yast::Execute.on_target("/sbin/ifdown", iface_name) if reload? end end end From a26ad8d3b7807b4c7d1faf14725590a4f0b97506 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Mon, 18 May 2020 14:06:38 +0100 Subject: [PATCH 04/10] Autoinst::ConfigReader: Copy only interfaces config --- src/lib/y2network/autoinst/config_reader.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/y2network/autoinst/config_reader.rb b/src/lib/y2network/autoinst/config_reader.rb index a7c570f3d..20cb2a84f 100644 --- a/src/lib/y2network/autoinst/config_reader.rb +++ b/src/lib/y2network/autoinst/config_reader.rb @@ -48,7 +48,10 @@ def initialize(section, original_config) # @return [Y2Network::Config] Network configuration def config - config = @original_config.copy + config = Y2Network::Config.new(source: :autoinst) + # We need the current interfaces in order to rewrite the config + # properly but the rest should be imported from the profile + config.interfaces = @original_config.interfaces.copy # apply at first udev rules, so interfaces names are correct UdevRulesReader.new(section.udev_rules).apply(config) if section.udev_rules From f526f10cbef1eefd35493a8684cf0789e9a2c689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Mon, 18 May 2020 16:17:53 +0100 Subject: [PATCH 05/10] Remove duplicate entry from .gitignore --- .gitignore | 2 +- .../y2network/sysconfig/interfaces_writer.rb | 6 ++++-- .../sysconfig/interfaces_writer_test.rb | 18 +++++++++++------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index 7296912ac..f2552d385 100644 --- a/.gitignore +++ b/.gitignore @@ -32,4 +32,4 @@ testsuite/run/ .yardoc /coverage *.pot -config.* +/test/data/scr_read/var/log diff --git a/src/lib/y2network/sysconfig/interfaces_writer.rb b/src/lib/y2network/sysconfig/interfaces_writer.rb index e8584be01..f48607af5 100644 --- a/src/lib/y2network/sysconfig/interfaces_writer.rb +++ b/src/lib/y2network/sysconfig/interfaces_writer.rb @@ -23,8 +23,6 @@ require "y2network/sysconfig/interface_file" require "y2network/sysconfig/routes_file" -Yast.import "Mode" - module Y2Network module Sysconfig # This class writes interfaces specific configuration @@ -33,6 +31,7 @@ module Sysconfig # hardware specific configuration through udev rules. # # @see Y2Network::InterfacesCollection + # @param reload [Boolean] whether the udev rules should be reloaded or not class InterfacesWriter def initialize(reload: true) @reload = reload @@ -48,6 +47,9 @@ def write(interfaces) private + # Whether the udev rules should be reloaded or not + # + # @return [Boolean] true if needs to be reloaded after written def reload? @reload end diff --git a/test/y2network/sysconfig/interfaces_writer_test.rb b/test/y2network/sysconfig/interfaces_writer_test.rb index a9ba78d75..a20c551ce 100644 --- a/test/y2network/sysconfig/interfaces_writer_test.rb +++ b/test/y2network/sysconfig/interfaces_writer_test.rb @@ -70,19 +70,23 @@ eth0.rename("eth1", renaming_mechanism) end - it "sets the interface down" do - expect(Yast::Execute).to receive(:on_target).with("/sbin/ifdown", "eth0") - subject.write(interfaces) + context "and the reload is forced" do + it "sets the interface down" do + expect(Yast::Execute).to receive(:on_target).with("/sbin/ifdown", "eth0") + subject.write(interfaces) + end end - context "during autoinstallation" do + context "and the reload is not forced (ie: autoinstallation)" do before do allow(Yast::Mode).to receive(:autoinst).and_return(true) end - it "does not set the interface down" do - expect(Yast::Execute).to_not receive(:on_target).with("/sbin/ifdown", any_args) - subject.write(interfaces) + context "if not forced the reload" do + it "does not set the interface down" do + expect(Yast::Execute).to receive(:on_target).with("/sbin/ifdown", any_args) + subject.write(interfaces) + end end end From d6e04919639fdc95b149448c71af8e983c1e2d23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Tue, 19 May 2020 15:12:03 +0100 Subject: [PATCH 06/10] Moved auto client under y2network namespace --- src/clients/lan_auto.rb | 199 +----------------- src/lib/network/network_autoyast.rb | 17 +- src/lib/y2network/autoinst/config.rb | 2 +- .../autoinst_profile/networking_section.rb | 4 +- src/lib/y2network/clients/auto.rb | 170 +++++++++++++++ src/modules/Lan.rb | 13 +- test/lan_auto_test.rb | 48 ----- test/y2network/clients/auto_test.rb | 126 +++++++++++ 8 files changed, 318 insertions(+), 261 deletions(-) create mode 100644 src/lib/y2network/clients/auto.rb delete mode 100755 test/lan_auto_test.rb create mode 100755 test/y2network/clients/auto_test.rb diff --git a/src/clients/lan_auto.rb b/src/clients/lan_auto.rb index e70b25794..d088e0a32 100644 --- a/src/clients/lan_auto.rb +++ b/src/clients/lan_auto.rb @@ -1,198 +1,3 @@ -# *************************************************************************** -# -# Copyright (c) 2012 Novell, Inc. -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or -# modify it under the terms of version 2 of the GNU General Public License as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program; if not, contact Novell, Inc. -# -# To contact Novell about this file by physical or electronic mail, -# you may find current contact information at www.novell.com -# -# ************************************************************************** +require "y2network/clients/auto" -require "network/network_autoyast" - -module Yast - # Client providing autoyast functionality - class LanAutoClient < Client - include Yast::Logger - - def main - Yast.import "UI" - - textdomain "network" - - Builtins.y2milestone("----------------------------------------") - Builtins.y2milestone("Lan autoinst client started") - - Yast.import "Lan" - Yast.import "Progress" - Yast.import "Map" - Yast.import "NetworkInterfaces" - Yast.import "LanItems" - Yast.include self, "network/lan/wizards.rb" - Yast.include self, "network/routines.rb" - - @func = "" - @param = {} - - # Check arguments - if Ops.greater_than(Builtins.size(WFM.Args), 0) && - Ops.is_string?(WFM.Args(0)) - @func = Convert.to_string(WFM.Args(0)) - if Ops.greater_than(Builtins.size(WFM.Args), 1) && - Ops.is_map?(WFM.Args(1)) - @param = Convert.to_map(WFM.Args(1)) - end - end - - Builtins.y2milestone("Lan autoinst callback: #{@func}") - - if @func == "Summary" - @ret = Lan.Summary("summary") - elsif @func == "Reset" - Lan.Import({}) - Yast::Lan.clear_configs - @ret = {} - elsif @func == "Change" - Yast::Lan.add_config(:yast, Y2Network::Config.from(:defaults)) unless Yast::Lan.yast_config - @ret = LanAutoSequence("") - elsif @func == "Import" - @new = Lan.FromAY(@param) - # see bnc#498993 - # in case keep_install_network is set to true (in AY) - # we'll keep values from installation - # and merge with XML data (bnc#712864) - @new = NetworkAutoYast.instance.merge_configs(@new) if @new["keep_install_network"] - - Lan.Import(@new) - @ret = true - elsif @func == "Read" - @progress_orig = Progress.set(false) - @ret = Lan.Read(:nocache) - Progress.set(@progress_orig) - elsif @func == "Packages" - @ret = Lan.AutoPackages - elsif @func == "SetModified" - @ret = Lan.SetModified - elsif @func == "GetModified" - @ret = Lan.Modified - elsif @func == "Export" - @settings2 = Lan.Export - Builtins.y2debug("settings: %1", @settings2) - @autoyast = ToAY(@settings2) - @ret = deep_copy(@autoyast) - elsif @func == "Write" - @progress_orig = Progress.set(false) - - result = Lan.WriteOnly - Builtins.y2error("Writing lan config failed") if !result - @ret &&= result - timeout = Lan.autoinst.ip_check_timeout || -1 - - if (timeout >= 0) && Lan.isAnyInterfaceDown - Builtins.y2debug("timeout %1", timeout) - error_text = _("Configuration Error: uninitialized interface.") - timeout == 0 ? Popup.Error(error_text) : Popup.TimedError(error_text, timeout) - end - - Progress.set(@progress_orig) - else - Builtins.y2error("unknown function: %1", @func) - @ret = false - end - - Builtins.y2milestone("Lan auto finished (#{@ret})") - Builtins.y2milestone("----------------------------------------") - @ret - end - - # Convert data from native network to autoyast for XML - # @param [Hash] settings native network settings - # @return [Hash] autoyast network settings - def ToAY(settings) - settings = deep_copy(settings) - interfaces = settings["interfaces"] || [] - Builtins.y2milestone("interfaces: #{interfaces.inspect})") - net_udev = settings["net-udev"] || [] - Builtins.y2milestone("net-udev: #{net_udev.inspect})") - - # Modules - s390_devices = settings["s390-devices"] || [] - Builtins.y2milestone("s390-devices: #{s390_devices.inspect})") - - modules = [] - Builtins.foreach(Ops.get_map(settings, "hwcfg", {})) do |device, mod| - newmap = {} - Ops.set(newmap, "device", device) - Ops.set(newmap, "module", Ops.get_string(mod, "MODULE", "")) - Ops.set(newmap, "options", Ops.get_string(mod, "MODULE_OPTIONS", "")) - modules = Builtins.add(modules, newmap) - end - - config = Ops.get_map(settings, "config", {}) - dhcp = Ops.get_map(config, "dhcp", {}) - dhcp_hostname = Ops.get_boolean(dhcp, "DHCLIENT_SET_HOSTNAME", false) - dns = Ops.get_map(settings, "dns", {}) - Ops.set(dns, "dhcp_hostname", dhcp_hostname) - dhcpopts = {} - if Builtins.haskey(dhcp, "DHCLIENT_HOSTNAME_OPTION") - Ops.set( - dhcpopts, - "dhclient_hostname_option", - Ops.get_string(dhcp, "DHCLIENT_HOSTNAME_OPTION", "AUTO") - ) - end - if Builtins.haskey(dhcp, "DHCLIENT_ADDITIONAL_OPTIONS") - Ops.set( - dhcpopts, - "dhclient_additional_options", - Ops.get_string(dhcp, "DHCLIENT_ADDITIONAL_OPTIONS", "") - ) - end - if Builtins.haskey(dhcp, "DHCLIENT_CLIENT_ID") - Ops.set( - dhcpopts, - "dhclient_client_id", - Ops.get_string(dhcp, "DHCLIENT_CLIENT_ID", "") - ) - end - - ret = {} - Ops.set(ret, "managed", Ops.get_boolean(settings, "managed", false)) - if Builtins.haskey(settings, "ipv6") - Ops.set(ret, "ipv6", Ops.get_boolean(settings, "ipv6", true)) - end - Ops.set( - ret, - "keep_install_network", - Ops.get_boolean(settings, "keep_install_network", true) - ) - Ops.set(ret, "modules", modules) if Ops.greater_than(Builtins.size(modules), 0) - Ops.set(ret, "dns", dns) if Ops.greater_than(Builtins.size(dns), 0) - Ops.set(ret, "dhcp_options", dhcpopts) if Ops.greater_than(Builtins.size(dhcpopts), 0) - if Ops.greater_than( - Builtins.size(Ops.get_map(settings, "routing", {})), - 0 - ) - Ops.set(ret, "routing", Ops.get_map(settings, "routing", {})) - end - Ops.set(ret, "interfaces", interfaces) if Ops.greater_than(Builtins.size(interfaces), 0) - Ops.set(ret, "s390-devices", s390_devices) if Ops.greater_than(Builtins.size(s390_devices), 0) - Ops.set(ret, "net-udev", net_udev) if Ops.greater_than(Builtins.size(net_udev), 0) - deep_copy(ret) - end - end -end - -Yast::LanAutoClient.new.main +Y2Network::Clients::Auto.new.run diff --git a/src/lib/network/network_autoyast.rb b/src/lib/network/network_autoyast.rb index fc6f83048..3aef15f9b 100644 --- a/src/lib/network/network_autoyast.rb +++ b/src/lib/network/network_autoyast.rb @@ -103,13 +103,14 @@ def set_network_service # Initializates NICs setup according AY profile # - # If the installer is running in 1st stage mode only, then the configuration - # is also written + # If the network was already written before the proposal or the second + # stage is not omitted, then, it returns without touching the config. # # @param [Boolean] write forces instant writing of the configuration # @return [Boolean] true when configuration was present and loaded from the profile def configure_lan(write: false) log.info("NetworkAutoYast: Lan configuration") + return false if !write && (Lan.autoinst.before_proposal || second_stage?) ay_configuration = Lan.FromAY(ay_networking_section) if keep_net_config? @@ -301,7 +302,7 @@ def ay_host_section # It imports the profile, configures the module and writes the configuration. # Writing the configuration is optional when second stage is available and mandatory # when running autoyast installation with first stage only. - def configure_submodule(yast_module, ay_config, write: false) + def configure_submodule(yast_module, ay_config, write: !second_stage?) return false if !ay_config yast_module.Import(ay_config) @@ -310,13 +311,17 @@ def configure_submodule(yast_module, ay_config, write: false) # Return true in order to not call the NetworkAutoconfiguration.configure_hosts return true unless AutoInstall.valid_imported_values - mode_section = ay_general_section.fetch("mode", {}) - write ||= !mode_section.fetch("second_stage", true) log.info("Write configuration instantly: #{write}") - yast_module.Write(gui: false) if write true end + + # Convenience method to check whether the second stage is enabled or not + # + # @return[Boolean] + def second_stage? + ay_general_section.fetch("mode", {}).fetch("second_stage", true) + end end end diff --git a/src/lib/y2network/autoinst/config.rb b/src/lib/y2network/autoinst/config.rb index ed751b6d3..6df715751 100644 --- a/src/lib/y2network/autoinst/config.rb +++ b/src/lib/y2network/autoinst/config.rb @@ -19,7 +19,7 @@ module Y2Network module Autoinst - # This class is responsible of storing network settings that are only + # This class is responsible of storing network settings that are only # relevant to the autoinstallation proccess. class Config # @return [Boolean] controls whether the network configuration should be diff --git a/src/lib/y2network/autoinst_profile/networking_section.rb b/src/lib/y2network/autoinst_profile/networking_section.rb index ca30be0fa..add23465b 100644 --- a/src/lib/y2network/autoinst_profile/networking_section.rb +++ b/src/lib/y2network/autoinst_profile/networking_section.rb @@ -35,7 +35,6 @@ module AutoinstProfile # # @see RoutingSection class NetworkingSection - # @return [Boolean] attr_accessor :setup_before_proposal # @return [Boolean] @@ -45,7 +44,6 @@ class NetworkingSection # @return [Integer] attr_accessor :strict_ip_check_timeout - # @return [RoutingSection] attr_accessor :routing # @return [DNSSection] @@ -107,7 +105,7 @@ def to_hashes "dns" => dns.to_hashes, "interfaces" => interfaces.to_hashes, "net-udev" => udev_rules.to_hashes, - "s390-devices" => s390_devices.to_hashes, + "s390-devices" => s390_devices.to_hashes } end end diff --git a/src/lib/y2network/clients/auto.rb b/src/lib/y2network/clients/auto.rb new file mode 100644 index 000000000..482d57f4d --- /dev/null +++ b/src/lib/y2network/clients/auto.rb @@ -0,0 +1,170 @@ +# Copyright (c) [2020] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. +require "yast" +require "installation/auto_client" + +Yast.import "Lan" +Yast.import "Progress" +Yast.import "Map" +Yast.import "LanItems" + +module Y2Network + module Clients + # This class is responsible of AutoYaST network configuration + class Auto < ::Installation::AutoClient + # Constructor + def initialize + textdomain "network" + + Yast.include self, "network/lan/wizards.rb" + end + + def read + @progress_orig = Yast::Progress.set(false) + ret = Yast::Lan.Read(:nocache) + Yast::Progress.set(@progress_orig) + ret + end + + def write + @progress_orig = Yast::Progress.set(false) + + result = Yast::Lan.WriteOnly + log.error("Writing lan config failed") if !result + @ret &&= result + timeout = Yast::Lan.autoinst.ip_check_timeout || -1 + + if (timeout >= 0) && Lan.isAnyInterfaceDown + Builtins.y2debug("timeout %1", timeout) + error_text = _("Configuration Error: uninitialized interface.") + (timeout == 0) ? Popup.Error(error_text) : Popup.TimedError(error_text, timeout) + end + + Yast::Progress.set(@progress_orig) + end + + def summary + Yast::Lan.Summary("summary") + end + + def reset + Yast::Lan.Import({}) + Yast::Lan.clear_configs + {} + end + + def change + if !Yast::Lan.yast_config + Yast::Lan.add_config(:yast, Y2Network::Config.new(:source, :config)) + end + LanAutoSequence("") + end + + def import(profile) + modified_profile = Yast::Lan.FromAY(profile) + + # see bnc#498993 + # in case keep_install_network is set to true (in AY) + # we'll keep values from installation + # and merge with XML data (bnc#712864) + if modified_profile.fetch("keep_install_network", true) + modified_profile = Yast::NetworkAutoYast.instance.merge_configs(modified_profile) + end + + Yast::Lan.Import(modified_profile) + true + end + + def packages + Yast::Lan.AutoPackages + end + + def modified + Yast::Lan.SetModified + end + + def modified? + Yast::Lan.Modified + end + + def export + raw_config = Yast::Lan.Export + log.debug("settings: #{raw_config.inspect}") + adapt_for_autoyast(raw_config) + end + + private + + def merge_current_config? + !!Yast::Lan.autoinst.keep_install_network + end + + # Convert data from native network to autoyast for XML + # @param [Hash] settings native network settings + # @return [Hash] autoyast network settings + def adapt_for_autoyast(settings) + settings = deep_copy(settings) + interfaces = settings["interfaces"] || [] + log.info("interfaces: #{interfaces.inspect}") + net_udev = settings["net-udev"] || [] + log.info("net-udev: #{net_udev.inspect}") + + # Modules + s390_devices = settings["s390-devices"] || [] + log.info("s390-devices: #{s390_devices.inspect}") + + modules = [] + settings.fetch("hwcfg", {}).each do |device, mod| + newmap = { "device" => device } + newmap["module"] = mod.fetch("MODULE", "") + newmap["options"] = mod.fetch("MODULE_OPTIONS", "") + modules << newmap + end + + config = settings.fetch("config", {}) + dhcp = config.fetch("dhcp", {}) + dhcp_hostname = dhcp.fetch("DHCLIENT_SET_HOSTNAME", false) + dns = settings.fetch("dns", {}) + dns["dhcp_hostname"] = dhcp_hostname + dhcpopts = {} + if dhcp.keys.include?("DHCLIENT_HOSTNAME_OPTION") + dhcpopts["dhclient_hostname_option"] = dhcp.fetch("DHCLIENT_HOSTNAME_OPTION", "AUTO") + end + if dhcp.keys.include?("DHCLIENT_ADDITIONAL_OPTIONS") + dhcpopts["dhclient_additional_options"] = dhcp.fetch("DHCLIENT_ADDITIONAL_OPTIONS", "") + end + if dhcp.keys.include?("DHCLIENT_CLIENT_ID") + dhcpopts["dhclient_client_id"] = dhcp.fetch("DHCLIENT_CLIENT_ID", "") + end + + ret = { "managed" => settings.fetch("managed", false) } + ret["ipv6"] = settings.fetch("ipv6", true) if settings.keys.include?("ipv6") + ret["keep_install_network"] = settings.fetch("keep_install_network", true) + ret["modules"] = modules unless modules.empty? + ret["dns"] = dns unless dns.empty? + ret["dhcp_options"] = dhcpopts unless dhcpopts.empty? + ret["routing"] = settings["routing"] unless settings.fetch("routing", {}).empty? + ret["interfaces"] = interfaces unless interfaces.empty? + ret["s390-devices"] = s390_devices unless s390_devices.empty? + ret["net-udev"] = net_udev unless net_udev.empty? + ret.dup + end + end + end +end diff --git a/src/modules/Lan.rb b/src/modules/Lan.rb index 819e72736..d970328ee 100644 --- a/src/modules/Lan.rb +++ b/src/modules/Lan.rb @@ -110,7 +110,7 @@ def main # @return [Boolean] attr_accessor :write_only # @return [Autoinst::Config] - attr_accessor :autoinst + attr_writer :autoinst #------------------ # GLOBAL FUNCTIONS @@ -675,8 +675,9 @@ def Import(settings) end # Export data. - # They need to be passed through {LanAutoClient#ToAY} to become - # what networking.rnc describes. + # They need to be converted to become what networking.rnc describes. + # @see Y2Network::Clients::Auto.adapt_for_autoyast + # # Most prominently, instead of a flat list called "interfaces" # we export a 2-level map of typed "devices" # @return dumped settings @@ -864,10 +865,10 @@ def autoinst_config(section) return unless autoinst_settings?(section) Y2Network::Autoinst::Config.new( - before_proposal: section.setup_before_proposal, - start_immediately: section.start_immediately, + before_proposal: section.setup_before_proposal, + start_immediately: section.start_immediately, keep_install_network: section.keep_install_network, - ip_check_timeout: section.strict_ip_check_timeout + ip_check_timeout: section.strict_ip_check_timeout ) end diff --git a/test/lan_auto_test.rb b/test/lan_auto_test.rb deleted file mode 100755 index a715d9188..000000000 --- a/test/lan_auto_test.rb +++ /dev/null @@ -1,48 +0,0 @@ -#!/usr/bin/env rspec - -# Copyright (c) [2019] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require_relative "test_helper" - -require "yast" -require_relative "../src/clients/lan_auto" - -describe Yast::LanAutoClient do - describe "#main" do - before do - allow(Yast::WFM).to receive(:Args).with(no_args).and_return([func]) - allow(Yast::WFM).to receive(:Args).with(0).and_return(func) - end - - context "when func is GetModified" do - let(:func) { "GetModified" } - - it "returns true if Lan.GetModified is true" do - expect(Yast::Lan).to receive(:Modified).and_return(true) - expect(subject.main).to eq(true) - end - - it "returns false if Lan.GetModified is false" do - expect(Yast::Lan).to receive(:Modified).and_return(false) - expect(subject.main).to eq(false) - end - end - end -end diff --git a/test/y2network/clients/auto_test.rb b/test/y2network/clients/auto_test.rb new file mode 100755 index 000000000..d43c19070 --- /dev/null +++ b/test/y2network/clients/auto_test.rb @@ -0,0 +1,126 @@ +#!/usr/bin/env rspec +# Copyright (c) [2020] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require_relative "../../test_helper" +require "y2network/clients/auto" + +describe Y2Network::Clients::Auto do + let(:network_autoyast) { Yast::NetworkAutoYast.instance } + + describe "#reset" do + it "clears Yast::Lan internal state" do + allow(Yast::Lan).to receive(:Import).with({}) + expect(Yast::Lan).to receive(:clear_configs) + subject.reset + end + end + + describe "#read" do + it "forces a Lan read" do + expect(Yast::Lan).to receive(:Read).with(:nocache) + subject.read + end + end + + describe "#import" do + let(:profile) { { "keep_install_network" => false } } + let(:from_ay_profile) { profile.dup } + + before do + allow(Yast::Lan).to receive(:Import) + allow(Yast::Lan).to receive(:FromAY).with(profile).and_return(from_ay_profile) + end + + it "prepares the given profile to be imported" do + expect(Yast::Lan).to receive(:FromAY).with(profile).and_return(from_ay_profile) + + subject.import(profile) + end + + it "imports the given profile" do + expect(Yast::Lan).to receive(:Import).with(from_ay_profile) + + subject.import(profile) + end + + context "unless the profile specifies that the current config should be ignored" do + let(:profile) { { "keep_install_network" => true } } + + it "merges the given profile with the current network config before import" do + expect(network_autoyast).to receive(:merge_configs).with(from_ay_profile) + + subject.import(profile) + end + end + + context "when the profile specifies that the current config should be ignored" do + it "does not merge the given profile with the current network config before import" do + expect(network_autoyast).to_not receive(:merge_configs) + + subject.import(profile) + end + end + end + + describe "#export" do + it "exports the current network configuration prepared for AutoYaST" do + expect(Yast::Lan).to receive(:Export).and_return(:exported_config) + expect(subject).to receive(:adapt_for_autoyast).with(:exported_config).and_return(:ay_config) + expect(subject.export).to eql(:ay_config) + end + end + + describe "#summary" do + let(:config_summary) { "config" } + + it "returns a text summary of the autoyast config network configuration" do + allow(Yast::Lan).to receive(:Summary).with("summary").and_return(config_summary) + + expect(subject.summary).to eql(config_summary) + end + end + + describe "#packages" do + let(:packages) { { "install" => ["wpa_supplicant"], "remove" => [] } } + + it "returns a hash with the packages that need to be installed and removed" do + allow(Yast::Lan).to receive(:AutoPackages).and_return(packages) + expect(subject.packages).to eql(packages) + end + end + + describe "#modified" do + it "sets the network config as modified" do + expect(Yast::Lan).to receive(:SetModified) + + subject.modified + end + end + + describe "#modified?" do + let(:modified) { :true_or_false } + + it "returns whether lan configuration has been modified or not" do + allow(Yast::Lan).to receive(:Modified).and_return(modified) + + expect(subject.modified?).to eql(modified) + end + end +end From 3ff60515c7a9dca8d7de05964f910ea3b12e52cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Wed, 20 May 2020 10:47:16 +0100 Subject: [PATCH 07/10] Added argument to the interfaces writer for write only --- src/lib/network/network_autoyast.rb | 6 +- src/lib/y2network/sysconfig/config_writer.rb | 2 +- test/network_autoyast_test.rb | 131 +++++++++++------- .../sysconfig/interfaces_writer_test.rb | 42 ++++-- 4 files changed, 114 insertions(+), 67 deletions(-) diff --git a/src/lib/network/network_autoyast.rb b/src/lib/network/network_autoyast.rb index 3aef15f9b..29e74cd6a 100644 --- a/src/lib/network/network_autoyast.rb +++ b/src/lib/network/network_autoyast.rb @@ -166,7 +166,7 @@ def configure_hosts(write: false) # Checks if the profile asks for keeping installation network configuration def keep_net_config? - ret = Lan.autoinst.keep_install_network + ret = ay_networking_section.fetch("keep_install_network", true) log.info("NetworkAutoYast: keep installation network: #{ret}") @@ -302,7 +302,7 @@ def ay_host_section # It imports the profile, configures the module and writes the configuration. # Writing the configuration is optional when second stage is available and mandatory # when running autoyast installation with first stage only. - def configure_submodule(yast_module, ay_config, write: !second_stage?) + def configure_submodule(yast_module, ay_config, write: false) return false if !ay_config yast_module.Import(ay_config) @@ -312,7 +312,7 @@ def configure_submodule(yast_module, ay_config, write: !second_stage?) return true unless AutoInstall.valid_imported_values log.info("Write configuration instantly: #{write}") - yast_module.Write(gui: false) if write + yast_module.Write(gui: false) if !second_stage? || write true end diff --git a/src/lib/y2network/sysconfig/config_writer.rb b/src/lib/y2network/sysconfig/config_writer.rb index 719283d0f..dbb93a686 100644 --- a/src/lib/y2network/sysconfig/config_writer.rb +++ b/src/lib/y2network/sysconfig/config_writer.rb @@ -188,7 +188,7 @@ def write_hostname_settings(config, old_config) # @param interfaces [Y2Network::InterfacesCollection] # @see Y2Network::Sysconfig::InterfacesWriter def write_interfaces(interfaces) - writer = Y2Network::Sysconfig::InterfacesWriter.new + writer = Y2Network::Sysconfig::InterfacesWriter.new(reload: !Yast::Lan.write_only) writer.write(interfaces) end diff --git a/test/network_autoyast_test.rb b/test/network_autoyast_test.rb index cc6d453ec..0773b1406 100755 --- a/test/network_autoyast_test.rb +++ b/test/network_autoyast_test.rb @@ -249,58 +249,50 @@ def nm_installed(installed) describe "#keep_net_config?" do let(:network_autoyast) { Yast::NetworkAutoYast.instance } + let(:profile) { { "networking" => { "keep_install_network" => true } } } - def keep_install_network_value(value) - allow(network_autoyast) - .to receive(:ay_networking_section) - .and_return(value) + before do + Yast::Lan.Import(Yast::Lan.FromAY(profile["networking"])) + allow(Yast::Profile).to receive(:current).and_return(profile) end - it "succeedes when keep_install_network is set in AY profile" do - keep_install_network_value("keep_install_network" => true) - expect(network_autoyast.keep_net_config?).to be true + context "when keep_install_network is true in AY profile" do + it "returns true" do + expect(network_autoyast.keep_net_config?).to be true + end end - it "fails when keep_install_network is not set in AY profile" do - keep_install_network_value("keep_install_network" => false) - expect(network_autoyast.keep_net_config?).to be false + context "when keep_install_network is false in AY profile" do + let(:profile) { { "networking" => { "keep_install_network" => false } } } + it "returns false" do + expect(network_autoyast.keep_net_config?).to be false + end end - it "succeedes when keep_install_network is not present in AY profile" do - keep_install_network_value({}) - expect(network_autoyast.keep_net_config?).to be true + context "when keep_install_network is not present in AY profile" do + let(:profile) { { "networking" => { "setup_before_proposal" => true } } } + + it "returns true" do + expect(network_autoyast.keep_net_config?).to be true + end end end describe "#configure_lan" do before do + Yast::Lan.autoinst = nil allow(Yast::Profile).to receive(:current) .and_return("general" => general_section, "networking" => networking_section) allow(Yast::AutoInstall).to receive(:valid_imported_values).and_return(true) + allow(Yast::Lan).to receive(:Write) + Yast::Lan.autoinst.before_proposal = before_proposal end let(:networking_section) { nil } let(:general_section) { nil } + let(:before_proposal) { false } - context "when second stage is disabled" do - let(:general_section) do - { "mode" => { "second_stage" => false } } - end - - it "writes the Lan module configuration" do - expect(Yast::Lan).to receive(:Write) - subject.configure_lan - end - end - - context "when writing the configuration is disabled" do - it "writes the Lan module configuration" do - expect(Yast::Lan).to_not receive(:Write) - subject.configure_lan(write: false) - end - end - - context "when second stage is enabled" do + context "when second stage is explicitly enabled" do let(:general_section) do { "mode" => { "second_stage" => true } } end @@ -309,37 +301,74 @@ def keep_install_network_value(value) expect(Yast::Lan).to_not receive(:Write) subject.configure_lan end + + it "returns false" do + expect(subject.configure_lan).to eql(false) + end end - context "when second stage is not explicitly enabled" do - let(:general_section) { nil } + context "when the configuration was written before the proposal" do + let(:before_proposal) { true } + let(:networking_section) { { "setup_before_proposal" => before_proposal } } - it "does not write the Lan module configuration" do + it "does not write anything" do expect(Yast::Lan).to_not receive(:Write) - subject.configure_lan + expect(subject.configure_lan).to eql(false) end - end - it "merges the installation configuration" do - expect(Yast::NetworkAutoYast.instance).to receive(:merge_configs) - subject.configure_lan + it "returns false" do + expect(subject.configure_lan).to eql(false) + end end - context "when the user wants to keep the installation network" do - let(:networking_section) { { "keep_install_network" => true } } + context "when the second stage is disabled" do + let(:before_proposal) { true } + let(:networking_section) { { "setup_before_proposal" => before_proposal } } + let(:general_section) do + { "mode" => { "second_stage" => false } } + end - it "merges the installation configuration" do - expect(Yast::NetworkAutoYast.instance).to receive(:merge_configs) - subject.configure_lan + context "and the configuration was not written before the proposal" do + let(:before_proposal) { false } + + context "if the user wants to keep the installation network" do + let(:networking_section) do + { "keep_install_network" => true, "setup_before_proposal" => before_proposal } + end + + it "merges the installation configuration" do + expect(Yast::NetworkAutoYast.instance).to receive(:merge_configs) + subject.configure_lan + end + end + + context "if the user does not want to keep the installation network" do + let(:networking_section) do + { "keep_install_network" => false, "setup_before_proposal" => before_proposal } + end + + it "does not merge the installation configuration" do + expect(Yast::NetworkAutoYast.instance).to_not receive(:merge_configs) + subject.configure_lan + end + end + + it "writes the Lan module configuration" do + expect(Yast::Lan).to receive(:Write) + subject.configure_lan + end end - end - context "when the user does not want to keep the installation network" do - let(:networking_section) { { "keep_install_network" => false } } + context "and the configuration was written before the proposal" do + it "does not write anything" do + expect(Yast::Lan).to_not receive(:Write) - it "does not merge the installation configuration" do - expect(Yast::NetworkAutoYast.instance).to_not receive(:merge_configs) - subject.configure_lan + subject.configure_lan + end + + it "returns false" do + expect(subject.configure_lan).to eql(false) + end end end end diff --git a/test/y2network/sysconfig/interfaces_writer_test.rb b/test/y2network/sysconfig/interfaces_writer_test.rb index a20c551ce..8217dff96 100644 --- a/test/y2network/sysconfig/interfaces_writer_test.rb +++ b/test/y2network/sysconfig/interfaces_writer_test.rb @@ -25,7 +25,8 @@ require "tmpdir" describe Y2Network::Sysconfig::InterfacesWriter do - subject(:writer) { described_class.new } + let(:reload) { false } + subject(:writer) { described_class.new(reload: reload) } describe "#write" do let(:interfaces) { Y2Network::InterfacesCollection.new([eth0]) } @@ -71,20 +72,18 @@ end context "and the reload is forced" do + let(:reload) { true } + it "sets the interface down" do expect(Yast::Execute).to receive(:on_target).with("/sbin/ifdown", "eth0") subject.write(interfaces) end end - context "and the reload is not forced (ie: autoinstallation)" do - before do - allow(Yast::Mode).to receive(:autoinst).and_return(true) - end - + context "and the reload is not forced (ie: end of the autoinstallation)" do context "if not forced the reload" do it "does not set the interface down" do - expect(Yast::Execute).to receive(:on_target).with("/sbin/ifdown", any_args) + expect(Yast::Execute).to_not receive(:on_target).with("/sbin/ifdown", any_args) subject.write(interfaces) end end @@ -201,11 +200,30 @@ end end - it "refreshes udev" do - expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "control", any_args) - expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "trigger", any_args) - expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "settle") - subject.write(interfaces) + context "after the udev rules have been written" do + context "when the reload is forced" do + let(:reload) { true } + + it "refreshes udev" do + expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "control", any_args) + expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "trigger", any_args) + expect(Yast::Execute).to receive(:on_target).with("/usr/bin/udevadm", "settle") + subject.write(interfaces) + end + end + + context "when the reload is not forced" do + it "does not refresh udev" do + expect(Yast::Execute) + .to_not receive(:on_target).with("/usr/bin/udevadm", "control", any_args) + expect(Yast::Execute) + .to_not receive(:on_target).with("/usr/bin/udevadm", "trigger", any_args) + expect(Yast::Execute) + .to_not receive(:on_target).with("/usr/bin/udevadm", "settle") + subject.write(interfaces) + end + end + end end end From 2702aa4c46ca1e0dc405dd82eca14e098e8ed350 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Wed, 20 May 2020 13:17:50 +0100 Subject: [PATCH 08/10] Added auto client write test. --- src/modules/Lan.rb | 1 + test/y2network/clients/auto_test.rb | 68 +++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/src/modules/Lan.rb b/src/modules/Lan.rb index d970328ee..530209e62 100644 --- a/src/modules/Lan.rb +++ b/src/modules/Lan.rb @@ -638,6 +638,7 @@ def FromAY(input) Ops.set(input, "config", "dhcp" => dhcp) if !Ops.get(input, "strict_IP_check_timeout").nil? + input["strict_ip_check_timeout"] = input.delete("strict_IP_check_timeout") Ops.set(input, ["config", "config"], "CHECK_DUPLICATE_IP" => true) end diff --git a/test/y2network/clients/auto_test.rb b/test/y2network/clients/auto_test.rb index d43c19070..2003f0772 100755 --- a/test/y2network/clients/auto_test.rb +++ b/test/y2network/clients/auto_test.rb @@ -23,6 +23,37 @@ describe Y2Network::Clients::Auto do let(:network_autoyast) { Yast::NetworkAutoYast.instance } + let(:eth0) { { "device" => "eth0", "bootproto" => "dhcp", "startmode" => "auto" } } + let(:interfaces) { [eth0] } + + let(:dns) { { "hostname" => "host", "dhcp_hostname" => true, "write_hostname" => true } } + let(:routes) do + [ + { + "destination" => "default", + "gateway" => "192.168.1.1", + "netmask" => "255.255.255.0", + "device" => "-" + }, + { + "destination" => "172.26.0.0/24", + "device" => "eth0" + } + ] + end + + let(:profile) do + { + "keep_install_network" => false, + "interfaces" => interfaces, + "routing" => { + "ipv4_forward" => true, + "ipv6_forward" => false, + "routes" => routes + }, + "dns" => dns + } + end describe "#reset" do it "clears Yast::Lan internal state" do @@ -123,4 +154,41 @@ expect(subject.modified?).to eql(modified) end end + + describe "#write" do + let(:system_config) { Y2Network::Config.new(source: :sysconfig) } + + before do + allow(Yast::Lan).to receive(:Read) + Y2Network::Config.add(:system, system_config) + allow(Yast::Lan).to receive(:WriteOnly) + subject.import(profile) + end + + it "writes the imported network configuration" do + expect(Yast::Lan).to receive(:WriteOnly) + + subject.write + end + + context "when the imported profile declares a strict ip check timeout" do + let(:profile) do + { + "strict_IP_check_timeout" => 15 + } + end + + context "and some interfaces is down" do + before do + allow(Yast::Lan).to receive(:isAnyInterfaceDown).and_return(true) + end + + it "shows an error popup with the given timeout" do + expect(Yast::Popup).to receive(:TimedError) + + subject.write + end + end + end + end end From 4d3bd60a130959bc28f0a743cb93f4d43a895670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Fri, 22 May 2020 00:32:16 +0100 Subject: [PATCH 09/10] Changes based on CR --- src/lib/y2network/clients/auto.rb | 5 +++++ src/lib/y2network/sysconfig/interfaces_writer.rb | 5 ++++- src/modules/Lan.rb | 8 ++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/lib/y2network/clients/auto.rb b/src/lib/y2network/clients/auto.rb index 482d57f4d..458db08bf 100644 --- a/src/lib/y2network/clients/auto.rb +++ b/src/lib/y2network/clients/auto.rb @@ -116,6 +116,11 @@ def merge_current_config? end # Convert data from native network to autoyast for XML + # + # @todo we should get rid of this method moving the logic to the + # Y2Network::AutoinstProfile::NetworkingSection in case we cannot remove + # it completely + # # @param [Hash] settings native network settings # @return [Hash] autoyast network settings def adapt_for_autoyast(settings) diff --git a/src/lib/y2network/sysconfig/interfaces_writer.rb b/src/lib/y2network/sysconfig/interfaces_writer.rb index f48607af5..167ac0e85 100644 --- a/src/lib/y2network/sysconfig/interfaces_writer.rb +++ b/src/lib/y2network/sysconfig/interfaces_writer.rb @@ -31,8 +31,11 @@ module Sysconfig # hardware specific configuration through udev rules. # # @see Y2Network::InterfacesCollection - # @param reload [Boolean] whether the udev rules should be reloaded or not class InterfacesWriter + + # Constructor + # + # @param reload [Boolean] whether the udev rules should be reloaded or not def initialize(reload: true) @reload = reload end diff --git a/src/modules/Lan.rb b/src/modules/Lan.rb index 530209e62..798cab628 100644 --- a/src/modules/Lan.rb +++ b/src/modules/Lan.rb @@ -436,7 +436,7 @@ def writeIPv6 # Update the SCR according to network settings # @return true on success - def Write(gui: true) + def Write(gui: true, apply_config: !write_only) Builtins.y2milestone("Writing configuration") # Query modified flag in all components, not just LanItems - DNS, @@ -472,7 +472,7 @@ def Write(gui: true) step_labels << _("Writing firewall configuration") if firewalld.installed? # Progress stage 9 - step_labels = Builtins.add(step_labels, _("Activate network services")) if !write_only + step_labels = Builtins.add(step_labels, _("Activate network services")) if apply_config # Progress stage 10 step_labels = Builtins.add(step_labels, _("Update configuration")) @@ -545,7 +545,7 @@ def Write(gui: true) Builtins.sleep(sl) end - if !write_only + if apply_config return false if Abort() # Progress step 9 @@ -560,7 +560,7 @@ def Write(gui: true) # Progress step 10 ProgressNextStage(_("Updating configuration...")) - update_mta_config if !write_only + update_mta_config if !apply_config Builtins.sleep(sl) if NetworkService.is_network_manager From 2535e9a9277d0cfe2699eb571768ab372cef6ad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Alejandro=20Anderssen=20Gonz=C3=A1lez?= Date: Fri, 22 May 2020 00:32:46 +0100 Subject: [PATCH 10/10] Copy /etc/hosts to the target system when written before proposal --- src/lib/network/clients/save_network.rb | 1 + src/lib/y2network/sysconfig/interfaces_writer.rb | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/network/clients/save_network.rb b/src/lib/network/clients/save_network.rb index e3cd4b18c..624997176 100644 --- a/src/lib/network/clients/save_network.rb +++ b/src/lib/network/clients/save_network.rb @@ -98,6 +98,7 @@ def CopyConfiguredNetworkFiles { dir: SYSCONFIG, file: "routes" }, { dir: ::File.join(ETC, "wicked"), file: "common.xml" }, { dir: ETC, file: DNSClass::HOSTNAME_FILE }, + { dir: ETC, file: "hosts" }, # Copy sysctl file as network writes there ip forwarding (bsc#1159295) { dir: ::File.join(ETC, "sysctl.d"), file: "70-yast.conf" } ] diff --git a/src/lib/y2network/sysconfig/interfaces_writer.rb b/src/lib/y2network/sysconfig/interfaces_writer.rb index 167ac0e85..02fcfa93d 100644 --- a/src/lib/y2network/sysconfig/interfaces_writer.rb +++ b/src/lib/y2network/sysconfig/interfaces_writer.rb @@ -32,7 +32,6 @@ module Sysconfig # # @see Y2Network::InterfacesCollection class InterfacesWriter - # Constructor # # @param reload [Boolean] whether the udev rules should be reloaded or not