Skip to content

Commit

Permalink
feat(HMS-2255): launch to location of the resource group by default
Browse files Browse the repository at this point in the history
We should allow users to follow Azure default and use the resource group location by default.
  • Loading branch information
ezr-ondrej committed Oct 20, 2023
1 parent f9724b4 commit f1a09be
Show file tree
Hide file tree
Showing 16 changed files with 191 additions and 83 deletions.
7 changes: 4 additions & 3 deletions api/openapi.gen.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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": {
Expand All @@ -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": {
Expand Down Expand Up @@ -1363,7 +1364,7 @@
"name": "GPL-3.0"
},
"title": "provisioning-api",
"version": "1.8.0"
"version": "1.9.0"
},
"openapi": "3.0.0",
"paths": {
Expand Down
7 changes: 4 additions & 3 deletions api/openapi.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion cmd/spec/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.8.0
1.9.0
2 changes: 1 addition & 1 deletion cmd/spec/example_reservation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
16 changes: 10 additions & 6 deletions internal/clients/http/azure/create_vm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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) {
Expand Down
18 changes: 9 additions & 9 deletions internal/clients/http/image_builder/image_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion internal/clients/instance_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions internal/clients/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
8 changes: 6 additions & 2 deletions internal/clients/stubs/azure_stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions internal/clients/stubs/image_builder_stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
23 changes: 19 additions & 4 deletions internal/jobs/launch_instance_azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package jobs
import (
"context"
"fmt"
"regexp"
"strconv"

"github.com/RHEnVision/provisioning-backend/internal/config"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit f1a09be

Please sign in to comment.