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

Fixes #36729 - Implement pruning outdated TFTP files #873

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fixes #36729 - Implement pruning outdated TFTP files #873

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
7 changes: 7 additions & 0 deletions modules/tftp/server.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ def delete_file(file)
logger.debug "TFTP: Skipping a request to delete a file which doesn't exists"
end
end

def self.outdated_files(age)
delete_before = Time.now - age
Dir.glob(File.join(Proxy::TFTP::Plugin.settings.tftproot, "boot", "*-{vmlinuz,initrd.img}")).filter do |file|
File.file?(file) && File.mtime(file) < delete_before
Copy link
Member

Choose a reason for hiding this comment

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

Could ctime be used? Assuming the file is written once and then never changed, would ctime ever change?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but then you still have the problem where:

  • Host X provisions RHEL 9
  • Host X's token expires
  • Host Y enters build mode for RHEL 9
  • Pruning happens
  • Host Y attempts to retrieve TFTP files

Note that's also a problem with the current foreman_maintain implementation. Perhaps few users hit this because they may use external Smart Proxies and never run the code on them.

Copy link
Member

Choose a reason for hiding this comment

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

I assume that Host Y enters build mode for RHEL 9 does a check if the files already exist on the smart-proxy, and if they do no operation is performed. And in theory, we would need to "touch" the files somehow to indicate. Which I think then is where you were getting at Foreman (or maybe the smart-proxy object?) having a canonical list of needed files?

Copy link
Member Author

Choose a reason for hiding this comment

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

When a host enters build mode (which can also be rebuilding an existing host), it immediately retrieves the files. Then the host still needs to boot. A BIOS (or UEFI) can take a significant amount of time on server class hardware (a minute is often on the low end). So if the pruning happens between those 2 events (Smart Proxy fetches TFTP files and host fetches TFTP files from Smart Proxy) you have a problem.

I think this requires a bit more thought on what the best way is, but bringing it back to the original issue: I'd recommend removing this from a health check and instead make it a separate step.

end
end
end

class Syslinux < Server
Expand Down
19 changes: 19 additions & 0 deletions modules/tftp/tftp_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,24 @@ def create_default(variant)
get "/serverName" do
{"serverName" => (Proxy::TFTP::Plugin.settings.tftp_servername || "")}.to_json
end

# Purge old entries
post "/prune" do
# how old a file can be, in seconds
age = params[:age]

if age.nil? || age.to_i <= 0
log_halt 400, "Invalid age; needs to be a valid integer > 0"
end

to_prune = Proxy::TFTP::Server.outdated_files(age.to_i)

to_prune.each do |file|
logger.debug("Removing outdated file", file)
File.unlink(file)
end

pruned.length
end
end
end