many: use image-builder upload in tests (HMS-10856)#2476
Conversation
|
This all probably won't work as it likely needs secrets we can't currently pass on the commandline of |
image-builder upload in testsimage-builder upload in tests (HMS-10856)
38fdcf5 to
406a31d
Compare
|
Ok; this needs to wire up |
406a31d to
cb828b9
Compare
cb828b9 to
def9c07
Compare
image-builder upload in tests (HMS-10856)image-builder upload in tests (HMS-10856)
|
I cannot see the Snyk issue but otherwise this is ready for review. |
023d25d to
981ad76
Compare
|
Ok; this is only failing on the dnf5 depsolver tests now; let's see if a rebase fixes that. |
981ad76 to
a5020af
Compare
|
Snyk errors are not relevant, the idiom already existed. |
brlane-rht
left a comment
There was a problem hiding this comment.
Looks pretty good, just a few small questions.
Change UploadAndRegister to return *UploadResult and an error instead of error. The UploadResult struct carries the provider name and the cloud-specific image identifier. This allows callers to obtain the image ID after upload or any additional information that might be relevant.
Add --format flag to the upload command. After a successful upload, the new UploadResult is serialized and printed to stdout as yaml or json.
Move the upload logic from cmd/boot-azure into a new azureUploader that implements the cloud.Uploader interface. This handles storage account setup, page blob upload, and image registration for both x86_64 (standard image) and aarch64 (gallery image).
Wire the new Azure uploader into the upload command.
Remove the upload and image registration logic from boot-aws. The command now takes a pre-uploaded AMI ID via --ami instead of a local image file. Upload should be done separately via `image-builder upload --to=aws`.
Remove the upload and image registration bits from boot-azure. The command now requires a pre-uploaded image via --image (or --snapshot for Windows/WSL). Upload should be done separately via image-builder upload --to=azure.
Update the test scripts to use the new workflow: 1. Upload via image-builder upload --to=aws/azure --format=json 2. Boot via boot-aws --ami or boot-azure --image Add upload_to_aws() and upload_to_azure() helpers that call image-builder upload and parse the JSON result for the image ID.
Bump the seed to ensure all images get built and boot tested with the new tools. Signed-off-by: Simon de Vlieger <cmdr@supakeen.com>
The two places that use `pb` directly were writing their output to stdout while normally we write output of progress to stderr. Fix that, and the tests. Signed-off-by: Simon de Vlieger <cmdr@supakeen.com>
Allow specifying the AWS boot mode for a given upload. Sometimes it's necessary to set this. Signed-off-by: Simon de Vlieger <cmdr@supakeen.com>
Take the boot mode from `describe` and pass it to `image-builder upload`. Signed-off-by: Simon de Vlieger <cmdr@supakeen.com>
a5020af to
cbe00ce
Compare
I've addressed all of them 🙂 |
|
Very nice and clean. Thank you! |
| requiredArgs := []string{"azure-client-id", "azure-client-secret", "azure-tenant", "azure-subscription", "azure-resource-group", "azure-image-name"} | ||
| for _, argName := range requiredArgs { | ||
| arg, err := cmd.Flags().GetString(argName) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if arg == "" { | ||
| missing = append(missing, fmt.Sprintf("--%s", argName)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Since all of the args are required, we could also remove whatever is in the function before, and just do:
| requiredArgs := []string{"azure-client-id", "azure-client-secret", "azure-tenant", "azure-subscription", "azure-resource-group", "azure-image-name"} | |
| for _, argName := range requiredArgs { | |
| arg, err := cmd.Flags().GetString(argName) | |
| if err != nil { | |
| return nil, err | |
| } | |
| if arg == "" { | |
| missing = append(missing, fmt.Sprintf("--%s", argName)) | |
| } | |
| } | |
| usedArgs := make(map[string]string) | |
| requiredArgs := []string{"azure-client-id", "azure-client-secret", "azure-tenant", "azure-subscription", "azure-resource-group", "azure-image-name"} | |
| for _, argName := range requiredArgs { | |
| arg, err := cmd.Flags().GetString(argName) | |
| if err != nil { | |
| return nil, err | |
| } | |
| if arg == "" { | |
| missing = append(missing, fmt.Sprintf("--%s", argName)) | |
| } else { | |
| usedArgs[argName] = arg | |
| } | |
| } |
And then we return values from the map. Not necessary, might complicate things once we start using arguments that are not required, but I think worth thinking about?
|
Failing with data race detections in the merge queue. |
|
I think we found this race here because we actually run with the race detector in this branch; I've opened a PR to address it and we'll need to rebase this PR (and likely others) after that lands: #2487 |
Replace the upload parts of
boot-(aws|azure)withimage-builder upload. Had to add some options and move all the Azure upload code since it didn't exist yet.