Skip to content
This repository has been archived by the owner on Oct 7, 2021. It is now read-only.

Add checks to quit if DefaultInterface address can't be resolved #133

Merged
merged 14 commits into from
Mar 20, 2017
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions cmd/todd-agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,20 @@ func main() {
// Watch for changes to group membership
go tc.Package.WatchForGroup()

// Get default IP address for the server.
// This address is primarily used 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)
defaultaddr, err := hostresources.GetDefaultInterfaceIP(cfg.LocalResources.DefaultInterface, cfg.LocalResources.IPAddrOverride)
if err != nil {
log.Fatalf("Unable to derive address from configured DefaultInterface: %v", err)
}

// Continually advertise agent status into message queue
for {

// Gather assets here as a map, and refer to a key in that map in the below struct
gatheredAssets := GetLocalAssets(cfg)

var defaultaddr string
if cfg.LocalResources.IPAddrOverride != "" {
defaultaddr = cfg.LocalResources.IPAddrOverride
} else {
defaultaddr = hostresources.GetIPOfInt(cfg.LocalResources.DefaultInterface).String()
}

fcts, err := facts.GetFacts(cfg)
if err != nil {
log.Errorf("Error gathering facts: %v", err)
Expand Down
12 changes: 11 additions & 1 deletion cmd/todd-server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/toddproject/todd/comms"
"github.com/toddproject/todd/config"
"github.com/toddproject/todd/db"
"github.com/toddproject/todd/hostresources"
"github.com/toddproject/todd/server/grouping"
)

Expand Down Expand Up @@ -81,9 +82,18 @@ func main() {
log.Fatalf("Problem connecting to comms: %v\n", err)
}

// Get default IP address for the server.
// This address is primarily used to inform the agents of the URL they should use to download assets
defaultaddr, err := hostresources.GetDefaultInterfaceIP(cfg.LocalResources.DefaultInterface, cfg.LocalResources.IPAddrOverride)
if err != nil {
log.Fatalf("Unable to derive address from configured DefaultInterface: %v", err)
}

assetURLPrefix := fmt.Sprintf("http://%s:%s", defaultaddr, cfg.Assets.Port)

go func() {
for {
err := tc.Package.ListenForAgent(assets)
err := tc.Package.ListenForAgent(assets, assetURLPrefix)
if err != nil {
log.Fatalf("Error listening for ToDD Agents")
}
Expand Down
4 changes: 2 additions & 2 deletions comms/comms.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ type Package interface {
// (agent advertisement to advertise)
AdvertiseAgent(defs.AgentAdvert) error

// (map of assets:hashes, lock for asset map)
ListenForAgent(assetProvider) error
// (map of assets:hashes, base asset URL)
ListenForAgent(assetProvider, string) error
Copy link
Member

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.

Copy link
Member Author

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


// (uuid)
ListenForTasks(string) error
Expand Down
12 changes: 2 additions & 10 deletions comms/rabbitmq.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/toddproject/todd/agent/tasks"
"github.com/toddproject/todd/config"
"github.com/toddproject/todd/db"
"github.com/toddproject/todd/hostresources"
)

const (
Expand Down Expand Up @@ -165,7 +164,7 @@ func (rmq rabbitMQComms) AdvertiseAgent(me defs.AgentAdvert) error {

// ListenForAgent will listen on the message queue for new agent advertisements.
// It is meant to be run as a goroutine
func (rmq rabbitMQComms) ListenForAgent(assets assetProvider) error {
func (rmq rabbitMQComms) ListenForAgent(assets assetProvider, assetURLPrefix string) error {

// TODO(mierdin): does func param need to be a pointer?

Expand Down Expand Up @@ -247,14 +246,7 @@ func (rmq rabbitMQComms) ListenForAgent(assets assetProvider) error {
if agentAssets[name] != hash {

// hashes do not match, so we need to append the asset download URL to the remediate list
var defaultIP string
if rmq.config.LocalResources.IPAddrOverride != "" { // TODO: set defaultIP to override byte default and check if empty
defaultIP = rmq.config.LocalResources.IPAddrOverride
} else {
defaultIP = hostresources.GetIPOfInt(rmq.config.LocalResources.DefaultInterface).String()
}
assetURL := fmt.Sprintf("http://%s:%s/%s/%s", defaultIP, rmq.config.Assets.Port, assetType, name)

assetURL := fmt.Sprintf("%s/%s/%s", assetURLPrefix, assetType, name)
assetList = append(assetList, assetURL)

}
Expand Down
35 changes: 29 additions & 6 deletions hostresources/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this to return getIPOfint(ifname).

Copy link
Member Author

Choose a reason for hiding this comment

The 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
}

}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like net.InterfaceByName() would make this easier.

Copy link
Member Author

Choose a reason for hiding this comment

The 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!

Copy link
Member Author

Choose a reason for hiding this comment

The 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")
}
7 changes: 4 additions & 3 deletions scripts/start-containers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ branch=$(git symbolic-ref -q HEAD)
branch=${branch##refs/heads/}
branch=${branch:-HEAD}

# Replace any slashes in branch with hyphen, so docker doesn't puke
branch=${branch/\//_}

toddimage=toddproject/todd:$branch

if [ ${PWD:(-7)} == "scripts" ]
Expand Down Expand Up @@ -67,12 +70,11 @@ function startinfra {
-initial-cluster etcd0=http://${HostIP}:2380 \
-initial-cluster-state new > /dev/null

# I have a rabbitmq server at home, but in case I'm working on my laptop only, spin this up too:
echo "Starting RabbitMQ"
docker run -d \
--net todd-network \
--name rabbit \
-p 8085:15672 \
-p 15672:15672 \
-p 5672:5672 \
-e RABBITMQ_DEFAULT_USER=guest \
-e RABBITMQ_DEFAULT_PASS=guest \
Expand All @@ -82,7 +84,6 @@ function startinfra {
docker run -d --net todd-network --volume=/var/influxdb:/data --name influx -p 8083:8083 -p 8086:8086 tutum/influxdb:0.9 > /dev/null
echo "Starting Grafana"
docker run -d --net todd-network --volume=/var/lib/grafana:/var/lib/grafana --name grafana -p 3000:3000 grafana/grafana > /dev/null

}

# arg $1: number of agents to use
Expand Down