-
Notifications
You must be signed in to change notification settings - Fork 980
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.
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
Fixes #36847 - Use initrd variable for Debian ipxe #10156
base: develop
Are you sure you want to change the base?
Conversation
@@ -36,7 +36,7 @@ ping --count 1 ${netX/dns} || echo Ping to DNS failed or ping command not availa | |||
<% kernel = boot_files_uris[0] -%> | |||
<% initrd = boot_files_uris[1] -%> | |||
|
|||
kernel <%= kernel %> initrd=initrd.img interface=auto url=<%= url %> ramdisk_size=10800 root=/dev/rd/0 rw auto <%= snippet("preseed_kernel_options", variables: {ipxe_net: true}).strip %> | |||
kernel <%= kernel %> initrd=<%= initrd %> interface=auto url=<%= url %> ramdisk_size=10800 root=/dev/rd/0 rw auto <%= snippet("preseed_kernel_options", variables: {ipxe_net: true}).strip %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at the snapshot failures, this should be something like basename(initrd)
(if such a thing exists)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or initrd.split('/').last
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same functionality can be used here, too
foreman/app/views/unattended/provisioning_templates/iPXE/autoyast_default_ipxe.erb
Line 33 in f0c5526
kernel <%= kernel %> initrd=initrd.img splash=silent install=<%= medium_uri %> autoyast=<%= foreman_url('provision') %> text-mode=1 useDHCP=1 <%= extra_args.join(' ')%> |
Previously the filename was hardcoded, but the OS layer actually provides this filename as a variable. This should match the actual filename on mirrors. The variables are already present in the environment, which means the manual calculation can be removed.
f0c5526
to
8d31dcc
Compare
This got a bit bigger, but I think this ends up cleaning it up nicely. |
<% subnet = @host.subnet -%> | ||
<% if subnet.respond_to?(:dhcp_boot_mode?) && subnet.dhcp_boot_mode? -%> | ||
kernel <%= kernel %> initrd=initrd.img splash=silent install=<%= medium_uri %> autoyast=<%= foreman_url('provision') %> text-mode=1 useDHCP=1 <%= extra_args.join(' ')%> | ||
kernel <%= @kernel_uri %> initrd=<%= @initrd_uri.split('/').last %> splash=silent install=<%= medium_uri %> autoyast=<%= foreman_url('provision') %> text-mode=1 useDHCP=1 <%= extra_args.join(' ')%> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, pretty cool cleanup!
I would prefer to have a method which returns @initrd_file and can be used in the various locations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I wasn't allowed to use File.basename
but given the repetition I don't think that woud have been pretty anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to have this including the new initrd_file method.
Previously the filename was hardcoded, but the OS layer actually provides this filename as a variable. This should match the actual filename on mirrors.