Skip to content
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

Jump to bottom

Consolidate logfile naming and destination filename #17438

Merged
merged 2 commits into from
May 21, 2018
Merged

Consolidate logfile naming and destination filename #17438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3625847
Add tests for destination_file_name before refactoring/changing
jrafanie May 16, 2018
53f2b84
Consolidate destination_file_name into LogFile#name
jrafanie May 16, 2018
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
13 changes: 10 additions & 3 deletions app/models/log_file.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,17 @@ def destination_directory
File.join("#{resource.zone.name}_#{resource.zone.id}", "#{resource.name}_#{resource.id}")
end

def self.logfile_name(resource, category = "Current", date_string = nil)
region = MiqRegion.my_region.try(:region) || "unknown"
[category, "region", region, resource.zone.name, resource.zone.id, resource.name, resource.id, date_string].compact.join(" ")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the .compact? Could anything be nil?

Copy link
Member Author

@jrafanie jrafanie May 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logfile_name is being used for non-log related materials such as automate dialogs/domains. date_string will be nil for them, but also in the unlikely event we couldn't find timestamps in the evm.log or compressed version of the evm.log.

end

def destination_file_name
date_string = "#{format_log_time(logging_started_on)}_#{format_log_time(logging_ended_on)}"
destname = historical ? "Archive_" : "Current_"
destname << "region_#{MiqRegion.my_region.try(:region) || "unknown"}_#{resource.zone.name}_#{resource.zone.id}_#{resource.name}_#{resource.id}_#{date_string}#{File.extname(local_file)}"
name.gsub(/\s+/, "_").concat(File.extname(local_file))
end

def name
super || self.class.logfile_name(resource)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to use the underscorized version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. I was trying to minimize change of the LogFile#name and it made sense to use underscores for a filename but didn't matter for LogFile#name.

end

def post_upload_tasks
Expand Down
15 changes: 5 additions & 10 deletions app/models/miq_server/log_management.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ def format_log_time(time)
def post_historical_logs(taskid, log_depot)
task = MiqTask.find(taskid)
log_prefix = "Task: [#{task.id}]"
log_type = "Archived"
log_type = "Archive"

# Post all compressed logs for a specific date + configs, creating a new row per day
VMDB::Util.compressed_log_patterns.each do |pattern|
log_start, log_end = log_start_and_end_for_pattern(pattern)
date_string = "#{format_log_time(log_start)} #{format_log_time(log_end)}" unless log_start.nil? && log_end.nil?
date_string = "#{format_log_time(log_start)}_#{format_log_time(log_end)}" unless log_start.nil? && log_end.nil?

cond = {:historical => true, :name => logfile_name(log_type, date_string), :state => 'available'}
cond = {:historical => true, :name => LogFile.logfile_name(self, log_type, date_string), :state => 'available'}
cond[:logging_started_on] = log_start unless log_start.nil?
cond[:logging_ended_on] = log_end unless log_end.nil?
logfile = log_files.find_by(cond)
Expand All @@ -39,14 +39,9 @@ def post_historical_logs(taskid, log_depot)
end
end

def logfile_name(category, date_string)
category = "Requested" if category == "Current"
"#{category} #{self.name} logs #{date_string} "
end

def log_patterns(log_type, base_pattern = nil)
case log_type.to_s.downcase
when "archived"
when "archive"
Array(::Settings.log.collection.archive.pattern).unshift(base_pattern)
when "current"
current_log_patterns
Expand Down Expand Up @@ -144,7 +139,7 @@ def post_one_log_pattern(pattern, logfile, log_type)
:local_file => local_file,
:logging_started_on => log_start,
:logging_ended_on => log_end,
:name => logfile_name(log_type, date_string),
:name => LogFile.logfile_name(self, log_type, date_string),
:description => "Logs for Zone #{zone.name rescue nil} Server #{self.name} #{date_string}",
)

Expand Down
4 changes: 2 additions & 2 deletions spec/models/file_depot_ftp_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
expect(vsftpd.content).to eq('uploads' =>
{"#{@zone.name}_#{@zone.id}" =>
{"#{@miq_server.name}_#{@miq_server.id}" =>
{"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}_unknown_unknown.txt" =>
{"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}.txt".gsub(/\s+/, "_") =>
log_file.local_file}}})
end

Expand All @@ -49,7 +49,7 @@
expect(vsftpd.content).to eq('uploads' =>
{"#{@zone.name}_#{@zone.id}" =>
{"#{@miq_server.name}_#{@miq_server.id}" =>
{"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}_unknown_unknown.txt" =>
{"Current_region_unknown_#{@zone.name}_#{@zone.id}_#{@miq_server.name}_#{@miq_server.id}.txt".gsub(/\s+/, "_") =>
log_file.local_file}}})
end

Expand Down
25 changes: 20 additions & 5 deletions spec/models/miq_server/log_management_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ def stub_vmdb_util_methods_for_collection_log
let(:log_end) { Time.parse("2018-05-11 15:34:16 UTC") }
let(:daily_log) { Rails.root.join("data", "user", "system", "evm_server_daily.zip").to_s}
let(:log_depot) { FactoryGirl.create(:file_depot) }
let!(:region) { MiqRegion.seed }
let(:zone) { @miq_server.zone }
before do
require 'vmdb/util'
allow(VMDB::Util).to receive(:compressed_log_patterns).and_return(compressed_log_patterns)
Expand All @@ -174,36 +176,49 @@ def stub_vmdb_util_methods_for_collection_log

it "no prior historical logfile" do
@miq_server.post_historical_logs(task.id, log_depot)
expect(@miq_server.reload.log_files.first).to have_attributes(
logfile = @miq_server.reload.log_files.first
expected_name = ["Archive", "region", region.region, zone.name, zone.id, @miq_server.name, @miq_server.id, "20180511_113312 20180511_153416"].join(" ")
expect(logfile).to have_attributes(
:file_depot => log_depot,
:local_file => daily_log,
:logging_started_on => log_start,
:logging_ended_on => log_end,
:name => "Archived #{@miq_server.name} logs 20180511_113312 20180511_153416 ",
:name => expected_name,
:description => "Logs for Zone #{@miq_server.zone.name} Server #{@miq_server.name} 20180511_113312 20180511_153416",
:miq_task_id => task.id
)

expected_filename = "Archive_region_#{region.region}_#{zone.name}_#{zone.id}_#{@miq_server.name}_#{@miq_server.id}_20180511_113312_20180511_153416.zip"
expected_filename.gsub!(/\s+/, "_")
expect(logfile.destination_file_name).to eq(expected_filename)

expect(task.reload).to have_attributes(
:message => "Archived log files from #{@miq_server.name} #{@miq_server.zone.name} MiqServer #{@miq_server.id} are posted",
:message => "Archive log files from #{@miq_server.name} #{@miq_server.zone.name} MiqServer #{@miq_server.id} are posted",
:state => "Active",
:status => "Ok",
)

end

it "no prior current logfile" do
allow(@miq_server).to receive(:current_log_patterns).and_return(current_log_patterns)
@miq_server.post_current_logs(task.id, log_depot)
expect(@miq_server.reload.log_files.first).to have_attributes(
logfile = @miq_server.reload.log_files.first
expected_name = ["Current", "region", region.region, zone.name, zone.id, @miq_server.name, @miq_server.id, "20180511_113312 20180511_153416"].join(" ")
expect(logfile).to have_attributes(
:file_depot => log_depot,
:local_file => daily_log,
:logging_started_on => log_start,
:logging_ended_on => log_end,
:name => "Requested #{@miq_server.name} logs 20180511_113312 20180511_153416 ",
:name => expected_name,
:description => "Logs for Zone #{@miq_server.zone.name} Server #{@miq_server.name} 20180511_113312 20180511_153416",
:miq_task_id => task.id
)

expected_filename = "Current_region_#{region.region}_#{zone.name}_#{zone.id}_#{@miq_server.name}_#{@miq_server.id}_20180511_113312_20180511_153416.zip"
expected_filename.gsub!(/\s+/, "_")
expect(logfile.destination_file_name).to eq(expected_filename)

expect(task.reload).to have_attributes(
:message => "Current log files from #{@miq_server.name} #{@miq_server.zone.name} MiqServer #{@miq_server.id} are posted",
:state => "Active",
Expand Down