particle-os/debian-forge-composer
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

cloudapi: drop ImageRequest.GetImageOptions() method

Browse source 
This method is not particularly useful anymore.  Its purpose was to
initialise the ImageOptions from an ImageRequest with the appropriate
size and partitioning mode.  However, the partitioning mode was also
being set later using request.GetPartitioningMode().  More importantly,
setting the size on the ImageOptions caused issues with the interaction
between filesystem and partitioning customizations as well as the image
request size (see #4705).  The correct thing to do here is to map the
ImageRequest.Size directly onto ImageOptions.Size, without taking into
account ImageType or the Blueprint Customizations.  The rest are
considered when generating the manifest in images, either when preparing
the Manifest() call or when generating the partition table.  This makes
it easier to trace and reason about the effect of each option.  This
kind of decision making in the API layer makes it difficult to maintain
the logic, since it requires duplicating the decision making or, as we
had now, making certain specific combinations impossible.
This commit is contained in:
Achilleas Koutsou 2025-05-05 15:56:16 +03:00 • committed by Sanne Raymaekers
parent c587e723a9
commit e621386caf
3 changed files with 8 additions and 104 deletions
Download patch file Download diff file Expand all files Collapse all files

10
internal/cloudapi/v2/compose.go
View file

@ -14,6 +14,7 @@ import (
"github.com/osbuild/images/pkg/customizations/subscription"
"github.com/osbuild/images/pkg/datasizes"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/distro"
"github.com/osbuild/images/pkg/distrofactory"
"github.com/osbuild/images/pkg/reporegistry"
"github.com/osbuild/images/pkg/rhsm/facts"
@ -1251,8 +1252,13 @@ func (request *ComposeRequest) GetImageRequests(distroFactory *distrofactory.Fac
repos = append(repos, dr...)
}
// Get the initial ImageOptions with image size set
imageOptions := ir.GetImageOptions(imageType, bp)
// Initialise the image options from the image request and
// customizations
imageOptions := distro.ImageOptions{}
if ir.Size != nil {
imageOptions.Size = *ir.Size
}
if request.Koji == nil {
imageOptions.Facts = &facts.ImageOptions{

23
internal/cloudapi/v2/imagerequest.go
View file

@ -7,8 +7,6 @@ import (
ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
"github.com/google/uuid"
"github.com/osbuild/blueprint/pkg/blueprint"
"github.com/osbuild/images/pkg/disk"
"github.com/osbuild/images/pkg/distro"
"github.com/osbuild/images/pkg/ostree"
"github.com/osbuild/images/pkg/platform"
@ -17,27 +15,6 @@ import (
"github.com/osbuild/osbuild-composer/internal/target"
)
// GetImageOptions returns the initial ImageOptions with Size and PartitioningMode set
// The size is set to the largest of:
// - Default size for the image type
// - Blueprint filesystem customizations
// - Requested size
//
// The partitioning mode is set to AutoLVM which will select LVM if there are additional mountpoints
func (ir *ImageRequest) GetImageOptions(imageType distro.ImageType, bp blueprint.Blueprint) distro.ImageOptions {
// NOTE: The size is in bytes
var size uint64
minSize := bp.Customizations.GetFilesystemsMinSize()
if ir.Size == nil {
size = imageType.Size(minSize)
} else if bp.Customizations != nil && minSize > 0 && minSize > *ir.Size {
size = imageType.Size(minSize)
} else {
size = imageType.Size(*ir.Size)
}
return distro.ImageOptions{Size: size, PartitioningMode: disk.AutoLVMPartitioningMode}
}
func newAWSTarget(options UploadOptions, imageType distro.ImageType) (*target.Target, error) {
var awsUploadOptions AWSEC2UploadOptions
jsonUploadOptions, err := json.Marshal(options)

79
internal/cloudapi/v2/imagerequest_test.go
View file

@ -3,7 +3,6 @@ package v2
import (
"testing"
"github.com/osbuild/blueprint/pkg/blueprint"
"github.com/osbuild/images/pkg/arch"
"github.com/osbuild/images/pkg/distro/rhel/rhel9"
"github.com/osbuild/images/pkg/distro/test_distro"
@ -14,84 +13,6 @@ import (
"github.com/stretchr/testify/require"
)
func TestImageRequestSize(t *testing.T) {
distro := test_distro.DistroFactory(test_distro.TestDistro1Name)
require.NotNil(t, distro)
arch, err := distro.GetArch(test_distro.TestArchName)
if err != nil {
panic(err)
}
imageType, err := arch.GetImageType(test_distro.TestImageTypeName)
if err != nil {
panic(err)
}
// A blueprint with no filesystem customizations
bp := blueprint.Blueprint{
Name: "image-request-test",
Description: "Empty Blueprint",
Version: "0.0.1",
}
// #1 With no size request
ir := ImageRequest{
Architecture: test_distro.TestArchName,
ImageType: test_distro.TestImageTypeName,
Size: nil,
}
imageOptions := ir.GetImageOptions(imageType, bp)
// The test_distro default size is 1GiB
assert.Equal(t, uint64(1073741824), imageOptions.Size)
// #2 With size request
ir = ImageRequest{
Architecture: test_distro.TestArchName,
ImageType: test_distro.TestImageTypeName,
Size: common.ToPtr(uint64(5368709120)),
}
imageOptions = ir.GetImageOptions(imageType, bp)
// The test_distro default size is actually 5GiB
assert.Equal(t, uint64(5368709120), imageOptions.Size)
// A blueprint with filesystem customizations
bp = blueprint.Blueprint{
Name: "image-request-test",
Description: "Customized Filesystem",
Version: "0.0.1",
Customizations: &blueprint.Customizations{
Filesystem: []blueprint.FilesystemCustomization{
blueprint.FilesystemCustomization{
Mountpoint: "/",
MinSize: 2147483648,
},
},
},
}
// #3 With no size request
ir = ImageRequest{
Architecture: test_distro.TestArchName,
ImageType: test_distro.TestImageTypeName,
Size: nil,
}
imageOptions = ir.GetImageOptions(imageType, bp)
// The test_distro default size is actually 2GiB
assert.Equal(t, uint64(2147483648), imageOptions.Size)
// #4 With size request
ir = ImageRequest{
Architecture: test_distro.TestArchName,
ImageType: test_distro.TestImageTypeName,
Size: common.ToPtr(uint64(5368709120)),
}
imageOptions = ir.GetImageOptions(imageType, bp)
// The test_distro default size is actually 5GiB
assert.Equal(t, uint64(5368709120), imageOptions.Size)
}
func TestGetOstreeOptions(t *testing.T) {
// No Ostree settings
ir := ImageRequest{