launch: Use appconfig.Compute for Plan guest information. (#3219)

Moves us towards supporting multiple compute sections in a launch plan.
This means, among other things, that we'll support round-tripping a full guest config, with fields such as GPU support or host dedication IDs.

Also fixes a couple bugs and outstanding weirdnesses with fly.toml import launches.

Squashlog:

* `launch`: use Compute instead of MachineGuest

This moves us closer to how fly.tomls get serialized

Unfortunately, we'll have to do some weird plan patching in a later commit to make this compatible with the plan shape the UI currently expects

* `deploy`: remove optionalGuest param, now that it's unused

* `launch`: polyfill `Plan.Compute` now with existing guest fields

This means we can write code *now* that expects the Launch Plan to have full compute fields, but also means that when the UI gets support for returning them, then we'll automatically start supporting that.

* `Flatten()`: move compute determination to own func

This allows launch to piggyback off the same logic. Initially I wanted to just copy-paste this into Launch and be done with it, but I had to copy `matchesGroups` too which just turns it into a big mess of unmaintainable code.

Not that this is *much* better, but it's a little better (I hope)

* `launch`: replace compute determination with `ComputeForGroup()`

* `fly_launch_test`: memory_mb: 1024 -> memory: "1gb"

* `launch`: maintain guest options that can't be set from web UI

internal/appconfig/processgroups.go

@@ -58,6 +58,36 @@ func (c *Config) DefaultProcessName() string {
 	return c.ProcessNames()[0]
 }
 
+// Checks if `toCheck` is a process group name that should target `target`
+//
+// Returns true if target == toCheck or if target is the default process name and toCheck is empty
+func (c *Config) flattenGroupMatches(target, toCheck string) bool {
+	if target == "" {
+		target = c.DefaultProcessName()
+	}
+	switch {
+	case toCheck == target:
+		return true
+	case toCheck == "" && target == c.DefaultProcessName():
+		return true
+	default:
+		return false
+	}
+}
+
+// Checks if any of the process group names in `toCheck` should target the group `target`
+//
+// Returns true if any of the groups in toCheck would return true for `flattenGroupMatches`,
+// or if toCheck is empty, returns true if target is the default process name
+func (c *Config) flattenGroupsMatch(target string, toCheck []string) bool {
+	if len(toCheck) == 0 {
+		return c.flattenGroupMatches(target, "")
+	}
+	return lo.SomeBy(toCheck, func(x string) bool {
+		return c.flattenGroupMatches(target, x)
+	})
+}
+
 // Flatten generates a machine config specific to a process_group.
 //
 // Only services, mounts, checks, metrics & files specific to the provided progress group will be in the returned config.
@@ -70,21 +100,8 @@ func (c *Config) Flatten(groupName string) (*Config, error) {
 	if groupName == "" {
 		groupName = defaultGroupName
 	}
-	matchesGroup := func(x string) bool {
-		switch {
-		case x == groupName:
-			return true
-		case x == "" && groupName == defaultGroupName:
-			return true
-		default:
-			return false
-		}
-	}
 	matchesGroups := func(xs []string) bool {
-		if len(xs) == 0 {
-			return matchesGroup("")
-		}
-		return lo.SomeBy(xs, matchesGroup)
+		return c.flattenGroupsMatch(groupName, xs)
 	}
 
 	dst := helpers.Clone(c)
@@ -93,7 +110,7 @@ func (c *Config) Flatten(groupName string) (*Config, error) {
 
 	// [processes]
 	dst.Processes = lo.PickBy(dst.Processes, func(k, v string) bool {
-		return matchesGroup(k)
+		return dst.flattenGroupMatches(groupName, k)
 	})
 
 	// [checks]
@@ -146,21 +163,7 @@ func (c *Config) Flatten(groupName string) (*Config, error) {
 	}
 
 	// [[vm]]
-	// Find the most specific VM compute requirements for this process group
-	// In reality there are only four valid cases:
-	//   1. No [[vm]] section
-	//   2. One [[vm]] section with `processes = [groupname]`
-	//   3. Previous case plus global [[compute]] without processes
-	//   4. Only a [[vm]] section without processes set which applies to all groups
-	compute := lo.MaxBy(
-		// grab only the compute that matches or have no processes set
-		lo.Filter(dst.Compute, func(x *Compute, _ int) bool {
-			return len(x.Processes) == 0 || matchesGroups(x.Processes)
-		}),
-		// Next find the most specific
-		func(item *Compute, _ *Compute) bool {
-			return slices.Contains(item.Processes, groupName)
-		})
+	compute := dst.ComputeForGroup(groupName)
 
 	dst.Compute = nil
 	if compute != nil {
@@ -175,6 +178,30 @@ func (c *Config) Flatten(groupName string) (*Config, error) {
 	return dst, nil
 }
 
+// ComputeForGroup finds the most specific VM compute requirements for this process group
+// In reality there are only four valid cases:
+//  1. No [[vm]] section
+//  2. One [[vm]] section with `processes = [groupName]`
+//  3. Previous case plus global [[compute]] without processes
+//  4. Only a [[vm]] section without processes set which applies to all groups
+func (c *Config) ComputeForGroup(groupName string) *Compute {
+
+	if groupName == "" {
+		groupName = c.DefaultProcessName()
+	}
+
+	compute := lo.MaxBy(
+		// grab only the compute that matches or have no processes set
+		lo.Filter(c.Compute, func(x *Compute, _ int) bool {
+			return len(x.Processes) == 0 || c.flattenGroupsMatch(groupName, x.Processes)
+		}),
+		// Next find the most specific
+		func(item *Compute, _ *Compute) bool {
+			return slices.Contains(item.Processes, groupName)
+		})
+	return compute
+}
+
 func (c *Config) InitCmd(groupName string) ([]string, error) {
 	if groupName == "" {
 		groupName = c.DefaultProcessName()

internal/command/deploy/deploy.go

@@ -224,10 +224,10 @@ func run(ctx context.Context) error {
 	span.SetAttributes(attribute.StringSlice("gpu.kinds", gpuKinds))
 	span.SetAttributes(attribute.StringSlice("cpu.kinds", cpuKinds))
 
-	return DeployWithConfig(ctx, appConfig, flag.GetYes(ctx), nil)
+	return DeployWithConfig(ctx, appConfig, flag.GetYes(ctx))
 }
 
-func DeployWithConfig(ctx context.Context, appConfig *appconfig.Config, forceYes bool, optionalGuest *api.MachineGuest) (err error) {
+func DeployWithConfig(ctx context.Context, appConfig *appconfig.Config, forceYes bool) (err error) {
 	io := iostreams.FromContext(ctx)
 	appName := appconfig.NameFromContext(ctx)
 	apiClient := client.FromContext(ctx).API()
@@ -256,7 +256,7 @@ func DeployWithConfig(ctx context.Context, appConfig *appconfig.Config, forceYes
 	}
 
 	fmt.Fprintf(io.Out, "\nWatch your deployment at https://fly.io/apps/%s/monitoring\n\n", appName)
-	if err := deployToMachines(ctx, appConfig, appCompact, img, optionalGuest); err != nil {
+	if err := deployToMachines(ctx, appConfig, appCompact, img); err != nil {
 		return err
 	}
 
@@ -300,7 +300,6 @@ func deployToMachines(
 	appConfig *appconfig.Config,
 	appCompact *api.AppCompact,
 	img *imgsrc.DeploymentImage,
-	guest *api.MachineGuest,
 ) (err error) {
 	// It's important to push appConfig into context because MachineDeployment will fetch it from there
 	ctx = appconfig.WithConfig(ctx, appConfig)
@@ -330,11 +329,9 @@ func deployToMachines(
 		return err
 	}
 
-	if guest == nil {
-		guest, err = flag.GetMachineGuest(ctx, nil)
-		if err != nil {
-			return err
-		}
+	guest, err := flag.GetMachineGuest(ctx, nil)
+	if err != nil {
+		return err
 	}
 
 	excludeRegions := make(map[string]interface{})

internal/command/launch/deploy.go

@@ -83,7 +83,7 @@ func (state *launchState) firstDeploy(ctx context.Context) error {
 				return err
 			}
 		}
-		return deploy.DeployWithConfig(ctx, state.appConfig, flag.GetBool(ctx, "now"), state.Plan.Guest())
+		return deploy.DeployWithConfig(ctx, state.appConfig, flag.GetBool(ctx, "now"))
 	}
 
 	// Alternative deploy documentation if our standard deploy method is not correct

internal/command/launch/launch.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/samber/lo"
 	"github.com/superfly/flyctl/api"
 	"github.com/superfly/flyctl/client"
 	"github.com/superfly/flyctl/flaps"
@@ -22,6 +21,10 @@ func (state *launchState) Launch(ctx context.Context) error {
 	// I'm assuming *not* for now, because it's confusing UX and this
 	// is the perfect time to remove it.
 
+	if err := state.updateComputeFromDeprecatedGuestFields(ctx); err != nil {
+		return err
+	}
+
 	state.updateConfig(ctx)
 
 	org, err := state.Org(ctx)
@@ -78,6 +81,32 @@ func (state *launchState) Launch(ctx context.Context) error {
 	return nil
 }
 
+// Apply the freestanding Guest fields to the appConfig's Compute field
+// This is temporary, but allows us to start using Compute-based plans in flyctl *now* while the UI catches up in time.
+func (state *launchState) updateComputeFromDeprecatedGuestFields(ctx context.Context) error {
+
+	if len(state.Plan.Compute) != 0 {
+		// If the UI returns a compute field, then we don't need to do any forward-compat patching.
+		return nil
+	}
+	// Fallback for versions of the UI that don't support a Compute field in the Plan.
+
+	defer func() {
+		// Set the plan's compute field to the calculated compute field.
+		// This makes sure that code expecting a compute definition in the plan is able to find it
+		// (and that it's up-to-date)
+		state.Plan.Compute = state.appConfig.Compute
+	}()
+
+	if compute := state.appConfig.ComputeForGroup(state.appConfig.DefaultProcessName()); compute != nil {
+		applyGuestToCompute(compute, state.Plan.Guest())
+	} else {
+		state.appConfig.Compute = append(state.appConfig.Compute, guestToCompute(state.Plan.Guest()))
+	}
+
+	return nil
+}
+
 // updateConfig populates the appConfig with the plan's values
 func (state *launchState) updateConfig(ctx context.Context) {
 	state.appConfig.AppName = state.Plan.AppName
@@ -99,12 +128,7 @@ func (state *launchState) updateConfig(ctx context.Context) {
 	} else {
 		state.appConfig.HTTPService = nil
 	}
-	state.appConfig.Compute = []*appconfig.Compute{
-		{
-			MachineGuest: state.Plan.Guest(),
-			Processes:    lo.Keys(state.appConfig.Processes),
-		},
-	}
+	state.appConfig.Compute = state.Plan.Compute
 }
 
 // createApp creates the fly.io app for the plan

internal/command/launch/plan/plan.go

@@ -2,6 +2,7 @@ package plan
 
 import (
 	"github.com/superfly/flyctl/api"
+	"github.com/superfly/flyctl/internal/appconfig"
 	"github.com/superfly/flyctl/internal/version"
 )
 
@@ -12,10 +13,18 @@ type LaunchPlan struct {
 	RegionCode       string `json:"region"`
 	HighAvailability bool   `json:"ha"`
 
-	CPUKind  string `json:"vm_cpukind,omitempty"`
-	CPUs     int    `json:"vm_cpus,omitempty"`
-	MemoryMB int    `json:"vm_memory,omitempty"`
-	VmSize   string `json:"vm_size,omitempty"`
+	// Deprecated: The UI currently returns this instead of Compute, but new development should use Compute.
+	CPUKind string `json:"vm_cpukind,omitempty"`
+	// Deprecated: The UI currently returns this instead of Compute, but new development should use Compute.
+	CPUs int `json:"vm_cpus,omitempty"`
+	// Deprecated: The UI currently returns this instead of Compute, but new development should use Compute.
+	MemoryMB int `json:"vm_memory,omitempty"`
+	// Deprecated: The UI currently returns this instead of Compute, but new development should use Compute.
+	VmSize string `json:"vm_size,omitempty"`
+
+	// In the future, we'll use this over CPUKind, CPUs, MemoryMB, and VmSize.
+	// As of writing this, however, the UI does not return this field.
+	Compute []*appconfig.Compute `json:"compute"`
 
 	HttpServicePort int `json:"http_service_port,omitempty"`
 
@@ -27,6 +36,8 @@ type LaunchPlan struct {
 	FlyctlVersion version.Version `json:"flyctl_version"`
 }
 
+// Guest returns the guest described by the *raw* guest fields in a Plan.
+// When the UI starts returning Compute, this will be deprecated.
 func (p *LaunchPlan) Guest() *api.MachineGuest {
 	// TODO(Allison): Determine whether we should use VmSize or CPUKind/CPUs
 	guest := api.MachineGuest{
@@ -39,10 +50,3 @@ func (p *LaunchPlan) Guest() *api.MachineGuest {
 	guest.MemoryMB = p.MemoryMB
 	return &guest
 }
-
-func (p *LaunchPlan) SetGuestFields(guest *api.MachineGuest) {
-	p.CPUs = guest.CPUs
-	p.CPUKind = guest.CPUKind
-	p.MemoryMB = guest.MemoryMB
-	p.VmSize = guest.ToSize()
-}