Delay saving host data until after bootstrapping

[mbarnes]

Resolves #177

Couple subtle semantic changes:

• A newly created host is not added to a cluster until after its bootstrapping successfully completes. Otherwise if a cluster operation were to be started in the interim, the new host may not yet be in a usable state and the cluster operation would fail.
• We lose some granularity in host status; namely, the "bootstrapping" status is no longer observable since host data is not saved until bootstapping is finished.

[ashcrow]

@mbarnes Based on this early WIP I like where this is going!

[mbarnes]

:warning: So I've disabled end-to-end tests marked @slow (all of which are creating hosts) for Travis CI because they now rely on bootstrapping to actually work, and Travis doesn't provide a suitable environment.

I'm trying to get these @slow tests working in our Vagrant environment but I'm bumping into the issue of SSH keys: the two "development nodes" don't have any. I think I know how to sort this out but I'm tempted to defer it to a separate PR.

Just need to do a bit more actual Kubernetes testing before I yank the WIP label...

[mbarnes]

@ashcrow :arrow_up: Seems to work now with Kubernetes. So many got'chas...

Have at it.

[ashcrow]

Cool. I'll start reviewing now but likely won't finish today.

[ashcrow]

I think I found a bug while doing some testing. At the moment flanneld required etcd so we have to configure an etcdstorehandler no matter what. That's all fine, however, I believe that the storehandler does not get registered since it has no models associated with it. This results in the URI used by flanneld to fallback to the default 127.0.0.1 instead of what is set up in the configuration file.

A few ideas:

1. Allow for registration without models.
2. Ignore for now as it will be "fixed" once I'm finished with the server/client PR for flannel without etcd configuration.

@mbarnes Thoughts?

[mbarnes]

1. Allow for registration without models.

This seems reasonable to me. I'm expecting more corner cases like what you described. Should be fairly easy.

[ashcrow]

@mbarnes ok cool. Let me know when it's re-review time. Otherwise things look good!

[mbarnes]

@ashcrow :arrow_up: I think I addressed the flanneld issue.

[ashcrow]

Looking now.

[ashcrow]

Functionality is working as I expect! Good work @mbarnes!

One last thing, running e2e on slow doesn't pass. Please update them to pass or add a new tag such as requires_ssh if it's not something that can run without keys and use that instead of jumping over all the slow items.

Failing scenarios:
  features/cluster/hosts.feature:231  Creating a new host with a valid cluster name (1)
  features/cluster/hosts.feature:243  Creating a new host with a valid cluster name (2)
  features/cluster/hosts.feature:255  Creating a new host with a valid cluster name (3)

1 feature passed, 1 failed, 11 skipped
2 scenarios passed, 3 failed, 58 skipped
19 steps passed, 3 failed, 343 skipped, 0 undefined
Took 0m58.970s

[mbarnes]

One last thing, running e2e on slow doesn't pass. Please update them to pass or add a new tag such as requires_ssh if it's not something that can run without keys and use that instead of jumping over all the slow items.

The @slow tests require -D use-vagrant and should pass again once I add SSH keys to the Vagrant cluster hosts. I'm working on that now. Do you know if behave offers a way to enable/disable certain tags programmatically? (e.g. automatically disable @slow if not using Vagrant)

[ashcrow]

@mbarnes not directly. You can probably do it someway with before_tag() or after_tag() if you want to sense for it.

[mbarnes]

@ashcrow :arrow_up: Stack Overflow to the rescue.

I'm tempted to rename the @slow tag to @vagrant...

[ashcrow]

@mbarnes renaming it would be fine with me.

[ashcrow]

This all looks good!

Will you do docs in a follow up or within this PR?

[mbarnes]

I'll do docs for register-store-handler config separately. This PR was really just a bug fix.

[ashcrow]

LGTM

@mbarnes mind taking off the WIP prefix if done?

[mbarnes]

Oh whoops... done.

[ashcrow]

@rh-atomic-bot r+

[rh-atomic-bot]

:pushpin: Commit 51acfcf has been approved by ashcrow

[rh-atomic-bot]

:hourglass: Testing commit 51acfcf with merge 9ec61d1...

[ashcrow]

@mbarnes homu still unhappy (likely still confused by Travis for some reason ...). Mind squashing?

[mbarnes]

Squashed and verified tests locally.