Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deal with starting / stopping weave #867

Merged
merged 1 commit into from
Feb 9, 2016
Merged

Conversation

tomwilkie
Copy link
Contributor

Fixes #510, fixes #784, fixes #825

A few things rolled up in this:

  • Factor out all the stuff that talks to weave into a client library
  • Have everything try and talk to weave by default (ie its no longer enabled iff weave is detected at startup)
  • Have everything that talks to weave back off if it fails; when it hits max back off, we stop logging about it. All the back off code is common.
  • Have the app register with weave dns, and the probe poll system dns and weave dns

This has a few nice effects:

  • You can start weave after scope, at it should still cluster up automatically.
  • Similarly you can restart weave with no problems.
  • You can now split the app and the probe on a weave network, at it will all behave as expected (ie weave scope launch --no-app on every machine and weave scope launch --no-probe on a few)

@tomwilkie tomwilkie self-assigned this Jan 28, 2016
@tomwilkie tomwilkie force-pushed the 510-detect-weave branch 3 times, most recently from 8c00a18 to d4f8eb6 Compare February 1, 2016 17:25
@tomwilkie tomwilkie changed the title [WIP] Deal with starting / stopping weave Deal with starting / stopping weave Feb 1, 2016
@tomwilkie tomwilkie removed their assignment Feb 1, 2016
@tomwilkie tomwilkie force-pushed the 510-detect-weave branch 10 times, most recently from e1a5125 to 1f75219 Compare February 3, 2016 01:06
@tomwilkie
Copy link
Contributor Author

@paulbellamy I'm working through the test breakages, but I think this is ready to be reviewed. I'd really like to get this in before I go off on holiday (Friday)

return "", err
}

switch v := addrs[0].(type) {

This comment was marked as abuse.

@paulbellamy
Copy link
Contributor

This doesn't seem to be attributing weave endpoints to containers. It seems to just attribute them to "TheInternet".

@tomwilkie
Copy link
Contributor Author

Sweet, you found the bug!

On Wednesday, 3 February 2016, Paul Bellamy notifications@github.com
wrote:

This doesn't seem to be attributing weave endpoints to containers. It
seems to just attribute them to "TheInternet".


Reply to this email directly or view it on GitHub
#867 (comment).

@tomwilkie
Copy link
Contributor Author

Should also use the weave client api: https://github.com/weaveworks/weave/tree/master/api

@tomwilkie tomwilkie force-pushed the 510-detect-weave branch 2 times, most recently from 6b120fc to cda5835 Compare February 4, 2016 19:43
@tomwilkie tomwilkie assigned tomwilkie and unassigned paulbellamy Feb 5, 2016
@tomwilkie tomwilkie force-pushed the 510-detect-weave branch 2 times, most recently from cf05e54 to b758d56 Compare February 5, 2016 12:43
@tomwilkie
Copy link
Contributor Author

I can't explain why the coverage has dropped so much, all the new code has reasonably good coverage.

- Run the Weave integrations regardless of if weave is detected.
- Make everything backoff and not spam the logs.
- Add miekg dns to vendor.
- Have the app periodically register with weaveDNS, and the probe do lookups there.
- Decide what the local networks are at runtime, not once at startup.
- Correctly resolve app ids, fixes #825
paulbellamy added a commit that referenced this pull request Feb 9, 2016
Deal with starting / stopping weave
@paulbellamy paulbellamy merged commit 4d153bd into master Feb 9, 2016
@paulbellamy paulbellamy deleted the 510-detect-weave branch February 9, 2016 14:51
@2opremio 2opremio mentioned this pull request Feb 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants