-
Notifications
You must be signed in to change notification settings - Fork 30
Add checks to quit if DefaultInterface address can't be resolved #133
Changes from 10 commits
ba5329e
88c868d
e4bffad
3c9c509
766a1af
0066021
5fb6243
82f2be5
3bc6930
419cfe4
7dec374
dcaad51
c21de2d
b3de4cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,33 +9,56 @@ | |
package hostresources | ||
|
||
import ( | ||
"errors" | ||
"net" | ||
) | ||
|
||
// GetIPOfInt will iterate over all addresses for the given network interface, but will return only | ||
// GetDefaultInterfaceIP determines the appropriate IP address to use for either the server or agent | ||
// based on configuration parameters passed in as arguments | ||
// | ||
// The server uses this address to inform the agents of the URL they should use to download assets | ||
// | ||
// The agents use this address so that the server knows how to orchestrate tests. | ||
// (i.e. This agent publishes it's default address, and the server instructs other agents to target it in tests) | ||
func GetDefaultInterfaceIP(ifname, ipAddrOverride string) (string, error) { | ||
|
||
if ipAddrOverride != "" { | ||
return ipAddrOverride, nil | ||
} | ||
|
||
defaultaddr, err := getIPOfInt(ifname) | ||
if err != nil { | ||
return "", err | ||
} else { | ||
return defaultaddr, nil | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can simplify this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup - fixed in 7dec374 |
||
|
||
} | ||
|
||
// getIPOfInt will iterate over all addresses for the given network interface, but will return only | ||
// the first one it finds. TODO(mierdin): This has obvious drawbacks, particularly with IPv6. Need to figure out a better way. | ||
func GetIPOfInt(ifname string) net.IP { | ||
func getIPOfInt(ifname string) (string, error) { | ||
interfaces, err := net.Interfaces() | ||
if err != nil { | ||
panic(err) | ||
return "", err | ||
} | ||
|
||
for _, iface := range interfaces { | ||
if iface.Name == ifname { | ||
|
||
addrs, err := iface.Addrs() | ||
if err != nil { | ||
panic(err) | ||
return "", err | ||
} | ||
for _, addr := range addrs { | ||
if ipnet, ok := addr.(*net.IPNet); ok { | ||
if ipnet.IP.To4() != nil { | ||
return ipnet.IP | ||
return ipnet.IP.To4().String(), nil | ||
} | ||
|
||
} | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Damn - and it looks like this has existed for some time. Can't believe I missed that! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool - implemented this and got rid of some of my wheel reinvention in c21de2d |
||
return nil | ||
return "", errors.New("No DefaultInterface address found") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this
ListenForAgent(_ assetProvider, assetURLPrefix string) error
instead of putting the description in the comments. Although it will be a bit inconsistent with the rest of the interface definition.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, and I think we already discussed this in a previous review (so that's on me). I opened #137 to do this in a future PR