Conversation
fbbc078 to
802e63c
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the functional test suite to support Ubuntu 22.04 and 24.04 cloud images as base images for the image customizer. It adds comprehensive test coverage for Ubuntu-specific customization scenarios including services management, kernel modules, script execution, and image history tracking.
Changes:
- Added CI workflow integration for downloading and converting Ubuntu cloud images to raw format
- Renamed Azure Linux-specific test helpers and split base image lists by distro
- Added Ubuntu-specific test infrastructure (mount points, connection helpers, preview feature mapping)
- Added functional tests validating Ubuntu customization for services, kernel modules, run scripts, and image history
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/scripts/download-ubuntu-image.sh | New script to download Ubuntu cloud images and convert from qcow2 to raw format |
| .github/workflows/tests-functional.yml | Updated CI workflow to download Ubuntu base images and pass them to tests |
| toolkit/tools/pkg/imagecustomizerlib/main_test.go | Added Ubuntu base image constants, flags, and test info structures; split base image lists by distro |
| toolkit/tools/pkg/imagecustomizerlib/imagecustomizer_test.go | Added Ubuntu mount points, connection helper, and four comprehensive Ubuntu functional tests |
| toolkit/tools/pkg/imagecustomizerlib/*_test.go (multiple files) | Renamed connectToCoreEfiImage to connectToAzureLinuxCoreEfiImage and updated tests to use baseImageAzureLinuxAll instead of baseImageAll |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
802e63c to
3c3c59d
Compare
| verifyFstabEntries(t, imageConnection, coreLegacyMountPoints, partitions) | ||
| verifyFstabEntries(t, imageConnection, azureLinuxBaremetalMountPoints, partitions) | ||
| verifyBootGrubCfg(t, imageConnection, "", | ||
| partitions[coreLegacyMountPoints[0].PartitionNum], |
There was a problem hiding this comment.
Why was coreLegacyMountPoints renamed to azureLinuxBaremetalMountPoints?
There was a problem hiding this comment.
To better tie it to testBaseImageAzl*BareMetal structs, where it's used, and which gets their names from baseImageAzureLinuxVariantBareMetal="bare-metal" and the params base-image-bare-metal-*. This azl variant is called core legacy in other places in the codebase, so it would be worth aligning one way or the other. I'm not partial to either name, just noticed the inconsistency in main_test.go.
There was a problem hiding this comment.
I think you are misreading the code. coreLegacyMountPoints is used for the BIOS boot partition layout, not for the bare-metal variant.
There was a problem hiding this comment.
I see the disconnect. Renaming the variable from the context of verifyLegacyBootImage does seem odd.
My previous comment was referring to the changes I've made in main_test.go, where three image variants are defined: azl core-efi, azl bare-metal, and ubuntu azure-cloud. I added the MountPoints field to testBaseImageInfo, and azl bare-metal needed the mount points under discussion. Let me know if those changes in main_test.go make sense. I went ahead and renamed to azureLinuxCoreLegacyMountPoints to prevent reader's confusion in this function.
| return imagecustomizerapi.SELinuxModeDefault, err | ||
| } | ||
|
|
||
| imageSELinuxMode, err := bootCustomizer.GetSELinuxMode(buildDir, imageChroot) |
There was a problem hiding this comment.
Out of interest, what is the behavior of bootCustomizer.GetSELinuxMode on Ubuntu? Ideally, it should return SELinuxModeDisabled if SELinux isn't installed on the system.
There was a problem hiding this comment.
Here's an example error message when I comment out lines 43-51 in customizeselinux.go (the distroHandler.SupportsSELinux check):
customizefiles_test.go:247:
Error Trace: .../azure-linux-image-tools/toolkit/tools/pkg/imagecustomizerlib/customizefiles_test.go:247
.../azure-linux-image-tools/toolkit/tools/pkg/imagecustomizerlib/customizefiles_test.go:224
Error: Received unexpected error:
failed to customize OS:
failed to get current SELinux mode:
failed to find SELinux args in grub file (/etc/default/grub):
failed to find any of the specified GRUB variables in /etc/default/grub: [GRUB_CMDLINE_LINUX_DEFAULT GRUB_CMDLINE_LINUX]
Test: TestCustomizeImageAdditionalDirs/Ubuntu2404AzureCloud
So it returns return imagecustomizerapi.SELinuxModeDefault, err
There was a problem hiding this comment.
That's probably a bug. If the variables (GRUB_CMDLINE_LINUX_DEFAULT and GRUB_CMDLINE_LINUX) don't exist, then we should assume there are empty.
toolkit/tools/pkg/imagecustomizerlib/testdata/adddirs-config.yaml
Outdated
Show resolved
Hide resolved
…ImageAll -> baseImageAzureLinuxAll
…nd baseImageVariantBareMetal -> baseImageAzureLinuxVariantBareMetal
…acyMountPoints -> azureLinuxCoreLegacyMountPoints
…ultAzureLinuxImage
- New flags: --base-image-azure-cloud-ubuntu2204 and --base-image-azure-cloud-ubuntu2404 - New functions: findFirstAvailableImage, for code reuse, and checkSkipForCustomizeDefaultImages, returning one default image for each distro, skipping on no availability. - New constants: paramBaseImageCoreEfiAzl2, paramBaseImageCoreEfiAzl3, paramBaseImageBareMetalAzl2, paramBaseImageBareMetalAzl3, paramBaseImageAzureCloudUbuntu2204, paramBaseImageAzureCloudUbuntu2404, paramLogLevel
…DefaultShell to testBaseImageInfo
Extend the functional test suite to validate image customizer operations on Ubuntu 22.04 and 24.04 cloud images. This covers services, kernel modules, scripts, and image history customization scenarios.