Skip to content

Commit

Permalink
aws: allow the user to customize their AMI name
Browse files Browse the repository at this point in the history
In the frontend a new field is allowing user to setup customized names
for their images (see
osbuild/image-builder-frontend#1136).

To allow the customization to effectively take place, this commit is
slightly changing how EC2 images are activated.

The Name tag and the AMI Name are not always sharing the same value
exactly anymore. If the user provides a SnaphsotName, the raw value is
set in the `Name` tag and the AMI name is set to `$SnaphsotName-$UUID`
(to make it unique).

This means that the `Name` tag can't be used anymore for the maintenance
tooling. The tooling will have to use the new Build-By tag to identify
the images built by osbuild-composer.
  • Loading branch information
lavocatt committed Feb 2, 2024
1 parent 3e31ebf commit bad6068
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 21 deletions.
2 changes: 1 addition & 1 deletion cmd/osbuild-upload-aws/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func main() {
bootModePtr = &bootMode
}

ami, err := a.Register(imageName, bucketName, keyName, share, arch, bootModePtr)
ami, err := a.Register(imageName, bucketName, keyName, imageName, share, arch, bootModePtr)
if err != nil {
println(err.Error())
return
Expand Down
7 changes: 6 additions & 1 deletion cmd/osbuild-worker/jobimpl-osbuild.go
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,12 @@ func (impl *OSBuildJobImpl) Run(job worker.Job) error {
break
}

ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, targetOptions.ShareWithAccounts, arch.Current().String(), targetOptions.BootMode)
// giving a Name tag value is optional, default to ImageName if missing.
tagName := jobTarget.ImageName
if targetOptions.TagName != "" {
tagName = targetOptions.TagName
}
ami, err := a.Register(jobTarget.ImageName, bucket, targetOptions.Key, tagName, targetOptions.ShareWithAccounts, arch.Current().String(), targetOptions.BootMode)
if err != nil {
targetResult.TargetError = clienterrors.WorkerClientError(clienterrors.ErrorImportingImage, err.Error(), nil)
break
Expand Down
2 changes: 1 addition & 1 deletion internal/boot/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func UploadImageToAWS(c *awsCredentials, imagePath string, imageName string) err
if err != nil {
return fmt.Errorf("cannot upload the image: %v", err)
}
_, err = uploader.Register(imageName, c.Bucket, imageName, nil, arch.Current().String(), nil)
_, err = uploader.Register(imageName, c.Bucket, imageName, imageName, nil, arch.Current().String(), nil)
if err != nil {
return fmt.Errorf("cannot register the image: %v", err)
}
Expand Down
29 changes: 20 additions & 9 deletions internal/cloud/awscloud/awscloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func WaitUntilImportSnapshotTaskCompletedWithContext(c *ec2.EC2, ctx aws.Context
// The caller can optionally specify the boot mode of the AMI. If the boot
// mode is not specified, then the instances launched from this AMI use the
// default boot mode value of the instance type.
func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch string, bootMode *string) (*string, error) {
func (a *AWS) Register(imageName string, bucket string, s3key string, tagName string, shareWith []string, rpmArch string, bootMode *string) (*string, error) {
rpmArchToEC2Arch := map[string]string{
"x86_64": "x86_64",
"aarch64": "arm64",
Expand All @@ -229,15 +229,18 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str
}
}

logrus.Infof("[AWS] 📥 Importing snapshot from image: %s/%s", bucket, key)
snapshotDescription := fmt.Sprintf("Image Builder AWS Import of %s", name)
// Note that the key value is only used in the upload transition phase in S3. It's deleted once the snapshot is done
// importing. So it's content has not much impact. The key is not conveyed anymore, compute a unique one based on
// the nameTag's value.
logrus.Infof("[AWS] 📥 Importing snapshot from image: %s/%s", bucket, s3key)
snapshotDescription := fmt.Sprintf("Image Builder AWS Import of %s", imageName)
importTaskOutput, err := a.ec2.ImportSnapshot(
&ec2.ImportSnapshotInput{
Description: aws.String(snapshotDescription),
DiskContainer: &ec2.SnapshotDiskContainer{
UserBucket: &ec2.UserBucket{
S3Bucket: aws.String(bucket),
S3Key: aws.String(key),
S3Key: aws.String(s3key),
},
},
},
Expand All @@ -261,10 +264,10 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str
}

// we no longer need the object in s3, let's just delete it
logrus.Infof("[AWS] 🧹 Deleting image from S3: %s/%s", bucket, key)
logrus.Infof("[AWS] 🧹 Deleting image from S3: %s/%s", bucket, s3key)
_, err = a.s3.DeleteObject(&s3.DeleteObjectInput{
Bucket: aws.String(bucket),
Key: aws.String(key),
Key: aws.String(s3key),
})
if err != nil {
return nil, err
Expand All @@ -290,7 +293,11 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String(name),
Value: aws.String(tagName),
},
{
Key: aws.String("Build-by"),
Value: aws.String("osbuild-composer"),
},
},
},
Expand All @@ -306,7 +313,7 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str
Architecture: aws.String(ec2Arch),
BootMode: bootMode,
VirtualizationType: aws.String("hvm"),
Name: aws.String(name),
Name: aws.String(imageName),
RootDeviceName: aws.String("/dev/sda1"),
EnaSupport: aws.Bool(true),
BlockDeviceMappings: []*ec2.BlockDeviceMapping{
Expand All @@ -332,7 +339,11 @@ func (a *AWS) Register(name, bucket, key string, shareWith []string, rpmArch str
Tags: []*ec2.Tag{
{
Key: aws.String("Name"),
Value: aws.String(name),
Value: aws.String(tagName),
},
{
Key: aws.String("Build-by"),
Value: aws.String("osbuild-composer"),
},
},
},
Expand Down
20 changes: 12 additions & 8 deletions internal/cloudapi/v2/imagerequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,8 @@ func newAWSTarget(options UploadOptions, imageType distro.ImageType) (*target.Ta
return nil, HTTPError(ErrorJSONUnMarshallingError)
}

// For service maintenance, images are discovered by the "Name:composer-api-*"
// tag filter. Currently all image names in the service are generated, so they're
// guaranteed to be unique as well. If users are ever allowed to name their images,
// an extra tag should be added.
key := fmt.Sprintf("composer-api-%s", uuid.New().String())
targetUuid := uuid.New()
key := fmt.Sprintf("composer-api-%s", targetUuid.String())

var amiBootMode *string
switch imageType.BootMode() {
Expand All @@ -63,16 +60,23 @@ func newAWSTarget(options UploadOptions, imageType distro.ImageType) (*target.Ta
amiBootMode = common.ToPtr(ec2.BootModeValuesLegacyBios)
}

// tag name is optional, not providing one will make the Name tag default to the ImageName
tagName := ""
if awsUploadOptions.SnapshotName != nil {
tagName = *awsUploadOptions.SnapshotName
}

t := target.NewAWSTarget(&target.AWSTargetOptions{
Region: awsUploadOptions.Region,
Key: key,
TagName: tagName,
ShareWithAccounts: awsUploadOptions.ShareWithAccounts,
BootMode: amiBootMode,
})
t.ImageName = key
if awsUploadOptions.SnapshotName != nil {
t.ImageName = *awsUploadOptions.SnapshotName
} else {
t.ImageName = key
// When registering an image the ImageName is used to set the ami's name. AMIs names must be unique.
t.ImageName = fmt.Sprintf("%s-%s", *awsUploadOptions.SnapshotName, targetUuid.String())
}
t.OsbuildArtifact.ExportFilename = imageType.Filename()
return t, nil
Expand Down
3 changes: 3 additions & 0 deletions internal/target/aws_target.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ type AWSTargetOptions struct {
Key string `json:"key"`
ShareWithAccounts []string `json:"shareWithAccounts"`

// Optional tag value if not provided will default to the ImageName's value.
TagName string `json:"tag_name,omitempty"`

// Boot mode of the AMI (optional)
// Supported values:
// - ec2.BootModeValuesLegacyBios
Expand Down
2 changes: 1 addition & 1 deletion test/cases/api/aws.sh
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function checkUploadStatusOptions() {
function verify() {
$AWS_CMD ec2 describe-images \
--owners self \
--filters Name=name,Values="$AWS_SNAPSHOT_NAME" \
--filters Name=name,Values="$AWS_SNAPSHOT_NAME-*" \
> "$WORKDIR/ami.json"

AMI_IMAGE_ID=$(jq -r '.Images[].ImageId' "$WORKDIR/ami.json")
Expand Down

0 comments on commit bad6068

Please sign in to comment.