kubuntu-council team mailing list archive

[Steve Langasek <mp+453468@xxxxxxxxxxxxxxxxxx>]

Review: Needs Fixing

Looks good, but there are a few things that should be adjusted before landing this.  Inline comments apply to both test cases for the most part

Diff comments:

> diff --git a/testcases/image/1734_PXE UEFI Netboot (Server live) b/testcases/image/1734_PXE UEFI Netboot (Server live)
> new file mode 100644
> index 0000000..8b6d576
> --- /dev/null
> +++ b/testcases/image/1734_PXE UEFI Netboot (Server live)
> @@ -0,0 +1,63 @@
> +<p>
> +This testcase is to run netboot installation on amd64 machines with <b>UEFI</b>
> +firmware using live images. You need an installed Ubuntu machine in the same
> +network, configured with a static IP, which will be functioning as a PXE server
> +with DHCP.
> +</p>
> +
> +<em>
> +    Proceed in your native language if you wish. Instructions will remain in
> +    English.
> +</em>
> +
> +<dl>
> +    <dt>HOST is the IP address of the machine acting as the PXE/DHCP server.</dt>
> +    <dt>NETBOOT-URL is the compressed netboot artifacts archive, as listed in the
> +        download links for the test case.</dt>
> +    <dt>INTERFACE is the relevant network interface on the HOST.</dt>
> +    <dt>STARTADDR and ENDADDR are the start and end points for dhcp addresses to
> +        be handed out on this test network.  Choose appropriate values to match
> +        the static network configuration of the HOST, and see the manpage for
> +        dnsmasq for more details.</dt>
> +</dl>
> +
> +
> +<p>Install packages for the various services:</p>

"[...] on the PXE server" or "[...] on HOST"

> +<dl>
> +    <dt>sudo apt install -y curl dnsmasq tftpd-hpa</dt>

This is ok as it's an incremental improvement on the existing test case text, but there's no need to install curl for this when wget is part of the base system...

> +</dl>
> +
> +<p>Configure /etc/dnsmasq.d/pxe.conf:</p>
> +
> +<pre>
> +interface=INTERFACE,lo

the ",lo" confuses me, I wouldn't expect you to ever be serving over loopback

> +bind-interfaces
> +dhcp-range=INTERFACE,STARTADDR,ENDADDR
> +dhcp-boot=bootx64.efi

As described on https://wiki.ubuntu.com/UEFI/SecureBoot/PXE-IPv6 please use:

dhcp-match=set:efi-x86_64,option:client-arch,7
dhcp-boot=tag:efi-x86_64,bootx64.efi

Or just point to that wiki page.

Also, if this is supposed to be a "UEFI" test case rather than an "amd64" test case, we should just include the options for all of our architectures, so at least:

dhcp-match=set:efi-x86_64,option:client-arch,7
dhcp-boot=tag:efi-x86_64,amd64/bootx64.efi
dhcp-match=set:efi-arm64,option:client-arch,11
dhcp-boot=tag:efi-arm64,arm64/bootaa64.efi

Note also that this addresses the fact that the existing netboot tarball unpacks into a subdirectory named after the architecture, so qualifying with the architecture name in the path listed in the dnsmasq config lets us use the same config file and the same tftp root for multiple architectures.

> +enable-tftp

Then you shouldn't install tftp-hpa, I think?  Since this means using dnsmasq's built-in TFTP server.

> +tftp-root=/srv/tftp
> +</pre>
> +
> +<p>Restart dnsmasq:</p>
> +
> +<dl>
> +    <dt>sudo systemctl restart dnsmasq.service</dt>
> +</dl>
> +
> +<p>Download the netboot artifacts:</p>
> +<dl>

nitpick: semantically this looks like a <ul> not a <dl>

> +    <dt>sudo curl NETBOOT-URL -o /tmp/netboot-artifacts.tar.gz</dt>

wget

> +    <dt>sudo tar -zxvf /tmp/netboot-artifacts.tar.gz --strip-components=2 -C /srv/tftp/</dt>

>From above, if we put the architecture path in the name then we can avoid using --strip-components and also use the same tftp root across multiple archs.  (I also want to redo the layout of the netboot tarball, in which case we would want to update this test case again.)

> +</dl>
> +
> +
> +
> +<p>
> +Power on the test machine and ensure that it boots PXE. Complete the installation

The previous wording was "Power on your amd64 machine and boot from ethernet" which I think was clearer.  Since we're changing it, I suggest:  "Power on the test machine and ensure that it boots from the network device."

> +(using the provided defaults where possible) and remove the device from the network once it's finished. Reboot the machine

There is no reason to remove the device from the network.  At most, we would at next boot change which device is booted from by default.

> +and ensure that you can log into the system with the username and password you
> +provided.
> +</p>
> +
> +<strong>If you finish the installation, please <a href="results#add_result">submit</a> a 'passed' result.
> +    If any action fails, or produces an unexpected result, please <a href="results#add_result">submit</a> a 'failed' result and <a href="../../buginstructions">file a bug</a>. Please be sure to include the bug number when you <a href="results#add_result">submit</a> your result.</strong>


-- 
https://code.launchpad.net/~cpete/ubuntu-manual-tests/+git/ubuntu-manual-tests/+merge/453468
Your team Ubuntu Testcase Admins is subscribed to branch ubuntu-manual-tests:main.