From 008fded264ebf41386fb400ab9994dd953ecf460 Mon Sep 17 00:00:00 2001 From: Ondrej Ezr Date: Wed, 18 Oct 2023 15:57:57 +0200 Subject: [PATCH] feat(HMS-2255): launch to location of the resource group by default We should allow users to follow Azure default and use the resource group location by default. --- api/openapi.gen.json | 7 +- api/openapi.gen.yaml | 7 +- cmd/spec/VERSION | 2 +- cmd/spec/example_reservation.go | 2 +- internal/clients/http/azure/create_vm.go | 16 ++-- .../http/image_builder/image_client.go | 20 ++--- internal/clients/instance_params.go | 2 +- internal/clients/interface.go | 14 ++-- internal/clients/stubs/azure_stub.go | 8 +- internal/clients/stubs/image_builder_stub.go | 4 +- internal/jobs/launch_instance_azure.go | 23 ++++- internal/jobs/launch_instance_azure_test.go | 83 +++++++++++++------ internal/payloads/reservation_payload.go | 8 +- .../services/azure_reservation_service.go | 38 +++++---- .../azure_reservation_service_test.go | 41 ++++++++- .../reservation-create-azure.http | 2 +- 16 files changed, 192 insertions(+), 85 deletions(-) diff --git a/api/openapi.gen.json b/api/openapi.gen.json index fd3d4027..aa5b8593 100644 --- a/api/openapi.gen.json +++ b/api/openapi.gen.json @@ -66,7 +66,7 @@ "amount": 1, "image_id": "composer-api-081fc867-838f-44a5-af03-8b8def808431", "instance_size": "Basic_A0", - "location": "useast_1", + "location": "useast", "name": "my-instance", "poweroff": false, "pubkey_id": 42, @@ -680,6 +680,7 @@ "type": "string" }, "location": { + "description": "Location (also known as region) to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location, or 'eastus' when also creating the resource group.", "type": "string" }, "name": { @@ -693,7 +694,7 @@ "type": "integer" }, "resource_group": { - "description": "Azure resource group name to deploy the VM resources into. Optional, defaults to 'redhat-deployed'.", + "description": "Azure resource group name to deploy the VM resources into. Optional, defaults to images resource group and when not found to 'redhat-deployed'.", "type": "string" }, "source_id": { @@ -1363,7 +1364,7 @@ "name": "GPL-3.0" }, "title": "provisioning-api", - "version": "1.8.0" + "version": "1.9.0" }, "openapi": "3.0.0", "paths": { diff --git a/api/openapi.gen.yaml b/api/openapi.gen.yaml index 48bb4f20..6687b1e3 100644 --- a/api/openapi.gen.yaml +++ b/api/openapi.gen.yaml @@ -95,6 +95,7 @@ components: type: string location: type: string + description: Location (also known as region) to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location, or 'eastus' when also creating the resource group. name: type: string poweroff: @@ -104,7 +105,7 @@ components: format: int64 resource_group: type: string - description: Azure resource group name to deploy the VM resources into. Optional, defaults to 'redhat-deployed'. + description: Azure resource group name to deploy the VM resources into. Optional, defaults to images resource group and when not found to 'redhat-deployed'. source_id: type: string v1.AzureReservationResponse: @@ -664,7 +665,7 @@ components: amount: 1 image_id: composer-api-081fc867-838f-44a5-af03-8b8def808431 instance_size: Basic_A0 - location: useast_1 + location: useast name: my-instance poweroff: false pubkey_id: 42 @@ -952,7 +953,7 @@ info: description: Provisioning service API license: name: GPL-3.0 - version: 1.8.0 + version: 1.9.0 paths: /availability_status/sources: post: diff --git a/cmd/spec/VERSION b/cmd/spec/VERSION index 27f9cd32..f8e233b2 100644 --- a/cmd/spec/VERSION +++ b/cmd/spec/VERSION @@ -1 +1 @@ -1.8.0 +1.9.0 diff --git a/cmd/spec/example_reservation.go b/cmd/spec/example_reservation.go index 339fe685..ebb77ea2 100644 --- a/cmd/spec/example_reservation.go +++ b/cmd/spec/example_reservation.go @@ -122,7 +122,7 @@ var AwsReservationResponsePayloadDoneExample = payloads.AWSReservationResponse{ var AzureReservationRequestPayloadExample = payloads.AzureReservationRequest{ PubkeyID: 42, SourceID: "654321", - Location: "useast_1", + Location: "useast", ResourceGroup: "redhat-hcc", InstanceSize: "Basic_A0", Amount: 1, diff --git a/internal/clients/http/azure/create_vm.go b/internal/clients/http/azure/create_vm.go index 8c060c5a..4e859723 100644 --- a/internal/clients/http/azure/create_vm.go +++ b/internal/clients/http/azure/create_vm.go @@ -33,6 +33,10 @@ const ( vmPollFrequency = 10 * time.Second ) +func newAzureResourceGroup(group armresources.ResourceGroup) clients.AzureResourceGroup { + return clients.AzureResourceGroup{ID: *group.ID, Name: *group.Name, Location: *group.Location} +} + func (c *client) BeginCreateVM(ctx context.Context, networkInterface *armnetwork.Interface, vmParams clients.AzureInstanceParams, vmName string) (string, error) { ctx, span := telemetry.StartSpan(ctx, "BeginCreateVM") defer span.End() @@ -154,16 +158,16 @@ func (c *client) prepareVMNetworking(ctx context.Context, subnet *armnetwork.Sub return networkInterface, publicIP, nil } -func (c *client) EnsureResourceGroup(ctx context.Context, name string, location string) (*string, error) { +func (c *client) EnsureResourceGroup(ctx context.Context, name string, location string) (clients.AzureResourceGroup, error) { resourceGroupClient, err := c.newResourceGroupsClient(ctx) if err != nil { - return nil, err + return clients.AzureResourceGroup{}, err } logger := logger(ctx) getResp, err := resourceGroupClient.Get(ctx, name, nil) if err == nil { - return getResp.ResourceGroup.ID, nil + return newAzureResourceGroup(getResp.ResourceGroup), nil } if err != nil { var azErr *azcore.ResponseError @@ -171,7 +175,7 @@ func (c *client) EnsureResourceGroup(ctx context.Context, name string, location logger.Debug().Msgf("resource group %s not found, creating", name) // 404 is expected, continue } else { - return nil, fmt.Errorf("failed to fetch resource group: %w", err) + return clients.AzureResourceGroup{}, fmt.Errorf("failed to fetch resource group: %w", err) } } @@ -183,10 +187,10 @@ func (c *client) EnsureResourceGroup(ctx context.Context, name string, location resp, err := resourceGroupClient.CreateOrUpdate(ctx, name, parameters, nil) if err != nil { - return nil, fmt.Errorf("cannot create resource group: %w", err) + return clients.AzureResourceGroup{}, fmt.Errorf("cannot create resource group: %w", err) } - return resp.ResourceGroup.ID, nil + return newAzureResourceGroup(resp.ResourceGroup), nil } func (c *client) createVirtualNetwork(ctx context.Context, location string, resourceGroupName string, name string) (*armnetwork.VirtualNetwork, error) { diff --git a/internal/clients/http/image_builder/image_client.go b/internal/clients/http/image_builder/image_client.go index db7e482b..6afec2f3 100644 --- a/internal/clients/http/image_builder/image_client.go +++ b/internal/clients/http/image_builder/image_client.go @@ -88,46 +88,46 @@ func (c *ibClient) GetAWSAmi(ctx context.Context, composeUUID uuid.UUID, instanc return uploadStatus.Ami, nil } -func (c *ibClient) GetAzureImageID(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, error) { +func (c *ibClient) GetAzureImageInfo(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, string, error) { logger := logger(ctx) logger.Trace().Msgf("Getting Azure ID of image %v", composeUUID.String()) composeStatus, err := c.getComposeStatus(ctx, composeUUID) if err != nil { - return "", err + return "", "", err } if composeStatus == nil { logger.Warn().Msg("Compose status is not ready") - return "", fmt.Errorf("getting azure id: %w", http.ErrImageStatus) + return "", "", fmt.Errorf("getting azure id: %w", http.ErrImageStatus) } logger.Trace().Msgf("Verifying Azure type") if composeStatus.ImageStatus.UploadStatus == nil { - return "", fmt.Errorf("%w: upload status is nil", http.ErrUploadStatus) + return "", "", fmt.Errorf("%w: upload status is nil", http.ErrUploadStatus) } if composeStatus.ImageStatus.UploadStatus.Type != UploadTypesAzure { - return "", fmt.Errorf("%w: expected image type Azure, got %s", http.ErrUnknownImageType, composeStatus.ImageStatus.UploadStatus.Type) + return "", "", fmt.Errorf("%w: expected image type Azure, got %s", http.ErrUnknownImageType, composeStatus.ImageStatus.UploadStatus.Type) } if len(composeStatus.Request.ImageRequests) < 1 { logger.Error().Msg(http.ErrImageRequestNotFound.Error()) - return "", http.ErrImageRequestNotFound + return "", "", http.ErrImageRequestNotFound } imageArch, archErr := clients.MapArchitectures(ctx, string(composeStatus.Request.ImageRequests[0].Architecture)) if archErr != nil || imageArch != instanceType.Architecture { - return "", http.ErrImageArchInvalid + return "", "", http.ErrImageArchInvalid } uploadOptions, err := composeStatus.ImageStatus.UploadStatus.Options.AsAzureUploadStatus() if err != nil { - return "", fmt.Errorf("%w: not an Azure status", http.ErrUploadStatus) + return "", "", fmt.Errorf("%w: not an Azure status", http.ErrUploadStatus) } azureUploadRequest, err := composeStatus.Request.ImageRequests[0].UploadRequest.Options.AsAzureUploadRequestOptions() if err != nil { - return "", fmt.Errorf("failed to decode Azure upload request from IB: %w", err) + return "", "", fmt.Errorf("failed to decode Azure upload request from IB: %w", err) } - return fmt.Sprintf("/resourceGroups/%s/providers/Microsoft.Compute/images/%s", azureUploadRequest.ResourceGroup, uploadOptions.ImageName), nil + return azureUploadRequest.ResourceGroup, uploadOptions.ImageName, nil } func (c *ibClient) GetGCPImageName(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, error) { diff --git a/internal/clients/instance_params.go b/internal/clients/instance_params.go index e49f7ed1..ee03f3c5 100644 --- a/internal/clients/instance_params.go +++ b/internal/clients/instance_params.go @@ -56,7 +56,7 @@ type AWSInstanceParams struct { // AzureInstanceParams define parameters for a single instance launch on Azure. type AzureInstanceParams struct { - // Location - to deploy into + // Location - to deploy into, defaults to Resource Group location Location string // ResourceGroupName to launch the instance in diff --git a/internal/clients/interface.go b/internal/clients/interface.go index 2677f0ab..1ac0a34f 100644 --- a/internal/clients/interface.go +++ b/internal/clients/interface.go @@ -7,6 +7,12 @@ import ( "github.com/google/uuid" ) +type AzureResourceGroup struct { + ID string + Name string + Location string +} + // GetSourcesClient returns Sources interface implementation. There are currently // two implementations available: HTTP and stub var GetSourcesClient func(ctx context.Context) (Sources, error) @@ -39,11 +45,9 @@ type ImageBuilder interface { // It also verifies the image is built successfully and for the right architecture. GetAWSAmi(ctx context.Context, composeUUID uuid.UUID, instanceType InstanceType) (string, error) - // GetAzureImageID returns partial image id, that is missing the subscription prefix - // Full name is /subscriptions//resourceGroups//providers/Microsoft.Compute/images/ - // GetAzureImageID returns /resourceGroups//providers/Microsoft.Compute/images/ + // GetAzureImageInfo returns Resource Group name and image name from the image builder info. // It also verifies the image is built successfully and for the right architecture. - GetAzureImageID(ctx context.Context, composeUUID uuid.UUID, instanceType InstanceType) (string, error) + GetAzureImageInfo(ctx context.Context, composeUUID uuid.UUID, instanceType InstanceType) (string, string, error) // GetGCPImageName returns GCP image name // It also verifies the image is built successfully and for the right architecture. @@ -117,7 +121,7 @@ type Azure interface { TenantId(ctx context.Context) (AzureTenantId, error) // EnsureResourceGroup makes sure that group with give name exists in a location - EnsureResourceGroup(ctx context.Context, name string, location string) (*string, error) + EnsureResourceGroup(ctx context.Context, name string, location string) (AzureResourceGroup, error) // CreateVMs creates multiple Azure virtual machines // Returns array of instance IDs and error if something went wrong diff --git a/internal/clients/stubs/azure_stub.go b/internal/clients/stubs/azure_stub.go index ccae65d3..03de284b 100644 --- a/internal/clients/stubs/azure_stub.go +++ b/internal/clients/stubs/azure_stub.go @@ -92,16 +92,20 @@ func (stub *AzureClientStub) WaitForVM(ctx context.Context, resumeToken string) return "", ErrNotStartedVM } -func (stub *AzureClientStub) EnsureResourceGroup(ctx context.Context, name string, location string) (*string, error) { +func (stub *AzureClientStub) EnsureResourceGroup(ctx context.Context, name string, location string) (clients.AzureResourceGroup, error) { id := strconv.Itoa(len(stub.createdRgs) + 1) + if name == "existing" { + return clients.AzureResourceGroup{ID: "/subscriptions/123/resourceGroups/mockedID", Name: "existing", Location: "westeurope"}, nil + } + rg := armresources.ResourceGroup{ ID: &id, Name: &name, Location: &location, } stub.createdRgs = append(stub.createdRgs, &rg) - return &id, nil + return clients.AzureResourceGroup{ID: id, Name: name, Location: location}, nil } func (stub *AzureClientStub) TenantId(ctx context.Context) (clients.AzureTenantId, error) { diff --git a/internal/clients/stubs/image_builder_stub.go b/internal/clients/stubs/image_builder_stub.go index 73b62ee8..c8c2ff1b 100644 --- a/internal/clients/stubs/image_builder_stub.go +++ b/internal/clients/stubs/image_builder_stub.go @@ -38,8 +38,8 @@ func (mock *ImageBuilderClientStub) GetAWSAmi(ctx context.Context, composeUUID u return "ami-0c830793775595d4b-test", nil } -func (mock *ImageBuilderClientStub) GetAzureImageID(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, error) { - return "/resourceGroups/redhat-deployed/providers/Microsoft.Compute/images/composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", nil +func (mock *ImageBuilderClientStub) GetAzureImageInfo(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, string, error) { + return "myTestGroup", "composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", nil } func (mock *ImageBuilderClientStub) GetGCPImageName(ctx context.Context, composeUUID uuid.UUID, instanceType clients.InstanceType) (string, error) { diff --git a/internal/jobs/launch_instance_azure.go b/internal/jobs/launch_instance_azure.go index 0162a547..bd061ada 100644 --- a/internal/jobs/launch_instance_azure.go +++ b/internal/jobs/launch_instance_azure.go @@ -3,6 +3,7 @@ package jobs import ( "context" "fmt" + "regexp" "strconv" "github.com/RHEnVision/provisioning-backend/internal/config" @@ -31,7 +32,7 @@ type LaunchInstanceAzureTaskArgs struct { // Associated reservation ReservationID int64 - // Location to provision the instances into + // Location to provision the instances into when blank, uses the Resource Group location Location string // Associated public key @@ -72,6 +73,15 @@ func HandleLaunchInstanceAzure(ctx context.Context, job *worker.Job) { logger.Debug().Msg("Resource group has not been set, defaulting to 'redhat-deployed'") args.ResourceGroupName = DefaultAzureResourceGroupName } + if args.Location != "" { + // Match for availability zone suffix and removes it. + // For backwards compatibility, we accept both forms and adjust here, + // but we want to accept only region without the zone info in the future. + res, e := regexp.MatchString(`_[1-6]\z`, args.Location) + if e == nil && res { + args.Location = args.Location[0 : len(args.Location)-2] + } + } // ensure panic finishes the job defer func() { @@ -116,13 +126,18 @@ func DoEnsureAzureResourceGroup(ctx context.Context, args *LaunchInstanceAzureTa return fmt.Errorf("cannot create new Azure client: %w", err) } - resourceGroupID, err := azureClient.EnsureResourceGroup(ctx, args.ResourceGroupName, location) + resourceGroup, err := azureClient.EnsureResourceGroup(ctx, args.ResourceGroupName, location) if err != nil { span.SetStatus(codes.Error, "cannot create resource group") logger.Error().Err(err).Msg("Cannot create resource group") return fmt.Errorf("failed to ensure resource group: %w", err) } - logger.Trace().Msgf("Using resource group id=%s", *resourceGroupID) + logger.Trace().Msgf("Using resource group id=%s", resourceGroup) + + if args.Location == "" { + logger.Debug().Str("azure_location", resourceGroup.Location).Msg("Using location from Resource Group") + args.Location = resourceGroup.Location + } return nil } @@ -168,7 +183,7 @@ func DoLaunchInstanceAzure(ctx context.Context, args *LaunchInstanceAzureTaskArg } vmParams := clients.AzureInstanceParams{ - Location: location, + Location: args.Location, ResourceGroupName: args.ResourceGroupName, ImageID: args.AzureImageID, Pubkey: pubkey, diff --git a/internal/jobs/launch_instance_azure_test.go b/internal/jobs/launch_instance_azure_test.go index 869606be..9b48f953 100644 --- a/internal/jobs/launch_instance_azure_test.go +++ b/internal/jobs/launch_instance_azure_test.go @@ -51,32 +51,63 @@ func prepareAzureReservation(t *testing.T, ctx context.Context, pk *models.Pubke } func TestDoEnsureAzureResourceGroup(t *testing.T) { - ctx := prepareAzureContext(t) - - pk := factories.NewPubkeyRSA() - err := daoStubs.AddPubkey(ctx, pk) - require.NoError(t, err, "failed to add stubbed key") - - res := prepareAzureReservation(t, ctx, pk) - - rDao := dao.GetReservationDao(ctx) - err = rDao.CreateAzure(ctx, res) - require.NoError(t, err, "failed to add stubbed reservation") - - args := &jobs.LaunchInstanceAzureTaskArgs{ - AzureImageID: "/subscriptions/subUUID/rgName/images/uuid2", - Location: "useast", - PubkeyID: pk.ID, - ReservationID: res.ID, - SourceID: "2", - Subscription: clients.NewAuthentication("subUUID", models.ProviderTypeAzure), - ResourceGroupName: "testGroup", - } - - err = jobs.DoEnsureAzureResourceGroup(ctx, args) - require.NoError(t, err, "the ensure resource group failed to run") - - assert.True(t, clientStubs.DidCreateAzureResourceGroup(ctx, "testGroup")) + t.Run("verify it calls the Azure client with correct params", func(t *testing.T) { + ctx := prepareAzureContext(t) + + pk := factories.NewPubkeyRSA() + err := daoStubs.AddPubkey(ctx, pk) + require.NoError(t, err, "failed to add stubbed key") + + res := prepareAzureReservation(t, ctx, pk) + + rDao := dao.GetReservationDao(ctx) + err = rDao.CreateAzure(ctx, res) + require.NoError(t, err, "failed to add stubbed reservation") + + args := &jobs.LaunchInstanceAzureTaskArgs{ + AzureImageID: "/subscriptions/subUUID/rgName/images/uuid2", + Location: "useast", + PubkeyID: pk.ID, + ReservationID: res.ID, + SourceID: "2", + Subscription: clients.NewAuthentication("subUUID", models.ProviderTypeAzure), + ResourceGroupName: "testGroup", + } + + err = jobs.DoEnsureAzureResourceGroup(ctx, args) + require.NoError(t, err, "the ensure resource group failed to run") + + assert.True(t, clientStubs.DidCreateAzureResourceGroup(ctx, "testGroup")) + }) + + t.Run("it fetches the Resource Group location", func(t *testing.T) { + ctx := prepareAzureContext(t) + + pk := factories.NewPubkeyRSA() + err := daoStubs.AddPubkey(ctx, pk) + require.NoError(t, err, "failed to add stubbed key") + + res := prepareAzureReservation(t, ctx, pk) + + rDao := dao.GetReservationDao(ctx) + err = rDao.CreateAzure(ctx, res) + require.NoError(t, err, "failed to add stubbed reservation") + + args := &jobs.LaunchInstanceAzureTaskArgs{ + AzureImageID: "/subscriptions/subUUID/rgName/images/uuid2", + Location: "", + PubkeyID: pk.ID, + ReservationID: res.ID, + SourceID: "2", + Subscription: clients.NewAuthentication("subUUID", models.ProviderTypeAzure), + ResourceGroupName: "existing", + } + + err = jobs.DoEnsureAzureResourceGroup(ctx, args) + require.NoError(t, err, "the ensure resource group failed to run") + + assert.Equal(t, "westeurope", args.Location) + }) } func TestDoLaunchInstanceAzure(t *testing.T) { diff --git a/internal/payloads/reservation_payload.go b/internal/payloads/reservation_payload.go index 5fc5f854..3ea48e0a 100644 --- a/internal/payloads/reservation_payload.go +++ b/internal/payloads/reservation_payload.go @@ -198,10 +198,12 @@ type AzureReservationRequest struct { ImageID string `json:"image_id" yaml:"image_id"` // ResourceGroup to use to deploy the resources into - ResourceGroup string `json:"resource_group" yaml:"resource_group" description:"Azure resource group name to deploy the VM resources into. Optional, defaults to 'redhat-deployed'."` + ResourceGroup string `json:"resource_group" yaml:"resource_group" description:"Azure resource group name to deploy the VM resources into. Optional, defaults to images resource group and when not found to 'redhat-deployed'."` - // Azure Location to deploy into. - Location string `json:"location" yaml:"location"` + // Azure Location also known as region to deploy the VM into. + // Be aware it needs to be the same as the image location. + // Defaults to the Resource group location or 'eastus' if new resource group is also created in this request. + Location string `json:"location" yaml:"location" description:"Location (also known as region) to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location, or 'eastus' when also creating the resource group."` // Azure Instance type. InstanceSize string `json:"instance_size" yaml:"instance_size"` diff --git a/internal/services/azure_reservation_service.go b/internal/services/azure_reservation_service.go index b7fabbc3..b8c21d80 100644 --- a/internal/services/azure_reservation_service.go +++ b/internal/services/azure_reservation_service.go @@ -34,13 +34,16 @@ func CreateAzureReservation(w http.ResponseWriter, r *http.Request) { pkDao := dao.GetPubkeyDao(r.Context()) rDao := dao.GetReservationDao(r.Context()) - // Check for preloaded region - if payload.Location == "" { - payload.Location = "eastus_1" - } - if !preload.AzureInstanceType.ValidateRegion(payload.Location) { - renderError(w, r, payloads.NewInvalidRequestError(r.Context(), "Unsupported location", ErrUnsupportedRegion)) - return + // validate region + // it needs to be in region format, but also + if payload.Location != "" && !preload.AzureInstanceType.ValidateRegion(payload.Location+"_1") { + // Location format is accepted now for backwards compatibility, but we should deprecate it + if preload.AzureInstanceType.ValidateRegion(payload.Location) { + logger.Warn().Msgf("Azure region passed with location suffix (%s), this is deprecated behaviour format", payload.Location) + } else { + renderError(w, r, payloads.NewInvalidRequestError(r.Context(), "Unsupported location", ErrUnsupportedRegion)) + return + } } // Validate pubkey @@ -80,31 +83,34 @@ func CreateAzureReservation(w http.ResponseWriter, r *http.Request) { } var azureImageName string + resourceGroupName := payload.ResourceGroup // Azure image IDs are "free form", if it's a UUID we treat it like a compose ID if composeUUID, pErr := uuid.Parse(payload.ImageID); pErr == nil { // Composer-built image - instanceType := preload.AzureInstanceType.FindInstanceType(clients.InstanceTypeName(payload.InstanceSize)) if instanceType == nil { renderError(w, r, payloads.NewInvalidRequestError(r.Context(), "Instance size is not a valid Azure instance size", nil)) return } - azureImageName, err = ibClient.GetAzureImageID(r.Context(), composeUUID, *instanceType) + var imageResourceGroupName string + imageResourceGroupName, azureImageName, err = ibClient.GetAzureImageInfo(r.Context(), composeUUID, *instanceType) if err != nil { renderError(w, r, payloads.NewClientError(r.Context(), err)) return } - azureImageName = fmt.Sprintf("/subscriptions/%s%s", authentication.Payload, azureImageName) + if resourceGroupName == "" { + resourceGroupName = imageResourceGroupName + } + azureImageName = fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", authentication.Payload, imageResourceGroupName, azureImageName) } else { // Format Image ID for image names passed manually in here. // Assumes the image is in the resource group we want to deploy into. if strings.HasPrefix(payload.ImageID, "composer-api") { - rg := payload.ResourceGroup - if rg == "" { - rg = jobs.DefaultAzureResourceGroupName + if resourceGroupName == "" { + resourceGroupName = jobs.DefaultAzureResourceGroupName } - azureImageName = fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", authentication.Payload, rg, payload.ImageID) + azureImageName = fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Compute/images/%s", authentication.Payload, resourceGroupName, payload.ImageID) } else { // Anything else is treated like a direct Azure image ID (e.g. from https://imagedirectory.cloud) azureImageName = payload.ImageID @@ -125,7 +131,7 @@ func CreateAzureReservation(w http.ResponseWriter, r *http.Request) { name := config.Application.InstancePrefix + payload.Name detail := &models.AzureDetail{ Location: payload.Location, - ResourceGroup: payload.ResourceGroup, + ResourceGroup: resourceGroupName, InstanceSize: payload.InstanceSize, Amount: payload.Amount, PowerOff: payload.PowerOff, @@ -154,9 +160,9 @@ func CreateAzureReservation(w http.ResponseWriter, r *http.Request) { EdgeID: logging.EdgeRequestId(r.Context()), AccountID: identity.AccountId(r.Context()), Args: jobs.LaunchInstanceAzureTaskArgs{ + Location: reservation.Detail.Location, ReservationID: reservation.ID, ResourceGroupName: reservation.Detail.ResourceGroup, - Location: reservation.Detail.Location, PubkeyID: pk.ID, SourceID: reservation.SourceID, AzureImageID: azureImageName, diff --git a/internal/services/azure_reservation_service_test.go b/internal/services/azure_reservation_service_test.go index eacf567d..5d0df08f 100644 --- a/internal/services/azure_reservation_service_test.go +++ b/internal/services/azure_reservation_service_test.go @@ -68,7 +68,7 @@ func TestCreateAzureReservationHandler(t *testing.T) { assert.IsType(t, jobs.LaunchInstanceAzureTaskArgs{}, stub.EnqueuedJobs(ctx)[0].Args, "Unexpected type of arguments for the planned job") jobArgs := stub.EnqueuedJobs(ctx)[0].Args.(jobs.LaunchInstanceAzureTaskArgs) assert.Equal(t, "testGroup", jobArgs.ResourceGroupName) - assert.Equal(t, "/subscriptions/4b9d213f-712f-4d17-a483-8a10bbe9df3a/resourceGroups/redhat-deployed/providers/Microsoft.Compute/images/composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", jobArgs.AzureImageID, "Expected translated image to real name - one from IB client stub") + assert.Equal(t, "/subscriptions/4b9d213f-712f-4d17-a483-8a10bbe9df3a/resourceGroups/myTestGroup/providers/Microsoft.Compute/images/composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", jobArgs.AzureImageID, "Expected translated image to real name - one from IB client stub") }) t.Run("successful reservation with azure image name translated to full azure ID", func(t *testing.T) { @@ -137,4 +137,43 @@ func TestCreateAzureReservationHandler(t *testing.T) { assert.Contains(t, rr.Body.String(), "Unsupported location") require.Equal(t, http.StatusBadRequest, rr.Code, "Handler returned wrong status code") }) + + t.Run("successful reservation with blank location and resource group", func(t *testing.T) { + ctx := stubs.WithReservationDao(sharedCtx) + ctx = stub.WithEnqueuer(ctx) + + var err error + values := map[string]interface{}{ + "source_id": source.ID, + "location": "", + "resource_group": "", + "image_id": "92ea98f8-7697-472e-80b1-7454fa0e7fa7", + "amount": 1, + "instance_size": "Basic_A0", + "pubkey_id": pk.ID, + } + if json_data, err = json.Marshal(values); err != nil { + t.Fatalf("unable to marshal values to json: %v", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", "/api/provisioning/reservations/azure", bytes.NewBuffer(json_data)) + require.NoError(t, err, "failed to create request") + req.Header.Add("Content-Type", "application/json") + + rr := httptest.NewRecorder() + handler := http.HandlerFunc(services.CreateAzureReservation) + handler.ServeHTTP(rr, req) + + stubCount := stubs.AzureReservationStubCount(ctx) + assert.Equal(t, 1, stubCount, "Reservation has not been Created through DAO") + + assert.Equal(t, 1, len(stub.EnqueuedJobs(ctx)), "Expected exactly one job to be planned") + assert.IsType(t, jobs.LaunchInstanceAzureTaskArgs{}, stub.EnqueuedJobs(ctx)[0].Args, "Unexpected type of arguments for the planned job") + jobArgs := stub.EnqueuedJobs(ctx)[0].Args.(jobs.LaunchInstanceAzureTaskArgs) + // from Image + assert.Equal(t, "myTestGroup", jobArgs.ResourceGroupName) + // plan the job with empty location - the job should default it to the RG location + assert.Equal(t, "", jobArgs.Location) + assert.Equal(t, "/subscriptions/4b9d213f-712f-4d17-a483-8a10bbe9df3a/resourceGroups/myTestGroup/providers/Microsoft.Compute/images/composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", jobArgs.AzureImageID, "Expected translated image to real name - one from IB client stub") + }) } diff --git a/scripts/rest_examples/reservation-create-azure.http b/scripts/rest_examples/reservation-create-azure.http index b7c618e7..3641dbea 100644 --- a/scripts/rest_examples/reservation-create-azure.http +++ b/scripts/rest_examples/reservation-create-azure.http @@ -5,7 +5,7 @@ X-Rh-Identity: {{identity}} { "name": "azure-linux-us-east", - "location": "eastus_1", + "location": "eastus", "source_id": "{{source_id_azure}}", "image_id": "composer-api-e7e1c242-4ce8-4d5e-a5d0-75720e91afca", "amount": 1,