Merge ~ltrager/curtin:centos8 into curtin:master

Proposed by Lee Trager
Status: Merged
Approved by: Ryan Harper
Approved revision: 084a368ac74eb3d69a4c740191f96d5ed0bf63bb
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~ltrager/curtin:centos8
Merge into: curtin:master
Diff against target: 551 lines (+313/-51)
5 files modified
curtin/commands/curthooks.py (+56/-33)
curtin/distro.py (+14/-2)
helpers/common (+84/-13)
tests/unittests/test_curthooks.py (+88/-3)
tests/unittests/test_distro.py (+71/-0)
Reviewer Review Type Date Requested Status
Ryan Harper Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+374335@code.launchpad.net

Commit message

centos: Add centos/rhel 8 support, enable UEFI Secure Boot

This branch adds support for deploying CentOS and RHEL 8 images. As a
side effect this also enables secure UEFI boot for CentOS 7 when the
grub2-efi-x64 package is installed.

LP: #1788088

Description of the change

A few things to note

* I added support for secure boot on both CentOS/RHEL 7 and 8. Curtin will use the old method of generating an unsigned grub bootloader if only the grub2-x64-modules package is installed. This allows old images to continue to be deployable without having to access the Internet.
* Upstream has removed support for btrfs[1]. btrfs use will be prevented from being configured in MAAS.
* cloud-init on RHEL/CentOS 8 is configured to create the default user as cloud-user.

Test image:
https://private-fileshare.canonical.com/~ltrager/centos8.tar.gz

[1] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/considerations_in_adopting_rhel_8/file-systems-and-storage_considerations-in-adopting-rhel-8

To post a comment you must log in.
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for putting this together. A couple of comments on the notes:

On btrfs, like bcache, MAAS doesn't allow such storage configs; I'd expect
MAAS to avoid creating btrfs filesystems for centos8 targets.

Per network-scripts, I've replied in the bug you filed; this isn't an issue
as long as the centos8 image includes the ifcfg-rh network-manager plugin.

W.r.t the cloud-init-el-release, we should continue to install the repo in
the case that we need to provide an update/hotfix prior to what would show
up in downstream centos8.

I've replied in-line below with some comments/questions on the changes.

review: Needs Fixing
~ltrager/curtin:centos8 updated
6cdc28f... by Lee Trager

rharper fixes

Revision history for this message
Lee Trager (ltrager) wrote :

Thanks for the review. I've updated as suggested and responded inline.

I was able to get cloud-init to work with NetworkManager as you suggested once I made sure the plugin was installed.

I agree that we should have a cloud-init repository enabled however I think we need a new repository for CentOS 8. The current version is Python 2 while CentOS 8 has moved to Python 3.6. The version included with CentOS 8 also works with MAAS without requiring any packages from EPEL.

~ltrager/curtin:centos8 updated
c23710b... by Lee Trager

Update redhat_upgrade_cloud_init for CentOS 8

Revision history for this message
Ryan Harper (raharper) wrote :

You're correct w.r.t Centos8 (we need to build/upload a py3 version). Cloud-init does have a branch to enable py3 builds from master, here:

https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/368845

We do have Centos8 builds in the COPR repo; We could push in one of the srpms from that branch into el-testing. Ideally I'd like to verify that the daily cloud-init on centos8 (py3) works.

I think since it's a different release (el8) I think the same repo can hold all 3 version (el6/7 and 8).

W.r.t EPEL, yes I think in Centos8, the cloud-init deps are present and we won't need to enable EPEL.

~ltrager/curtin:centos8 updated
c9348f0... by Lee Trager

Merge branch 'master' into centos8

0cda0e9... by Lee Trager

Use helper to get RHEL version

Revision history for this message
Ryan Harper (raharper) wrote :

Sorry for the delay, a few more questions.

review: Needs Information
~ltrager/curtin:centos8 updated
2dcbdbd... by Lee Trager

Only install bridge-utils on RHEL8

3cded85... by Lee Trager

Merge branch 'master' into centos8

Revision history for this message
Lee Trager (ltrager) wrote :

Thanks for the review. I fixed installing bridge-utils on CentOS8 and responded below.

Revision history for this message
Ryan Harper (raharper) :
review: Needs Fixing
Revision history for this message
Ryan Harper (raharper) wrote :

One more comment around the efibootmgr change.

~ltrager/curtin:centos8 updated
d8e6046... by Lee Trager

raharper fixes

Revision history for this message
Lee Trager (ltrager) wrote :

Thanks for the review. I've updated as suggested. Curtin will now try to determine the system partition from the storage configuration given. If no storage configuration was sent Curtin will fall back on auto detection.

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for the refactor, some more in-line comments. Specifically I want not try to grok the edi_dev and partition number if we know it curthooks.

review: Needs Fixing
~ltrager/curtin:centos8 updated
3abb50c... by Lee Trager

Merge branch 'master' into centos8

327abcc... by Lee Trager

rharper fixes

Revision history for this message
Lee Trager (ltrager) wrote :

Fixed some things up and responded below. Let me know if that makes sense.

Revision history for this message
Ryan Harper (raharper) :
review: Needs Fixing
~ltrager/curtin:centos8 updated
e678aca... by Lee Trager

rharper fixes

Revision history for this message
Lee Trager (ltrager) wrote :

Updated as suggested and responded below.

~ltrager/curtin:centos8 updated
00a8d01... by Lee Trager

Merge branch 'master' into centos8

39af019... by Lee Trager

Fix RHEL UEFI deployments

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for meeting. here's a quick refactor of looking up the efi partition via storage config.

I think we should extract the list of 'grub_device' ids, and then any /boot/efi ids, then we can apply the get_path_to_storage_volume() to each item in the list of ids.

https://paste.ubuntu.com/p/JW58dz8WKv/

~ltrager/curtin:centos8 updated
ad2478e... by Lee Trager

Merge branch 'master' into centos8

7246e40... by Lee Trager

Fixes from meeting with rharper

Revision history for this message
Lee Trager (ltrager) wrote :

Updated to remove the while loop. MAAS sets all grub_device to True on all block devices when a RAID1 array is configured. It does this for BIOS systems to ensure GRUB is on the super block of all RAID 1 devices. Since UEFI GRUB only support one system partition MAAS assumes Curtin will select the partition /boot/efi is mounted at.

This could be fixed in MAAS but would cause problems if a newer version of Curtin is used with an older version of MAAS.

[1] http://paste.ubuntu.com/p/ZQSCHHDDX8/

Revision history for this message
Ryan Harper (raharper) wrote :

That's fine for now, though changes are coming here:

# UEFI-Hybrid mode
https://bugs.launchpad.net/curtin/+bug/1750920

And newer maas should define both a bios_grub partition and the efi partition to allow an image to boot either way.

And there's more work coming to enhance boot resilience

https://docs.google.com/document/d/1ew9-vrha9xrrbVqiX0vhlV3W7pAInaNAbJvftFUNE7w/edit?ts=5dd71f82

I'll get some vmtests on this.

Revision history for this message
Ryan Harper (raharper) wrote :
Revision history for this message
Ryan Harper (raharper) wrote :

vmtest results looked good. We have some known failures, IO timeouts that we expect. I'll verify those; there's one failure which is fixed if you rebase this branch to master and force push here. Let's do that so I only need to verify the timeouts. I'll rerun the 4 failures after you've rebased this branch on master.

~ltrager/curtin:centos8 updated
084a368... by Lee Trager

Merge branch 'master' into centos8

Revision history for this message
Lee Trager (ltrager) wrote :

Master has been merged :)

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
2index 4b36da2..43ebe8d 100644
3--- a/curtin/commands/curthooks.py
4+++ b/curtin/commands/curthooks.py
5@@ -469,12 +469,25 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
6
7 if storage_cfg_odict:
8 storage_grub_devices = []
9- for item_id, item in storage_cfg_odict.items():
10- if not item.get('grub_device'):
11- continue
12- LOG.debug("checking: %s", item)
13- storage_grub_devices.append(
14- get_path_to_storage_volume(item_id, storage_cfg_odict))
15+ if util.is_uefi_bootable():
16+ # Curtin only supports creating one EFI system partition. Thus the
17+ # grub_device can only be the default system partition mounted at
18+ # /boot/efi.
19+ for item_id, item in storage_cfg_odict.items():
20+ if item.get('path') == '/boot/efi':
21+ efi_dev_id = storage_cfg_odict[item['device']]['volume']
22+ LOG.debug("checking: %s", item)
23+ storage_grub_devices.append(get_path_to_storage_volume(
24+ efi_dev_id, storage_cfg_odict))
25+ break
26+ else:
27+ for item_id, item in storage_cfg_odict.items():
28+ if not item.get('grub_device'):
29+ continue
30+ LOG.debug("checking: %s", item)
31+ storage_grub_devices.append(
32+ get_path_to_storage_volume(item_id, storage_cfg_odict))
33+
34 if len(storage_grub_devices) > 0:
35 grubcfg['install_devices'] = storage_grub_devices
36
37@@ -575,7 +588,7 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
38 join_stdout_err = ['sh', '-c', 'exec "$0" "$@" 2>&1']
39 out, _err = util.subp(
40 join_stdout_err + args + instdevs, env=env, capture=True)
41- LOG.debug("%s\n%s\n", args, out)
42+ LOG.debug("%s\n%s\n", args + instdevs, out)
43
44 if util.is_uefi_bootable() and grubcfg.get('update_nvram', True):
45 uefi_reorder_loaders(grubcfg, target)
46@@ -978,10 +991,17 @@ def install_missing_packages(cfg, target, osfamily=DISTROS.debian):
47 # UEFI requires grub-efi-{arch}. If a signed version of that package
48 # exists then it will be installed.
49 if util.is_uefi_bootable():
50- uefi_pkgs = []
51+ uefi_pkgs = ['efibootmgr']
52 if osfamily == DISTROS.redhat:
53 # centos/redhat doesn't support 32-bit?
54- uefi_pkgs.extend(['grub2-efi-x64-modules'])
55+ if 'grub2-efi-x64-modules' not in installed_packages:
56+ # Previously Curtin only supported unsigned GRUB due to an
57+ # upstream bug. By default lp:maas-image-builder and
58+ # packer-maas have grub preinstalled. If grub2-efi-x64-modules
59+ # is already in the image use unsigned grub so the install
60+ # doesn't require Internet access. If grub is missing use the
61+ # signed version.
62+ uefi_pkgs.extend(['grub2-efi-x64', 'shim-x64'])
63 elif osfamily == DISTROS.debian:
64 arch = util.get_architecture()
65 if arch == 'i386':
66@@ -1261,40 +1281,43 @@ def redhat_upgrade_cloud_init(netcfg, target=None, osfamily=DISTROS.redhat):
67 cloud_init_yum_repo = (
68 paths.target_path(target,
69 'etc/yum.repos.d/curtin-cloud-init.repo'))
70+ rhel_ver = distro.rpm_get_dist_id(target)
71 # Inject cloud-init daily yum repo
72 util.write_file(cloud_init_yum_repo,
73- content=cloud_init_repo(
74- distro.rpm_get_dist_id(target)))
75+ content=cloud_init_repo(rhel_ver))
76
77- # we separate the installation of repository packages (epel,
78+ # ensure up-to-date ca-certificates to handle https mirror
79+ # connections for epel and cloud-init-el.
80+ packages = ['ca-certificates']
81+
82+ if int(rhel_ver) < 8:
83+ # cloud-init in RHEL < 8 requires EPEL for dependencies.
84+ packages += ['epel-release']
85+ # RHEL8+ no longer ships bridge-utils. This does not effect
86+ # bridge configuration. Only install on RHEL < 8 if not
87+ # available, do not upgrade.
88+ with util.ChrootableTarget(target) as in_chroot:
89+ try:
90+ in_chroot.subp(['rpm', '-q', 'bridge-utils'],
91+ capture=False, rcs=[0])
92+ except util.ProcessExecutionError:
93+ LOG.debug(
94+ 'Image missing bridge-utils package, installing')
95+ packages += ['bridge-utils']
96+
97+ packages += ['cloud-init-el-release', 'cloud-init']
98+
99+ # We separate the installation of repository packages (epel,
100 # cloud-init-el-release) as we need a new invocation of yum
101 # to read the newly installed repo files.
102-
103- # ensure up-to-date ca-certificates to handle https mirror
104- # connections
105- distro.install_packages(['ca-certificates'], target=target,
106- osfamily=osfamily)
107- distro.install_packages(['epel-release'], target=target,
108- osfamily=osfamily)
109- distro.install_packages(['cloud-init-el-release'], target=target,
110- osfamily=osfamily)
111- distro.install_packages(['cloud-init'], target=target,
112- osfamily=osfamily)
113+ for package in packages:
114+ distro.install_packages(
115+ [package], target=target, osfamily=osfamily)
116
117 # remove cloud-init el-stable bootstrap repo config as the
118 # cloud-init-el-release package points to the correct repo
119 util.del_file(cloud_init_yum_repo)
120
121- # install bridge-utils if needed
122- with util.ChrootableTarget(target) as in_chroot:
123- try:
124- in_chroot.subp(['rpm', '-q', 'bridge-utils'],
125- capture=False, rcs=[0])
126- except util.ProcessExecutionError:
127- LOG.debug('Image missing bridge-utils package, installing')
128- distro.install_packages(['bridge-utils'], target=target,
129- osfamily=osfamily)
130-
131 LOG.info('Passing network configuration through to target')
132 net.render_netconfig_passthrough(target, netconfig={'network': netcfg})
133
134diff --git a/curtin/distro.py b/curtin/distro.py
135index 9714515..b233e26 100644
136--- a/curtin/distro.py
137+++ b/curtin/distro.py
138@@ -284,7 +284,13 @@ def run_yum_command(mode, args=None, opts=None, env=None, target=None,
139 if opts is None:
140 opts = []
141
142- cmd = ['yum'] + defopts + opts + [mode] + args
143+ # dnf is a drop in replacement for yum. On newer RH based systems yum
144+ # is just a sym link to dnf.
145+ if which('dnf', target=target):
146+ cmd = ['dnf']
147+ else:
148+ cmd = ['yum']
149+ cmd += defopts + opts + [mode] + args
150 if not execute:
151 return env, cmd
152
153@@ -311,8 +317,14 @@ def yum_install(mode, packages=None, opts=None, env=None, target=None,
154 raise ValueError(
155 'Unsupported mode "%s" for yum package install/upgrade' % mode)
156
157+ # dnf is a drop in replacement for yum. On newer RH based systems yum
158+ # is just a sym link to dnf.
159+ if which('dnf', target=target):
160+ cmd = ['dnf']
161+ else:
162+ cmd = ['yum']
163 # download first, then install/upgrade from cache
164- cmd = ['yum'] + defopts + opts + [mode]
165+ cmd += defopts + opts + [mode]
166 dl_opts = ['--downloadonly', '--setopt=keepcache=1']
167 inst_opts = ['--cacheonly']
168
169diff --git a/helpers/common b/helpers/common
170index 5928150..6c8e4f0 100644
171--- a/helpers/common
172+++ b/helpers/common
173@@ -679,7 +679,7 @@ install_grub() {
174
175 # grub is not the bootloader you are looking for
176 if [ "${target_arch}" = "s390x" ]; then
177- return 0;
178+ return 0;
179 fi
180
181 # set correct grub package
182@@ -694,7 +694,7 @@ install_grub() {
183 x86_64)
184 case $rhel_ver in
185 6) grub_name="grub";;
186- 7) grub_name="grub2-pc";;
187+ 7|8) grub_name="grub2-pc";;
188 *)
189 error "Unknown rhel_ver [$rhel_ver]";
190 return 1;
191@@ -711,7 +711,10 @@ install_grub() {
192 case "$target_arch" in
193 x86_64)
194 # centos 7+, no centos6 support
195- grub_name="grub2-efi-x64-modules"
196+ # grub2-efi-x64 installs a signed grub bootloader while
197+ # curtin uses grub2-efi-x64-modules to generate grubx64.efi.
198+ # Either works just check that one of them is installed.
199+ grub_name="grub2-efi-x64 grub2-efi-x64-modules"
200 grub_target="x86_64-efi"
201 ;;
202 amd64)
203@@ -739,11 +742,13 @@ install_grub() {
204 error "failed to check if $grub_name installed";
205 return 1;
206 fi
207- case "$tmp" in
208- install\ ok\ installed) :;;
209- *) debug 1 "$grub_name not installed, not doing anything";
210- return 1;;
211- esac
212+ # Check that any of the packages in $grub_name are installed. If
213+ # grub_name contains multiple packages, as it does for CentOS 7+,
214+ # only one package has to be installed for this to pass.
215+ if ! echo $tmp | grep -q 'install ok installed'; then
216+ debug 1 "$grub_name not installed, not doing anything"
217+ return 1
218+ fi
219
220 local grub_d="etc/default/grub.d"
221 # ubuntu writes to /etc/default/grub.d/50-curtin-settings.cfg
222@@ -822,11 +827,31 @@ install_grub() {
223 if [ "$update_nvram" -ge 1 ]; then
224 nvram=""
225 fi
226+ if [ "${#grubdevs_new[@]}" -eq 1 ] && [ -f "${grubdevs_new[0]}" ]; then
227+ # Currently UEFI can only be pointed to one system partition. If
228+ # for some reason multiple install locations are given only use the
229+ # first.
230+ efi_dev="${grubdevs_new[0]}"
231+ elif [ "${#grubdevs_new[@]}" -gt 1 ]; then
232+ error "Only one grub device supported on UEFI!"
233+ exit 1
234+ else
235+ # If no storage configuration was given try to determine the system
236+ # partition.
237+ efi_dev=$(awk -v "MP=${mp}/boot/efi" '$2 == MP { print $1 }' /proc/mounts)
238+ fi
239+ # The partition number of block device name need to be determined here
240+ # so both getting the UEFI device from Curtin config and discovering it
241+ # work.
242+ efi_part_num=$(cat /sys/class/block/$(basename $efi_dev)/partition)
243+ efi_disk="/dev/$(lsblk -no pkname $efi_dev)"
244 debug 1 "curtin uefi: installing ${grub_name} to: /boot/efi"
245 chroot "$mp" env DEBIAN_FRONTEND=noninteractive sh -exc '
246 echo "before grub-install efiboot settings"
247 efibootmgr -v || echo "WARN: efibootmgr exited $?"
248 bootid="$4"
249+ efi_disk="$5"
250+ efi_part_num="$6"
251 grubpost=""
252 case $bootid in
253 debian|ubuntu)
254@@ -836,7 +861,15 @@ install_grub() {
255 ;;
256 centos|redhat|rhel)
257 grubcmd="grub2-install"
258- grubpost="grub2-mkconfig -o /boot/grub2/grub.cfg"
259+ # RHEL uses redhat instead of the os_variant rhel for the bootid.
260+ if [ "$bootid" = "rhel" ]; then
261+ bootid="redhat"
262+ fi
263+ if [ -f /boot/efi/EFI/$bootid/grubx64.efi ]; then
264+ grubpost="grub2-mkconfig -o /boot/efi/EFI/$bootid/grub.cfg"
265+ else
266+ grubpost="grub2-mkconfig -o /boot/grub2/grub.cfg"
267+ fi
268 ;;
269 *)
270 echo "Unsupported OS: $bootid" 1>&2
271@@ -852,10 +885,45 @@ install_grub() {
272 echo "$gi_out" | grep -q -- "$no_nvram" || no_nvram=""
273 echo "$gi_out" | grep -q -- "--target" || target=""
274 echo "$gi_out" | grep -q -- "--efi-directory" || efi_dir=""
275- $grubcmd $target $efi_dir \
276- --bootloader-id=$bootid --recheck $no_nvram
277+
278+ # Do not overwrite grubx64.efi if it already exists. grub-install
279+ # generates grubx64.efi and overwrites any existing binary in
280+ # /boot/efi/EFI/$bootid. This binary is not signed and will cause
281+ # secure boot to fail.
282+ #
283+ # CentOS, RHEL, Fedora ship the signed boot loader in the package
284+ # grub2-efi-x64 which installs the signed boot loader to
285+ # /boot/efi/EFI/$bootid/grubx64.efi. All Curtin has to do is
286+ # configure the firmware. This mirrors what Anaconda does.
287+ #
288+ # Debian and Ubuntu come with a patched version of grub which
289+ # add the install flag --uefi-secure-boot which is enabled by
290+ # default. When enabled if a signed version of grub exists on
291+ # the filesystem it will be copied into /boot/efi/EFI/$bootid.
292+ # Stock Ubuntu images do not ship with anything in /boot. Those
293+ # files are generated by installing a kernel and grub.
294+ if [ -f /boot/efi/EFI/$bootid/grubx64.efi ]; then
295+ if [ -z "$no_nvram" ]; then
296+ # UEFI firmware should be pointed to the shim if available to
297+ # enable secure boot.
298+ for boot_uefi in \
299+ /boot/efi/EFI/$bootid/shimx64.efi \
300+ /boot/efi/EFI/BOOT/BOOTX64.EFI \
301+ /boot/efi/EFI/$bootid/grubx64.efi; do
302+ if [ -f $boot_uefi ]; then
303+ break
304+ fi
305+ done
306+ loader=$(echo ${boot_uefi##/boot/efi} | sed "s|/|\\\|g")
307+ efibootmgr --create --write-signature --label $bootid \
308+ --disk $efi_disk --part $efi_part_num --loader $loader
309+ fi
310+ else
311+ $grubcmd $target $efi_dir \
312+ --bootloader-id=$bootid --recheck $no_nvram
313+ fi
314 [ -z "$grubpost" ] || $grubpost;' \
315- -- "${grub_name}" "${grub_target}" "$nvram" "$os_variant" </dev/null ||
316+ -- "$grub_name" "$grub_target" "$nvram" "$os_variant" "$efi_disk" "$efi_part_num" </dev/null ||
317 { error "failed to install grub!"; return 1; }
318
319 chroot "$mp" sh -exc '
320@@ -885,8 +953,11 @@ install_grub() {
321 centos|redhat|rhel)
322 case $bootver in
323 6) grubcmd="grub-install";;
324- 7) grubcmd="grub2-install"
325+ 7|8) grubcmd="grub2-install"
326 grubpost="grub2-mkconfig -o /boot/grub2/grub.cfg";;
327+ *)
328+ echo "Unknown rhel_ver [$bootver]"
329+ exit 1
330 esac
331 ;;
332 *)
333diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
334index a1e13de..8533f5b 100644
335--- a/tests/unittests/test_curthooks.py
336+++ b/tests/unittests/test_curthooks.py
337@@ -391,7 +391,8 @@ class TestInstallMissingPkgs(CiTestCase):
338 arch = 'amd64'
339 self.mock_arch.return_value = arch
340 self.mock_machine.return_value = 'x86_64'
341- expected_pkgs = ['grub-efi-%s' % arch,
342+ expected_pkgs = ['efibootmgr',
343+ 'grub-efi-%s' % arch,
344 'grub-efi-%s-signed' % arch,
345 'shim-signed']
346 self.mock_machine.return_value = 'x86_64'
347@@ -408,7 +409,7 @@ class TestInstallMissingPkgs(CiTestCase):
348 arch = 'i386'
349 self.mock_arch.return_value = arch
350 self.mock_machine.return_value = 'i386'
351- expected_pkgs = ['grub-efi-ia32']
352+ expected_pkgs = ['efibootmgr', 'grub-efi-ia32']
353 self.mock_machine.return_value = 'i686'
354 self.mock_uefi.return_value = True
355 target = "not-a-real-target"
356@@ -422,7 +423,7 @@ class TestInstallMissingPkgs(CiTestCase):
357 arch = 'arm64'
358 self.mock_arch.return_value = arch
359 self.mock_machine.return_value = 'aarch64'
360- expected_pkgs = ['grub-efi-%s' % arch]
361+ expected_pkgs = ['efibootmgr', 'grub-efi-%s' % arch]
362 self.mock_uefi.return_value = True
363 target = "not-a-real-target"
364 cfg = {}
365@@ -430,6 +431,38 @@ class TestInstallMissingPkgs(CiTestCase):
366 self.mock_install_packages.assert_called_with(
367 expected_pkgs, target=target, osfamily=self.distro_family)
368
369+ @patch.object(events, 'ReportEventStack')
370+ def test_install_packages_on_uefi_amd64_centos(self, mock_events):
371+ arch = 'amd64'
372+ self.mock_arch.return_value = arch
373+ self.mock_machine.return_value = 'x86_64'
374+ expected_pkgs = ['efibootmgr', 'grub2-efi-x64', 'shim-x64']
375+ self.mock_uefi.return_value = True
376+ self.mock_haspkg.return_value = True
377+ target = "not-a-real-target"
378+ cfg = {}
379+ curthooks.install_missing_packages(
380+ cfg, target=target, osfamily=distro.DISTROS.redhat)
381+ self.mock_install_packages.assert_called_with(
382+ expected_pkgs, target=target, osfamily=distro.DISTROS.redhat)
383+
384+ @patch.object(events, 'ReportEventStack')
385+ def test_install_packages_on_uefi_amd64_centos_legacy(self, mock_events):
386+ arch = 'amd64'
387+ self.mock_arch.return_value = arch
388+ self.mock_machine.return_value = 'x86_64'
389+ self.mock_get_installed_packages.return_value = [
390+ 'grub2-efi-x64-modules']
391+ expected_pkgs = ['efibootmgr']
392+ self.mock_uefi.return_value = True
393+ self.mock_haspkg.return_value = True
394+ target = "not-a-real-target"
395+ cfg = {}
396+ curthooks.install_missing_packages(
397+ cfg, target=target, osfamily=distro.DISTROS.redhat)
398+ self.mock_install_packages.assert_called_with(
399+ expected_pkgs, target=target, osfamily=distro.DISTROS.redhat)
400+
401
402 class TestSetupZipl(CiTestCase):
403
404@@ -539,6 +572,58 @@ class TestSetupGrub(CiTestCase):
405 self.target, '/dev/vdb'],),
406 self.mock_subp.call_args_list[0][0])
407
408+ @patch('curtin.block.is_valid_device')
409+ @patch('curtin.commands.curthooks.os.path.exists')
410+ def test_uses_grub_install_on_storage_config_uefi(
411+ self, m_exists, m_is_valid_device):
412+ self.mock_is_uefi_bootable.return_value = True
413+ cfg = {
414+ 'storage': {
415+ 'version': 1,
416+ 'config': [
417+ {
418+ 'id': 'vdb',
419+ 'type': 'disk',
420+ 'name': 'vdb',
421+ 'path': '/dev/vdb',
422+ 'ptable': 'gpt',
423+ },
424+ {
425+ 'id': 'vdb-part1',
426+ 'type': 'partition',
427+ 'device': 'vdb',
428+ 'number': 1,
429+ },
430+ {
431+ 'id': 'vdb-part1_format',
432+ 'type': 'format',
433+ 'volume': 'vdb-part1',
434+ 'fstype': 'fat32',
435+ },
436+ {
437+ 'id': 'vdb-part1_mount',
438+ 'type': 'mount',
439+ 'device': 'vdb-part1_format',
440+ 'path': '/boot/efi',
441+ },
442+ ]
443+ },
444+ 'grub': {
445+ 'update_nvram': False,
446+ },
447+ }
448+ self.subp_output.append(('', ''))
449+ m_exists.return_value = True
450+ m_is_valid_device.side_effect = (False, True)
451+ curthooks.setup_grub(cfg, self.target, osfamily=distro.DISTROS.redhat)
452+ self.assertEquals(
453+ ([
454+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
455+ 'install-grub', '--uefi',
456+ '--os-family=%s' % distro.DISTROS.redhat, self.target,
457+ '/dev/vdb1'],),
458+ self.mock_subp.call_args_list[0][0])
459+
460 def test_grub_install_installs_to_none_if_install_devices_None(self):
461 cfg = {
462 'grub': {
463diff --git a/tests/unittests/test_distro.py b/tests/unittests/test_distro.py
464index b079162..dc1038c 100644
465--- a/tests/unittests/test_distro.py
466+++ b/tests/unittests/test_distro.py
467@@ -254,6 +254,44 @@ class TestYumInstall(CiTestCase):
468 distro.install_packages(pkglist, osfamily=osfamily, target=target)
469 m_subp.assert_has_calls(expected_calls)
470
471+ @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
472+ @mock.patch('curtin.util.subp')
473+ @mock.patch('curtin.distro.which')
474+ def test_dnf_install(self, m_which, m_subp):
475+ pkglist = ['foobar', 'wark']
476+ target = 'mytarget'
477+ mode = 'install'
478+ m_which.return_value = '/usr/bin/dnf'
479+ expected_calls = [
480+ mock.call(['dnf', '--assumeyes', '--quiet', 'install',
481+ '--downloadonly', '--setopt=keepcache=1'] + pkglist,
482+ env=None, retries=[1] * 10,
483+ target=paths.target_path(target)),
484+ mock.call(['dnf', '--assumeyes', '--quiet', 'install',
485+ '--cacheonly'] + pkglist, env=None,
486+ target=paths.target_path(target))
487+ ]
488+
489+ # call yum_install directly
490+ self.assertFalse(m_subp.called)
491+ distro.yum_install(mode, pkglist, target=target)
492+ m_subp.assert_has_calls(expected_calls)
493+
494+ # call yum_install through run_yum_command; expect the same calls
495+ # so clear m_subp's call stack.
496+ m_subp.reset_mock()
497+ self.assertFalse(m_subp.called)
498+ distro.run_yum_command('install', pkglist, target=target)
499+ m_subp.assert_has_calls(expected_calls)
500+
501+ # call yum_install through install_packages; expect the same calls
502+ # so clear m_subp's call stack.
503+ m_subp.reset_mock()
504+ self.assertFalse(m_subp.called)
505+ osfamily = distro.DISTROS.redhat
506+ distro.install_packages(pkglist, osfamily=osfamily, target=target)
507+ m_subp.assert_has_calls(expected_calls)
508+
509
510 class TestSystemUpgrade(CiTestCase):
511
512@@ -289,6 +327,39 @@ class TestSystemUpgrade(CiTestCase):
513 m_subp.assert_has_calls(expected_calls)
514
515 @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
516+ @mock.patch('curtin.util.subp')
517+ @mock.patch('curtin.distro.which')
518+ def test_system_upgrade_redhat_dnf(self, m_which, m_subp):
519+ """system_upgrade osfamily=redhat calls run_yum_command mode=upgrade"""
520+ osfamily = distro.DISTROS.redhat
521+ target = 'mytarget'
522+ mode = 'upgrade'
523+ m_which.return_value = '/usr/bin/dnf'
524+ pkglist = []
525+ expected_calls = [
526+ mock.call(['dnf', '--assumeyes', '--quiet', mode,
527+ '--downloadonly', '--setopt=keepcache=1'] + pkglist,
528+ env=None, retries=[1] * 10,
529+ target=paths.target_path(target)),
530+ mock.call(['dnf', '--assumeyes', '--quiet', mode,
531+ '--cacheonly'] + pkglist, env=None,
532+ target=paths.target_path(target))
533+ ]
534+ # call system_upgrade via osfamily; note that we expect the same calls
535+ # call system_upgrade via osfamily; note that we expect the same calls
536+
537+ # call yum_install through run_yum_command
538+ distro.run_yum_command(mode, pkglist, target=target)
539+ m_subp.assert_has_calls(expected_calls)
540+
541+ # call system_upgrade via osfamily; note that we expect the same calls
542+ # but to prevent a false positive we clear m_subp's call stack.
543+ m_subp.reset_mock()
544+ self.assertFalse(m_subp.called)
545+ distro.system_upgrade(target=target, osfamily=osfamily)
546+ m_subp.assert_has_calls(expected_calls)
547+
548+ @mock.patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
549 @mock.patch('curtin.distro.os.environ')
550 @mock.patch('curtin.distro.apt_update')
551 @mock.patch('curtin.distro.which')

Subscribers

People subscribed via source and target branches