Merge pull request #477 from tidewise/machine-name-arg

feat: rename yml file before transferring (on top of #467)

Author: doudou

lib/syskit/cli/log_runtime_archive.rb

@@ -30,8 +30,8 @@ class LogRuntimeArchive
             #   to the ftp server @see process_root_folder_transfer
             # @param [Logger] logger the log structure
             def initialize(
-                root_dir, target_dir: nil,
-                logger: LogRuntimeArchive.null_logger
+                root_dir,
+                target_dir: nil, logger: LogRuntimeArchive.null_logger
             )
                 @last_archive_index = {}
                 @logger = logger
@@ -44,12 +44,13 @@ def initialize(
             #
             # @param [Params] server_params the FTP server parameters
             # @return [Array<TransferDatasetResult>]
-            def process_root_folder_transfer(server_params)
+            def process_root_folder_transfer(server_params, info_name: "info")
                 candidates = self.class.find_all_dataset_folders(@root_dir)
                 candidates.map do |child|
                     process_dataset_transfer(
                         child, server_params, @root_dir,
-                        full: !Roby::Application.log_dir_locked?(child.basename)
+                        full: !Roby::Application.log_dir_locked?(child.basename),
+                        info_name: info_name
                     )
                 end
             end
@@ -147,9 +148,10 @@ def process_dataset(child, full:, max_archive_size: DEFAULT_MAX_ARCHIVE_SIZE)
                 end
             end
 
-            def process_dataset_transfer(child, server, root, full:)
+            def process_dataset_transfer(child, server, root, full:, info_name: "info")
                 self.class.transfer_dataset(
-                    child, server, root, full: full, logger: @logger
+                    child, server, root,
+                    full: full, logger: @logger, info_name: info_name
                 )
             end
 
@@ -166,36 +168,74 @@ def failures
             end
 
             # Transfer the given dataset
-            def self.transfer_dataset(
+            def self.transfer_dataset( # rubocop:disable Metrics/ParameterLists
                 dataset_path, server, root,
-                full:, logger: null_logger
+                full:, info_name: "info", logger: null_logger
             )
                 logger.info(
                     "Transfering dataset #{dataset_path} in " \
                     "#{full ? 'full' : 'partial'} mode"
                 )
+                complete, paths_to_transfer =
+                    transfer_compute_paths(dataset_path, full, info_name)
+
+                transfer_results = paths_to_transfer.map do |source_path, target_name|
+                    result =
+                        transfer_file(source_path, server, root, target_name: target_name)
+                    source_path.unlink if result.success?
+
+                    result
+                end
+
+                result = TransferDatasetResult.new(
+                    complete: complete, transfer_results: transfer_results
+                )
+                log_transfer_results(dataset_path, result, logger: logger)
+            end
+
+            # @api private
+            #
+            # Compute paths to the files that should be transferred in a given dataset dir
+            #
+            # @param [Pathname] dataset_path the path to the dataset dir
+            # @param [Boolean] full if the transfer should be in full mode (all files
+            #   are transferred) or partial mode (only already rotated pocolog files
+            #   are transferred)
+            # @param [String] info_name name that should be given to the `info.yml`
+            #   file on the target machine
+            # @return [(Boolean,Array<Pathname>)] the boolean indicates whether this pass
+            #   is complete or not. The caller should call transfer_dataset until
+            #   the flag is true. The array is the list of paths to be transferred
+            #   in this pass
+            def self.transfer_compute_paths(dataset_path, full, info_name)
                 candidates =
                     each_file_from_path(dataset_path)
                     .reject { |p| p.extname == ".partial" }
 
-                complete, candidates =
+                complete, paths_to_transfer =
                     if full
                         archive_filter_candidates_full(candidates)
                     else
                         archive_filter_candidates_partial(candidates)
                     end
 
-                transfer_results = candidates.map do |child_path|
-                    result = transfer_file(child_path, server, root)
-                    child_path.unlink if result.success?
+                paths_to_transfer =
+                    transfer_compute_remote_names(paths_to_transfer, info_name)
 
-                    result
-                end
+                [complete, paths_to_transfer]
+            end
 
-                result = TransferDatasetResult.new(
-                    complete: complete, transfer_results: transfer_results
-                )
-                log_transfer_results(dataset_path, result, logger: logger)
+            # Renames the info.yml file
+            def self.transfer_compute_remote_names(paths, info_name)
+                paths.map do |p|
+                    basename = p.basename.to_s
+
+                    if basename == "info.yml"
+                        [p, "#{info_name}.yml"]
+                    else
+                        [p, basename]
+                    end
+                end
             end
 
             # Logs the transfer dataset results
@@ -232,10 +272,11 @@ def self.log_transfer_results(dataset_path, result, logger: null_logger)
             # Transfer a file to the central log server via FTP
             #
             # @return [LogUploadState:Result]
-            def self.transfer_file(file, server, root)
+            def self.transfer_file(file, server, root, target_name: File.basename(file))
                 ftp = RobyApp::LogTransferServer::FTPUpload.new(
                     server.host, server.port, server.certificate, server.user,
                     server.password, file,
+                    target_name: target_name,
                     max_upload_rate: server.max_upload_rate || Float::INFINITY,
                     implicit_ftps: server.implicit_ftps
                 )

lib/syskit/cli/log_runtime_archive_main.rb

@@ -76,6 +76,11 @@ def archive(root_dir, target_dir)
                    type: :boolean, default: false,
                    desc: "use implicit connection method for ftps " \
                          "(disable for 2.5 clients on 2.7 servers)"
+            option :info_name,
+                   type: :string, default: "info",
+                   desc: "basename (without extension) under which the info.yml " \
+                         "metadata file should be uploaded. Use for instance the " \
+                         "machine name"
             def watch_transfer( # rubocop:disable Metrics/ParameterLists
                 source_dir, host, port, certificate_path, user, password
             )
@@ -100,6 +105,11 @@ def watch_transfer( # rubocop:disable Metrics/ParameterLists
                    type: :boolean, default: false,
                    desc: "use implicit connection method for ftps " \
                          "(disable for 2.5 clients on 2.7 servers)"
+            option :info_name,
+                   type: :string, default: "info",
+                   desc: "basename (without extension) under which the info.yml " \
+                         "metadata file should be uploaded. Use for instance the " \
+                         "machine name"
             def transfer( # rubocop:disable Metrics/ParameterLists
                 source_dir, host, port, certificate_path, user, password
             )
@@ -112,7 +122,9 @@ def transfer( # rubocop:disable Metrics/ParameterLists
                     implicit_ftps: options[:implicit_ftps],
                     max_upload_rate: rate_mbps_to_bps(options[:max_upload_rate_mbps])
                 )
-                archiver.process_root_folder_transfer(server_params)
+                archiver.process_root_folder_transfer(
+                    server_params, info_name: options[:info_name]
+                )
             end
 
             desc "transfer_server TARGET_DIR HOST PORT CERTFILE_PATH USER PASSWORD",

lib/syskit/roby_app/log_transfer_server/ftp_upload.rb

@@ -10,7 +10,8 @@ class FTPUpload
                 def initialize( # rubocop:disable Metrics/ParameterLists
                     host, port, certificate, user, password, file,
                     max_upload_rate: Float::INFINITY,
-                    implicit_ftps: false
+                    implicit_ftps: false,
+                    target_name: File.basename(file)
                 )
 
                     @host = host
@@ -19,6 +20,7 @@ def initialize( # rubocop:disable Metrics/ParameterLists
                     @user = user
                     @password = password
                     @file = file
+                    @target_name = target_name
 
                     @max_upload_rate = Float(max_upload_rate)
                     if @max_upload_rate <= 0
@@ -89,17 +91,16 @@ def transfer(ftp, root)
                     last = Time.now
                     chdir_to_file_directory(ftp, root) if root
 
-                    target_name = File.basename(@file)
                     File.open(@file) do |file_io|
-                        ftp.storbinary("STOR #{target_name}.partial",
+                        ftp.storbinary("STOR #{@target_name}.partial",
                                        file_io, Net::FTP::DEFAULT_BLOCKSIZE) do |buf|
                             now = Time.now
                             rate_limit(buf.size, now, last)
                             last = Time.now
                         end
                     end
 
-                    ftp.rename("#{target_name}.partial", target_name)
+                    ftp.rename("#{@target_name}.partial", @target_name)
                 end
 
                 # @api private

test/cli/test_log_runtime_archive.rb

@@ -666,44 +666,63 @@ def create_server(params)
                 end
 
                 describe ".transfer_dataset" do
-                    before do
-                        @dataset = make_valid_folder("PATH")
-                        make_random_file "test.0.log", root: @dataset
-                    end
+                    describe "transfer of normal files" do
+                        before do
+                            @dataset = make_valid_folder("PATH")
+                            make_random_file "test.0.log", root: @dataset
+                        end
 
-                    it "transfers a dataset through FTP" do
-                        results = LogRuntimeArchive.transfer_dataset(
-                            @dataset, @params, @root, full: true
-                        )
+                        it "transfers a dataset through FTP" do
+                            results = LogRuntimeArchive.transfer_dataset(
+                                @dataset, @params, @root, full: true
+                            )
 
-                        assert results.success?
-                        # Datasets that have pocolog files are not complete
-                        refute results.complete
-                        assert(File.exist?(@target_dir / "PATH" / "test.0.log"))
-                    end
+                            assert results.success?
+                            # Datasets that have pocolog files are not complete
+                            refute results.complete
+                            assert(File.exist?(@target_dir / "PATH" / "test.0.log"))
+                        end
 
-                    it "removes the source file if the transfer was successful" do
-                        results = LogRuntimeArchive.transfer_dataset(
-                            @dataset, @params, @root, full: true
-                        )
+                        it "removes the source file if the transfer was successful" do
+                            results = LogRuntimeArchive.transfer_dataset(
+                                @dataset, @params, @root, full: true
+                            )
+
+                            assert results.success?
+                            refute((@dataset / "test.0.log").exist?)
+                        end
 
-                        assert results.success?
-                        refute((@dataset / "test.0.log").exist?)
+                        it "does not remove the source file if the transfer failed" do
+                            result = RobyApp::LogTransferServer::LogUploadState::Result
+                                     .new("/PATH", false, "message")
+                            flexmock(LogRuntimeArchive)
+                                .should_receive(:transfer_file)
+                                .and_return(result)
+                            results = LogRuntimeArchive.transfer_dataset(
+                                @dataset, @params, @root, full: true
+                            )
+
+                            refute results.success?
+                            assert((@dataset / "test.0.log").exist?)
+                        end
                     end
 
-                    it "does not remove the source file if the transfer failed" do
-                        result = RobyApp::LogTransferServer::LogUploadState::Result.new(
-                            "/PATH", false, "message"
-                        )
-                        flexmock(LogRuntimeArchive)
-                            .should_receive(:transfer_file)
-                            .and_return(result)
-                        results = LogRuntimeArchive.transfer_dataset(
-                            @dataset, @params, @root, full: true
-                        )
+                    describe "transfer of the info.yml file" do
+                        before do
+                            @dataset = make_valid_folder("PATH")
+                            @info_yml_content = make_random_file "info.yml",
+                                                                 root: @dataset
+                        end
 
-                        refute results.success?
-                        assert((@dataset / "test.0.log").exist?)
+                        it "renames info.yml during on the remote machine" do
+                            results = LogRuntimeArchive.transfer_dataset(
+                                @dataset, @params, @root, full: true, info_name: "test"
+                            )
+                            assert results.success?
+
+                            expected_path = @target_dir / "PATH" / "test.yml"
+                            assert_equal @info_yml_content, expected_path.read
+                        end
                     end
                 end
 

test/cli/test_log_runtime_archive_main.rb

@@ -186,7 +186,7 @@ def call_archive(root_path, archive_path, low_limit, freed_limit)
                     flexmock(LogRuntimeArchive)
                         .new_instances
                         .should_receive(:process_root_folder_transfer)
-                        .with(ftp_params)
+                        .with(ftp_params, info_name: "something")
                         .and_raise(quit)
 
                     assert_raises(quit) do
@@ -197,7 +197,8 @@ def call_archive(root_path, archive_path, low_limit, freed_limit)
                              "user", "password",
                              "--period", 0.5,
                              "--max_upload_rate_mbps", 10,
-                             "--implicit-ftps"]
+                             "--implicit-ftps",
+                             "--info-name", "something"]
                         )
                     end
                 end

test/process_managers/test_remote.rb

@@ -528,7 +528,7 @@ def wait_for_upload_completion(poll_period: 0.01, timeout: 1)
                 end
 
                 state = client.log_upload_state
-                return state if state.pending_count == 0
+                return state if state.each_result.first
 
                 sleep poll_period
             end