Skip to content

fix: cleanup the log transfer functionality #569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
doudou wants to merge 1 commit into from
Open

fix: cleanup the log transfer functionality #569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .rubocop_todo.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,7 +609,6 @@ Metrics/AbcSize:
- 'lib/syskit/robot/master_device_instance.rb'
- 'lib/syskit/robot/robot_definition.rb'
- 'lib/syskit/roby_app/configuration.rb'
- 'lib/syskit/roby_app/log_transfer_server/spawn_server.rb'
- 'lib/syskit/roby_app/logging_group.rb'
- 'lib/syskit/roby_app/plugin.rb'
- 'lib/syskit/roby_app/rest_api.rb'
Expand Down Expand Up @@ -799,7 +798,6 @@ Metrics/ParameterLists:
- 'lib/syskit/robot/communication_bus.rb'
- 'lib/syskit/robot/master_device_instance.rb'
- 'lib/syskit/roby_app/configuration.rb'
- 'lib/syskit/roby_app/log_transfer_server/spawn_server.rb'
- 'lib/syskit/runtime/connection_management.rb'

# Offense count: 119
Expand Down
2 changes: 0 additions & 2 deletions lib/syskit.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,8 @@ module ProcessManagers
end
end

require "syskit/roby_app/log_transfer_server"
require "syskit/process_managers/process_base"
require "syskit/process_managers/status"
require "syskit/roby_app/log_transfer_server/log_upload_state"
require "syskit/process_managers/remote/protocol"
require "syskit/process_managers/remote/loader"
require "syskit/process_managers/remote/manager"
Expand Down
4 changes: 2 additions & 2 deletions lib/syskit/cli/log_runtime_archive.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

require "archive/tar/minitar"
require "sys/filesystem"
require "syskit/roby_app/log_transfer_server/ftp_upload"
require "syskit/roby_app/log_transfer/ftp_upload"

module Syskit
module CLI
Expand Down Expand Up @@ -300,7 +300,7 @@ def self.log_transfer_results(result, logger: null_logger)
#
# @return [LogUploadState:Result]
def self.transfer_file(file, server, root, target_name: File.basename(file))
ftp = RobyApp::LogTransferServer::FTPUpload.new(
ftp = RobyApp::LogTransfer::FTPUpload.new(
server.host, server.port, server.certificate, server.user,
server.password, file,
target_name: target_name,
Expand Down
24 changes: 21 additions & 3 deletions lib/syskit/cli/log_runtime_archive_main.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
require "pathname"
require "thor"
require "syskit/cli/log_runtime_archive"
require "syskit/runtime/server/spawn_server"
require "syskit/roby_app/log_transfer/server"

module Syskit
module CLI
Expand Down Expand Up @@ -129,13 +129,30 @@ def transfer( # rubocop:disable Metrics/ParameterLists

desc "transfer_server TARGET_DIR HOST PORT CERTFILE_PATH USER PASSWORD",
"creates the log transfer FTP server that runs on the main computer"
long_desc <<~DOC
To create a certificate, run the following, replacing 10.16.1.1 by the
IP that will be used by clients to access the transfer server

openssl genrsa -out transfer_rsa_private_key.pem 2048

openssl req -new -x509 -key transfer_rsa_private_key.pem \
-out transfer_certificate.pem -days +3650 -subj "/CN=10.16.1.1"

cat transfer_rsa_private_key.pem transfer_certificate.pem \
> log-transfer-server.pem

Then use transfer_certificate.pem on the client machines
DOC
option :implicit_ftps,
type: :boolean, default: false,
desc: "use implicit connection method for ftps " \
"(disable for 2.5 clients on 2.7 servers)"
option :min_free_space,
type: :numeric, default: 0,
desc: "abort transfers if the disk space is below this limit (in MB)"
option :debug,
type: :boolean, default: false,
desc: "enable debug output"
def transfer_server( # rubocop:disable Metrics/ParameterLists
target_dir, host, port, certfile_path, user, password
)
Expand Down Expand Up @@ -220,13 +237,14 @@ def create_server( # rubocop:disable Metrics/ParameterLists
target_dir, host, port, certificate, user, password,
implicit_ftps, min_free_space
)
Runtime::Server::SpawnServer.new(
RobyApp::LogTransfer::Server.new(
target_dir, user, password,
certificate,
interface: host,
port: port,
implicit_ftps: implicit_ftps,
min_free_space: min_free_space
min_free_space: min_free_space,
debug: options[:debug]
)
end

Expand Down
36 changes: 0 additions & 36 deletions lib/syskit/process_managers/remote/manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,6 @@ module ProcessManagers
#
# @see Configuration#use_deployment DeploymentGroup#use_deployment
module Remote
# Type transferred between the server and the manager to report on log updates
#
# Defined here to make sure it is actually defined. Otherwise, the log
# state reporting would fail at runtime, and unit-testing for this is
# very hard.
LogUploadState = RobyApp::LogTransferServer::LogUploadState

# Syskit-side interface to the remote process server
class Manager
# Emitted when an operation fails
Expand Down Expand Up @@ -287,35 +280,6 @@ def queue_death_announcement(deadline:)
@death_queue.push(read_object(deadline: deadline))
end

# Initiate the upload of a file from the remote process server
#
# The transfer is asynchronous, use {#upload_state} to track the
# upload progress
def log_upload_file(
host, port, certificate, user, password, localfile,
max_upload_rate: Float::INFINITY,
implicit_ftps: Runtime::Server.use_implicit_ftps?
)
write_command(
COMMAND_LOG_UPLOAD_FILE,
[host, port, certificate, user, password, localfile,
max_upload_rate, implicit_ftps]
)

wait_for_ack
end

# Query the current state of log upload
#
# @return [UploadState]
def log_upload_state
write_command(COMMAND_LOG_UPLOAD_STATE)

deadline = compute_response_deadline
wait_for_ack
read_object(deadline: deadline)
end

# Wait for some data to be available on the socket
#
# This is really meant for unit tests. Do not use in live code.
Expand Down
2 changes: 0 additions & 2 deletions lib/syskit/process_managers/remote/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,5 @@ module Server
end

require "syskit/process_managers/remote/protocol"
require "syskit/roby_app/log_transfer_server/ftp_upload"
require "syskit/roby_app/log_transfer_server/log_upload_state"
require "syskit/process_managers/remote/server/process"
require "syskit/process_managers/remote/server/server"
88 changes: 0 additions & 88 deletions lib/syskit/process_managers/remote/server/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,6 @@ def initialize(
@name_service_ip = name_service_ip
@processes = {}
@all_ios = []
@log_upload_command_queue = Queue.new
@log_upload_results_queue = Queue.new
@log_upload_pending = Concurrent::AtomicFixnum.new
@log_upload_thread = Thread.new { log_upload_main }
end

def each_client(&block)
Expand Down Expand Up @@ -261,8 +257,6 @@ def announce_dead_processes(dead_processes)

# Helper method that stops all running processes
def quit_and_join # :nodoc:
@log_upload_command_queue << nil

info "stopping process server"
processes.each_value do |p|
info "killing #{p.name}"
Expand All @@ -273,8 +267,6 @@ def quit_and_join # :nodoc:
close_client(socket)
rescue SystemCallError, IOError # rubocop:disable Lint/SuppressedException
end

@log_upload_thread.join
end

# Helper method that deals with one client request
Expand Down Expand Up @@ -357,19 +349,6 @@ def handle_command(socket) # :nodoc:
socket.write Marshal.dump(ret)
elsif cmd_code == COMMAND_QUIT
quit
elsif cmd_code == COMMAND_LOG_UPLOAD_FILE
parameters = Marshal.load(socket)
begin
log_upload_file(socket, parameters)
send_ack(socket)
rescue StandardError => e
send_nack(socket, e.message)
end

elsif cmd_code == COMMAND_LOG_UPLOAD_STATE
state = log_upload_state
socket.write RET_YES
socket.write Marshal.dump(state)
elsif cmd_code == COMMAND_WAIT_RUNNING
result = {}
process_names = Marshal.load(socket)
Expand Down Expand Up @@ -552,73 +531,6 @@ def quit
@quit = true
@com_w&.write INTERNAL_QUIT
end

def log_upload_file(socket, parameters)
host, port, certificate, user, password, localfile,
max_upload_rate, implicit_ftps = parameters

debug "#{socket} requested uploading of #{localfile}"

begin
localfile = log_upload_sanitize_path(Pathname(localfile))
rescue Exception => e # rubocop:disable Lint/RescueException
@log_upload_results_queue <<
RobyApp::LogTransferServer::LogUploadState::Result.new(
localfile, false, e.message
)
return
end

info "queueing upload of #{localfile} to #{host}:#{port}"
@log_upload_command_queue <<
RobyApp::LogTransferServer::FTPUpload.new(
host, port, certificate,
user, password, localfile,
max_upload_rate: max_upload_rate || Float::INFINITY,
implicit_ftps: implicit_ftps
)
@log_upload_pending.increment
end

def log_upload_sanitize_path(path)
log_path = Pathname(app.log_dir)
full_path = path.realpath(log_path)
if full_path.to_s.start_with?(log_path.to_s + "/")
return full_path
end

raise ArgumentError,
"cannot upload files not within the app's log directory"
end

def log_upload_main
while (transfer = @log_upload_command_queue.pop)
@log_upload_results_queue << transfer.open_and_transfer
@log_upload_pending.decrement
end
end

def log_upload_state
results = []
loop do
results << @log_upload_results_queue.pop(true)
rescue ThreadError
break
end

log_dir = Pathname.new(app.log_dir)
results.each do |r|
if r.success?
r.file.unlink
r.file =
Pathname.new(r.file).relative_path_from(log_dir).to_s
end
end

RobyApp::LogTransferServer::LogUploadState.new(
@log_upload_pending.value, results
)
end
end
end
end
Expand Down
4 changes: 0 additions & 4 deletions lib/syskit/roby_app.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ module RobyApp
require "syskit/roby_app/plugin"
require "syskit/roby_app/single_file_dsl"

require "syskit/roby_app/tmp_root_ca"
require "syskit/roby_app/log_transfer_manager"
require "syskit/roby_app/log_transfer_server"

module Syskit
class << self
# The main configuration object
Expand Down
41 changes: 3 additions & 38 deletions lib/syskit/roby_app/configuration.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module RobyApp
#
# The main configuration instance is accessible as Syskit.conf or (if
# running in a Roby application) as Conf.syskit
class Configuration # rubocop:disable Metrics/ClassLength
class Configuration
# The application that we are configuring
# @return [Roby::Application]
attr_reader :app
Expand Down Expand Up @@ -59,25 +59,6 @@ class Configuration # rubocop:disable Metrics/ClassLength
# is 20s
attr_accessor :exception_transition_timeout

# Configuration of Syskit's log transfer functionality
#
# Minimum configuration: set `ip` to an IP which the process servers
# can reach. You must also configure log rotation
# ({#log_rotation_period}). Syskit will transfer the rotated logs to
# the main Syskit's instance log directory.
#
# If you want to transfer to another dir, also set {#target_dir}. If you do
# set {#target_dir}, local files will also be transferred. There is currently
# no optimization for this (the local logs will also be transferred through
# the network)
#
# If you want to use an external server, you must also provide its public
# certificate and set self_spawned to false. In this case, target_dir is
# ignored
#
# @return [LogTransferManager::Configuration]
attr_reader :log_transfer

# Period in seconds for triggering log rotation and transfer
#
# This is considered experimental, and is disabled by default
Expand Down Expand Up @@ -337,17 +318,6 @@ def initialize(app)
@resolution_time_slice = 0.5

@log_rotation_period = nil
@log_transfer = LogTransferManager::Configuration.new(
user: "syskit",
port: 20_301,
password: SecureRandom.base64(32),
self_spawned: true,
certificate: nil, # Use random generated self-signed certificate
target_dir: nil, # Use the app's log dir
default_max_upload_rate: Float::INFINITY,
max_upload_rates: {},
implicit_ftps: Runtime::Server.use_implicit_ftps?
)

clear
self.export_types = true
Expand Down Expand Up @@ -860,18 +830,17 @@ def register_remote_manager(
client = ProcessManagers::Remote::Manager.new(
host, port, root_loader: app.default_loader
)
config = register_process_server(
register_process_server(
name, client,
host_id: host_id || name,
logging_enabled: logging_enabled,
register_on_name_server: register_on_name_server
)
config.supports_log_transfer = true
client
end

ProcessServerConfig =
Struct.new :name, :client, :log_dir, :host_id, :supports_log_transfer,
Struct.new :name, :client, :log_dir, :host_id,
:logging_enabled, :register_on_name_server, :disabled,
keyword_init: true do
def manager
Expand All @@ -897,10 +866,6 @@ def loader
client.loader
end

def supports_log_transfer?
supports_log_transfer
end

def logging_enabled?
logging_enabled
end
Expand Down
4 changes: 2 additions & 2 deletions ...oby_app/log_transfer_server/ftp_upload.rb → ...yskit/roby_app/log_transfer/ftp_upload.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
# frozen_string_literal: true

require "syskit/roby_app/log_transfer_server/log_upload_state"
require "syskit/roby_app/log_transfer/log_upload_state"

module Syskit
module RobyApp
module LogTransferServer
module LogTransfer
# Encapsulation of the log file upload process
class FTPUpload
def initialize( # rubocop:disable Metrics/ParameterLists
Expand Down
Loading