-
Notifications
You must be signed in to change notification settings - Fork 148
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
Added foreman-bootloaders-redhat-201705231433 #1656
Conversation
Hmmm I need to find out how do I add a URL to annex:
|
Ok had Source0 in non-URL format, that's why it failed. |
Hmmm but it does exist:
|
I think this has something to do with github HEAD request, when I try this against Apache2 httpd it works just ok locally.
|
Changing distribution URL to http://downloads.theforeman.org/foreman-bootloaders/ instead of github. |
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.
Thanks @lzap , this is an useful addition, it takes time to find these bootloaders if you don't know where to look for them. Let me know what you think about the comments inline
%define _binaries_in_noarch_packages_terminate_build 0 | ||
|
||
Name: foreman-bootloaders-redhat | ||
Version: 201705231433 |
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 not version 1?
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.
This is standard fedora packaging versioning scheme for "from SCM" aka "snapshot" which is I believe better than just number:
Source0: http://downloads.theforeman.org/foreman-bootloaders/%{name}-%{version}.tar.bz2 | ||
BuildArch: noarch | ||
|
||
Requires: /var/lib/tftpboot |
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 TFTP proxy does not need this directory, in fact my TFTP server path is /var/lib/tftproot
. Would it be possible to get the directory from /etc/foreman-proxy/settings.d/tftp.yml
? There's an option tftproot
in that file that specifies which directory the proxy will use.
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.
What system are you on? For both RHEL and Fedora it's really tftpboot:
[lzap@lzapx ~]$ LC_ALL=C yum whatprovides /var/lib/tftproot
Last metadata expiration check: 1:55:32 ago on Tue May 30 12:09:43 2017.
Error: No Matches found
[lzap@lzapx ~]$ LC_ALL=C yum whatprovides /var/lib/tftpboot
Last metadata expiration check: 1:55:36 ago on Tue May 30 12:09:43 2017.
tftp-server-5.2-18.fc25.x86_64 : The server for the Trivial File Transfer Protocol (TFTP)
Repo : @System
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.
I'm on Fedora, but I changed it manually long ago to tftproot
- my point is that it's configurable by the proxy, so the package should read that configuration instead of extracting in the default location
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.
To expand on this, both Fedora and RHEL are really /var/lib/tftpboot
. That's also what we use in our installer (see tftp/manifests/params.pp). Also in the proxy it's the same default value:
config/settings.d/tftp.yml.example
5:#:tftproot: /var/lib/tftpboot
I think you have it wrong, unless you use Debian/Ubuntu where this path is different I believe @mmoll ?
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.
It's not a matter of right or wrong, it's completely configurable on the proxy and xinetd, the point is that the package should respect the existing configuration (otherwise it'd just not work, like on my installation). The default is the same in all operating systems AFAIK. Here are my config files:
https://gist.github.com/dLobatog/bd32aba094a91bf8b914a7f148d5db82
%{_sharedstatedir}/tftpboot/grub2/grubppc64le.elf | ||
%{_sharedstatedir}/tftpboot/grub2/shimaa64.efi | ||
%{_sharedstatedir}/tftpboot/grub2/shimia32-redhat.efi | ||
%{_sharedstatedir}/tftpboot/grub2/grubppc64.efi |
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.
This file is 0b in the tgz, why is it necessary?
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.
This is supposed to be symlink to *elf files (two of them) to workaround this problem: http://projects.theforeman.org/issues/16706 I will verify if these are really symlinks and fix this if not.
%{_sharedstatedir}/tftpboot/grub2/shimia32-redhat.efi | ||
%{_sharedstatedir}/tftpboot/grub2/grubppc64.efi | ||
%{_sharedstatedir}/tftpboot/grub2/shimx64.efi | ||
%{_sharedstatedir}/tftpboot/grub2/grubppc64le.efi |
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.
This file is 0b in the tgz, why is it necessary?
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.
Ditto.
|
||
|
||
%files | ||
%{_sharedstatedir}/tftpboot/grub/grubx64.efi |
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.
Nitpick, comparing this to the tgz contents is difficult, if it was alphabetically ordered it would be easier
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.
I do not sort files in the resulting tar by name, but I added this good idea: theforeman/foreman-bootloaders@dbdf41f
|
||
%install | ||
install -d -m0755 %{buildroot}%{_sharedstatedir}/tftpboot | ||
cp -p -r grub grub2 %{buildroot}%{_sharedstatedir}/tftpboot |
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.
-p
will preserve ownership by root, is that necessary or foreman-proxy
would be more appropriate?
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.
This is from Foreman installation:
[root@hp-dl2x170g6-01 ~]# ll /var/lib/tftpboot -d
drwxr-xr-x. 6 root root 135 May 29 12:38 /var/lib/tftpboot
[root@hp-dl2x170g6-01 ~]# ll /var/lib/tftpboot
total 136
drwxr-xr-x. 2 foreman-proxy root 166 May 29 12:10 boot
-rw-r--r--. 1 root root 20832 May 29 12:38 chain.c32
-rw-r--r--. 1 root root 26268 May 29 12:38 memdisk
-rw-r--r--. 1 root root 55140 May 29 12:38 menu.c32
drwxr-xr-x. 2 foreman-proxy root 6 May 29 12:38 poap.cfg
-rw-r--r--. 1 root root 26826 May 29 12:38 pxelinux.0
drwxr-xr-x. 2 foreman-proxy root 189 May 29 12:56 pxelinux.cfg
drwxr-xr-x. 2 foreman-proxy root 6 May 29 12:38 ztp.cfg
This is from Foreman 1.15 actually:
[root@zzzap ~]# ll /var/lib/tftpboot -d
drwxr-xr-x. 8 root root 177 30. kvě 16.09 /var/lib/tftpboot
[root@zzzap ~]# ll /var/lib/tftpboot
celkem 172
drwxr-xr-x. 2 foreman-proxy root 6 30. kvě 16.09 boot
drwxr-xr-x. 2 foreman-proxy root 6 30. kvě 16.09 grub
drwxr-xr-x. 2 foreman-proxy root 57 30. kvě 16.10 grub2
-rw-r--r--. 1 root root 20704 30. kvě 16.03 chain.c32
-rw-r--r--. 1 root root 33628 30. kvě 16.03 mboot.c32
-rw-r--r--. 1 root root 26140 30. kvě 16.03 memdisk
-rw-r--r--. 1 root root 55012 30. kvě 16.03 menu.c32
drwxr-xr-x. 2 foreman-proxy root 6 30. kvě 16.09 poap.cfg
drwxr-xr-x. 2 foreman-proxy root 6 30. kvě 16.09 pxelinux.cfg
-rw-r--r--. 1 root root 26764 30. kvě 16.03 pxelinux.0
drwxr-xr-x. 2 foreman-proxy root 6 30. kvě 16.09 ztp.cfg
You are right, I need to make sure that both grub and grub2 dirs are owned by foreman-proxy with 7xx perms, I will explicitly change that. New dependency will be required there.
Ok pushed, reordered and added explicit owner, added require and buildrequire for |
Ok new version that installs into
|
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.
@lzap Thanks for the update! I left a comment about the Requires, it should be on the tftpboot package.
Source0: http://downloads.theforeman.org/foreman-bootloaders/%{name}-%{version}.tar.bz2 | ||
BuildArch: noarch | ||
|
||
Requires: /var/lib/tftpboot |
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.
This Requires
should probably be moved now to the tftpboot package
Done, also few more changes to polish some warnings. |
I can get this added and built, I was a little curious about why the split out to the sub-package? Is it so you can use and install |
@ehelms thanks for testing, the idea is to install just Guys I need to get this in this week in order to match downstream deadlines, we've been working hard on this. If there are no other objections, let's merge this. Thanks! |
@ehelms The point is that It's a convenience we can do at the packaging level for users following the default directory for TFTP given that installer changes are a no-no at this point of 1.15. |
[test] |
@lzap instead of a new PR, can you update this one to update the |
Done, added to Foreman Core EL7/F24 comps and whitelists. This needs to be installed next to foreman-proxy package. |
Was wondering if @mmoll can help me with Debian package, this is very relevant in Debian context. Having ability to install Red Hat patched Grub2 would be big plus as Debian does not carry MAC address based config search. |
This might be late to the party but why name it |
Well, I have no current plans to build Debian loaders, but this naming allows this. Yes. |
So in short we should always install |
Yes that's the plan. Debian builds won't currently work due to missing patches, see https://github.com/theforeman/foreman-bootloaders#ubuntu-and-debian-support |
This adds shim, grub and grub2 bootloaders of multiple architectures
(supported by RHEL) into a package. The ultimate goal is to allow our
users to install it and use it without pain of getting them extracted
from DVD installation ISOs or RPM packages or building their own
versions. Long term, we can also ship PXELinux once syslinux team does
new release (EFI was not yet released).
Please read the upstream site for more info:
https://github.com/theforeman/foreman-bootloaders
A change in Foreman has been recently merged which follows the new
naming pattern this package brings:
theforeman/foreman#4552
This package will be followed by "fedora" version which carries most recent
builds of these packages. I also plan to do changes in the installer to
leverage this package rather than copying or building Grub2 from scratch.
Package for Debian systems will be available for review shortly.
For plugin updates, please indicate which repos this should be built into: