Support Building AWS Windows LI Images by marmijo · Pull Request #4069 · coreos/coreos-assembler

marmijo · 2025-04-08T22:56:31Z

Expand the functionality of cosa to build and replicate AWS Windows License Included CoreOS Images using ore.

See individual commits.

xfef: https://issues.redhat.com/browse/COS-3042

To build an aws-winli AMI, run the following command:

cosa imageupload-aws --upload --winli \
	--windows-ami ${windows_server_ami_id}  \
        --winli-instance-type ${instance_type} \
	--region ${region} \
	--credentials-file ${aws_config_file} \
	--arch x86_64 \
	--bucket s3://${bucket}

Edit:

2025-04-24: updated ami creation command now that buildextend is RIP

dustymabe

Initial review. Thank you for working on this!

marmijo · 2025-04-18T01:13:43Z

I added another commit to this PR to include the aws-winli information in the release metadata.

dustymabe · 2025-04-18T01:32:11Z

+func randomName(prefix string) string {
+	b := make([]byte, 5)
+	if _, err := rand.Read(b); err != nil {
+		plog.Errorf("randomName: failed to generate a random name: %v", err)
+	}
+	return fmt.Sprintf("%s-%x", prefix, b)
+}


I was thinking more that rather than copy/paste the function from azure/api.go we'd put it in util/ (like the retry functions) and use that everywhere (i.e. we'd delete the copy of this in azure/api.go too.

Ah, yeah that makes sense. I can make that change as a separate commit. Should I make the change to the azure API in a separate PR after this merges?

I would just put it in the same PR.

dustymabe · 2025-04-18T01:32:55Z

 	cmdUpload.Flags().StringVar(&uploadX86BootMode, "x86-boot-mode", "uefi-preferred", "Set boot mode (uefi-preferred, uefi)")
+	cmdUpload.Flags().BoolVar(&uploadCreateWinLIAMI, "winli", false, "Create a Windows LI AMI")
+	cmdUpload.Flags().StringVar(&uploadWinLIwindowsServerAMI, "windows-ami", "", "Windows Server AMI used to create a Windows LI AMI")
+	cmdUpload.Flags().StringVar(&uploadWinLIInstanceType, "winli-instance-type", "", "ec2 instance type used to create a Windows LI AMI (default: t2.large)")


Suggested change

cmdUpload.Flags().StringVar(&uploadWinLIInstanceType, "winli-instance-type", "", "ec2 instance type used to create a Windows LI AMI (default: t2.large)")

cmdUpload.Flags().StringVar(&uploadWinLIInstanceType, "winli-instance-type", "t2.large", "ec2 instance type used to create a Windows LI AMI")

I think you can just set a default this way and you won't have to do the conditional logic below.

dustymabe · 2025-04-18T01:34:22Z

                        action='append', default=[])
+    parser.add_argument("--winli", action="store_true", help="create an AWS Windows LI Ami")
+    parser.add_argument("--windows-ami", help="Windows Server AMI ID used to create AWS Windows LI image")
+    parser.add_argument("--winli-instance-type", help="ec2 instance type used to create AWS Windows LI image (default: t2.large)")


Suggested change

parser.add_argument("--winli-instance-type", help="ec2 instance type used to create AWS Windows LI image (default: t2.large)")

parser.add_argument("--winli-instance-type", help="ec2 instance type used to create AWS Windows LI image")

I'd just leave off this detail here as it could become stale.

SGTM. That's why I created the option in the first place, so we wouldn't hard code it. Just like the windows server ami, this can become stale. Although, much less frequently.

dustymabe

LGTM

dustymabe

still LGTM - maybe wait til next week to merge just in case this disrupts something.

Add command line options to ore to support building Windows LI images.

Expand the schema to include aws-winli amis. Also expand cmd-coreos-prune to hanlde the new schema and prune aws-winli images if they exist in the metadata. Auto-generated code using `make schema`

Add command line options to support building aws windows li images.

Adjust ore replicate to handle both the `aws` and `aws-winli` metadata keys when replicating to aws regions.

The new Windows License Included AMI creation process creates volumes that might not be deleted when the instance they're attached to is deleted. Expand AWS Garbage Colleciton to also clean up EC2 volumes that were created by mantle.

Add AWS Windows License Included AMI information to the rhel-coreos-extensions section of the release metadata. See: https://issues.redhat.com/browse/COS-3057

marmijo · 2025-04-22T20:26:11Z

Pushed some updates to make CI happy. I also edited the schema commit to include pruning aws-winli images based on the new schema.

Expand the replicate to clouds functionality to build AWS Windows License Included (winli) images for streams that support it before replicating AMIs to other regions. `cosa aws-replicate` will now replicate both traditional AMIs and aws-winli AMIs if present in the metadata. See: coreos/coreos-assembler#4069 Also add a `winli` knob to the pipeline config clouds.aws section to only build winli images for specific streams.

Expand the replicate to clouds functionality to build AWS Windows License Included (winli) images for streams that support it before replicating AMIs to other regions. `cosa aws-replicate` will now replicate both traditional AMIs and aws-winli AMIs if present in the metadata. See: coreos/coreos-assembler#4069 Also add `create_and_replicate_winli_ami` as a stream level knob to the pipeline config to enable building and replicating the AMIs for specific streams.

openshift-ci Bot added the do-not-merge/work-in-progress label Apr 8, 2025

marmijo marked this pull request as draft April 8, 2025 22:56

marmijo force-pushed the aws-winli branch 4 times, most recently from 7b3492a to 7a91a88 Compare April 9, 2025 23:15

dustymabe reviewed Apr 14, 2025

View reviewed changes

marmijo force-pushed the aws-winli branch from 7a91a88 to 34aac3b Compare April 17, 2025 00:14

marmijo removed the do-not-merge/work-in-progress label Apr 17, 2025

marmijo changed the title ~~WIP: Support Building AWS Windows LI Images~~ Support Building AWS Windows LI Images Apr 17, 2025

openshift-ci Bot added the do-not-merge/work-in-progress label Apr 17, 2025

marmijo removed the do-not-merge/work-in-progress label Apr 17, 2025

marmijo marked this pull request as ready for review April 17, 2025 00:16

dustymabe reviewed Apr 17, 2025

View reviewed changes

Comment thread mantle/platform/api/aws/images.go Outdated

dustymabe reviewed Apr 17, 2025

View reviewed changes

Comment thread mantle/cmd/ore/aws/upload.go Outdated

marmijo mentioned this pull request Apr 18, 2025

Add AWS WinLI images to RHELCoreOSExtensions coreos/stream-metadata-go#85

Merged

marmijo force-pushed the aws-winli branch from 34aac3b to bed9ebd Compare April 18, 2025 01:10

dustymabe reviewed Apr 18, 2025

View reviewed changes

marmijo force-pushed the aws-winli branch 2 times, most recently from b66b975 to 963bea5 Compare April 18, 2025 17:16

dustymabe previously approved these changes Apr 18, 2025

View reviewed changes

marmijo dismissed dustymabe’s stale review via 3ff8a7e April 18, 2025 19:24

marmijo force-pushed the aws-winli branch 2 times, most recently from 3ff8a7e to 238517b Compare April 18, 2025 19:27

dustymabe previously approved these changes Apr 18, 2025

View reviewed changes

marmijo dismissed dustymabe’s stale review via 06e2084 April 22, 2025 17:18

marmijo force-pushed the aws-winli branch from 238517b to 06e2084 Compare April 22, 2025 17:18

marmijo added 6 commits April 22, 2025 13:40

ore/aws: add options to create windows li images

46f597b

Add command line options to ore to support building Windows LI images.

add schema for aws-winli ami metadata

b56579b

Expand the schema to include aws-winli amis. Also expand cmd-coreos-prune to hanlde the new schema and prune aws-winli images if they exist in the metadata. Auto-generated code using `make schema`

cosalib/aws: add support for windows li image creation

d1c251a

Add command line options to support building aws windows li images.

cosalib/aws: expand ore_aws_replicate to also copy aws-winli images

a5bfbde

Adjust ore replicate to handle both the `aws` and `aws-winli` metadata keys when replicating to aws regions.

ore/aws: update GC to include EC2 volumes

b155c71

The new Windows License Included AMI creation process creates volumes that might not be deleted when the instance they're attached to is deleted. Expand AWS Garbage Colleciton to also clean up EC2 volumes that were created by mantle.

cmd-generate-release-meta: add aws-winli release metadata

54ba24e

Add AWS Windows License Included AMI information to the rhel-coreos-extensions section of the release metadata. See: https://issues.redhat.com/browse/COS-3057

marmijo force-pushed the aws-winli branch from d19e485 to 54ba24e Compare April 22, 2025 19:42

dustymabe approved these changes Apr 22, 2025

View reviewed changes

marmijo merged commit 5e3785f into coreos:main Apr 22, 2025
5 checks passed

marmijo mentioned this pull request Apr 23, 2025

libcloud: build and replicate aws-winli images for supported streams coreos/fedora-coreos-pipeline#1134

Merged

marmijo mentioned this pull request Jul 22, 2025

mantle/aws: rework AWS Windows LI image creation #4237

Merged

	cmdUpload.Flags().StringVar(&uploadWinLIInstanceType, "winli-instance-type", "", "ec2 instance type used to create a Windows LI AMI (default: t2.large)")
	cmdUpload.Flags().StringVar(&uploadWinLIInstanceType, "winli-instance-type", "t2.large", "ec2 instance type used to create a Windows LI AMI")

	parser.add_argument("--winli-instance-type", help="ec2 instance type used to create AWS Windows LI image (default: t2.large)")
	parser.add_argument("--winli-instance-type", help="ec2 instance type used to create AWS Windows LI image")

Conversation

marmijo commented Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marmijo commented Apr 18, 2025

Uh oh!

dustymabe Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

marmijo Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

dustymabe Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

dustymabe Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

dustymabe Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

marmijo Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

marmijo Apr 18, 2025

Choose a reason for hiding this comment

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

dustymabe left a comment

Choose a reason for hiding this comment

Uh oh!

marmijo commented Apr 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

marmijo commented Apr 8, 2025 •

edited

Loading