Skip to content

Commit 784edf6

Browse files
mihaitodormihaitodor
committed
Cleanup code and check a bunch of errors
1 parent 45322d7 commit 784edf6

17 files changed

+125
-64
lines changed

‎catalog/services_state.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,7 @@ func (state *ServicesState) BroadcastTombstones(fn func() []service.Service, loo
575575

576576
tombstones = append(tombstones, otherTombstones...)
577577

578-
if tombstones != nil && len(tombstones) > 0 {
578+
if len(tombstones) > 0 {
579579
state.SendServices(
580580
tombstones,
581581
director.NewTimedLooper(TOMBSTONE_COUNT, state.tombstoneRetransmit, nil),

‎catalog/services_state_test.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ func Test_TrackingAndBroadcasting(t *testing.T) {
271271
looper := director.NewFreeLooper(5, make(chan error))
272272
state.Broadcasts = make(chan [][]byte, 5)
273273
state.SendServices(services, looper)
274-
looper.Wait()
274+
err := looper.Wait()
275+
So(err, ShouldBeNil)
275276

276277
So(len(state.Broadcasts), ShouldEqual, 5)
277278
})
@@ -280,7 +281,8 @@ func Test_TrackingAndBroadcasting(t *testing.T) {
280281
looper := director.NewFreeLooper(1, make(chan error))
281282
go state.TrackNewServices(containerFn, looper)
282283
state.ProcessServiceMsgs(director.NewFreeLooper(2, nil))
283-
looper.Wait()
284+
err := looper.Wait()
285+
So(err, ShouldBeNil)
284286

285287
So(state.Servers[hostname].Services[svcId1], ShouldNotBeNil)
286288
So(state.Servers[hostname].Services[svcId2], ShouldNotBeNil)
@@ -328,7 +330,9 @@ func Test_TrackingAndBroadcasting(t *testing.T) {
328330
service1.Tombstone()
329331
service2.Tombstone()
330332
go state.SendServices([]service.Service{service1, service2}, looper)
331-
looper.Wait()
333+
334+
err := looper.Wait()
335+
So(err, ShouldBeNil)
332336

333337
// First go-round
334338
broadcasts := <-state.Broadcasts
@@ -639,7 +643,8 @@ func Test_DecodeStream(t *testing.T) {
639643
}
640644

641645
buf := bytes.NewBufferString(string(jsonBytes))
642-
DecodeStream(buf, mockCallback)
646+
err = DecodeStream(buf, mockCallback)
647+
So(err, ShouldBeNil)
643648
So(compareMap["api"][0].Hostname, ShouldEqual, "some-aws-host")
644649
So(compareMap["api"][0].Status, ShouldEqual, 1)
645650
})

‎catalog/url_listener_test.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,9 @@ func Test_Listen(t *testing.T) {
8080
listener.eventChannel <- ChangeEvent{}
8181
listener.Retries = 0
8282
listener.Watch(state)
83-
listener.looper.Wait()
83+
err := listener.looper.Wait()
8484

85+
So(err, ShouldBeNil)
8586
So(len(errors), ShouldEqual, 0)
8687
})
8788
})

‎cli.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ import (
88
)
99

1010
type CliOpts struct {
11-
AdvertiseIP *string
12-
ClusterIPs *[]string
13-
ClusterName *string
14-
CpuProfile *bool
15-
Discover *[]string
16-
LoggingLevel *string
11+
AdvertiseIP *string
12+
ClusterIPs *[]string
13+
ClusterName *string
14+
CpuProfile *bool
15+
Discover *[]string
16+
LoggingLevel *string
1717
}
1818

1919
func exitWithError(err error, message string) {

‎haproxy/haproxy.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,10 @@ func (h *HAproxy) WriteConfig(state *catalog.ServicesState, output io.Writer) er
184184
}
185185

186186
// This is the potentially slowest bit, do it outside the critical section
187-
io.Copy(output, buf)
187+
_, err = io.Copy(output, buf)
188+
if err != nil {
189+
return fmt.Errorf("Error writing template '%s': %s", h.Template, err.Error())
190+
}
188191

189192
return nil
190193
}

‎haproxy/haproxy_test.go

+25-9
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616

1717
var hostname1 = "indomitable"
1818
var hostname2 = "indefatigable"
19-
var hostname3 = "invincible"
2019

2120
func Test_HAproxy(t *testing.T) {
2221
Convey("End-to-end testing HAproxy functionality", t, func() {
@@ -30,9 +29,17 @@ func Test_HAproxy(t *testing.T) {
3029
ip := "127.0.0.1"
3130
ip3 := "127.0.0.3"
3231

33-
ports1 := []service.Port{{"tcp", 10450, 8080, ip}, {"tcp", 10020, 9000, ip}}
34-
ports2 := []service.Port{{"tcp", 9999, 8090, ip3}}
35-
ports3 := []service.Port{{"tcp", 32763, 8080, ip3}, {"tcp", 10020, 9000, ip3}}
32+
ports1 := []service.Port{
33+
{Type: "tcp", Port: 10450, ServicePort: 8080, IP: ip},
34+
{Type: "tcp", Port: 10020, ServicePort: 9000, IP: ip},
35+
}
36+
ports2 := []service.Port{
37+
{Type: "tcp", Port: 9999, ServicePort: 8090, IP: ip3},
38+
}
39+
ports3 := []service.Port{
40+
{Type: "tcp", Port: 32763, ServicePort: 8080, IP: ip3},
41+
{Type: "tcp", Port: 10020, ServicePort: 9000, IP: ip3},
42+
}
3643

3744
services := []service.Service{
3845
{
@@ -130,7 +137,9 @@ func Test_HAproxy(t *testing.T) {
130137
Image: "some-svc",
131138
Hostname: "titanic",
132139
Updated: baseTime.Add(5 * time.Second),
133-
Ports: []service.Port{{"tcp", 666, 6666, "127.0.0.1"}},
140+
Ports: []service.Port{
141+
{Type: "tcp", Port: 666, ServicePort: 6666, IP: "127.0.0.1"},
142+
},
134143
}
135144

136145
// It had 1 before
@@ -178,7 +187,9 @@ func Test_HAproxy(t *testing.T) {
178187
Hostname: "titanic",
179188
Status: service.UNHEALTHY,
180189
Updated: baseTime.Add(5 * time.Second),
181-
Ports: []service.Port{{"tcp", 666, 6666, "127.0.0.1"}},
190+
Ports: []service.Port{
191+
{Type: "tcp", Port: 666, ServicePort: 6666, IP: "127.0.0.1"},
192+
},
182193
}
183194
badSvc2 := service.Service{
184195
ID: "0000bad00001",
@@ -187,13 +198,16 @@ func Test_HAproxy(t *testing.T) {
187198
Hostname: "titanic",
188199
Status: service.UNKNOWN,
189200
Updated: baseTime.Add(5 * time.Second),
190-
Ports: []service.Port{{"tcp", 666, 6666, "127.0.0.1"}},
201+
Ports: []service.Port{
202+
{Type: "tcp", Port: 666, ServicePort: 6666, IP: "127.0.0.1"},
203+
},
191204
}
192205
state.AddServiceEntry(badSvc)
193206
state.AddServiceEntry(badSvc2)
194207

195208
buf := bytes.NewBuffer(make([]byte, 0, 2048))
196-
proxy.WriteConfig(state, buf)
209+
err := proxy.WriteConfig(state, buf)
210+
So(err, ShouldBeNil)
197211

198212
output := buf.Bytes()
199213
// Look for a few things we should NOT see
@@ -249,7 +263,9 @@ func Test_HAproxy(t *testing.T) {
249263
Image: "some-svc",
250264
Hostname: hostname2,
251265
Updated: newTime,
252-
Ports: []service.Port{{"tcp", 1337, 8090, "127.0.0.1"}},
266+
Ports: []service.Port{
267+
{Type: "tcp", Port: 1337, ServicePort: 8090, IP: "127.0.0.1"},
268+
},
253269
}
254270
OUTER:
255271
for {

‎healthy/service_bridge.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,11 @@ func (m *Monitor) templateCheckArgs(check *Check, svc *service.Service) string {
117117
}
118118

119119
var output bytes.Buffer
120-
t.Execute(&output, svc)
120+
err = t.Execute(&output, svc)
121+
if err != nil {
122+
log.Errorf("Unable to execute template: '%s'", check.Args)
123+
return check.Args
124+
}
121125

122126
return output.String()
123127
}

‎healthy/service_bridge_test.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import (
44
"testing"
55
"time"
66

7+
"github.com/Nitro/sidecar/discovery"
78
"github.com/Nitro/sidecar/service"
89
"github.com/relistan/go-director"
910
. "github.com/smartystreets/goconvey/convey"
10-
"github.com/Nitro/sidecar/discovery"
1111
)
1212

1313
var hostname string = "indefatigable"
@@ -137,7 +137,10 @@ func Test_ServicesBridge(t *testing.T) {
137137
Convey("Responds to changes in a list of services", func() {
138138
So(len(monitor.Checks), ShouldEqual, 4)
139139

140-
ports := []service.Port{{"udp", 11234, 8080, "127.0.0.1"}, {"tcp", 1234, 8081, "127.0.0.1"}}
140+
ports := []service.Port{
141+
{Type: "udp", Port: 11234, ServicePort: 8080, IP: "127.0.0.1"},
142+
{Type: "tcp", Port: 1234, ServicePort: 8081, IP: "127.0.0.1"},
143+
}
141144
svc := service.Service{ID: "babbacabba", Name: "testing-12312312", Ports: ports}
142145
svcList := []service.Service{svc}
143146

@@ -165,8 +168,8 @@ func Test_CheckForService(t *testing.T) {
165168
Convey("When building a default check", t, func() {
166169
svcId1 := "deadbeef123"
167170
ports := []service.Port{
168-
{"udp", 11234, 8080, "127.0.0.1"},
169-
{"tcp", 1234, 8081, "127.0.0.1"},
171+
{Type: "udp", Port: 11234, ServicePort: 8080, IP: "127.0.0.1"},
172+
{Type: "tcp", Port: 1234, ServicePort: 8081, IP: "127.0.0.1"},
170173
}
171174
service1 := service.Service{ID: svcId1, Hostname: hostname, Ports: ports}
172175

‎logging_bridge_test.go

+6-3
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,24 @@ func Test_LoggingBridge(t *testing.T) {
1111
bridge := LoggingBridge{testing: true}
1212

1313
Convey("Properly splits apart and re-levels log messages", func() {
14-
bridge.Write([]byte("2016/06/24 11:45:33 [DEBUG] memberlist: TCP connection from=172.16.106.1:59598"))
14+
_, err := bridge.Write([]byte("2016/06/24 11:45:33 [DEBUG] memberlist: TCP connection from=172.16.106.1:59598"))
15+
So(err, ShouldBeNil)
1516

1617
So(string(bridge.lastLevel), ShouldEqual, "[DEBUG]")
1718
So(string(bridge.lastMessage), ShouldEqual, "memberlist: TCP connection from=172.16.106.1:59598")
1819

19-
bridge.Write([]byte("2016/06/24 11:45:33 [WARN] memberlist: Something something"))
20+
_, err = bridge.Write([]byte("2016/06/24 11:45:33 [WARN] memberlist: Something something"))
21+
So(err, ShouldBeNil)
2022

2123
So(string(bridge.lastLevel), ShouldEqual, "[WARN]")
2224
So(string(bridge.lastMessage), ShouldEqual, "memberlist: Something something")
2325
})
2426

2527
Convey("Handles writes that have more than one message", func() {
26-
bridge.Write(
28+
_, err := bridge.Write(
2729
[]byte("2016/06/24 11:45:33 [DEBUG] memberlist: TCP connection from=172.16.106.1:59598\nasdf"),
2830
)
31+
So(err, ShouldBeNil)
2932

3033
So(string(bridge.lastMessage), ShouldEqual, "memberlist: TCP connection from=172.16.106.1:59598")
3134
})

‎main.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,8 @@ func configureCpuProfiler(opts *CliOpts) {
189189
if *opts.CpuProfile {
190190
profilerFile, err := os.Create("sidecar.cpu.prof")
191191
exitWithError(err, "Can't write profiling file")
192-
pprof.StartCPUProfile(profilerFile)
192+
err = pprof.StartCPUProfile(profilerFile)
193+
exitWithError(err, "Can't start the CPU profiler")
193194
log.Debug("Profiling!")
194195
}
195196
}
@@ -367,7 +368,8 @@ func main() {
367368
})
368369

369370
if !config.HAproxy.Disable {
370-
proxy.WriteAndReload(state)
371+
err := proxy.WriteAndReload(state)
372+
exitWithError(err, "Failed to reload HAProxy config")
371373
}
372374

373375
select {}

‎receiver/http.go

+8-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ func UpdateHandler(response http.ResponseWriter, req *http.Request, rcvr *Receiv
2222
if err != nil {
2323
message, _ := json.Marshal(ApiErrors{[]string{err.Error()}})
2424
response.WriteHeader(http.StatusInternalServerError)
25-
response.Write(message)
25+
_, err := response.Write(message)
26+
if err != nil {
27+
log.Errorf("Error replying to client when failed to read the request body: %s", err)
28+
}
2629
return
2730
}
2831

@@ -31,7 +34,10 @@ func UpdateHandler(response http.ResponseWriter, req *http.Request, rcvr *Receiv
3134
if err != nil {
3235
message, _ := json.Marshal(ApiErrors{[]string{err.Error()}})
3336
response.WriteHeader(http.StatusInternalServerError)
34-
response.Write(message)
37+
_, err := response.Write(message)
38+
if err != nil {
39+
log.Errorf("Error replying to client when failed to unmarshal the request JSON: %s", err)
40+
}
3541
return
3642
}
3743

‎receiver/http_test.go

+5-10
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,7 @@ func Test_updateHandler(t *testing.T) {
7373
Convey("updates the state and enqueues an update", func() {
7474
startTime := rcvr.CurrentState.LastChanged
7575

76-
var evtState *catalog.ServicesState
77-
evtState = deepcopy.Copy(state).(*catalog.ServicesState)
76+
evtState := deepcopy.Copy(state).(*catalog.ServicesState)
7877
evtState.LastChanged = time.Now().UTC()
7978

8079
change := catalog.StateChangedEvent{
@@ -107,8 +106,7 @@ func Test_updateHandler(t *testing.T) {
107106
})
108107

109108
Convey("enqueus all updates if no Subscriptions are provided", func() {
110-
var evtState *catalog.ServicesState
111-
evtState = deepcopy.Copy(state).(*catalog.ServicesState)
109+
evtState := deepcopy.Copy(state).(*catalog.ServicesState)
112110
evtState.LastChanged = time.Now().UTC()
113111

114112
change := catalog.StateChangedEvent{
@@ -136,8 +134,7 @@ func Test_updateHandler(t *testing.T) {
136134
})
137135

138136
Convey("does not enqueue updates if the service is not subscribed to", func() {
139-
var evtState *catalog.ServicesState
140-
evtState = deepcopy.Copy(state).(*catalog.ServicesState)
137+
evtState := deepcopy.Copy(state).(*catalog.ServicesState)
141138
evtState.LastChanged = time.Now().UTC()
142139

143140
change := catalog.StateChangedEvent{
@@ -167,8 +164,7 @@ func Test_updateHandler(t *testing.T) {
167164
})
168165

169166
Convey("enqueues updates if the service is subscribed to", func() {
170-
var evtState *catalog.ServicesState
171-
evtState = deepcopy.Copy(state).(*catalog.ServicesState)
167+
evtState := deepcopy.Copy(state).(*catalog.ServicesState)
172168
evtState.LastChanged = time.Now().UTC()
173169

174170
change := catalog.StateChangedEvent{
@@ -198,8 +194,7 @@ func Test_updateHandler(t *testing.T) {
198194
})
199195

200196
Convey("a copy of the state is passed to the OnUpdate func", func() {
201-
var evtState *catalog.ServicesState
202-
evtState = deepcopy.Copy(state).(*catalog.ServicesState)
197+
evtState := deepcopy.Copy(state).(*catalog.ServicesState)
203198
evtState.LastChanged = time.Now().UTC()
204199

205200
change := catalog.StateChangedEvent{

‎receiver/receiver.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ func (rcvr *Receiver) ProcessUpdates() {
145145
// Copy the state while locked so we don't have it change
146146
// under us while writing and we don't hold onto the lock the
147147
// whole time we're writing to disk (e.g. in haproxy-api).
148-
var tmpState *catalog.ServicesState
149-
tmpState = deepcopy.Copy(rcvr.CurrentState).(*catalog.ServicesState)
148+
tmpState := deepcopy.Copy(rcvr.CurrentState).(*catalog.ServicesState)
150149
rcvr.StateLock.Unlock()
151150

152151
rcvr.OnUpdate(tmpState)

‎service/service.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func buildPortFor(port *docker.APIPort, container *docker.APIContainers, ip stri
184184
if err != nil {
185185
log.Errorf("Error converting label value for %s to integer: %s",
186186
svcPortLabel,
187-
err.Error(),
187+
err,
188188
)
189189
return returnPort
190190
}

0 commit comments

Comments
 (0)