Split application state from config struct

[tgross]

@justenwalker proposed the following in #123 and I want to make sure we come back to it as a future refactoring.

---

I can see how this would be painful. I still think we are complicating the idea of external configuration (ie from a file) with our application state (internal configuration) - I believe the fact that the structures are very similar is incidental.

Perhaps this isn't the time to undertake such a refactor, but does the idea of separating these concepts from each other seem like a good goal? Maybe we can create another issue for that.

Here are some of my thoughts:

instead of

// Config is the top-level Containerbuddy Configuration
type Config struct {
    Consul       string                    `json:"consul,omitempty"`
    Etcd         json.RawMessage           `json:"etcd,omitempty"`
    LogConfig    *LogConfig                `json:"logging,omitempty"`
    OnStart      json.RawMessage           `json:"onStart,omitempty"`
    PreStart     json.RawMessage           `json:"preStart,omitempty"`
    PreStop      json.RawMessage           `json:"preStop,omitempty"`
    PostStop     json.RawMessage           `json:"postStop,omitempty"`
    StopTimeout  int                       `json:"stopTimeout"`
    Services     []*services.ServiceConfig `json:"services"`
    Backends     []*backends.BackendConfig `json:"backends"`
    PreStartCmd  *exec.Cmd
    PreStopCmd   *exec.Cmd
    PostStopCmd  *exec.Cmd
    Command      *exec.Cmd
    QuitChannels []chan bool
    ConfigFlag   string
}

something like

type ApplicationState struct {
    PreStartCmd  *exec.Cmd
    PreStopCmd   *exec.Cmd
    PostStopCmd  *exec.Cmd
    Command      *exec.Cmd
    QuitChannels []chan bool
    Backends []backends.Backend
    Service  []services.Service
    ConfigFlag   string
}

Inside loadConfig some logic happens to parse the map[string]interface{} into several config structs
which will be passed as arguments to the NewX functions in packages which are responsible for converting them to the appropriate interfaces (github.com/mitchellh/mapstructure might help). Those config structs live in the package itself.

For instance, after loading up the var backendConfig backends.BackendConfig we get backend := backends.NewBackend(backendConfig) which is put into the ApplicationState struct

More concrete steps:

• loadConfig(config map[string]interface{}) will be a pure function which takes a raw config and produces a ApplicationState
• We store it as the currentState
• Then it is Run()
• Later, the application might want to reloadConfig() which:
  • config := loadConfigMap()
  • newState := loadConfig(config)
  • currentState.Stop()
  • currentState := newState
  • currentState.Run()

In retrospect I realize I oversimplified, this does not account for the fact that we don't re-run the shimmed application and preStart commands but it is part of the ApplicationState.

[tgross]

Ongoing work here #138

[tgross]

Released in https://github.com/joyent/containerpilot/releases/tag/2.1.1

[tgross]

@justenwalker at this point I think we're in pretty good shape for future work, or do you have other refactoring work in mind?

[justenwalker]

@tgross I'm pretty much done here. I left out the linting work - so maybe I'll create a separate issue for that.

[tgross]

@tgross I'm pretty much done here. I left out the linting work - so maybe I'll create a separate issue for that.

Sounds good. There are a smattering of low-hanging fruit issues that I want to encourage community members to work on and now that things have settled a bit we may be able to get those contributions in easier.