Fix "weave ps" #2418
Fix "weave ps" #2418
Conversation
| func GetBridgeNetDev(bridgeName string) (NetDev, error) { | ||
| var netdev NetDev | ||
|
|
||
| err := weavenet.WithNetNSLinkByPid(1, bridgeName, func(link netlink.Link) error { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
|
||
| containers := make(map[string]*docker.Container) | ||
| for _, cid := range containerIDs { | ||
| if containers[cid], err = client.InspectContainer(cid); err != nil { |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
| return common.GetBridgeNetDev(bridgeName) | ||
| } | ||
|
|
||
| container, err := c.InspectContainer(containerID) |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
- Optimize. Use LinkByName instead of iterating over list of available links. Linux does not support interfaces with the same names. - Remove unnecessary indirection in utility functions which return common.NetDev.
Make sure that any network namespace changes happens at the end of the cmd invocation. Otherwise, we are risking to end up in a situation in which OS scheduling threads are in a "wrong" namespace. See WithNetNS for more details.
Please see #2388 (comment) for more details.
- Do not re-enter the root netns when querying weave bridge. - Enter the root netns when constructing the predicate.
| indexes := make(map[int]struct{}) | ||
|
|
||
| // Scan devices in root namespace to find those attached to weave bridge | ||
| err := weavenet.WithNetNSLinkByPidUnsafe(1, bridgeName, |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
LGTM assuming my questions for clarification come out the way I think. |
The #2418 has changed a behaviour of "weave ps" which outputs "Link not found" when weave is not running. The previous behaviour was to output an empty line.
The #2418 has changed a behaviour of "weave ps" which outputs "Link not found" in the case of weave not running. The previous behaviour was to output an empty line.
This PR fixes the problem described in #2388. In addition, the PR optimizes the
GetBridgeNetDevsandcontainerAddrsfunctions.Keep in mind, that the fix is specific for this case, because we move all the code depending on a non-host netns to the bottom of
containerAddrsinstead of makingWithNetNSsafe.To address the safety problem, I've filed #2419 .
Fixes #2388.