[mic][iso] generate PXE-bootable ISO images. #10595

gmileka · 2024-10-01T21:24:32Z

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

Summary

This changes updates the Azures Linux Image Customizer to emit iso images that are PXE-ready.
The changes include:

interface:
- configuration: new configuration to specify the download url for the iso.
- command-line: new parameter to export the iso image contents into a folder for easier deployment unto a pxe server.
implementation:
implemented the above interface changes.
extended the set of parameters that must be saved between runs to include the new pxe parameters.
updated the code to emit a pxe grub.cfg and a pxe dracut config.
update the dracut package to include an additional download script that will be triggered in the pxe scenario.

Change Log

Does this affect the toolchain?

NO

Associated issues

[AUTO-CHERRYPICK] Fix fluent-bit CVE-2024-34250 with a patch - branch main #9293

Links to CVEs

n/a

Test Methodology

Deploy a PXE server on the network.
Create an iso and specify the new --output-pxe-artifacts-dir
Deploy the iso to the pxe server and boot a PXE client.
Deploy the artifacts folder contents to a the pxe server and boot a PXE client.
Tested iso-to-iso customization and ensured kernel parameters are augmented.

cwize1 · 2024-10-03T17:26:20Z

SPECS/dracut/add-livenet-download-service.patch

@@ -0,0 +1,147 @@
+From 2888695d6e1e03f727fe45d6e38fc71f7b5be123 Mon Sep 17 00:00:00 2001


This change should probably be split off into a separate PR so that it can reviewed separately from the MIC PXE feature.

cwize1 · 2024-10-03T17:37:07Z

SPECS/dracut/add-livenet-download-service.patch

@@ -0,0 +1,147 @@
+From 2888695d6e1e03f727fe45d6e38fc71f7b5be123 Mon Sep 17 00:00:00 2001


This patch means that the MIC PXE feature will require a minimum version of Azure Linux 3.0 to be able to function. In addition, you will be unlikely to be able to apply this patch to Azure Linux 2.0.

At a minimum, MIC should check for this patch in some way and error out if it isn't available. That will at least avoid the situation where it silently fails for the user.

Though, it might be good to investigate ways we could make MIC PXE feature independent of the Azure Linux version.

cwize1 · 2024-10-03T17:40:55Z

toolkit/tools/imagecustomizer/docs/cli.md

+
+Below is a list of the files necessary for PXE booting:
+
+```


What file format is this? It isn't something I recognize.

Also, you seem to be using both // and # for comments.

Would it make sense to use a Markdown table instead?

cwize1 · 2024-10-03T17:43:42Z

toolkit/tools/imagecustomizer/docs/configuration.md

+
+Specifies the PXE-specific configuration for the generated OS artifacts.
+
+<div id="pxe-isoimageurl"></div>


Don't add div unless the header is ambiguous. Not all Markdown tools handle div. So, limiting their use is goodness.

cwize1 · 2024-10-03T17:50:46Z

toolkit/tools/imagecustomizer/docs/samples/deploy-pxe-artifacts.sh

+    fi
+    copy_file $artifactsRootDir/boot/grub2/grubenv $tftpLocalDir/boot/grub2/grubenv    
+
+    # replace every '/' with a '\/' to avoid breaking sed search/replace syntax.


sed can take any character after the s. It might be goodness to use a character not supported in URLs. e.g. s|<old>|<new>|g

cwize1 · 2024-10-03T17:51:33Z

toolkit/tools/imagecustomizer/docs/samples/deploy-pxe-artifacts.sh

+function mount_iso() {
+    local isoPath=$1
+    local mountDir=$2
+    sudo rm -rf $mountDir


Don't use sudo in scripts. If the script needs root privileges, then the entire script should be run as sudo.

cwize1 · 2024-10-03T17:55:54Z

toolkit/tools/imagecustomizer/docs/samples/deploy-pxe-artifacts.sh

+
+function show_usage() {
+    echo
+    echo "$(basename ${BASH_SOURCE[0]}) <source-path>"


Doesn't <source-path> need the -s prefix?

cwize1 · 2024-10-03T21:07:03Z

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go

 	if err != nil {
 		return fmt.Errorf("failed to create iso image:\n%w", err)
 	}

+	err = populatePXEArtifactsDir(isoImagePath, b.workingDirs.isoBuildDir, outputPXEArtifactsDir, outputImageBase)


populatePXEArtifactsDir is called at the end of both createLiveOSIsoImage and createImageFromUnchangedOS. Would it make sense to move the populatePXEArtifactsDir call to convertWriteableFormatToOutputImage instead?

cwize1 · 2024-10-03T21:08:56Z

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go

 		}
 	}

-	err = b.updateGrubCfg(b.artifacts.savedKernelArgsFilePath, b.artifacts.grubCfgPath, extraCommandLine)
+	err = b.updateGrubCfg(b.artifacts.savedConfigsFilePath, b.artifacts.isoGrubCfgPath, extraCommandLine, b.artifacts.pxeGrubCfgPath, pxeIsoImageUrl, outputImageBase)


Please line wrap at 120.

cwize1 · 2024-10-03T21:13:23Z

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go

+	}
+
+	isoGrubCfg := filepath.Join(outputPXEArtifactsDir, "boot/grub2/grub.cfg")
+	pxeGrubCfg := filepath.Join(outputPXEArtifactsDir, "boot/grub2/grub-pxe.cfg")


You should place these 2 file paths in constants.

cwize1 · 2024-10-03T21:17:15Z

toolkit/tools/pkg/imagecustomizerlib/liveosisobuilder.go

@@ -810,7 +957,7 @@ func (b *LiveOSIsoBuilder) prepareArtifactsFromFullImage(inputIsoSavedKernelArgs
 //
 // ouptuts:
 //   - create a LiveOS ISO.
-func (b *LiveOSIsoBuilder) createIsoImage(additionalIsoFiles []safechroot.FileToCopy, isoOutputDir, isoOutputBaseName string) error {
+func (b *LiveOSIsoBuilder) createIsoImage(additionalIsoFiles []safechroot.FileToCopy, isoOutputDir, isoOutputBaseName string) (isoRepoDirPath string, err error) {


Should this be isoImagePath instead of isoRepoDirPath?

cwize1 · 2024-10-03T21:25:30Z

toolkit/tools/imagecustomizer/docs/configuration.md

+
+<div id="pxe-isoimageurl"></div>
+
+### isoImageUrl [string]


It might be better to have 2 mutually exclusive fields: isoImageBaseUrl (which adds ISO file name) and isoImageFileUrl (which doesn't add the ISO file name). That way, it avoids the behavior changing based on the format of the URL.

vinceaperri · 2024-10-03T22:08:06Z

SPECS/dracut/add-livenet-download-service.patch

+ create mode 100644 modules.d/90livenet/liveos-artifacts-download.service
+ create mode 100755 modules.d/90livenet/liveos-artifacts-download.sh
+
+diff --git a/modules.d/90livenet/liveos-artifacts-download.service b/modules.d/90livenet/liveos-artifacts-download.service


this file and another may be more maintainable as additional sources instead of baking them into the patch, since they are entirely new files. you would copy them to the appropriate location before the build.

%prep
%autosetup -p1 -n %{name}-ng-%{version}
cp %{SOURCE1} .
cp %{SOURCEX} modules.d/90livenet/liveos-artifacts-download.service
...

gmileka force-pushed the gmileka/mic-iso-pxe-official-pr branch from f2825b8 to d6fd3b9 Compare October 2, 2024 00:22

gmileka marked this pull request as ready for review October 2, 2024 19:35

gmileka requested review from a team as code owners October 2, 2024 19:35

cwize1 reviewed Oct 3, 2024

View reviewed changes

cwize1 requested changes Oct 3, 2024

View reviewed changes

vinceaperri requested changes Oct 3, 2024

View reviewed changes

gmileka added 4 commits October 22, 2024 17:42

Create a PXE bootable iso.

0705719

Fix tests.

167a77e

Minor doc update.

0b6b734

Fix build break post rebase.

39eea29

gmileka force-pushed the gmileka/mic-iso-pxe-official-pr branch from e5ee6be to 39eea29 Compare October 23, 2024 00:58

gmileka requested a review from a team as a code owner October 23, 2024 00:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mic][iso] generate PXE-bootable ISO images. #10595

[mic][iso] generate PXE-bootable ISO images. #10595

gmileka commented Oct 1, 2024 •

edited

Loading

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

cwize1 Oct 3, 2024

vinceaperri Oct 3, 2024

gmileka Oct 18, 2024

		@@ -0,0 +1,147 @@
		From 2888695d6e1e03f727fe45d6e38fc71f7b5be123 Mon Sep 17 00:00:00 2001


		Specifies the PXE-specific configuration for the generated OS artifacts.

		<div id="pxe-isoimageurl"></div>

[mic][iso] generate PXE-bootable ISO images. #10595

Are you sure you want to change the base?

[mic][iso] generate PXE-bootable ISO images. #10595

Conversation

gmileka commented Oct 1, 2024 • edited Loading

Merge Checklist

Summary

Change Log

Does this affect the toolchain?

Associated issues

Links to CVEs

Test Methodology

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmileka commented Oct 1, 2024 •

edited

Loading