Review feedback

This commit is contained in:
Alfonso Acosta
2016-07-01 16:51:57 +00:00
parent 6f1e52cd0d
commit c0a672c02a
7 changed files with 28 additions and 33 deletions

View File

@@ -43,7 +43,7 @@ func TestControl(t *testing.T) {
Value: "foo",
}
})
client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler, false)
client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, controlHandler)
if err != nil {
t.Fatal(err)
}

View File

@@ -83,7 +83,7 @@ func TestPipeClose(t *testing.T) {
probeConfig := appclient.ProbeConfig{
ProbeID: "foo",
}
client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil, false)
client, err := appclient.NewAppClient(probeConfig, ip+":"+port, ip+":"+port, nil)
if err != nil {
t.Fatal(err)
}

View File

@@ -57,12 +57,11 @@ type appClient struct {
readers chan io.Reader
// For controls
noControls bool
control xfer.ControlHandler
control xfer.ControlHandler
}
// NewAppClient makes a new appClient.
func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler, noControls bool) (AppClient, error) {
func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlHandler) (AppClient, error) {
httpTransport, err := pc.getHTTPTransport(hostname)
if err != nil {
return nil, err
@@ -83,10 +82,9 @@ func NewAppClient(pc ProbeConfig, hostname, target string, control xfer.ControlH
wsDialer: websocket.Dialer{
TLSClientConfig: httpTransport.TLSClientConfig,
},
conns: map[string]xfer.Websocket{},
readers: make(chan io.Reader, 2),
noControls: noControls,
control: control,
conns: map[string]xfer.Websocket{},
readers: make(chan io.Reader, 2),
control: control,
}, nil
}
@@ -233,9 +231,6 @@ func (c *appClient) controlConnection() (bool, error) {
}
func (c *appClient) ControlConnection() {
if c.noControls {
return
}
go func() {
log.Infof("Control connection to %s starting", c.target)
defer log.Infof("Control connection to %s exiting", c.target)

View File

@@ -109,7 +109,7 @@ func TestAppClientPublish(t *testing.T) {
Insecure: false,
}
p, err := NewAppClient(pc, u.Host, s.URL, nil, false)
p, err := NewAppClient(pc, u.Host, s.URL, nil)
if err != nil {
t.Fatal(err)
}
@@ -158,7 +158,7 @@ func TestAppClientDetails(t *testing.T) {
ProbeID: "",
Insecure: false,
}
p, err := NewAppClient(pc, u.Host, s.URL, nil, false)
p, err := NewAppClient(pc, u.Host, s.URL, nil)
if err != nil {
t.Fatal(err)
}
@@ -203,7 +203,7 @@ func TestStop(t *testing.T) {
Insecure: false,
}
p, err := NewAppClient(pc, u.Host, s.URL, nil, false)
p, err := NewAppClient(pc, u.Host, s.URL, nil)
if err != nil {
t.Fatal(err)
}

View File

@@ -23,11 +23,12 @@ type ClientFactory func(string, string) (AppClient, error)
type multiClient struct {
clientFactory ClientFactory
mtx sync.Mutex
sema semaphore
clients map[string]AppClient // holds map from app id -> client
ids map[string]report.IDList // holds map from hostname -> app ids
quit chan struct{}
mtx sync.Mutex
sema semaphore
clients map[string]AppClient // holds map from app id -> client
ids map[string]report.IDList // holds map from hostname -> app ids
quit chan struct{}
noControls bool
}
type clientTuple struct {
@@ -53,14 +54,15 @@ type MultiAppClient interface {
}
// NewMultiAppClient creates a new MultiAppClient.
func NewMultiAppClient(clientFactory ClientFactory) MultiAppClient {
func NewMultiAppClient(clientFactory ClientFactory, noControls bool) MultiAppClient {
return &multiClient{
clientFactory: clientFactory,
sema: newSemaphore(maxConcurrentGET),
clients: map[string]AppClient{},
ids: map[string]report.IDList{},
quit: make(chan struct{}),
sema: newSemaphore(maxConcurrentGET),
clients: map[string]AppClient{},
ids: map[string]report.IDList{},
quit: make(chan struct{}),
noControls: noControls,
}
}
@@ -100,9 +102,7 @@ func (c *multiClient) Set(hostname string, endpoints []string) {
hostIDs := report.MakeIDList()
for tuple := range clients {
hostIDs = hostIDs.Add(tuple.ID)
_, ok := c.clients[tuple.ID]
if !ok {
if _, ok := c.clients[tuple.ID]; !ok && !c.noControls {
c.clients[tuple.ID] = tuple.AppClient
tuple.AppClient.ControlConnection()
}

View File

@@ -67,7 +67,7 @@ func TestMultiClient(t *testing.T) {
}
)
mp := appclient.NewMultiAppClient(factory)
mp := appclient.NewMultiAppClient(factory, false)
defer mp.Stop()
// Add two hostnames with overlapping apps, check we don't add the same app twice
@@ -89,7 +89,7 @@ func TestMultiClient(t *testing.T) {
}
func TestMultiClientPublish(t *testing.T) {
mp := appclient.NewMultiAppClient(factory)
mp := appclient.NewMultiAppClient(factory, false)
defer mp.Stop()
sum := func() int { return a1.publish + a2.publish + b2.publish + b3.publish }

View File

@@ -113,13 +113,13 @@ func probeMain(flags probeFlags) {
ProbeID: probeID,
Insecure: flags.insecure,
}
clients := appclient.NewMultiAppClient(func(hostname, endpoint string) (appclient.AppClient, error) {
clientFactory := func(hostname, endpoint string) (appclient.AppClient, error) {
return appclient.NewAppClient(
probeConfig, hostname, endpoint,
xfer.ControlHandlerFunc(controls.HandleControlRequest),
flags.noControls,
)
})
}
clients := appclient.NewMultiAppClient(clientFactory, flags.noControls)
defer clients.Stop()
dnsLookupFn := net.LookupIP