From 95bd6b4ae29c9b18c0a0b214cd2d3ad84764daa5 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Tue, 1 Dec 2015 12:14:39 -0500 Subject: [PATCH 1/8] Make golint happy. --- testutil/server.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/testutil/server.go b/testutil/server.go index 1476d48e2..4f5a00302 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -43,7 +43,7 @@ type TestServerConfig struct { Stdout, Stderr io.Writer `json:"-"` } -// Ports is used to configure the network ports we use. +// PortsConfig is used to configure the network ports we use. type PortsConfig struct { HTTP int `json:"http,omitempty"` RPC int `json:"rpc,omitempty"` @@ -97,10 +97,10 @@ type TestServer struct { HTTPAddr string SerfAddr string - HttpClient *http.Client + HTTPClient *http.Client } -// NewTestServerConfig creates a new TestServer, and makes a call to +// NewTestServer creates a new TestServer, and makes a call to // an optional callback function to modify the configuration. func NewTestServer(t *testing.T, cb ServerConfigCallback) *TestServer { if path, err := exec.LookPath("nomad"); err != nil || path == "" { @@ -167,7 +167,7 @@ func NewTestServer(t *testing.T, cb ServerConfigCallback) *TestServer { HTTPAddr: fmt.Sprintf("127.0.0.1:%d", nomadConfig.Ports.HTTP), SerfAddr: fmt.Sprintf("127.0.0.1:%d", nomadConfig.Ports.Serf), - HttpClient: client, + HTTPClient: client, } // Wait for the server to be ready @@ -195,7 +195,7 @@ func (s *TestServer) Stop() { // but will likely return before a leader is elected. func (s *TestServer) waitForAPI() { WaitForResult(func() (bool, error) { - resp, err := s.HttpClient.Get(s.url("/v1/agent/self")) + resp, err := s.HTTPClient.Get(s.url("/v1/agent/self")) if err != nil { return false, err } @@ -216,7 +216,7 @@ func (s *TestServer) waitForAPI() { func (s *TestServer) waitForLeader() { WaitForResult(func() (bool, error) { // Query the API and check the status code - resp, err := s.HttpClient.Get(s.url("/v1/jobs")) + resp, err := s.HTTPClient.Get(s.url("/v1/jobs")) if err != nil { return false, err } @@ -256,7 +256,7 @@ func (s *TestServer) put(path string, body io.Reader) *http.Response { if err != nil { s.t.Fatalf("err: %s", err) } - resp, err := s.HttpClient.Do(req) + resp, err := s.HTTPClient.Do(req) if err != nil { s.t.Fatalf("err: %s", err) } @@ -269,7 +269,7 @@ func (s *TestServer) put(path string, body io.Reader) *http.Response { // get performs a new HTTP GET request. func (s *TestServer) get(path string) *http.Response { - resp, err := s.HttpClient.Get(s.url(path)) + resp, err := s.HTTPClient.Get(s.url(path)) if err != nil { s.t.Fatalf("err: %s", err) } From 0cb341adfb8d2fe083ec88b37e8b66597d6ea9b3 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Tue, 1 Dec 2015 12:16:21 -0500 Subject: [PATCH 2/8] Make testutil.TestServer work correctly on Windows. --- testutil/server.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/testutil/server.go b/testutil/server.go index 4f5a00302..e2464f244 100644 --- a/testutil/server.go +++ b/testutil/server.go @@ -91,7 +91,7 @@ func defaultServerConfig() *TestServerConfig { // TestServer is the main server wrapper struct. type TestServer struct { - PID int + cmd *exec.Cmd Config *TestServerConfig t *testing.T @@ -117,6 +117,7 @@ func NewTestServer(t *testing.T, cb ServerConfigCallback) *TestServer { defer os.RemoveAll(dataDir) t.Fatalf("err: %s", err) } + defer configFile.Close() nomadConfig := defaultServerConfig() nomadConfig.DataDir = dataDir @@ -162,7 +163,7 @@ func NewTestServer(t *testing.T, cb ServerConfigCallback) *TestServer { server := &TestServer{ Config: nomadConfig, - PID: cmd.Process.Pid, + cmd: cmd, t: t, HTTPAddr: fmt.Sprintf("127.0.0.1:%d", nomadConfig.Ports.HTTP), @@ -184,10 +185,13 @@ func NewTestServer(t *testing.T, cb ServerConfigCallback) *TestServer { func (s *TestServer) Stop() { defer os.RemoveAll(s.Config.DataDir) - cmd := exec.Command("kill", "-9", fmt.Sprintf("%d", s.PID)) - if err := cmd.Run(); err != nil { + if err := s.cmd.Process.Kill(); err != nil { s.t.Errorf("err: %s", err) } + + // wait for the process to exit to be sure that the data dir can be + // deleted on all platforms. + s.cmd.Wait() } // waitForAPI waits for only the agent HTTP endpoint to start From 79f1d76e9241d97fd5076ad726fab0e9ff0e49a1 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Tue, 1 Dec 2015 14:53:01 -0500 Subject: [PATCH 3/8] Test request timing with a server slow enough to measure with low granularity system clocks. --- api/api_test.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ api/util_test.go | 3 --- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/api/api_test.go b/api/api_test.go index 8a64bd974..8f27dc7bd 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1,7 +1,9 @@ package api import ( + "encoding/json" "net/http" + "net/http/httptest" "os" "testing" "time" @@ -46,6 +48,54 @@ func makeClient(t *testing.T, cb1 configCallback, return client, server } +func TestRequestTime(t *testing.T) { + t.Parallel() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(100 * time.Millisecond) + d, err := json.Marshal(struct{ Done bool }{true}) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.Write(d) + })) + defer srv.Close() + + conf := DefaultConfig() + conf.Address = srv.URL + + client, err := NewClient(conf) + if err != nil { + t.Fatalf("err: %v", err) + } + + var out interface{} + + qm, err := client.query("/", &out, nil) + if err != nil { + t.Fatalf("query err: %v", err) + } + if qm.RequestTime == 0 { + t.Errorf("bad request time: %d", qm.RequestTime) + } + + wm, err := client.write("/", struct{ S string }{"input"}, &out, nil) + if err != nil { + t.Fatalf("write err: %v", err) + } + if wm.RequestTime == 0 { + t.Errorf("bad request time: %d", wm.RequestTime) + } + + wm, err = client.delete("/", &out, nil) + if err != nil { + t.Fatalf("delete err: %v", err) + } + if wm.RequestTime == 0 { + t.Errorf("bad request time: %d", wm.RequestTime) + } +} + func TestDefaultConfig_env(t *testing.T) { t.Parallel() url := "http://1.2.3.4:5678" diff --git a/api/util_test.go b/api/util_test.go index a4ebc1e45..1c1073ed7 100644 --- a/api/util_test.go +++ b/api/util_test.go @@ -8,9 +8,6 @@ func assertQueryMeta(t *testing.T, qm *QueryMeta) { if qm.LastIndex == 0 { t.Fatalf("bad index: %d", qm.LastIndex) } - if qm.RequestTime == 0 { - t.Fatalf("bad request time: %d", qm.RequestTime) - } if !qm.KnownLeader { t.Fatalf("expected known leader, got none") } From 780c960dfd8fbd1b1ce136cb3bd8da7cb81dc32a Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Wed, 2 Dec 2015 13:57:40 -0500 Subject: [PATCH 4/8] Work around bugs in package `net` in Go < 1.6. --- command/agent/config.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/command/agent/config.go b/command/agent/config.go index 5f0bfdf78..840b6e83c 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -257,12 +257,28 @@ func DefaultConfig() *Config { } } -// GetListener can be used to get a new listener using a custom bind address. +// Listener can be used to get a new listener using a custom bind address. // If the bind provided address is empty, the BindAddr is used instead. func (c *Config) Listener(proto, addr string, port int) (net.Listener, error) { if addr == "" { addr = c.BindAddr } + + // Do our own range check to avoid bugs in package net. + // + // golang.org/issue/11715 + // golang.org/issue/13447 + // + // Both of the above bugs were fixed by golang.org/cl/12447 which will be + // included in Go 1.6. The error returned below is the same as what Go 1.6 + // will return. + if 0 > port || port > 65535 { + return nil, &net.OpError{ + Op: "listen", + Net: proto, + Err: &net.AddrError{Err: "invalid port", Addr: fmt.Sprint(port)}, + } + } return net.Listen(proto, fmt.Sprintf("%s:%d", addr, port)) } From 38ede674bd366d95665d747e866c9a23508d14e7 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Wed, 2 Dec 2015 13:59:27 -0500 Subject: [PATCH 5/8] Do not leak listeners in test. --- command/agent/config_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index c13ff91f3..961ae9363 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -292,13 +292,16 @@ func TestConfig_Listener(t *testing.T) { config := DefaultConfig() // Fails on invalid input - if _, err := config.Listener("tcp", "nope", 8080); err == nil { + if ln, err := config.Listener("tcp", "nope", 8080); err == nil { + ln.Close() t.Fatalf("expected addr error") } - if _, err := config.Listener("nope", "127.0.0.1", 8080); err == nil { + if ln, err := config.Listener("nope", "127.0.0.1", 8080); err == nil { + ln.Close() t.Fatalf("expected protocol err") } - if _, err := config.Listener("tcp", "127.0.0.1", -1); err == nil { + if ln, err := config.Listener("tcp", "127.0.0.1", -1); err == nil { + ln.Close() t.Fatalf("expected port error") } From d53591052e03f055ea120062f4ec721d66b15daf Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Tue, 8 Dec 2015 08:14:13 -0800 Subject: [PATCH 6/8] Sending the user process sigint during shutdown on linux --- client/driver/executor/exec_linux.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/driver/executor/exec_linux.go b/client/driver/executor/exec_linux.go index 26d2d7145..256cc39f4 100644 --- a/client/driver/executor/exec_linux.go +++ b/client/driver/executor/exec_linux.go @@ -215,8 +215,15 @@ func (e *LinuxExecutor) Wait() *cstructs.WaitResult { return res } +// Shutdown sends the user process an interrupt signal indicating that it is +// about to be forcefully shutdown in sometime func (e *LinuxExecutor) Shutdown() error { - return e.ForceStop() + proc, err := os.FindProcess(e.spawn.UserPid) + if err != nil { + return fmt.Errorf("Failed to find user processes %v: %v", e.spawn.UserPid, err) + } + + return proc.Signal(os.Interrupt) } // ForceStop immediately exits the user process and cleans up both the task From ba3f9dc6a88f89c72f2e05eb38cf03cd568618ae Mon Sep 17 00:00:00 2001 From: Camilo Aguilar Date: Tue, 8 Dec 2015 18:57:06 -0500 Subject: [PATCH 7/8] Sets default syslog facility As advertised by documentation. --- command/agent/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command/agent/config.go b/command/agent/config.go index 5f0bfdf78..6d8c890bb 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -254,6 +254,7 @@ func DefaultConfig() *Config { Server: &ServerConfig{ Enabled: false, }, + SyslogFacility: "LOCAL0", } } From f5dc92daa8d70263d2c54e73e320da6204d632e7 Mon Sep 17 00:00:00 2001 From: Camilo Aguilar Date: Tue, 8 Dec 2015 19:05:15 -0500 Subject: [PATCH 8/8] Addresses Go lint suggestions --- command/agent/config.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 5f0bfdf78..bc295ff06 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -257,7 +257,7 @@ func DefaultConfig() *Config { } } -// GetListener can be used to get a new listener using a custom bind address. +// Listener can be used to get a new listener using a custom bind address. // If the bind provided address is empty, the BindAddr is used instead. func (c *Config) Listener(proto, addr string, port int) (net.Listener, error) { if addr == "" { @@ -267,8 +267,8 @@ func (c *Config) Listener(proto, addr string, port int) (net.Listener, error) { } // Merge merges two configurations. -func (a *Config) Merge(b *Config) *Config { - var result Config = *a +func (c *Config) Merge(b *Config) *Config { + result := *c if b.Region != "" { result.Region = b.Region @@ -371,7 +371,7 @@ func (a *Config) Merge(b *Config) *Config { // Merge is used to merge two server configs together func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { - var result ServerConfig = *a + result := *a if b.Enabled { result.Enabled = true @@ -400,7 +400,7 @@ func (a *ServerConfig) Merge(b *ServerConfig) *ServerConfig { // Merge is used to merge two client configs together func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { - var result ClientConfig = *a + result := *a if b.Enabled { result.Enabled = true @@ -448,7 +448,7 @@ func (a *ClientConfig) Merge(b *ClientConfig) *ClientConfig { // Merge is used to merge two telemetry configs together func (a *Telemetry) Merge(b *Telemetry) *Telemetry { - var result Telemetry = *a + result := *a if b.StatsiteAddr != "" { result.StatsiteAddr = b.StatsiteAddr @@ -464,7 +464,7 @@ func (a *Telemetry) Merge(b *Telemetry) *Telemetry { // Merge is used to merge two port configurations. func (a *Ports) Merge(b *Ports) *Ports { - var result Ports = *a + result := *a if b.HTTP != 0 { result.HTTP = b.HTTP @@ -480,7 +480,7 @@ func (a *Ports) Merge(b *Ports) *Ports { // Merge is used to merge two address configs together. func (a *Addresses) Merge(b *Addresses) *Addresses { - var result Addresses = *a + result := *a if b.HTTP != "" { result.HTTP = b.HTTP @@ -496,7 +496,7 @@ func (a *Addresses) Merge(b *Addresses) *Addresses { // Merge merges two advertise addrs configs together. func (a *AdvertiseAddrs) Merge(b *AdvertiseAddrs) *AdvertiseAddrs { - var result AdvertiseAddrs = *a + result := *a if b.RPC != "" { result.RPC = b.RPC @@ -509,7 +509,7 @@ func (a *AdvertiseAddrs) Merge(b *AdvertiseAddrs) *AdvertiseAddrs { // Merge merges two Atlas configurations together. func (a *AtlasConfig) Merge(b *AtlasConfig) *AtlasConfig { - var result AtlasConfig = *a + result := *a if b.Infrastructure != "" { result.Infrastructure = b.Infrastructure @@ -536,9 +536,8 @@ func LoadConfig(path string) (*Config, error) { if fi.IsDir() { return LoadConfigDir(path) - } else { - return LoadConfigFile(path) } + return LoadConfigFile(path) } // LoadConfigString is used to parse a config string