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

@@ -680,6 +680,7 @@
             "type": "string"
           },
           "location": {
+            "description": "Location to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location",
             "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": {

api/openapi.gen.yaml

@@ -95,6 +95,7 @@ components:
                     type: string
                 location:
                     type: string
+                    description: Location to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location
                 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:

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,24 +158,24 @@ 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
 		if errors.As(err, &azErr) && azErr.StatusCode == http.StatusNotFound {
 			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) {

internal/clients/http/image_builder/image_client.go

@@ -87,41 +87,41 @@ func (c *ibClient) GetAWSAmi(ctx context.Context, composeID string) (string, err
 	return uploadStatus.Ami, nil
 }
 
-func (c *ibClient) GetAzureImageID(ctx context.Context, composeID string) (string, error) {
+func (c *ibClient) GetAzureImageInfo(ctx context.Context, composeID string) (string, string, error) {
 	logger := logger(ctx)
 	logger.Trace().Msgf("Getting Azure ID of image %v", composeID)
 
 	composeStatus, err := c.getComposeStatus(ctx, composeID)
 	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
 	}
 
 	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, composeID string) (string, error) {

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

internal/clients/interface.go

@@ -6,6 +6,12 @@ import (
 	"github.com/RHEnVision/provisioning-backend/internal/models"
 )
 
+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)
@@ -37,10 +43,8 @@ type ImageBuilder interface {
 	// GetAWSAmi returns related AWS image AMI identifier
 	GetAWSAmi(ctx context.Context, composeID string) (string, error)
 
-	// GetAzureImageID returns partial image id, that is missing the subscription prefix
-	// Full name is /subscriptions/<subscription-id>/resourceGroups/<Group>/providers/Microsoft.Compute/images/<ImageName>
-	// GetAzureImageID returns /resourceGroups/<Group>/providers/Microsoft.Compute/images/<ImageName>
-	GetAzureImageID(ctx context.Context, composeID string) (string, error)
+	// GetAzureImageInfo returns Resource Group name and image name from the image builder info.
+	GetAzureImageInfo(ctx context.Context, composeID string) (string, string, error)
 
 	// GetGCPImageName returns GCP image name
 	GetGCPImageName(ctx context.Context, composeID string) (string, error)
@@ -113,7 +117,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

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) {

internal/clients/stubs/image_builder_stub.go

@@ -37,8 +37,8 @@ func (mock *ImageBuilderClientStub) GetAWSAmi(ctx context.Context, composeID str
 	return "ami-0c830793775595d4b-test", nil
 }
 
-func (mock *ImageBuilderClientStub) GetAzureImageID(ctx context.Context, composeID string) (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, composeID string) (string, string, error) {
+	return "myTestGroup", "composer-api-92ea98f8-7697-472e-80b1-7454fa0e7fa7", nil
 }
 
 func (mock *ImageBuilderClientStub) GetGCPImageName(ctx context.Context, composeID string) (string, error) {

internal/jobs/launch_instance_azure.go

@@ -31,7 +31,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
@@ -116,13 +116,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 +173,7 @@ func DoLaunchInstanceAzure(ctx context.Context, args *LaunchInstanceAzureTaskArg
 	}
 
 	vmParams := clients.AzureInstanceParams{
-		Location:          location,
+		Location:          args.Location,
 		ResourceGroupName: args.ResourceGroupName,
 		ImageID:           args.AzureImageID,
 		Pubkey:            pubkey,

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) {

internal/payloads/reservation_payload.go

@@ -198,10 +198,10 @@ 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"`
+	Location string `json:"location" yaml:"location" description:"Location to deploy the VM into, be aware it needs to be the same as the image location. Defaults to the Resource Group location"`
 
 	// Azure Instance type.
 	InstanceSize string `json:"instance_size" yaml:"instance_size"`