Iftrit v2: Boilerplate cleanup

[tedsuo]

trafficstars

Ifrit currently does not have a version number, but let's call the current head of master v1.0.0 for the purpose of this PR. There are a set of changes – which I never got around to making while at Pivotal – which I feel could clean up the boilerplate code in main() functions that use ifrit. This code is usuallly copy/pasted around, but it may improve on-boarding and comprehension to clean it up.

• port grouper and sigmon packages into the main ifrit package. the additional package names create confusion when reading code, and they are almost always used together.
• create a ifrit.Main method which wraps up the most common setup for ifrit into a single function.
• allow nil members.
• create an ifrit.EventListener interface (at least for use in ifrit.Main), which allows logging boilerplate to be shared.
• write proper intro docs ;)
• possibly there are other cleanups which could be included in the same push.

Example of the above changes being applied: https://gist.github.com/tedsuo/1e99de0d465330526ff2

Personally, I find the end result to be much more readable.

By porting the new functionality to the main package, ifrit could remain backwards compatible simply by leaving the current packages alone.

@nimakaviani @jvshahid @emalm since y’all make the heaviest use of ifrit, let me know if this of interest.

[jvshahid]

I like the change, specially allowing nil values and wrapping the boilerplate, but have a few minor comments:

1. It is unclear what type of events will be emitted
2. It is unclear whether the new design allows custom exit codes. Diego doesn't have different exit codes but other users might be interested in exiting with different codes depending on the error
3. I would like to address the linter warnings that are usually printed in the members slice, because they use positional fields instead of setting them by name.

[nimakaviani]

I like the example I see in the gist but I had the same question about the event listener and what type of events are to be emitted.

As for the nil members, I like how it makes the code cleaner, but I see the logic becoming harder to comprehend particularly where the nil member is not introduced intentionally.

[tedsuo]

@jvshahid A couple answers

1. I imagine that events would be for the standard ifrit lifecycle: invoked, ready, signaled. and exited. The reason for this feature is that logging/metrics code always appears interspersed with the startup code at these points, so creating a single-line Main function needs a way to integrate these observers.
2. For signaling, Main would take a list of signals, like sigmon:

func Main(EventListener, Runner, ...os.Signal)

3. The shorthand of positional fields is nice and clean, but I agree that linters don't like it for a good reason. An alternative would be to use constructors:

  group := ifrit.NewOrderedGroup(ifrit.Members{
    // allow nil members that are ignored, like a disabled debug server
    ifrit.NewMember("debug-server", cf_debug_server.Runner(dbgAddr, reconfigurableSink)),
    ifrit.NewMember("http_server", httpServer),
    ifrit.NewMember("presence", initializeCellPresence(address, serviceClient, executorClient)),
    ifrit.NewMember("bulker", harmonizer.NewBulker(logger, *pollingInterval)),
    ifrit.NewMember("event-consumer", harmonizer.NewEventConsumer(logger, opGenerator, queue)),
    ifrit.NewMember("evacuator", evacuator}),
  })

@nimakaviani how would a nil member get introduced unintentionally? Is there a pattern you have seen?

[tedsuo]

Sorry @jvshahid, just realized you were talking about exit codes, not signals.

Doing a quick survey of diego main files, I noticed they occasionally have little squiggles in there which make centralized EventHandler impractical, so I'm backing away from that proposal.

Instead, Invoke and Background could take an optional list of signals. So there's would be no need for an additional Main function or sigmon-like object.

I updated the example to reflect this: https://gist.github.com/tedsuo/1e99de0d465330526ff2

[tedsuo]

Since I doubt there is appetite for improvement at this point, I'm inclined to let sleeping bears lie. But I'm welcome to a v2 if CF peeps want to work on it.