Changes based on CR

src/lib/network/clients/save_network.rb

@@ -20,7 +20,7 @@
 require "network/network_autoconfiguration"
 require "network/network_autoyast"
 require "y2network/proposal_settings"
-require "y2network/config_copier"
+require "y2network/helpers"
 
 Yast.import "Installation"
 Yast.import "DNS"
@@ -29,6 +29,7 @@
 
 module Yast
   class SaveNetworkClient < Client
+    include Y2Network::Helpers
     include Logger
 
     def initialize
@@ -46,28 +47,17 @@ def main
 
   private
 
-    ETC = "/etc".freeze
-    NETWORK_MANAGER = "/etc/NetworkManager".freeze
-
-    COMMON_FILES = [
-      { 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" }
-    ].freeze
-
+    SYSCTL_PATH = "/etc/sysctl.d/70-yast.conf".freeze
     # Directory containing udev rules
     UDEV_RULES_DIR = "/etc/udev/rules.d".freeze
     PERSISTENT_RULES = "70-persistent-net.rules".freeze
 
     def copy_udev_rules
-      copy_files_to_target([PERSISTENT_RULES], UDEV_RULES_DIR)
-
-      return unless Arch.s390
-
+      files = [PERSISTENT_RULES]
       # chzdev creates the rules starting with "41-"
-      log.info("Copy S390 specific udev rule files (/etc/udev/rules/41*)")
-      copy_files_to_target(["41-*"], UDEV_RULES_DIR)
+      files << "41-*" if Arch.s390
+
+      copy_to_target(UDEV_RULES_DIR, include: files)
 
       nil
     end
@@ -109,55 +99,42 @@ def copy_from_instsys
 
         log.info("Copy network configuration files from 1st stage into installed system")
         copy_dhcp_info
-        COMMON_FILES.each { |e| copy_files_to_target([e[:file]], e[:dir]) }
-        copy_config_for(backend)
+        config_copier_for(backend)&.copy
       end
 
       nil
     end
 
+    # Convenience method to obtain a config copier for the given backend
+    #
+    # @param source [Symbol, String]
+    def config_copier_for(source)
+      require "y2network/#{source}/config_copier"
+
+      modname = source.to_s.split("_").map(&:capitalize).join
+      klass = Y2Network.const_get("#{modname}::ConfigCopier")
+      klass.new
+    rescue LoadError, NameError => e
+      log.info("There is no config copier for #{source}. #{e.inspect}")
+      nil
+    end
+
     # For copying dhcp-client leases
     # FIXME: We probably could omit the copy of these leases as we are using
     # wicked during the installation instead of dhclient.
     DHCPV4_PATH = "/var/lib/dhcp/".freeze
     DHCPV6_PATH = "/var/lib/dhcp6/".freeze
     DHCP_FILES = ["*.leases"].freeze
 
-    # Convenience method for copying dhcp files
-    def copy_dhcp_info
-      copy_files_to_target(DHCP_FILES, DHCPV4_PATH)
-      copy_files_to_target(DHCP_FILES, DHCPV6_PATH)
-    end
-
-    # Copy the network configuration of the given backend
-    #
-    # @param backend [Symbol]
-    def copy_config_for(backend)
-      return unless backend
-
-      log.info("Copying #{backend} specific config to the target system.")
-      Y2Network::ConfigCopier.for(backend)&.copy
+    def copy_common_files
+      copy_to_target("/etc", include: ["hosts", DNSClass::HOSTNAME_FILE])
+      copy_to_target(SYSCTL_PATH)
     end
 
-    # Make unit testing possible
-    ROOT_PATH = "/".freeze
-
-    # Convenvenience method for copying a list of files into the target system.
-    # It takes care of creating the target directory but only if some file
-    # needs to be copied
-    #
-    # @param files [Array<String>] list of short filenames to be copied
-    # @param path [String] path where the files resides and where will be
-    # copied in the target system
-    # @return [Boolean] whether some file was copied
-    def copy_files_to_target(files, path)
-      dest_dir = ::File.join(Installation.destdir, path)
-      glob_files = ::Dir.glob(files.map { |f| File.join(ROOT_PATH, path, f) })
-      return false if glob_files.empty?
-
-      ::FileUtils.mkdir_p(dest_dir)
-      ::FileUtils.cp(glob_files, dest_dir, preserve: true)
-      true
+    # Convenience method for copying dhcp files
+    def copy_dhcp_info
+      copy_to_target(DHCPV4_PATH, include: DHCP_FILES)
+      copy_to_target(DHCPV6_PATH, include: DHCP_FILES)
     end
 
     # Creates target's /etc/hosts configuration
@@ -212,7 +189,7 @@ def configure_network_manager
       return unless proposal_backend == :network_manager
 
       if Yast::Lan.system_config.backend&.id == :network_manager
-        copy_config_for(:network_manager)
+        config_copier_for(:network_manager)&.copy
       else
         Yast::Lan.yast_config.backend = :network_manager
         Yast::Lan.write_config

src/lib/y2network/config_copier.rb → src/lib/y2network/helpers.rb

@@ -1,4 +1,4 @@
-# Copyright (c) [2019] SUSE LLC
+# Copyright (c) [2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -17,42 +17,15 @@
 # To contact SUSE LLC about this file by physical or electronic mail, you may
 # find current contact information at www.suse.com.
 
-require "yast"
 
-Yast.import "Installation"
+require "yast"
 
 module Y2Network
-  # This class is responsible for copying the backend specific configuration to the target system
-  #
-  # It is expect that a configuration copier exists for each supported backend
-  # by inheriting from this class.
-  class ConfigCopier
+  module Helpers
     include Yast::Logger
-    include Yast::I18n
 
-    ETC = "/etc/".freeze
     ROOT_PATH = "/".freeze
 
-    class << self
-      # Returns a configuration writer for a given source
-      #
-      # @param source [Symbol] Source name (e.g., :wicked)
-      # @return [Y2Network::ConfigCopier]
-      #
-      # @see Y2Network::ConfigCopier
-      def for(source)
-        require "y2network/#{source}/config_copier"
-        modname = source.to_s.split("_").map(&:capitalize).join
-        klass = Y2Network.const_get("#{modname}::ConfigCopier")
-        klass.new
-      end
-    end
-
-    # Copies the configuration to the target system
-    def copy; end
-
-  private
-
     def inst_dir
       Yast::Installation.destdir
     end
@@ -61,18 +34,18 @@ def inst_dir
     # It takes care of creating the target directory but only if some file
     # needs to be copied
     #
-    # @param files [Array<String>] list of short filenames to be copied
     # @param path [String] path where the files resides and where will be
     # copied in the target system
     # @return [Boolean] whether some file was copied
-    def copy_files_to_target(files, path)
-      dest_dir = ::File.join(Yast::Installation.destdir, path)
-      glob_files = ::Dir.glob(files.map { |f| File.join(ROOT_PATH, path, f) })
+    def copy_to_target(path, include: nil, target: inst_dir)
+      dest_dir = ::File.join(target, path)
+      files = include ? include.map { |f| File.join(ROOT_PATH, path, f) } : path
+      glob_files = ::Dir.glob(files)
       return false if glob_files.empty?
 
       log.info("Copying '#{glob_files.join(",")}' to '#{dest_dir}'.")
 
-      ::FileUtils.mkdir_p(dest_dir)
+      ::FileUtils.mkdir_p(dest_dir) if include
       ::FileUtils.cp(glob_files, dest_dir, preserve: true)
       true
     end

src/lib/y2network/network_manager/config_copier.rb

@@ -1,4 +1,4 @@
-# Copyright (c) [2019] SUSE LLC
+# Copyright (c) [2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -18,16 +18,17 @@
 # find current contact information at www.suse.com.
 
 require "yast"
-require "y2network/config_copier"
+require "y2network/helpers"
 
 module Y2Network
   module NetworkManager
-    # This class copies Wicked specific configuration to the target system
-    class ConfigCopier < Y2Network::ConfigCopier
-      NETWORK_MANAGER = "/etc/NetworkManager".freeze
+    # This class copies NetworkManager specific configuration to the target system
+    class ConfigCopier
+      include Helpers
+      NETWORK_MANAGER_PATH = "/etc/NetworkManager".freeze
 
       def copy
-        copy_files_to_target(["*"], File.join(NETWORK_MANAGER, "system-connections"))
+        copy_to_target(File.join(NETWORK_MANAGER_PATH, "system-connections"), include: ["*"])
       end
     end
   end

src/lib/y2network/wicked/config_copier.rb

@@ -1,4 +1,4 @@
-# Copyright (c) [2019] SUSE LLC
+# Copyright (c) [2023] SUSE LLC
 #
 # All Rights Reserved.
 #
@@ -19,25 +19,29 @@
 
 require "yast"
 require "y2storage"
-require "y2network/config_copier"
 require "cfa/generic_sysconfig"
+require "y2network/helpers"
 require "shellwords"
 
 module Y2Network
   module Wicked
     # This class copies Wicked specific configuration to the target system
-    class ConfigCopier < Y2Network::ConfigCopier
+    class ConfigCopier
+      include Yast::Logger
+      include Y2Network::Helpers
+
       SYSCONFIG = "/etc/sysconfig/network".freeze
+      WICKED_PATH = "/etc/wicked".freeze
       WICKED_DHCP_PATH = "/var/lib/wicked/".freeze
       WICKED_ENTRIES = [
         { dir: SYSCONFIG, files: ["ifcfg-*", "ifroute-*", "routes"] },
         { dir: WICKED_DHCP_PATH, files: ["duid.xml", "iaid.xml", "lease*.xml"] },
-        { dir: ::File.join(ETC, "wicked"), files: ["common.xml"] }
+        { dir: WICKED_PATH, files: ["common.xml"] }
       ].freeze
 
       def copy
         adjust_files_for_network_disks!
-        WICKED_ENTRIES.each { |e| copy_files_to_target(e[:files], e[:dir]) }
+        WICKED_ENTRIES.each { |e| copy_to_target(e[:dir], include: e[:files]) }
         merge_sysconfig_files
       end
 

test/save_network_test.rb

@@ -45,8 +45,7 @@
     let(:selected_backend) { :wicked }
 
     before do
-      stub_const("Yast::SaveNetworkClient::ROOT_PATH", scr_root)
-      stub_const("Y2Network::ConfigCopier::ROOT_PATH", scr_root)
+      stub_const("Y2Network::Helpers::ROOT_PATH", scr_root)
       allow(Yast::Installation).to receive(:destdir).and_return(destdir)
       allow(Yast::Package).to receive(:Installed).and_return(false)
 

test/y2network/wicked/config_copier_test.rb

@@ -26,18 +26,14 @@
   let(:scr_root) { File.join(DATA_PATH, "instsys") }
 
   before do
-    stub_const("Y2Network::ConfigCopier::ROOT_PATH", scr_root)
+    stub_const("Y2Network::Helpers::ROOT_PATH", scr_root)
     allow(Yast::Installation).to receive(:destdir).and_return(destdir)
   end
 
   after do
     FileUtils.remove_entry(destdir) if Dir.exist?(destdir)
   end
 
-  describe "#copy" do
-
-  end
-
   describe "#adjust_files_for_network_disks!" do
     let(:template_file) { File.join(SCRStub::DATA_PATH, "ifcfg-eth0.template") }
     let(:file) { File.join(scr_root, sysconfig, "ifcfg-eth0") }