Break core functionality out to library?

[tgross]

After #76 landed we ended up with a main.go and separate containerbuddy package. This opens up the suggestion of making the core functionality of Containerbuddy a library that other applications could import. Then the Containerbuddy binary that we ship would be the main.go and probably the configuration loaders? I've experimented with this a little bit back before we added a lot of our features but it seems like it's feasible and useful.

This would definitely be a post-1.0 release item and needs some discussion about where the "seams" are before we start putting up a bunch of PRs. I do want to avoid making the project unapproachably over-factored.

[justenwalker]

👍 I'd recommend that a config package be factored out to help facilitate other tools programmatically creating a Containerbuddy config.

[justenwalker]

Also it sort of makes sense to break the backends up into their own package and use init()
to register them

[tgross]

Agreed on those points.

Also it sort of makes sense to break the backends up into their own package and use init()
to register them

Something that came up in the metrics discussion was the notion of conditionally compiling backends. We could have the release build include all the backends for simplicity but provide make flags so that folks who only want to support Consul (for example) in their environment can do that.

[tgross]

Starting to work thru #27 I'm thinking that the way to do this would be:

main.go

package core
        getPollables(NewConfig)
        startPolling
        Pollable
        func (*Pollable) PollTime()
        func (*Pollable) PollFunc

package config
        Config
        NewConfig(parse)

package discovery
        DiscoveryService
        NewDiscoveryService(Config.Discovery)
        Consul impl DiscoveryService
        Etcd impl DiscoveryService

package services
        Service impl Pollable
        NewService(Config.Services)

package backends
        Backend impl Pollable
        NewBackend(Config.Backends)

package metrics
        Metrics impl Pollable
        NewMetrics()

package tasks
        Task impl Pollable
        NewTask()

I think I've worked it out that if Config imports Discovery, Service, Backends, etc. to call NewDiscovery, NewService, etc., and then Core only imports Config but calls Config.Services.DoStuff then we can avoid circular imports.

To incrementally refactor:

• We can make pollingFunc a method of Pollable.
• Make checkHealth a Pollable implementation method of ServiceConfig and make checkForChanges a Pollable implementation method of BackendConfig
• Add metrics and periodic tasks to work out the kinks in that implementation
• Split out everything else

(did a minor edit here: Pollable doesn't need to be in its own package because of the way go duck-types, so I rolled that back into core)

[tgross]

I'm doing a little proof of concept of how we'd break some of the other packages out and I'm finding that having the discovery service methods take ServiceConfig and BackendConfig is another source of circular references. Having those take the parameters they need rather than the whole Config fixes it. (This includes having ServiceConfig embed a ServiceDefinition struct for sanity as well.)

[justenwalker]

@tgross That sounds good.

Right now we have configs with non-serializable fields which are populated as part of initialization - this smells a bit off to me. One thing I'm also thinking about is extracting the config object from the instance object.

in other words: ServiceConfig is just a serializer and it only used for configuration. You then construct a Service (Interface?) with NewService(serviceConfig)

[tgross]

in other words: ServiceConfig is just a serializer, you then connstruct a Service (Interface?) with NewService(serviceConfig)

Yeah, that's more or less along the lines of where I'm heading with it. I'll have a PR up within the next hour or so showing what I'd like to propose.

[tgross]

Done and included in the 2.0 release: https://github.com/joyent/containerpilot/releases/tag/2.0.0.
Further explorations in #128