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.

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 #25120 - use medium provider interface #69

Merged
merged 1 commit into from Oct 15, 2018

Conversation

timogoebel
Copy link
Member

This commit makes use of the new medium provider interface to build the full host image as the old way of calling the methods is deprecated and will be removed in 1.22.
The embedded PXElinux template is now rendered in it's own template scope (yay, new renderer!) so we can avoid ugly gsub string replacement hacks.

This also prepares foreman_bootdisk for frozen strings.

And all of this removes this loop, which is very hard to digest:

files.map! do |bootfile_info|
bootfile_info.map do |f|
suffix = f[1].split('/').last
iso_f0 = iso9660_filename(f[0].to_s + '_' + suffix)
tmpl.gsub!(f[0].to_s + '-' + suffix, iso_f0)
ForemanBootdisk.logger.debug("Boot file #{iso_f0}, source #{f[1]}")
[iso_f0, f[1]]
end
end

:files => [[['BOOT/NUXATOMIC_7_3_X86_64_VMLINUZ', '/tmp/pxeboot/vmlinuz']],
[['BOOT/ATOMIC_7_3_X86_64_INITRD_IMG', '/tmp/pxeboot/initrd.img']]]}, anything)
ForemanBootdisk::ISOGenerator.generate_full_host(@host)
end
Copy link
Member

Choose a reason for hiding this comment

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

This test is actually for iso9660_filename to make sure that filenames gets shortened to maximum size for ISO format, can you keep it in some way or another? Note boot/RedHatEnterpriseLinuxAtomic-7.3-x86_64 -> BOOT/NUXATOMIC_7_3_X86_64_VMLINUZ (shortened automatically).

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I can add some tests for iso9660_filename method. We should definitely have them.

Copy link
Member

Choose a reason for hiding this comment

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

Yep should be probably a unit test, whatever you prefer.

@timogoebel timogoebel force-pushed the 25120-medium-provider-interface branch from ba7b268 to 9f2d97a Compare October 15, 2018 10:26
@timogoebel
Copy link
Member Author

@lzap: added the unit test.

@lzap lzap merged commit 5d5e91e into theforeman:master Oct 15, 2018
@lzap
Copy link
Member

lzap commented Oct 15, 2018

I was able to generate all images just fine, thanks!

@timogoebel
Copy link
Member Author

Nice, standby for the next chores. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants