fix: fix mac-pool workflow

this commit will change several deatils around the mac-pool workflow and the expectation on how the values for the machine should be treated

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

Makefile

@@ -2,8 +2,6 @@ VERSION ?= 0.8.0-dev
 CONTAINER_MANAGER ?= podman
 # Image URL to use all building/pushing image targets
 IMG ?= quay.io/redhat-developer/mapt:v${VERSION}
-# IMG ?= quay.io/rhqp/mapt:serverless-10-amd64
-
 TKN_IMG ?= quay.io/redhat-developer/mapt:v${VERSION}-tkn
 
 # Go and compilation related variables

cmd/mapt/cmd/aws/services/mac-pool.go

@@ -157,8 +157,6 @@ func getRequest() *cobra.Command {
 
 			if err := macpool.Request(
 				&maptContext.ContextArgs{
-					ProjectName:   viper.GetString(params.ProjectName),
-					BackedURL:     viper.GetString(params.BackedURL),
 					ResultsOutput: viper.GetString(params.ConnectionDetailsOutput),
 					Debug:         viper.IsSet(params.Debug),
 					DebugLevel:    viper.GetUint(params.DebugLevel),
@@ -196,12 +194,8 @@ func getRelease() *cobra.Command {
 
 			if err := macpool.Release(
 				&maptContext.ContextArgs{
-					ProjectName: viper.GetString(params.ProjectName),
-					BackedURL:   viper.GetString(params.BackedURL),
-					Serverless:  viper.IsSet(params.Serverless),
-					Debug:       viper.IsSet(params.Debug),
-					DebugLevel:  viper.GetUint(params.DebugLevel),
-					Tags:        viper.GetStringMapString(params.Tags),
+					Debug:      viper.IsSet(params.Debug),
+					DebugLevel: viper.GetUint(params.DebugLevel),
 				},
 				viper.GetString(awsParams.MACDHID)); err != nil {
 				logging.Error(err)

pkg/manager/context/context.go

@@ -119,6 +119,9 @@ func StackNameByProject(stackName string) string {
 }
 
 func addCommonTags() {
+	if mc.tags == nil {
+		mc.tags = make(map[string]string)
+	}
 	mc.tags[tagKeyOrigin] = origin
 	mc.tags[TagKeyProjectName] = mc.projectName
 }

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

@@ -17,20 +17,14 @@ import (
 	"github.com/redhat-developer/mapt/pkg/util/logging"
 )
 
-// request and release (same approach as mac standard, but request never create machine underneath, this is handled by pool-capacity-keeper)
-// TODO Important
-// release and request will not behave the same if the run as targer vs as selfhosted runner. In that case for release we do not
-// want it be added as selfhosted but only on request??
+// Create works as an orchestrator for create n machines based on offered capacity
+// if pool already exists just change the params for the HouseKeeper
+// also the HouseKeep will take care of regulate the capacity
 
+// Even if we want to destroy the pool we will set params to max size 0
 func Create(ctx *maptContext.ContextArgs, r *MacPoolRequestArgs) error {
 	// Create mapt Context
 	maptContext.Init(ctx)
-
-	// Initially create pool with number of machines matching available capacity
-	// this is the number of machines free to accept workloads
-	// if err := validateNoExistingPool(); err != nil {
-	// 	return err
-	// }
 	if err := r.addMachinesToPool(r.OfferedCapacity); err != nil {
 		return err
 	}
@@ -53,21 +47,25 @@ func HouseKeeper(ctx *maptContext.ContextArgs, r *MacPoolRequestArgs) error {
 		return err
 	}
 	// Pool under expected offered capacity
-	if p.currentOfferedCapacity() < p.offeredCapacity {
-		if p.currentPoolSize() < p.maxSize {
+	if p.currentOfferedCapacity() < r.OfferedCapacity {
+		if p.currentPoolSize() < r.MaxSize {
+			logging.Debug("house keeper will try to add machines as offered capacity is lower than expected")
 			return r.addCapacity(p)
 		}
 		// if number of machines in the pool + to max machines
 		// we do nothing
+		logging.Debug("house keeper will not do any action as pool size is currently at max size")
 		return nil
 	}
 	// Pool over expected offered capacity need to destroy machines
-	if p.currentOfferedCapacity() > p.offeredCapacity {
+	if p.currentOfferedCapacity() > r.OfferedCapacity {
 		if len(p.destroyableMachines) > 0 {
+			logging.Debug("house keeper will try to destroy machines as offered capacity is higher than expected")
 			// Need to check if any offered can be destroy
 			return r.destroyCapacity(p)
 		}
 	}
+	logging.Debug("house keeper will not do any action as offered capacity is met by the pool")
 	// Otherwise nonLockedMachines meet Capacity so we do nothing
 	return nil
 }
@@ -123,7 +121,7 @@ func (r *MacPoolRequestArgs) addMachinesToPool(n int) error {
 			return err
 		}
 		mr := r.fillMacRequest()
-		if err = mr.CreateMacMachine(dh); err != nil {
+		if err = mr.CreateAvailableMacMachine(dh); err != nil {
 			return err
 		}
 	}
@@ -203,13 +201,15 @@ func validateBackedURL() error {
 // This function will fill information about machines in the pool
 // depending on their state and age full fill the struct to easily
 // manage them
-func getPool(poolName, arch, osVersion string) (p *pool, err error) {
+func getPool(poolName, arch, osVersion string) (*pool, error) {
 	// Get machines in the pool
 	poolID := &macHost.PoolID{
 		PoolName:  poolName,
 		Arch:      arch,
 		OSVersion: osVersion,
 	}
+	var p pool
+	var err error
 	p.machines, err = macHost.GetPoolDedicatedHostsInformation(poolID)
 	if err != nil {
 		return nil, err
@@ -232,7 +232,7 @@ func getPool(poolName, arch, osVersion string) (p *pool, err error) {
 			return h.Host.AllocationTime.UTC().Before(macAgeDestroyRequeriemnt)
 		})
 	p.name = poolName
-	return
+	return &p, nil
 }
 
 // This is a boilerplate function to pick the best machine for

pkg/provider/aws/action/mac-pool/types.go

@@ -8,7 +8,7 @@ const (
 	houseKeepingCommand            = "aws mac-pool house-keep --name %s --arch %s --version %s --offered-capacity %d --max-size %d --serverless "
 	houseKeepingFixedLocationParam = "--fixed-location "
 	// https://docs.aws.amazon.com/eventbridge/latest/userguide/eb-scheduled-rule-pattern.html#eb-rate-expressions
-	houseKeepingInterval = "15 minutes"
+	houseKeepingInterval = "27 minutes"
 )
 
 type MacPoolRequestArgs struct {
@@ -62,3 +62,17 @@ type pool struct {
 
 func (p *pool) currentOfferedCapacity() int { return len(p.currentOfferedMachines) }
 func (p *pool) currentPoolSize() int        { return len(p.machines) }
+
+// func (p *pool) currentOfferedCapacity() int {
+// 	return util.If(
+// 		p.currentOfferedMachines == nil,
+// 		0,
+// 		len(p.currentOfferedMachines))
+// }
+
+// func (p *pool) currentPoolSize() int {
+// 	return util.If(
+// 		p.machines == nil,
+// 		0,
+// 		len(p.machines))
+// }

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

@@ -147,7 +147,8 @@ func create(r *MacRequestArgs, dh *mac.HostInformation) (err error) {
 	mr := r.fillMacRequest()
 	// Setup the topology and install the mac machine
 	if !r.Airgap {
-		return mr.CreateMacMachine(dh)
+
+		return mr.CreateAndLockMacMachine(dh)
 	}
 	return mr.CreateAirgapMacMachine(dh)
 }

pkg/provider/aws/modules/mac/machine/machine.go

@@ -92,12 +92,20 @@ func (r *Request) ReplaceUserAccess(h *mac.HostInformation) error {
 	return r.manageMacMachine(h)
 }
 
-// Release will set the lock as false
-func (r *Request) CreateMacMachine(h *mac.HostInformation) error {
+// This create the machine and set as locked....meaning that it will return a way
+// for it to be used (i.e mac action)
+func (r *Request) CreateAndLockMacMachine(h *mac.HostInformation) error {
 	r.lock = true
 	return r.manageMacMachine(h)
 }
 
+// This create the machine and set it as ready to be used (i.e when mac-pool action)
+// in this case machines are added to the pool as ready to be used by request
+func (r *Request) CreateAvailableMacMachine(h *mac.HostInformation) error {
+	r.lock = false
+	return r.manageMacMachine(h)
+}
+
 // this creates the stack for the mac machine
 func (r *Request) manageMacMachine(h *mac.HostInformation) error {
 	return r.manageMacMachineTargets(h, nil)
@@ -132,12 +140,12 @@ func (r *Request) manageMacMachineTargets(h *mac.HostInformation, targetURNs []s
 // this creates the stack for the mac machine
 func (r *Request) CreateAirgapMacMachine(h *mac.HostInformation) error {
 	r.airgapPhaseConnectivity = network.ON
-	err := r.CreateMacMachine(h)
+	err := r.CreateAndLockMacMachine(h)
 	if err != nil {
 		return nil
 	}
 	r.airgapPhaseConnectivity = network.OFF
-	return r.CreateMacMachine(h)
+	return r.CreateAndLockMacMachine(h)
 }
 
 // Main function to deploy all requried resources to azure
@@ -148,7 +156,10 @@ func (r *Request) deployerMachine(ctx *pulumi.Context) error {
 	// Lookup AMI
 	aN := fmt.Sprintf(amiRegex, r.Version)
 	bdt := blockDeviceType
-	arch := awsArchIDbyArch[r.Architecture]
+	arch := util.If(
+		isAWSArchID(r.Architecture),
+		r.Architecture,
+		awsArchIDbyArch[r.Architecture])
 	ami, err := data.GetAMI(
 		data.ImageRequest{
 			Name:            &aN,

pkg/provider/aws/modules/mac/machine/types.go

@@ -62,3 +62,5 @@ var awsArchIDbyArch = map[string]string{
 	"x86": "x86_64_mac",
 	"m1":  "arm64_mac",
 	"m2":  "arm64_mac"}
+
+func isAWSArchID(a string) bool { return a == "x86_64_mac" || a == "arm64_mac" }

pkg/provider/aws/modules/mac/util/util.go

@@ -44,8 +44,8 @@ func PickHost(prefix string, his []*mac.HostInformation) (*mac.HostInformation,
 
 func IsMachineLocked(h *mac.HostInformation) (bool, error) {
 	s, err := manager.CheckStack(manager.Stack{
-		StackName:   maptContext.StackNameByProject(StackMacMachine),
-		ProjectName: maptContext.ProjectName(),
+		StackName:   fmt.Sprintf("%s-%s", StackMacMachine, *h.ProjectName),
+		ProjectName: *h.ProjectName,
 		BackedURL:   *h.BackedURL,
 		ProviderCredentials: aws.GetClouProviderCredentials(
 			map[string]string{

pkg/provider/util/output/output.go

@@ -9,9 +9,11 @@ import (
 )
 
 func Write(stackResult auto.UpResult, destinationFolder string, results map[string]string) (err error) {
-	for k, v := range results {
-		if err = writeOutput(stackResult, k, destinationFolder, v); err != nil {
-			return err
+	if len(destinationFolder) > 0 {
+		for k, v := range results {
+			if err = writeOutput(stackResult, k, destinationFolder, v); err != nil {
+				return err
+			}
 		}
 	}
 	return