Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 30 additions & 13 deletions internal/appconfig/machines.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (c *Config) ToReleaseMachineConfig() (*fly.MachineConfig, error) {

// Guest
if v := c.Deploy.ReleaseCommandCompute; v != nil {
guest, err := c.computeToGuest(v)
guest, err := c.computeToGuest(v, mConfig.Guest)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -378,7 +378,11 @@ func (c *Config) updateMachineConfig(src *fly.MachineConfig) (*fly.MachineConfig
fly.MergeFiles(mConfig, c.MergedFiles)

// Guest
if guest, err := c.toMachineGuest(); err != nil {
//
// Pass the existing machine guest (carried over from src via the clone above)
// so that a partial [[compute]] section preserves values previously set by
// `fly scale`/`--vm-*` instead of resetting them to the preset defaults.
if guest, err := c.toMachineGuest(mConfig.Guest); err != nil {
return nil, err
} else if guest != nil {
// Only override machine's Guest if app config knows what to set
Expand Down Expand Up @@ -429,7 +433,7 @@ func (c *Config) tomachineSetStopConfig(mConfig *fly.MachineConfig) error {
return nil
}

func (c *Config) toMachineGuest() (*fly.MachineGuest, error) {
func (c *Config) toMachineGuest(existing *fly.MachineGuest) (*fly.MachineGuest, error) {
// XXX: Don't be extra smart here, keep it backwards compatible with apps that don't have a [[compute]] section.
// Think about apps that counts on `fly deploy` to respect whatever was set by `fly scale` or the --vm-* family flags.
// It is important to return a `nil` guest when fly.toml doesn't contain a [[compute]] section for the process group.
Expand All @@ -440,21 +444,34 @@ func (c *Config) toMachineGuest() (*fly.MachineGuest, error) {
}

// At most one compute after group flattening
return c.computeToGuest(c.Compute[0])
return c.computeToGuest(c.Compute[0], existing)
}

func (c *Config) computeToGuest(compute *Compute) (*fly.MachineGuest, error) {
size := fly.DefaultVMSize
func (c *Config) computeToGuest(compute *Compute, existing *fly.MachineGuest) (*fly.MachineGuest, error) {
// Pick the guest to build on top of:
// - an explicit `size` -> seed that preset
// - a GPU compute -> seed the default GPU preset
// - an existing machine guest -> preserve it (e.g. values set by `fly scale`)
// and only override the fields the user explicitly listed in the [[compute]] section
// - otherwise -> seed the default preset
// Preserving the existing guest for a partial [[compute]] keeps `fly deploy`
// from resetting machines back to shared-cpu-1x/256MB. See issue #4544.
guest := &fly.MachineGuest{}
switch {
case compute.Size != "":
size = compute.Size
if err := guest.SetSize(compute.Size); err != nil {
return nil, err
}
case compute.MachineGuest != nil && compute.GPUKind != "":
size = fly.DefaultGPUVMSize
}

guest := &fly.MachineGuest{}
if err := guest.SetSize(size); err != nil {
return nil, err
if err := guest.SetSize(fly.DefaultGPUVMSize); err != nil {
return nil, err
}
case existing != nil:
guest = helpers.Clone(existing)
default:
if err := guest.SetSize(fly.DefaultVMSize); err != nil {
return nil, err
}
}

if c.HostDedicationID != "" {
Expand Down
45 changes: 45 additions & 0 deletions internal/appconfig/machines_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,51 @@ func TestToMachineConfig_compute_none(t *testing.T) {
}
}

// Regression test for https://github.com/superfly/flyctl/issues/4544
//
// A partial [[vm]] section (e.g. only cpu_kind) must not reset a machine that
// was previously scaled up via `fly scale vm/--memory` back to the
// shared-cpu-1x / 256MB defaults. `fly deploy` passes the live (scaled) machine
// config as src, and any field the user did not explicitly set in fly.toml must
// be preserved from it.
func TestToMachineConfig_partialComputePreservesScale(t *testing.T) {
// fly.toml with a partial [[vm]] block: only cpu_kind is set, no size and
// no explicit cpus/memory.
cfg, err := unmarshalTOML([]byte(`
app = "pfval-4544"
primary_region = "iad"

[build]
image = "nginx"

[[vm]]
cpu_kind = "shared"
`))
require.NoError(t, err)

// The machine as it currently exists on the platform, after the user ran
// fly scale vm shared-cpu-4x --memory 1024
src := &fly.MachineConfig{
Image: "nginx",
Guest: &fly.MachineGuest{
CPUKind: "shared",
CPUs: 4,
MemoryMB: 1024,
},
}

got, err := cfg.ToMachineConfig("", src)
require.NoError(t, err)

// A partial [[vm]] should preserve the scaled CPUs/Memory and only change
// the field the user actually specified.
assert.Equal(t, &fly.MachineGuest{
CPUKind: "shared",
CPUs: 4,
MemoryMB: 1024,
}, got.Guest)
}

func TestToMachineConfig_hostdedicationid(t *testing.T) {
cfg, err := LoadConfig("./testdata/tomachine-hostdedicationid.toml")
require.NoError(t, err)
Expand Down
Loading