Skip to content

Commit

Permalink
Turn getProcessGroup into a method, make everything use this fn
Browse files Browse the repository at this point in the history
Now this function is public, and a method of `api.Machine`

There were a couple of places that still expected either "process_group" or "fly_process_group", so this normalizes everything (but should retain backwards compatibility!)

This should be tested just to be sure that nothing depends on a specific key in the backend, though :)
  • Loading branch information
alichay committed Mar 15, 2023
1 parent b2964d6 commit 0e1d0c9
Show file tree
Hide file tree
Showing 8 changed files with 93 additions and 93 deletions.
31 changes: 30 additions & 1 deletion api/machine_types.go
Expand Up @@ -81,8 +81,16 @@ func (m *Machine) IsActive() bool {
return m.State != MachineStateDestroyed && m.State != MachineStateDestroying
}

func (m *Machine) ProcessGroup() string {

if m.Config == nil {
return ""
}
return m.Config.ProcessGroup()
}

func (m *Machine) HasProcessGroup(desired string) bool {
return m.Config != nil && m.Config.Metadata[MachineConfigMetadataKeyFlyProcessGroup] == desired
return m.Config != nil && m.ProcessGroup() == desired
}

func (m *Machine) ImageVersion() string {
Expand Down Expand Up @@ -391,6 +399,27 @@ type MachineConfig struct {
DisableMachineAutostart bool `json:"disable_machine_autostart,omitempty"`
}

func (c *MachineConfig) ProcessGroup() string {
// backwards compatible process_group getter.
// from poking around, "fly_process_group" used to be called "process_group"
// and since it's a metadata value, it's like a screenshot.
// so we have 3 scenarios
// - machines with only 'process_group'
// - machines with both 'process_group' and 'fly_process_group'
// - machines with only 'fly_process_group'

if c.Metadata == nil {
return ""
}

flyProcessGroup := c.Metadata[MachineConfigMetadataKeyFlyProcessGroup]
if flyProcessGroup != "" {
return flyProcessGroup
}

return c.Metadata["process_group"]
}

type Static struct {
GuestPath string `toml:"guest_path" json:"guest_path" validate:"required"`
UrlPrefix string `toml:"url_prefix" json:"url_prefix" validate:"required"`
Expand Down
54 changes: 54 additions & 0 deletions api/machine_types_test.go
Expand Up @@ -66,3 +66,57 @@ func TestIsReleaseCommandMachine(t *testing.T) {
}

}

func TestGetProcessGroup(t *testing.T) {

type testcase struct {
name string
machine *Machine
expected string
}

cases := []testcase{
{
name: "machine with only 'process_group'",
expected: "web",
machine: &Machine{

Config: &MachineConfig{
Metadata: map[string]string{
"process_group": "web",
},
},
},
},
{
name: "machine with both 'process_group' & 'fly_process_group'",
expected: "app",
machine: &Machine{

Config: &MachineConfig{
Metadata: map[string]string{
"process_group": "web",
"fly_process_group": "app",
},
},
},
},
{
name: "machine with only 'fly_process_group'",
expected: "web",
machine: &Machine{

Config: &MachineConfig{
Metadata: map[string]string{
"fly_process_group": "web",
},
},
},
},
}

for _, tc := range cases {
require.Equal(t, tc.expected, tc.machine.ProcessGroup(), tc.name)
}

}
9 changes: 3 additions & 6 deletions internal/command/deploy/machines.go
Expand Up @@ -229,7 +229,7 @@ func (md *machineDeployment) resolveProcessGroupMachineChanges() ProcessGroupMac

for _, leasableMachine := range md.machineSet.GetMachines() {
mach := leasableMachine.Machine()
machGroup := mach.Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup]
machGroup := mach.ProcessGroup()
groupMatch := ""
for name := range md.processConfigs {
if machGroup == name {
Expand Down Expand Up @@ -603,7 +603,7 @@ func (md *machineDeployment) validateProcessesConfig() error {
func (md *machineDeployment) validateVolumeConfig() error {
for _, m := range md.machineSet.GetMachines() {
mid := m.Machine().ID
if m.Machine().Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup] == api.MachineProcessGroupFlyAppReleaseCommand {
if m.Machine().ProcessGroup() == api.MachineProcessGroupFlyAppReleaseCommand {
continue
}
mountsConfig := m.Machine().Config.Mounts
Expand Down Expand Up @@ -738,10 +738,7 @@ func (md *machineDeployment) resolveUpdatedMachineConfig(origMachineRaw *api.Mac
launchInput.Config.Mounts[0].Path = md.volumeDestination
}

processGroup := launchInput.Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup]
if processGroup == "" {
processGroup = api.MachineProcessGroupApp
}
processGroup := launchInput.Config.ProcessGroup()
if processConfig, ok := md.processConfigs[processGroup]; ok {
launchInput.Config.Services = processConfig.Services
launchInput.Config.Checks = processConfig.Checks
Expand Down
2 changes: 1 addition & 1 deletion internal/command/machine/list.go
Expand Up @@ -118,7 +118,7 @@ func runMachineList(ctx context.Context) (err error) {

}

if processGroup, ok := machine.Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup]; ok {
if processGroup := machine.ProcessGroup(); processGroup != "" {
machineProcessGroup = processGroup

}
Expand Down
24 changes: 3 additions & 21 deletions internal/command/machine/status.go
Expand Up @@ -9,7 +9,8 @@ import (

"github.com/alecthomas/chroma/quick"
"github.com/spf13/cobra"
"github.com/superfly/flyctl/api"
"github.com/superfly/flyctl/flaps"
"github.com/superfly/flyctl/internal/appconfig"
"github.com/superfly/flyctl/internal/command"
"github.com/superfly/flyctl/internal/flag"
"github.com/superfly/flyctl/internal/render"
Expand Down Expand Up @@ -45,25 +46,6 @@ func newStatus() *cobra.Command {
return cmd
}

func getProcessGroup(m *api.Machine) string {
// backwards compatible process_group getter.
// from poking around, "fly_process_group" used to be called "process_group"
// and since it's a metadata value, it's like a screenshot.
// so we have 3 scenarios
// - machines with only 'process_group'
// - machines with both 'process_group' and 'fly_process_group'
// - machines with only 'fly_process_group'

processGroup := m.Config.Metadata["process_group"]
flyProcessGroup := m.Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup]

if flyProcessGroup != "" {
return flyProcessGroup
}

return processGroup
}

func runMachineStatus(ctx context.Context) (err error) {
io := iostreams.FromContext(ctx)

Expand All @@ -86,7 +68,7 @@ func runMachineStatus(ctx context.Context) (err error) {
machine.Name,
machine.PrivateIP,
machine.Region,
getProcessGroup(machine),
machine.ProcessGroup(),
fmt.Sprint(machine.Config.Guest.CPUKind),
fmt.Sprint(machine.Config.Guest.CPUs),
fmt.Sprint(machine.Config.Guest.MemoryMB),
Expand Down
62 changes: 0 additions & 62 deletions internal/command/machine/status_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion internal/command/status/machines.go
Expand Up @@ -25,7 +25,7 @@ func getFromMetadata(m *api.Machine, key string) string {
}

func getProcessgroup(m *api.Machine) string {
name := getFromMetadata(m, api.MachineConfigMetadataKeyFlyProcessGroup)
name := m.ProcessGroup()

if name != "" {
return name
Expand Down
2 changes: 1 addition & 1 deletion internal/machine/leasable_machine.go
Expand Up @@ -86,7 +86,7 @@ func (lm *leasableMachine) FormattedMachineId() string {
if lm.Machine().Config.Metadata == nil {
return res
}
procGroup := lm.Machine().Config.Metadata[api.MachineConfigMetadataKeyFlyProcessGroup]
procGroup := lm.Machine().ProcessGroup()
if procGroup == "" || lm.Machine().IsFlyAppsReleaseCommand() {
return res
}
Expand Down

0 comments on commit 0e1d0c9

Please sign in to comment.