diff --git a/internal/appconfig/machines.go b/internal/appconfig/machines.go index ecb851f105..83508eb4d0 100644 --- a/internal/appconfig/machines.go +++ b/internal/appconfig/machines.go @@ -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 } @@ -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 @@ -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. @@ -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 != "" { diff --git a/internal/appconfig/machines_test.go b/internal/appconfig/machines_test.go index 3642fe5156..a8ee456592 100644 --- a/internal/appconfig/machines_test.go +++ b/internal/appconfig/machines_test.go @@ -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)