feat: refactor mac request / release flow

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
 }