-
Notifications
You must be signed in to change notification settings - Fork 993
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 - Rename initrd for Debian #9870
Conversation
Can one of the admins verify this patch? |
2 similar comments
Can one of the admins verify this patch? |
Can one of the admins verify this patch? |
ok to test I've verified the URLs provided in https://projects.theforeman.org/issues/36847 so I think this is correct. |
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.
Test failures are relevant: the snapshots need to be updated.
Can you please update the snapshots @obol89 It should only be about these 2 files. Should be easys to update them manually on disk without recording the snapshots completely: https://github.com/theforeman/foreman/blob/develop/test/unit/foreman/renderer/snapshots/ProvisioningTemplate/iPXE/Preseed_default_iPXE.ubuntu4dhcp.snap.txt |
I just updated the files. I'm not sure if I did it correctly from the merge request / github perspective, but please let me know if there is anything else to do. |
Ready to merge @ekohl ? |
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.
Checking versions:
- Debian 10: https://ftp.debian.org/debian/dists/buster/main/installer-amd64/current/images/netboot/debian-installer/amd64/
- Debian 11: https://ftp.debian.org/debian/dists/bullseye/main/installer-amd64/current/images/netboot/debian-installer/amd64/
- Debian 12: https://ftp.debian.org/debian/dists/bookworm/main/installer-amd64/current/images/netboot/debian-installer/amd64/
So all supported Debian versions have this.
It's also weird we have a line initrd
below it which is correct. Looks like the specific line was introduced in 91de027. That came from theforeman/community-templates@8deda37. In theforeman/community-templates#496 it is explicitly discussed, but I suppose nobody verified it for Debian.
@@ -5,7 +5,7 @@ echo Trying to ping DNS: ${netX/dns} | |||
ping --count 1 ${netX/dns} || echo Ping to DNS failed or ping command not available. | |||
|
|||
|
|||
kernel http://ftp.debian.org/debian/dists/wheezy/main/installer-amd64/current/images/netboot/debian-installer/amd64/linux initrd=initrd.img interface=auto url=http://foreman.example.com/unattended/provision ramdisk_size=10800 root=/dev/rd/0 rw auto BOOTIF=01-${netX/mac:hexhyp} auto=true netcfg/disable_dhcp=false locale=en_US hostname=snapshot-ipv4-dhcp-deb10 domain=snap.example.com | |||
kernel http://ftp.debian.org/debian/dists/wheezy/main/installer-amd64/current/images/netboot/debian-installer/amd64/linux initrd=initrd.gz interface=auto url=http://foreman.example.com/unattended/provision ramdisk_size=10800 root=/dev/rd/0 rw auto BOOTIF=01-${netX/mac:hexhyp} auto=true netcfg/disable_dhcp=false locale=en_US hostname=snapshot-ipv4-dhcp-deb10 domain=snap.example.com |
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.
We should really update our tests to have something newer than Wheezy, but that's outside the scope of this PR so I opened #9972.
@@ -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.gz 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.
Why doesn't this use the initrd
variable that's defined on line 37? Shouldn't it be:
kernel <%= kernel %> initrd=initrd.gz 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 %> |
I'd expect it to then correctly use this code:
PXEFILES = {:kernel => "linux", :initrd => "initrd.gz"} |
@@ -5,7 +5,7 @@ echo Trying to ping DNS: ${netX/dns} | |||
ping --count 1 ${netX/dns} || echo Ping to DNS failed or ping command not available. | |||
|
|||
|
|||
kernel http://archive.ubuntu.com/ubuntu/dists/focal/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/linux initrd=initrd.img interface=auto url=http://foreman.example.com/unattended/provision ramdisk_size=10800 root=/dev/rd/0 rw auto BOOTIF=01-${netX/mac:hexhyp} console-setup/ask_detect=false console-setup/layout=USA console-setup/variant=USA keyboard-configuration/layoutcode=us localechooser/translation/warn-light=true localechooser/translation/warn-severe=true netcfg/disable_dhcp=false locale=en_US hostname=snapshot-ipv4-dhcp-ubuntu18 domain=snap.example.com |
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.
Confusingly enough, this combination is never valid. For Focal you only have legacy-images
and that directory only has boot.img.gz
, not initrd.*
.
Thank you for your contribution! This PR has been inactive for 3 months, closing for now. Feel free to reopen when you return to it. This is an automated process. |
I'm going to reopen this because I think it's still needed. |
[test] |
I opened #10156 with #9870 (comment) and resolved the merge conflicts. |
Thank you for your contribution! This PR has been inactive for 3 months, closing for now. Feel free to reopen when you return to it. This is an automated process. |
initrd
in official Debian repositories has.gz
extension, not.img
as it is in the template currently.