feat: refactor mac request / release flow

    This commit move the time consuming effort for having fresh machines on the mac pool from the request action to the release. Now release will manage the root volume replace task and request will create new user credentials and set machine as locked to avoid new workloads to make use of it

Signed-off-by: Adrian Riobo <[email protected]>

cmd/mapt/cmd/aws/hosts/mac.go

@@ -23,10 +23,10 @@ const (
 	dhIDDesc          string = "id for the dedicated host"
 	arch              string = "arch"
 	archDesc          string = "mac architecture allowed values x86, m1, m2"
-	archDefault       string = "m2"
+	archDefault       string = mac.DefaultArch
 	osVersion         string = "version"
 	osVersionDesc     string = "macos operating system vestion 11, 12 on x86 and m1/m2; 13, 14 on all archs"
-	osDefault         string = "14"
+	osDefault         string = mac.DefaultOSVersion
 	fixedLocation     string = "fixed-location"
 	fixedLocationDesc string = "if this flag is set the host will be created only on the region set by the AWS Env (AWS_DEFAULT_REGION)"
 )
@@ -96,14 +96,6 @@ func getMacRequest() *cobra.Command {
 	flagSet.Bool(airgap, false, airgapDesc)
 	flagSet.AddFlagSet(params.GetGHActionsFlagset())
 	c.PersistentFlags().AddFlagSet(flagSet)
-	err := c.MarkPersistentFlagRequired(arch)
-	if err != nil {
-		logging.Error(err)
-	}
-	err = c.MarkPersistentFlagRequired(osVersion)
-	if err != nil {
-		logging.Error(err)
-	}
 	return c
 }
 

pkg/provider/aws/action/mac/constants.go

@@ -7,6 +7,7 @@ const (
 	awsMacMachineID = "amm"
 
 	customResourceTypeLock = "rh:qe:aws:mac:lock"
+	customResourceTypeKey  = "rh:mapt:aws:mac:key"
 
 	outputLock              = "ammLock"
 	outputHost              = "ammHost"
@@ -19,8 +20,8 @@ const (
 	outputRegion            = "ammRegion"
 
 	amiRegex         = "amzn-ec2-macos-%s*"
-	archDefault      = "m2"
-	osVersionDefault = "14"
+	DefaultArch      = "m2"
+	DefaultOSVersion = "15"
 
 	vncDefaultPort  int    = 5900
 	diskSize        int    = 100

pkg/provider/aws/action/mac/mac-machine.go

@@ -83,38 +83,25 @@ func (r *MacRequest) replaceMachine(h *HostInformation) error {
 		return err
 	}
 	logging.Debugf("Replacing root volume for AMI %s", *ami.Image.ImageId)
-	_, err = qEC2.ReplaceRootVolume(
+	if _, err = qEC2.ReplaceRootVolume(
 		qEC2.ReplaceRootVolumeRequest{
 			Region:     *h.Region,
 			InstanceID: *h.Host.Instances[0].InstanceId,
 			// Needto lookup for AMI + check if copy is required
 			AMIID: *ami.Image.ImageId,
-		})
-	if err != nil {
-		return err
-	}
-	r.lock = true
-	if err := r.manageMacMachine(h); err != nil {
+			Wait:  true,
+		}); err != nil {
 		return err
 	}
-	// replace will run again the boostrap script to generate
-	// and set new keys to access the machine
-	r.replace = true
+	r.lock = false
 	return r.manageMacMachine(h)
 }
 
-// Release will set the lock as false
-func (r *MacRequest) releaseLock(h *HostInformation) error {
-	r.lock = false
-	lockURN := fmt.Sprintf("urn:pulumi:%s::%s::%s::%s",
-		maptContext.StackNameByProject(stackMacMachine),
-		maptContext.ProjectName(),
-		customResourceTypeLock,
-		resourcesUtil.GetResourceName(
-			r.Prefix, awsMacMachineID, "mac-lock"))
-
-	// rh:qe:aws:mac:lock main-amm-mac-lock
-	return r.manageMacMachineTargets(h, []string{lockURN})
+// Run the bootstrap script creating new access credentials for the user
+func (r *MacRequest) replaceUserAccess(h *HostInformation) error {
+	r.replace = true
+	r.lock = true
+	return r.manageMacMachine(h)
 }
 
 // Release will set the lock as false
@@ -236,14 +223,15 @@ func (r *MacRequest) deployerMachine(ctx *pulumi.Context) error {
 	ctx.Export(fmt.Sprintf("%s-%s", r.Prefix, outputUserPassword), userPassword.Result)
 	ctx.Export(fmt.Sprintf("%s-%s", r.Prefix, outputUserPrivateKey),
 		ukp.PrivateKey.PrivateKeyPem)
-	// Create a lock on the machine
-	if err := machineLock(ctx,
-		resourcesUtil.GetResourceName(
-			r.Prefix, awsMacMachineID, "mac-lock"), r.lock); err != nil {
+	readiness, err := r.readiness(ctx, i, ukp.PrivateKey, bastion, []pulumi.Resource{bc})
+	if err != nil {
 		return err
 	}
 	ctx.Export(fmt.Sprintf("%s-%s", r.Prefix, outputLock), pulumi.Bool(r.lock))
-	return r.readiness(ctx, i, ukp.PrivateKey, bastion, []pulumi.Resource{bc})
+	return machineLock(ctx,
+		resourcesUtil.GetResourceName(
+			r.Prefix, awsMacMachineID, "mac-lock"), r.lock,
+		pulumi.DependsOn([]pulumi.Resource{readiness}))
 }
 
 // Write exported values in context to files o a selected target folder
@@ -400,8 +388,8 @@ func (r *MacRequest) readiness(ctx *pulumi.Context,
 	m *ec2.Instance,
 	mk *tls.PrivateKey,
 	b *bastion.Bastion,
-	dependecies []pulumi.Resource) error {
-	_, err := remote.NewCommand(ctx,
+	dependecies []pulumi.Resource) (*remote.Command, error) {
+	return remote.NewCommand(ctx,
 		resourcesUtil.GetResourceName(r.Prefix, awsMacMachineID, "readiness-cmd"),
 		&remote.CommandArgs{
 			Connection: remoteCommandArgs(m, mk, b),
@@ -412,7 +400,6 @@ func (r *MacRequest) readiness(ctx *pulumi.Context,
 				Create: remoteTimeout,
 				Update: remoteTimeout}),
 		pulumi.DependsOn(dependecies))
-	return err
 }
 
 // helper function to set the connection args

pkg/provider/aws/action/mac/mac.go

@@ -10,11 +10,9 @@ import (
 	"github.com/redhat-developer/mapt/pkg/provider/aws/data"
 	"github.com/redhat-developer/mapt/pkg/provider/aws/services/tag"
 	"github.com/redhat-developer/mapt/pkg/util/logging"
-
-	ec2Types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 )
 
-// Request could be interpreted as a general way to create / release
+// Request could be interpreted as a general way to create / get a machine
 //
 // Some project will request a mac machine
 // based on tags it will check if there is any existing mac machine (based on labels + arch + MaxPoolSize)
@@ -50,7 +48,7 @@ func Request(r *MacRequest) error {
 		}
 		return create(r, hi)
 	}
-	err = r.replaceMachine(hi)
+	err = r.replaceUserAccess(hi)
 	if err != nil {
 		return err
 	}
@@ -78,13 +76,14 @@ func Release(prefix string, hostID string) error {
 	maptContext.InitBase(
 		*hi.ProjectName,
 		*hi.BackedURL)
+
 	// Set a default request
 	r := &MacRequest{
 		Prefix:       prefix,
-		Architecture: archDefault,
-		Version:      osVersionDefault,
+		Architecture: DefaultArch,
+		Version:      DefaultOSVersion,
 	}
-	return r.releaseLock(hi)
+	return r.replaceMachine(hi)
 }
 
 // Initial scenario consider 1 machine
@@ -100,28 +99,25 @@ func Destroy(prefix, hostID string) error {
 	maptContext.InitBase(
 		*hi.ProjectName,
 		*hi.BackedURL)
-	// Check if dh is available and it has no instance on it
-	// otherwise we can not release it
-	if hi.Host.State == ec2Types.AllocationStateAvailable &&
-		len(host.Instances) == 0 {
-		return aws.DestroyStack(aws.DestroyStackRequest{
-			Stackname: stackDedicatedHost,
-			// TODO check if needed to add region for backedURL
-			Region:    *hi.Region,
-			BackedURL: *hi.BackedURL,
-		})
-	}
 	// Dedicated host is not on a valid state to be deleted
 	// With same backedURL check if machine is locked
 	machineLocked, err := isMachineLocked(prefix, hi)
 	if err != nil {
 		return err
 	}
 	if !machineLocked {
-		return aws.DestroyStack(aws.DestroyStackRequest{
+		if err := aws.DestroyStack(aws.DestroyStackRequest{
 			Stackname: stackMacMachine,
 			Region:    *hi.Region,
 			BackedURL: *hi.BackedURL,
+		}); err != nil {
+			return err
+		}
+		return aws.DestroyStack(aws.DestroyStackRequest{
+			Stackname: stackDedicatedHost,
+			// TODO check if needed to add region for backedURL
+			Region:    *hi.Region,
+			BackedURL: *hi.BackedURL,
 		})
 	}
 	logging.Debug("nothing to be destroyed")

pkg/provider/aws/services/ec2/compute/compute.go

@@ -2,33 +2,59 @@ package compute
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/aws/aws-sdk-go-v2/config"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
+	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
 )
 
 type ReplaceRootVolumeRequest struct {
 	Region     string
 	InstanceID string
 	AMIID      string
+	Wait       bool
 }
 
 // This function will replace the root volume for a running ec2 instance
 // and will delete the replaced volume
-func ReplaceRootVolume(r ReplaceRootVolumeRequest) (*ec2.CreateReplaceRootVolumeTaskOutput, error) {
+// If wait flag is true on request the funcion will wait until the replace task succeed
+// otherwise it will trigger the task and return the id to reference it
+func ReplaceRootVolume(r ReplaceRootVolumeRequest) (*string, error) {
+	ctx := context.Background()
 	cfg, err := config.LoadDefaultConfig(
-		context.Background(),
+		ctx,
 		config.WithRegion(r.Region))
 	if err != nil {
 		return nil, err
 	}
 	client := ec2.NewFromConfig(cfg)
 	deleteReplacedRootVolume := true
-	return client.CreateReplaceRootVolumeTask(
-		context.Background(),
+	// rrvt, err :=
+	rrvt, err := client.CreateReplaceRootVolumeTask(
+		ctx,
 		&ec2.CreateReplaceRootVolumeTaskInput{
 			InstanceId:               &r.InstanceID,
 			DeleteReplacedRootVolume: &deleteReplacedRootVolume,
 			ImageId:                  &r.AMIID,
 		})
+	if err != nil {
+		return rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId, err
+	}
+	taskState := rrvt.ReplaceRootVolumeTask.TaskState
+	for r.Wait && taskState != types.ReplaceRootVolumeTaskStateSucceeded {
+		drvt, err := client.DescribeReplaceRootVolumeTasks(ctx, &ec2.DescribeReplaceRootVolumeTasksInput{
+			ReplaceRootVolumeTaskIds: []string{*rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId},
+		})
+		if err != nil {
+			return rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId, nil
+		}
+		if len(drvt.ReplaceRootVolumeTasks) == 0 {
+			return rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId,
+				fmt.Errorf("something wrong happened checkding the replace root volume task with id %s",
+					*rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId)
+		}
+		taskState = drvt.ReplaceRootVolumeTasks[0].TaskState
+	}
+	return rrvt.ReplaceRootVolumeTask.ReplaceRootVolumeTaskId, nil
 }