From c07b129b561b9969f06738fce22d2791ad4a8996 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 15:57:54 +0100 Subject: [PATCH 1/8] Fix reading MaxFails default for UpstreamServer --- client/nginx.go | 4 ++-- tests/client_test.go | 49 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/client/nginx.go b/client/nginx.go index 32c8aff9..4844a8e0 100644 --- a/client/nginx.go +++ b/client/nginx.go @@ -30,7 +30,7 @@ type UpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails int `json:"max_fails"` + MaxFails int `json:"max_fails,omitempty"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } @@ -40,7 +40,7 @@ type StreamUpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails int `json:"max_fails"` + MaxFails int `json:"max_fails,omitempty"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } diff --git a/tests/client_test.go b/tests/client_test.go index 9a2ceb64..1db98398 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -171,9 +171,11 @@ func TestStreamUpstreamServerSlowStart(t *testing.T) { // Add a server with slow_start // (And FailTimeout, since the default is 10s) + // (And MaxFails, since the default is 1) streamServer := client.StreamUpstreamServer{ Server: "127.0.0.1:2000", SlowStart: "11s", + MaxFails: 1, FailTimeout: "10s", } err = c.AddStreamServer(streamUpstream, streamServer) @@ -363,9 +365,11 @@ func TestUpstreamServerSlowStart(t *testing.T) { // Add a server with slow_start // (And FailTimeout, since the default is 10s) + // (And MaxFails, sice the default is 1) server := client.UpstreamServer{ Server: "127.0.0.1:2000", SlowStart: "11s", + MaxFails: 1, FailTimeout: "10s", } err = c.AddHTTPServer(upstream, server) @@ -547,6 +551,51 @@ func TestStreamStats(t *testing.T) { t.Errorf("Couldn't remove stream servers: %v", err) } } + +func TestUpstreamServerDefaultParameters(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + } + + expected := client.UpstreamServer{ + Server: "127.0.0.1:2000", + MaxConns: 0, + MaxFails: 1, + FailTimeout: "10s", + SlowStart: "0s", + } + + err = c.AddHTTPServer(upstream, server) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetHTTPServers(upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(expected, servers[0]) { + t.Errorf("Expected: %v Got: %v", expected, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + func TestKeyValue(t *testing.T) { zoneName := "zone_one" httpClient := &http.Client{} From f2ca2042d1148b212d196bab423b602d6b3a9346 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 16:12:55 +0100 Subject: [PATCH 2/8] Add upstreamServer test coverage --- tests/client_test.go | 245 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 243 insertions(+), 2 deletions(-) diff --git a/tests/client_test.go b/tests/client_test.go index 1db98398..0bfa0c53 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -161,7 +161,6 @@ func TestStreamClient(t *testing.T) { } } -// Test adding the slow_start property on an upstream server func TestStreamUpstreamServerSlowStart(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") @@ -203,6 +202,128 @@ func TestStreamUpstreamServerSlowStart(t *testing.T) { } } +func TestStreamUpstreamServerMaxConns(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with max_conns + // (And SlowStart, since the default is 0s) + // (And MaxFails, since the default is 1) + // (And FailTimeout, since the default is 10s) + streamServer := client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "0s", + MaxConns: 16, + MaxFails: 1, + FailTimeout: "10s", + } + err = c.AddStreamServer(streamUpstream, streamServer) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetStreamServers(streamUpstream) + if err != nil { + t.Fatalf("Error getting stream servers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(streamServer, servers[0]) { + t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + +func TestStreamUpstreamServerMaxFails(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with max_fails + // (And FailTimeout, since the default is 10s) + streamServer := client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "11s", + MaxFails: 32, + FailTimeout: "10s", + } + err = c.AddStreamServer(streamUpstream, streamServer) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetStreamServers(streamUpstream) + if err != nil { + t.Fatalf("Error getting stream servers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(streamServer, servers[0]) { + t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + +func TestStreamUpstreamServerFailTimeout(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with fail_timeout + streamServer := client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "11s", + MaxFails: 1, + FailTimeout: "20s", + } + err = c.AddStreamServer(streamUpstream, streamServer) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetStreamServers(streamUpstream) + if err != nil { + t.Fatalf("Error getting stream servers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(streamServer, servers[0]) { + t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + func TestClient(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") @@ -355,7 +476,127 @@ func TestClient(t *testing.T) { } } -// Test adding the slow_start property on an upstream server +func TestUpstreamServerMaxConns(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with max_conns + // (And FailTimeout, since the default is 10s) + // (And MaxFails, sice the default is 1) + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "11s", + MaxFails: 1, + FailTimeout: "10s", + MaxConns: 64, + } + err = c.AddHTTPServer(upstream, server) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetHTTPServers(upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(server, servers[0]) { + t.Errorf("Expected: %v Got: %v", server, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + +func TestUpstreamServerMaxFails(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with max_fails + // (And FailTimeout, since the default is 10s) + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "11s", + MaxFails: 16, + FailTimeout: "10s", + } + err = c.AddHTTPServer(upstream, server) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetHTTPServers(upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(server, servers[0]) { + t.Errorf("Expected: %v Got: %v", server, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + +func TestUpstreamServerFailTimeout(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + // Add a server with fail_timeout + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "11s", + MaxFails: 16, + FailTimeout: "15s", + } + err = c.AddHTTPServer(upstream, server) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetHTTPServers(upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(server, servers[0]) { + t.Errorf("Expected: %v Got: %v", server, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + func TestUpstreamServerSlowStart(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") From 102ea86f9a7953933f7eb360181278514afad116 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 16:28:01 +0100 Subject: [PATCH 3/8] Refactor tests to use defaultUpstreamServer --- tests/client_test.go | 165 ++++++++++++++++++++++--------------------- 1 file changed, 84 insertions(+), 81 deletions(-) diff --git a/tests/client_test.go b/tests/client_test.go index 0bfa0c53..bc20c9ab 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -169,14 +169,8 @@ func TestStreamUpstreamServerSlowStart(t *testing.T) { } // Add a server with slow_start - // (And FailTimeout, since the default is 10s) - // (And MaxFails, since the default is 1) - streamServer := client.StreamUpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 1, - FailTimeout: "10s", - } + streamServer := createDefaultStreamUpstreamServer() + streamServer.SlowStart = "11s" err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -210,16 +204,8 @@ func TestStreamUpstreamServerMaxConns(t *testing.T) { } // Add a server with max_conns - // (And SlowStart, since the default is 0s) - // (And MaxFails, since the default is 1) - // (And FailTimeout, since the default is 10s) - streamServer := client.StreamUpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "0s", - MaxConns: 16, - MaxFails: 1, - FailTimeout: "10s", - } + streamServer := createDefaultStreamUpstreamServer() + streamServer.MaxConns = 16 err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -253,13 +239,8 @@ func TestStreamUpstreamServerMaxFails(t *testing.T) { } // Add a server with max_fails - // (And FailTimeout, since the default is 10s) - streamServer := client.StreamUpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 32, - FailTimeout: "10s", - } + streamServer := createDefaultStreamUpstreamServer() + streamServer.MaxFails = 32 err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -293,12 +274,8 @@ func TestStreamUpstreamServerFailTimeout(t *testing.T) { } // Add a server with fail_timeout - streamServer := client.StreamUpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 1, - FailTimeout: "20s", - } + streamServer := createDefaultStreamUpstreamServer() + streamServer.FailTimeout = "20s" err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -484,15 +461,8 @@ func TestUpstreamServerMaxConns(t *testing.T) { } // Add a server with max_conns - // (And FailTimeout, since the default is 10s) - // (And MaxFails, sice the default is 1) - server := client.UpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 1, - FailTimeout: "10s", - MaxConns: 64, - } + server := createDefaultUpstreamServer() + server.MaxConns = 64 err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -526,13 +496,8 @@ func TestUpstreamServerMaxFails(t *testing.T) { } // Add a server with max_fails - // (And FailTimeout, since the default is 10s) - server := client.UpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 16, - FailTimeout: "10s", - } + server := createDefaultUpstreamServer() + server.MaxFails = 16 err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -566,12 +531,8 @@ func TestUpstreamServerFailTimeout(t *testing.T) { } // Add a server with fail_timeout - server := client.UpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 16, - FailTimeout: "15s", - } + server := createDefaultUpstreamServer() + server.FailTimeout = "15s" err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -605,14 +566,8 @@ func TestUpstreamServerSlowStart(t *testing.T) { } // Add a server with slow_start - // (And FailTimeout, since the default is 10s) - // (And MaxFails, sice the default is 1) - server := client.UpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "11s", - MaxFails: 1, - FailTimeout: "10s", - } + server := createDefaultUpstreamServer() + server.SlowStart = "11s" err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -723,6 +678,43 @@ func TestStats(t *testing.T) { } } +func TestUpstreamServerDefaultParameters(t *testing.T) { + httpClient := &http.Client{} + c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") + if err != nil { + t.Fatalf("Error connecting to nginx: %v", err) + } + + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + } + + expected := createDefaultUpstreamServer() + err = c.AddHTTPServer(upstream, server) + if err != nil { + t.Errorf("Error adding upstream server: %v", err) + } + servers, err := c.GetHTTPServers(upstream) + if err != nil { + t.Fatalf("Error getting HTTPServers: %v", err) + } + if len(servers) != 1 { + t.Errorf("Too many servers") + } + // don't compare IDs + servers[0].ID = 0 + + if !reflect.DeepEqual(expected, servers[0]) { + t.Errorf("Expected: %v Got: %v", expected, servers[0]) + } + + // remove upstream servers + _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + if err != nil { + t.Errorf("Couldn't remove servers: %v", err) + } +} + func TestStreamStats(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") @@ -793,47 +785,40 @@ func TestStreamStats(t *testing.T) { } } -func TestUpstreamServerDefaultParameters(t *testing.T) { +func TestStreamUpstreamServerDefaultParameters(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") if err != nil { t.Fatalf("Error connecting to nginx: %v", err) } - server := client.UpstreamServer{ + streamServer := client.StreamUpstreamServer{ Server: "127.0.0.1:2000", } - expected := client.UpstreamServer{ - Server: "127.0.0.1:2000", - MaxConns: 0, - MaxFails: 1, - FailTimeout: "10s", - SlowStart: "0s", - } - - err = c.AddHTTPServer(upstream, server) + expected := createDefaultStreamUpstreamServer() + err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) } - servers, err := c.GetHTTPServers(upstream) + streamServers, err := c.GetStreamServers(streamUpstream) if err != nil { - t.Fatalf("Error getting HTTPServers: %v", err) + t.Fatalf("Error getting stream servers: %v", err) } - if len(servers) != 1 { + if len(streamServers) != 1 { t.Errorf("Too many servers") } // don't compare IDs - servers[0].ID = 0 + streamServers[0].ID = 0 - if !reflect.DeepEqual(expected, servers[0]) { - t.Errorf("Expected: %v Got: %v", expected, servers[0]) + if !reflect.DeepEqual(expected, streamServers[0]) { + t.Errorf("Expected: %v Got: %v", expected, streamServers[0]) } - // remove upstream servers - _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) + // cleanup stream upstream servers + _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) if err != nil { - t.Errorf("Couldn't remove servers: %v", err) + t.Errorf("Couldn't remove stream servers: %v", err) } } @@ -1158,3 +1143,21 @@ func compareStreamUpstreamServers(x []client.StreamUpstreamServer, y []client.St return reflect.DeepEqual(xServers, yServers) } + +func createDefaultUpstreamServer() client.UpstreamServer { + return client.UpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "0s", + MaxFails: 1, + FailTimeout: "10s", + } +} + +func createDefaultStreamUpstreamServer() client.StreamUpstreamServer { + return client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "0s", + MaxFails: 1, + FailTimeout: "10s", + } +} From 51efbf1a71014efe1ade6547eca5e609f9cb71a0 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 21:13:15 +0100 Subject: [PATCH 4/8] Fix MaxFails set --- client/nginx.go | 14 ++++++++++++-- tests/client_test.go | 10 ++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/client/nginx.go b/client/nginx.go index 4844a8e0..ca108610 100644 --- a/client/nginx.go +++ b/client/nginx.go @@ -30,7 +30,7 @@ type UpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails int `json:"max_fails,omitempty"` + MaxFails *int `json:"max_fails"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } @@ -40,7 +40,7 @@ type StreamUpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails int `json:"max_fails,omitempty"` + MaxFails *int `json:"max_fails"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } @@ -386,6 +386,11 @@ func (client *NginxClient) AddHTTPServer(upstream string, server UpstreamServer) return fmt.Errorf("failed to add %v server to %v upstream: server already exists", server.Server, upstream) } + if server.MaxFails == nil { + defaultMaxFails := 1 + server.MaxFails = &defaultMaxFails + } + path := fmt.Sprintf("http/upstreams/%v/servers/", upstream) err = client.post(path, &server) if err != nil { @@ -611,6 +616,11 @@ func (client *NginxClient) AddStreamServer(upstream string, server StreamUpstrea return fmt.Errorf("failed to add %v stream server to %v upstream: server already exists", server.Server, upstream) } + if server.MaxFails == nil { + defaultMaxFails := 1 + server.MaxFails = &defaultMaxFails + } + path := fmt.Sprintf("stream/upstreams/%v/servers/", upstream) err = client.post(path, &server) if err != nil { diff --git a/tests/client_test.go b/tests/client_test.go index bc20c9ab..a803aa59 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -240,7 +240,7 @@ func TestStreamUpstreamServerMaxFails(t *testing.T) { // Add a server with max_fails streamServer := createDefaultStreamUpstreamServer() - streamServer.MaxFails = 32 + *streamServer.MaxFails = 32 err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -497,7 +497,7 @@ func TestUpstreamServerMaxFails(t *testing.T) { // Add a server with max_fails server := createDefaultUpstreamServer() - server.MaxFails = 16 + *server.MaxFails = 16 err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -1145,19 +1145,21 @@ func compareStreamUpstreamServers(x []client.StreamUpstreamServer, y []client.St } func createDefaultUpstreamServer() client.UpstreamServer { + defaultMaxFails := 1 return client.UpstreamServer{ Server: "127.0.0.1:2000", SlowStart: "0s", - MaxFails: 1, + MaxFails: &defaultMaxFails, FailTimeout: "10s", } } func createDefaultStreamUpstreamServer() client.StreamUpstreamServer { + defaultMaxFails := 1 return client.StreamUpstreamServer{ Server: "127.0.0.1:2000", SlowStart: "0s", - MaxFails: 1, + MaxFails: &defaultMaxFails, FailTimeout: "10s", } } From a1bef581cf8e7befb03a87b172c39070ba970321 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 21:19:55 +0100 Subject: [PATCH 5/8] Combine upstreamServer related tests --- tests/client_test.go | 233 ++++--------------------------------------- 1 file changed, 17 insertions(+), 216 deletions(-) diff --git a/tests/client_test.go b/tests/client_test.go index a803aa59..dcfeebf4 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -161,121 +161,21 @@ func TestStreamClient(t *testing.T) { } } -func TestStreamUpstreamServerSlowStart(t *testing.T) { +func TestStreamUpstreamServer(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") if err != nil { t.Fatalf("Error connecting to nginx: %v", err) } - // Add a server with slow_start - streamServer := createDefaultStreamUpstreamServer() - streamServer.SlowStart = "11s" - err = c.AddStreamServer(streamUpstream, streamServer) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetStreamServers(streamUpstream) - if err != nil { - t.Fatalf("Error getting stream servers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(streamServer, servers[0]) { - t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestStreamUpstreamServerMaxConns(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) - } - - // Add a server with max_conns - streamServer := createDefaultStreamUpstreamServer() - streamServer.MaxConns = 16 - err = c.AddStreamServer(streamUpstream, streamServer) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetStreamServers(streamUpstream) - if err != nil { - t.Fatalf("Error getting stream servers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(streamServer, servers[0]) { - t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestStreamUpstreamServerMaxFails(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) - } - - // Add a server with max_fails - streamServer := createDefaultStreamUpstreamServer() - *streamServer.MaxFails = 32 - err = c.AddStreamServer(streamUpstream, streamServer) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetStreamServers(streamUpstream) - if err != nil { - t.Fatalf("Error getting stream servers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(streamServer, servers[0]) { - t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestStreamUpstreamServerFailTimeout(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) + maxFails := 64 + streamServer := client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + MaxConns: 321, + MaxFails: &maxFails, + FailTimeout: "21s", + SlowStart: "12s", } - - // Add a server with fail_timeout - streamServer := createDefaultStreamUpstreamServer() - streamServer.FailTimeout = "20s" err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -453,112 +353,7 @@ func TestClient(t *testing.T) { } } -func TestUpstreamServerMaxConns(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) - } - - // Add a server with max_conns - server := createDefaultUpstreamServer() - server.MaxConns = 64 - err = c.AddHTTPServer(upstream, server) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetHTTPServers(upstream) - if err != nil { - t.Fatalf("Error getting HTTPServers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(server, servers[0]) { - t.Errorf("Expected: %v Got: %v", server, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestUpstreamServerMaxFails(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) - } - - // Add a server with max_fails - server := createDefaultUpstreamServer() - *server.MaxFails = 16 - err = c.AddHTTPServer(upstream, server) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetHTTPServers(upstream) - if err != nil { - t.Fatalf("Error getting HTTPServers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(server, servers[0]) { - t.Errorf("Expected: %v Got: %v", server, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestUpstreamServerFailTimeout(t *testing.T) { - httpClient := &http.Client{} - c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") - if err != nil { - t.Fatalf("Error connecting to nginx: %v", err) - } - - // Add a server with fail_timeout - server := createDefaultUpstreamServer() - server.FailTimeout = "15s" - err = c.AddHTTPServer(upstream, server) - if err != nil { - t.Errorf("Error adding upstream server: %v", err) - } - servers, err := c.GetHTTPServers(upstream) - if err != nil { - t.Fatalf("Error getting HTTPServers: %v", err) - } - if len(servers) != 1 { - t.Errorf("Too many servers") - } - // don't compare IDs - servers[0].ID = 0 - - if !reflect.DeepEqual(server, servers[0]) { - t.Errorf("Expected: %v Got: %v", server, servers[0]) - } - - // remove upstream servers - _, _, err = c.UpdateHTTPServers(upstream, []client.UpstreamServer{}) - if err != nil { - t.Errorf("Couldn't remove servers: %v", err) - } -} - -func TestUpstreamServerSlowStart(t *testing.T) { +func TestUpstreamServer(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") if err != nil { @@ -566,8 +361,14 @@ func TestUpstreamServerSlowStart(t *testing.T) { } // Add a server with slow_start - server := createDefaultUpstreamServer() - server.SlowStart = "11s" + maxFails := 64 + server := client.UpstreamServer{ + Server: "127.0.0.1:2000", + MaxConns: 321, + MaxFails: &maxFails, + FailTimeout: "21s", + SlowStart: "12s", + } err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) From 4365af8307468f1147bc88e3e1ac14341a798f16 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 21:31:19 +0100 Subject: [PATCH 6/8] Make defaultMaxFail global. Remove helpers --- client/nginx.go | 4 ++-- tests/client_test.go | 36 ++++++++++++++---------------------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/client/nginx.go b/client/nginx.go index ca108610..86bdf265 100644 --- a/client/nginx.go +++ b/client/nginx.go @@ -17,6 +17,8 @@ const pathNotFoundCode = "PathNotFound" const streamContext = true const httpContext = false +var defaultMaxFails = 1 + // NginxClient lets you access NGINX Plus API. type NginxClient struct { apiEndpoint string @@ -387,7 +389,6 @@ func (client *NginxClient) AddHTTPServer(upstream string, server UpstreamServer) } if server.MaxFails == nil { - defaultMaxFails := 1 server.MaxFails = &defaultMaxFails } @@ -617,7 +618,6 @@ func (client *NginxClient) AddStreamServer(upstream string, server StreamUpstrea } if server.MaxFails == nil { - defaultMaxFails := 1 server.MaxFails = &defaultMaxFails } diff --git a/tests/client_test.go b/tests/client_test.go index dcfeebf4..4a0c90ca 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -16,6 +16,8 @@ const ( streamZoneSync = "zone_test_sync" ) +var defaultMaxFails = 1 + func TestStreamClient(t *testing.T) { httpClient := &http.Client{} c, err := client.NewNginxClient(httpClient, "http://127.0.0.1:8080/api") @@ -490,7 +492,12 @@ func TestUpstreamServerDefaultParameters(t *testing.T) { Server: "127.0.0.1:2000", } - expected := createDefaultUpstreamServer() + expected := client.UpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "0s", + MaxFails: &defaultMaxFails, + FailTimeout: "10s", + } err = c.AddHTTPServer(upstream, server) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -597,7 +604,12 @@ func TestStreamUpstreamServerDefaultParameters(t *testing.T) { Server: "127.0.0.1:2000", } - expected := createDefaultStreamUpstreamServer() + expected := client.StreamUpstreamServer{ + Server: "127.0.0.1:2000", + SlowStart: "0s", + MaxFails: &defaultMaxFails, + FailTimeout: "10s", + } err = c.AddStreamServer(streamUpstream, streamServer) if err != nil { t.Errorf("Error adding upstream server: %v", err) @@ -944,23 +956,3 @@ func compareStreamUpstreamServers(x []client.StreamUpstreamServer, y []client.St return reflect.DeepEqual(xServers, yServers) } - -func createDefaultUpstreamServer() client.UpstreamServer { - defaultMaxFails := 1 - return client.UpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "0s", - MaxFails: &defaultMaxFails, - FailTimeout: "10s", - } -} - -func createDefaultStreamUpstreamServer() client.StreamUpstreamServer { - defaultMaxFails := 1 - return client.StreamUpstreamServer{ - Server: "127.0.0.1:2000", - SlowStart: "0s", - MaxFails: &defaultMaxFails, - FailTimeout: "10s", - } -} From f33fe9614443ae3e12554a28279142ce4cb0d1cd Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Tue, 20 Aug 2019 23:43:15 +0100 Subject: [PATCH 7/8] Cleanup --- tests/client_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/client_test.go b/tests/client_test.go index 4a0c90ca..ba81ed5a 100644 --- a/tests/client_test.go +++ b/tests/client_test.go @@ -196,7 +196,7 @@ func TestStreamUpstreamServer(t *testing.T) { t.Errorf("Expected: %v Got: %v", streamServer, servers[0]) } - // remove upstream servers + // remove stream upstream servers _, _, err = c.UpdateStreamServers(streamUpstream, []client.StreamUpstreamServer{}) if err != nil { t.Errorf("Couldn't remove servers: %v", err) @@ -362,7 +362,6 @@ func TestUpstreamServer(t *testing.T) { t.Fatalf("Error connecting to nginx: %v", err) } - // Add a server with slow_start maxFails := 64 server := client.UpstreamServer{ Server: "127.0.0.1:2000", From 8715745005a3be1948b23628af3a6865bb9034e1 Mon Sep 17 00:00:00 2001 From: Dean Coakley Date: Thu, 22 Aug 2019 10:45:13 +0100 Subject: [PATCH 8/8] Get defaults from NGINX Plus api --- client/nginx.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/client/nginx.go b/client/nginx.go index 86bdf265..8741664a 100644 --- a/client/nginx.go +++ b/client/nginx.go @@ -17,8 +17,6 @@ const pathNotFoundCode = "PathNotFound" const streamContext = true const httpContext = false -var defaultMaxFails = 1 - // NginxClient lets you access NGINX Plus API. type NginxClient struct { apiEndpoint string @@ -32,7 +30,7 @@ type UpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails *int `json:"max_fails"` + MaxFails *int `json:"max_fails,omitempty"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } @@ -42,7 +40,7 @@ type StreamUpstreamServer struct { ID int `json:"id,omitempty"` Server string `json:"server"` MaxConns int `json:"max_conns"` - MaxFails *int `json:"max_fails"` + MaxFails *int `json:"max_fails,omitempty"` FailTimeout string `json:"fail_timeout,omitempty"` SlowStart string `json:"slow_start,omitempty"` } @@ -388,10 +386,6 @@ func (client *NginxClient) AddHTTPServer(upstream string, server UpstreamServer) return fmt.Errorf("failed to add %v server to %v upstream: server already exists", server.Server, upstream) } - if server.MaxFails == nil { - server.MaxFails = &defaultMaxFails - } - path := fmt.Sprintf("http/upstreams/%v/servers/", upstream) err = client.post(path, &server) if err != nil { @@ -617,10 +611,6 @@ func (client *NginxClient) AddStreamServer(upstream string, server StreamUpstrea return fmt.Errorf("failed to add %v stream server to %v upstream: server already exists", server.Server, upstream) } - if server.MaxFails == nil { - server.MaxFails = &defaultMaxFails - } - path := fmt.Sprintf("stream/upstreams/%v/servers/", upstream) err = client.post(path, &server) if err != nil {