Ignore Link Local Addresses #627
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The code assumed that 'go a(); go b()' would always call a() first, but that isn't guaranteed by the runtime. There was a bunch of other code that happened to work because of bad assumptions, so I went around and cleaned them up (it assumed that it was ok for results to be of length 100 when making a request for 10 ids, which only worked because the aggregation code looped over the input rather than looping over the result of the request).
…why the machine address updater keeps saying that it is updating addresses with values that haven't changed.
When storing the addresses of a Machine, we won't ever want to use the ScopeLinkLocal addresses, both PublicAddress and PrivateAddress will explicitly filter them out, and they just confuse things when dealing with addresses (at one point they were candidates for Clients to try to connect to the API server).
| @@ -34,6 +35,9 @@ type testInstance struct { | |||
|
|
|||
| var _ instance.Instance = (*testInstance)(nil) | |||
|
|
|||
| func (t *testInstance) Id() instance.Id { | |||
| return t.id | |||
| } | |||
There was a problem hiding this comment.
Add a blank line after (we might use "o" for this (vim shortcuts), like we use "d" for Delete this line :)
|
LGTM, thanks for doing this! Just a couple of comments. |
|
|
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
added a commit
that referenced
this pull request
Sep 1, 2014
Ignore Link Local Addresses This addresses 3 different issues, I can split them up if requested, but it is easiest to just land them together. 1) Race condition in aggregate_test.go. It had an assumption that: go funcA() go funcB() would always call funcA before funcB. (Because the result list was a fixed response, and the processing code doesn't do any mapping of result ids to actual request ids.) There was a fair bit in other tests that only really worked by happenstance, so I cleaned them up as well. 2) Add some Trace level logging about why 2 address lists don't match. I've seen a bunch of log entries that say "updating machine addresses" when nothing appears to have actually changed. I can just shelve this, and instead only test manually and see if I find a fix, but I figure that trace level logging is cheap. 3) Actually fix bug #1362453 which is that when looking at Machine addresses, we include their LinkLocal address. Which isn't a useful address in any sense. And right now both PrivateAddress and PublicAddress take care to filter them out. However I thought it might be part of why (2) is getting confused, and certainly in the past I was seeing them end up in the list of APIHostPorts which is a bit silly (there is no reason to say that you can connect to a machine on its LinkLocal address.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses 3 different issues, I can split them up if requested, but it is easiest to just land them together.
Race condition in aggregate_test.go. It had an assumption that:
go funcA()
go funcB()
would always call funcA before funcB. (Because the result list was a fixed response, and the processing code doesn't do any mapping of result ids to actual request ids.)
There was a fair bit in other tests that only really worked by happenstance, so I cleaned them up as well.
Add some Trace level logging about why 2 address lists don't match. I've seen a bunch of log entries that say "updating machine addresses" when nothing appears to have actually changed.
I can just shelve this, and instead only test manually and see if I find a fix, but I figure that trace level logging is cheap.