From b784246dfb012b510f6d855088cd29a368c5a069 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 29 Jun 2021 16:49:55 +0000 Subject: [PATCH 001/123] Add basic /healthcheck directory structure --- healthcheck/README.md | 0 healthcheck/healthcheck.go | 6 ++++++ 2 files changed, 6 insertions(+) create mode 100644 healthcheck/README.md create mode 100644 healthcheck/healthcheck.go diff --git a/healthcheck/README.md b/healthcheck/README.md new file mode 100644 index 000000000..e69de29bb diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go new file mode 100644 index 000000000..9c4dd5da8 --- /dev/null +++ b/healthcheck/healthcheck.go @@ -0,0 +1,6 @@ +package healthcheck + +var ( + live bool + ready bool +) From fe5a7bc1255687662af0ce1b0e1748372d84cbd2 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 16:57:51 +0000 Subject: [PATCH 002/123] Initial commit of healthcheck --- healthcheck/healthcheck.go | 1 + 1 file changed, 1 insertion(+) create mode 100644 healthcheck/healthcheck.go diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go new file mode 100644 index 000000000..85f0393b7 --- /dev/null +++ b/healthcheck/healthcheck.go @@ -0,0 +1 @@ +package main \ No newline at end of file From fa57263d982ba09397d53569746faa3eb4aeb17f Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 17:28:15 +0000 Subject: [PATCH 003/123] create function to initialize HTTP endpoints --- healthcheck/healthcheck.go | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 9c4dd5da8..aa24bdf81 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -1,6 +1,30 @@ package healthcheck +import "net/http" + var ( live bool ready bool ) + +func initHealthCheck() { + http.HandleFunc("/readiness", func(w http.ResponseWriter, r *http.Request) { + if ready { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } + }) + + http.HandleFunc("/liveness", func(w http.ResponseWriter, r *http.Request) { + if live { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } + }) + + go http.ListenAndServe(":8080", nil) +} From 0d5d6aef0104939fc933e8722d1af4409e7ddc03 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 18:08:50 +0000 Subject: [PATCH 004/123] add documentation for package healthcheck --- healthcheck/healthcheck.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index aa24bdf81..d7f9ec6fd 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -1,3 +1,5 @@ +// Package healthcheck tests and communicates the health of the Cloud SQL +// Proxy Client. package healthcheck import "net/http" @@ -9,21 +11,21 @@ var ( func initHealthCheck() { http.HandleFunc("/readiness", func(w http.ResponseWriter, r *http.Request) { - if ready { - w.WriteHeader(200) - w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - } + if ready { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } }) http.HandleFunc("/liveness", func(w http.ResponseWriter, r *http.Request) { - if live { - w.WriteHeader(200) - w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - } + if live { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } }) go http.ListenAndServe(":8080", nil) From b03576afb4e6d5eeac25d1badedeca287e5b74b0 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 18:18:29 +0000 Subject: [PATCH 005/123] add flag indicating whether to use healthcheck or not --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 177d3e234..62afb54ab 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -132,6 +132,9 @@ unavailable.`, `When set, the proxy uses this host as the base API path. Example: https://sqladmin.googleapis.com`, ) + + // Settings for healthcheck + useHealthCheck = flag.Bool("use_health_check", false, "When set, periodically checks and communicates the health of the proxy client.") ) const ( From c0a954d31e28d4bdefe0cc8036ebdb672185559c Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 29 Jun 2021 19:19:19 +0000 Subject: [PATCH 006/123] Add basic structure for liveness + readiness tests --- healthcheck/healthcheck.go | 55 +++++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index aa24bdf81..db7709baa 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -1,6 +1,23 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This file contains health checks for the Cloud SQL Auth proxy. package healthcheck -import "net/http" +import ( + "net/http" +) var ( live bool @@ -9,22 +26,34 @@ var ( func initHealthCheck() { http.HandleFunc("/readiness", func(w http.ResponseWriter, r *http.Request) { - if ready { - w.WriteHeader(200) - w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - } + ready = readinessTest() + if ready { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } }) http.HandleFunc("/liveness", func(w http.ResponseWriter, r *http.Request) { - if live { - w.WriteHeader(200) - w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - } + live = livenessTest() + if live { + w.WriteHeader(200) + w.Write([]byte("ok\n")) + } else { + w.WriteHeader(500) + } }) go http.ListenAndServe(":8080", nil) } + +// livenessTest returns true as long as the proxy is running +func livenessTest() bool { + return true +} + +// TODO(monazhn): Proxy is not ready when MaxConnections has been reached +func readinessTest() bool { + return true +} From c5fe04d077d007e07684ae8f48c0fa0e11fbe2a3 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 19:22:22 +0000 Subject: [PATCH 007/123] Initial commit --- healthcheck/healthcheck_test.go | 1 + 1 file changed, 1 insertion(+) create mode 100644 healthcheck/healthcheck_test.go diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go new file mode 100644 index 000000000..f17fc6764 --- /dev/null +++ b/healthcheck/healthcheck_test.go @@ -0,0 +1 @@ +package healthcheck \ No newline at end of file From 36388e30d2fa902273823d4b24ec4d7aceafb360 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 29 Jun 2021 22:03:50 +0000 Subject: [PATCH 008/123] add exported function for proxy client to sya its ready --- healthcheck/healthcheck.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 452d5581e..2f678503d 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -48,6 +48,10 @@ func InitHealthCheck() { go http.ListenAndServe(":8080", nil) } +func Ready() { + ready = true +} + // livenessTest returns true as long as the proxy is running func livenessTest() bool { return true @@ -56,4 +60,4 @@ func livenessTest() bool { // TODO(monazhn): Proxy is not ready when MaxConnections has been reached func readinessTest() bool { return true -} +} \ No newline at end of file From 30ea13ee1efadcc5ded75461c15b36e3b43967d2 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 30 Jun 2021 16:21:22 +0000 Subject: [PATCH 009/123] make proxy succeed readiness test when ready for new connections --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 5 +++++ healthcheck/healthcheck.go | 7 +++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 62afb54ab..5d504a91b 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -34,6 +34,7 @@ import ( "syscall" "time" + "github.com/GoogleCloudPlatform/cloudsql-proxy/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/certs" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/fuse" @@ -628,6 +629,10 @@ func main() { } logging.Infof("Ready for new connections") + + if *useHealthCheck { + healthcheck.NotifyReady() + } signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGTERM, syscall.SIGINT) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 2f678503d..08ee29903 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -26,8 +26,7 @@ var ( func InitHealthCheck() { http.HandleFunc("/readiness", func(w http.ResponseWriter, r *http.Request) { - ready = readinessTest() - if ready { + if readinessTest() { w.WriteHeader(200) w.Write([]byte("ok\n")) } else { @@ -48,7 +47,7 @@ func InitHealthCheck() { go http.ListenAndServe(":8080", nil) } -func Ready() { +func NotifyReady() { ready = true } @@ -59,5 +58,5 @@ func livenessTest() bool { // TODO(monazhn): Proxy is not ready when MaxConnections has been reached func readinessTest() bool { - return true + return ready } \ No newline at end of file From 5c5e5403c37459ab068dffa6b338302376166a0f Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 30 Jun 2021 18:01:05 +0000 Subject: [PATCH 010/123] Add hc struct, max connections check for readiness --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 12 ++++- healthcheck/healthcheck.go | 63 +++++++++++++++++++------- 2 files changed, 57 insertions(+), 18 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 5d504a91b..ebaa7e5e5 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -460,6 +460,7 @@ func Main(timeout time.Duration) { } func main() { + flag.Parse() if *version { @@ -591,6 +592,12 @@ func main() { RefreshCfgBuffer: refreshCfgBuffer, } + var hc *healthcheck.HealthCheck + + if *useHealthCheck { + hc = healthcheck.InitHealthCheck(proxyClient) + } + // Initialize a source of new connections to Cloud SQL instances. var connSrc <-chan proxy.Conn if *useFuse { @@ -629,9 +636,10 @@ func main() { } logging.Infof("Ready for new connections") - + if *useHealthCheck { - healthcheck.NotifyReady() + healthcheck.NotifyReady(hc) + logging.Infof("Ready notification sent") } signals := make(chan os.Signal, 1) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 08ee29903..1a9e1aa95 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -17,38 +17,54 @@ package healthcheck import ( "net/http" -) + "sync/atomic" -var ( - live bool - ready bool + "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) -func InitHealthCheck() { - http.HandleFunc("/readiness", func(w http.ResponseWriter, r *http.Request) { - if readinessTest() { +type HealthCheck struct { + live bool + ready bool + startup bool +} + +func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { + + hc := &HealthCheck{ + live: true, + ready: false, + startup: false, + } + + http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { + hc.ready = readinessTest(proxyClient, hc) + if hc.ready { w.WriteHeader(200) w.Write([]byte("ok\n")) } else { w.WriteHeader(500) + w.Write([]byte("error\n")) } }) - http.HandleFunc("/liveness", func(w http.ResponseWriter, r *http.Request) { - live = livenessTest() - if live { + http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { + hc.live = livenessTest() + if hc.live { w.WriteHeader(200) w.Write([]byte("ok\n")) } else { w.WriteHeader(500) + w.Write([]byte("error\n")) } }) go http.ListenAndServe(":8080", nil) + + return hc } -func NotifyReady() { - ready = true +func NotifyReady(hc *HealthCheck) { + hc.startup = true } // livenessTest returns true as long as the proxy is running @@ -56,7 +72,22 @@ func livenessTest() bool { return true } -// TODO(monazhn): Proxy is not ready when MaxConnections has been reached -func readinessTest() bool { - return ready -} \ No newline at end of file +// readinessTest checks for the proxy having started up, but not having reached MaxConnections +func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { + + if !hc.startup { + return false + } + + // Parts of this code is taken from client.go + active := atomic.AddUint64(&proxyClient.ConnectionsCounter, 1) + + // Defer decrementing ConnectionsCounter upon connections closing + defer atomic.AddUint64(&proxyClient.ConnectionsCounter, ^uint64(0)) + + if proxyClient.MaxConnections > 0 && active > proxyClient.MaxConnections { + return false + } + + return true +} From e77ecc8e14d498c6c185a12302509cc2632c841f Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 30 Jun 2021 18:33:40 +0000 Subject: [PATCH 011/123] Error check ListenAndServe --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 -- healthcheck/healthcheck.go | 8 +++++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index ebaa7e5e5..0a1596bb8 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -460,7 +460,6 @@ func Main(timeout time.Duration) { } func main() { - flag.Parse() if *version { @@ -639,7 +638,6 @@ func main() { if *useHealthCheck { healthcheck.NotifyReady(hc) - logging.Infof("Ready notification sent") } signals := make(chan os.Signal, 1) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 1a9e1aa95..b76ad05cd 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -19,6 +19,7 @@ import ( "net/http" "sync/atomic" + "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) @@ -58,7 +59,12 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { } }) - go http.ListenAndServe(":8080", nil) + go func() { + err := http.ListenAndServe(":8080", nil) + if err != nil { + logging.Errorf("Failed to start endpoint(s).") + } + }() return hc } From 405fbff755b5aca3f204e70edb6c91df8c3006e0 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 30 Jun 2021 18:57:23 +0000 Subject: [PATCH 012/123] Add comments to healthcheck.go --- healthcheck/healthcheck.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index b76ad05cd..c261c4894 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -23,10 +23,15 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +// HealthCheck is a type used to implement health checks for the proxy. type HealthCheck struct { - live bool - ready bool - startup bool + // live and ready correspond to liveness and readiness probing in Kubernetes + // health checks + live bool + ready bool + // started is used to support readiness probing and should not be confused + // for relating to startup probing. + started bool } func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { @@ -34,9 +39,10 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { hc := &HealthCheck{ live: true, ready: false, - startup: false, + started: false, } + // Handlers used to set up HTTP endpoint for communicating proxy health. http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { hc.ready = readinessTest(proxyClient, hc) if hc.ready { @@ -70,25 +76,27 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { } func NotifyReady(hc *HealthCheck) { - hc.startup = true + hc.started = true } -// livenessTest returns true as long as the proxy is running +// livenessTest returns true as long as the proxy is running. func livenessTest() bool { return true } -// readinessTest checks for the proxy having started up, but not having reached MaxConnections +// readinessTest checks several criteria before determining the proxy is ready. func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { - if !hc.startup { + // Wait until the 'Ready For Connections' log to mark the proxy client as ready. + if !hc.started { return false } - // Parts of this code is taken from client.go + // Mark not ready if the proxy client is at MaxConnections + // (Parts of this code are taken from client.go) active := atomic.AddUint64(&proxyClient.ConnectionsCounter, 1) - // Defer decrementing ConnectionsCounter upon connections closing + // Defer decrementing ConnectionsCounter upon connections closing. defer atomic.AddUint64(&proxyClient.ConnectionsCounter, ^uint64(0)) if proxyClient.MaxConnections > 0 && active > proxyClient.MaxConnections { From b3836b657404525dec2801880e6d38e906afa7af Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 30 Jun 2021 20:09:25 +0000 Subject: [PATCH 013/123] set up testing --- healthcheck/healthcheck_test.go | 47 ++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index f17fc6764..d67999d8c 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1 +1,46 @@ -package healthcheck \ No newline at end of file +package healthcheck + +import ( + "net/http/httptest" + "testing" + + "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" +) + +func newClient(mc uint64) *proxy.Client { + return &proxy.Client{ + MaxConnections: mc, + } +} + +func newHealthCheck(l bool, r bool, s bool) *HealthCheck { + return &HealthCheck{ + live: l, + ready: r, + started: s, + } +} + +func TestLiveness(t *testing.T) { + +} + +func TestUnexpectedTermination(t *testing.T) { + +} + +func TestBadStartup(t *testing.T) { + +} + +func TestNotifyReady(t *testing.T) { + +} + +func TestReadiness(t *testing.T) { + +} + +func TestMaxConnections(t *testing.T) { + +} \ No newline at end of file From d0be8f7f8cd80f0a41288895035c26177e575ad2 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 30 Jun 2021 23:39:22 +0000 Subject: [PATCH 014/123] wrote tests for liveness, bad startup, and successful startup --- healthcheck/healthcheck_test.go | 44 ++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index d67999d8c..2e3972676 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1,6 +1,7 @@ package healthcheck import ( + "net/http" "net/http/httptest" "testing" @@ -22,19 +23,56 @@ func newHealthCheck(l bool, r bool, s bool) *HealthCheck { } func TestLiveness(t *testing.T) { - + proxyClient := newClient(0) + InitHealthCheck(proxyClient) + + resp, err := http.Get("/liveness") + if err != nil { + t.Errorf("failed to GET from /liveness") + } + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } + if resp.Status != "ok\n" { + t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) + } } func TestUnexpectedTermination(t *testing.T) { - + } func TestBadStartup(t *testing.T) { + proxyClient := newClient(0) + InitHealthCheck(proxyClient) + resp, err := http.Get("/readiness") + if err != nil { + t.Errorf("failed to GET from /readiness") + } + if resp.StatusCode != 500 { + t.Errorf("got status code %v instead of 500", resp.StatusCode) + } + if resp.Status != "error\n" { + t.Errorf("got status %v instead of \"error\\n\"", resp.Status) + } } -func TestNotifyReady(t *testing.T) { +func TestSuccessfulStartup(t *testing.T) { + proxyClient := newClient(0) + hc := InitHealthCheck(proxyClient) + NotifyReady(hc) + resp, err := http.Get("/readiness") + if err != nil { + t.Errorf("failed to GET from /readiness") + } + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } + if resp.Status != "ok\n" { + t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) + } } func TestReadiness(t *testing.T) { From a31adb3b95fd2b591e808a0abe51d78ce338bb4c Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 1 Jul 2021 17:03:37 +0000 Subject: [PATCH 015/123] Include tests for Readiness and MaxConnections --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- healthcheck/healthcheck.go | 51 +++++++++++++++++++++----- healthcheck/healthcheck_test.go | 40 +++++++++++++------- 3 files changed, 69 insertions(+), 24 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 0a1596bb8..490c2a7f6 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -637,7 +637,7 @@ func main() { logging.Infof("Ready for new connections") if *useHealthCheck { - healthcheck.NotifyReady(hc) + healthcheck.NotifyReadyForConnections(hc) } signals := make(chan os.Signal, 1) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index c261c4894..d033adc11 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -17,7 +17,6 @@ package healthcheck import ( "net/http" - "sync/atomic" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -68,14 +67,14 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { go func() { err := http.ListenAndServe(":8080", nil) if err != nil { - logging.Errorf("Failed to start endpoint(s).") + logging.Errorf("Failed to start endpoint(s): %v", err) } }() return hc } -func NotifyReady(hc *HealthCheck) { +func NotifyReadyForConnections(hc *HealthCheck) { hc.started = true } @@ -93,15 +92,47 @@ func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { } // Mark not ready if the proxy client is at MaxConnections - // (Parts of this code are taken from client.go) - active := atomic.AddUint64(&proxyClient.ConnectionsCounter, 1) - - // Defer decrementing ConnectionsCounter upon connections closing. - defer atomic.AddUint64(&proxyClient.ConnectionsCounter, ^uint64(0)) - - if proxyClient.MaxConnections > 0 && active > proxyClient.MaxConnections { + if proxyClient.MaxConnections > 0 && proxyClient.ConnectionsCounter >= proxyClient.MaxConnections { return false } return true } + +// Broken/Unfinished/Unused Test for Unexpected Termination +/*func TestUnexpectedTermination(t *testing.T) { + proxyClient := newClient(0) + InitHealthCheck(proxyClient) + + resp, err := http.Get("http://localhost:8080/liveness") + if err != nil { + t.Errorf("failed to GET from /liveness: %v", err) + } + + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } + + shutdown := make(chan bool, 1) + go func() { + proxyClient.Shutdown(0) + shutdown <- true + }() + finishedShutdown := false + select { + case <-time.After(100 * time.Millisecond): + case finishedShutdown = <-shutdown: + } + if !finishedShutdown { + t.Errorf("Shutdown should have been quick because no connections needed to be closed.") + } + + //TODO(monazhn): close health check endpoints + + resp, err = http.Get("http://localhost:8080/liveness") + if err == nil { + t.Log(resp.StatusCode) // TEST IS FAILING... need a CloseHealthCheck function + close them in cloud_sql_proxy.go + t.Errorf("GET /liveness is expected to, but does not return an error after Shutdown is complete.") + } + +}*/ diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 2e3972676..fe0fd0e2d 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -2,7 +2,6 @@ package healthcheck import ( "net/http" - "net/http/httptest" "testing" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -14,14 +13,6 @@ func newClient(mc uint64) *proxy.Client { } } -func newHealthCheck(l bool, r bool, s bool) *HealthCheck { - return &HealthCheck{ - live: l, - ready: r, - started: s, - } -} - func TestLiveness(t *testing.T) { proxyClient := newClient(0) InitHealthCheck(proxyClient) @@ -53,15 +44,15 @@ func TestBadStartup(t *testing.T) { if resp.StatusCode != 500 { t.Errorf("got status code %v instead of 500", resp.StatusCode) } - if resp.Status != "error\n" { - t.Errorf("got status %v instead of \"error\\n\"", resp.Status) + if resp.Status != "ok\n" { + t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) } } func TestSuccessfulStartup(t *testing.T) { proxyClient := newClient(0) hc := InitHealthCheck(proxyClient) - NotifyReady(hc) + NotifyReadyForConnections(hc) resp, err := http.Get("/readiness") if err != nil { @@ -76,9 +67,32 @@ func TestSuccessfulStartup(t *testing.T) { } func TestReadiness(t *testing.T) { + proxyClient := newClient(0) // No specified limit for MaxConnections (MaxConnections check always passes) + hc := InitHealthCheck(proxyClient) + NotifyReadyForConnections(hc) // Set hc.started = true + resp, err := http.Get("http://localhost:8080/readiness") + if err != nil { + t.Errorf("failed to GET from /readiness") + } + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } } func TestMaxConnections(t *testing.T) { + proxyClient := newClient(10) // MaxConnections == 10 + hc := InitHealthCheck(proxyClient) + NotifyReadyForConnections(hc) + + proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections -} \ No newline at end of file + resp, err := http.Get("http://localhost:8080/readiness") + if err != nil { + t.Errorf("failed to GET from /readiness") + } + + if resp.StatusCode != 500 { + t.Errorf("got status code %v instead of 500", resp.StatusCode) + } +} From c47bbc4bd90cc5b8a82d22c878c51d6208434379 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 17:11:28 +0000 Subject: [PATCH 016/123] fixing tests --- healthcheck/healthcheck.go | 11 +++++- healthcheck/healthcheck_test.go | 59 ++++++++++++++++++--------------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index c261c4894..4a7f3655f 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -16,6 +16,7 @@ package healthcheck import ( + "fmt" "net/http" "sync/atomic" @@ -35,7 +36,7 @@ type HealthCheck struct { } func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { - + fmt.Printf("initializing healthcheck\n") hc := &HealthCheck{ live: true, ready: false, @@ -44,6 +45,7 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { // Handlers used to set up HTTP endpoint for communicating proxy health. http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { + fmt.Printf("hit readiness endpoint\n") hc.ready = readinessTest(proxyClient, hc) if hc.ready { w.WriteHeader(200) @@ -55,6 +57,7 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { }) http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { + fmt.Printf("hit liveness endpoint\n") hc.live = livenessTest() if hc.live { w.WriteHeader(200) @@ -65,6 +68,8 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { } }) + fmt.Printf("endpoints created\n") + go func() { err := http.ListenAndServe(":8080", nil) if err != nil { @@ -76,19 +81,23 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { } func NotifyReady(hc *HealthCheck) { + fmt.Printf("ready notification received\n") hc.started = true } // livenessTest returns true as long as the proxy is running. func livenessTest() bool { + fmt.Printf("running liveness test\n") return true } // readinessTest checks several criteria before determining the proxy is ready. func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { + fmt.Printf("running readiness test\n") // Wait until the 'Ready For Connections' log to mark the proxy client as ready. if !hc.started { + fmt.Printf("readiness test failed because startup was not completed\n") return false } diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 2e3972676..c6c5feeca 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1,6 +1,7 @@ package healthcheck import ( + "log" "net/http" "net/http/httptest" "testing" @@ -8,6 +9,10 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +var handler = func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "something failed", http.StatusInternalServerError) +} + func newClient(mc uint64) *proxy.Client { return &proxy.Client{ MaxConnections: mc, @@ -23,18 +28,18 @@ func newHealthCheck(l bool, r bool, s bool) *HealthCheck { } func TestLiveness(t *testing.T) { - proxyClient := newClient(0) - InitHealthCheck(proxyClient) + InitHealthCheck(newClient(0)) - resp, err := http.Get("/liveness") + req, err := http.NewRequest("GET", "127.0.0.1:8080/liveness", nil) if err != nil { - t.Errorf("failed to GET from /liveness") - } - if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) + log.Fatal(err) } - if resp.Status != "ok\n" { - t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) + + resp := httptest.NewRecorder() + handler(resp, req) + + if resp.Code != 200 { + t.Errorf("got status code %v instead of 200", resp.Code) } } @@ -43,35 +48,35 @@ func TestUnexpectedTermination(t *testing.T) { } func TestBadStartup(t *testing.T) { - proxyClient := newClient(0) - InitHealthCheck(proxyClient) + InitHealthCheck(newClient(0)) - resp, err := http.Get("/readiness") + req, err := http.NewRequest("GET", "/readiness", nil) if err != nil { - t.Errorf("failed to GET from /readiness") - } - if resp.StatusCode != 500 { - t.Errorf("got status code %v instead of 500", resp.StatusCode) + log.Fatal(err) } - if resp.Status != "error\n" { - t.Errorf("got status %v instead of \"error\\n\"", resp.Status) + + resp := httptest.NewRecorder() + handler(resp, req) + + if resp.Code != 500 { + t.Errorf("got status code %v instead of 500", resp.Code) } } func TestSuccessfulStartup(t *testing.T) { - proxyClient := newClient(0) - hc := InitHealthCheck(proxyClient) + hc := InitHealthCheck(newClient(0)) NotifyReady(hc) - resp, err := http.Get("/readiness") + req, err := http.NewRequest("GET", "/readiness", nil) if err != nil { - t.Errorf("failed to GET from /readiness") - } - if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) + log.Fatal(err) } - if resp.Status != "ok\n" { - t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) + + resp := httptest.NewRecorder() + handler(resp, req) + + if resp.Code != 200 { + t.Errorf("got status code %v instead of 200", resp.Code) } } From 9a0328b440737bd75f4f30f83eca413c21b983a4 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 1 Jul 2021 17:20:43 +0000 Subject: [PATCH 017/123] Revert code pushed by mistake :-( --- healthcheck/healthcheck.go | 40 +-------------------------------- healthcheck/healthcheck_test.go | 4 ++-- 2 files changed, 3 insertions(+), 41 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index d033adc11..f10e0d6ef 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -97,42 +97,4 @@ func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { } return true -} - -// Broken/Unfinished/Unused Test for Unexpected Termination -/*func TestUnexpectedTermination(t *testing.T) { - proxyClient := newClient(0) - InitHealthCheck(proxyClient) - - resp, err := http.Get("http://localhost:8080/liveness") - if err != nil { - t.Errorf("failed to GET from /liveness: %v", err) - } - - if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) - } - - shutdown := make(chan bool, 1) - go func() { - proxyClient.Shutdown(0) - shutdown <- true - }() - finishedShutdown := false - select { - case <-time.After(100 * time.Millisecond): - case finishedShutdown = <-shutdown: - } - if !finishedShutdown { - t.Errorf("Shutdown should have been quick because no connections needed to be closed.") - } - - //TODO(monazhn): close health check endpoints - - resp, err = http.Get("http://localhost:8080/liveness") - if err == nil { - t.Log(resp.StatusCode) // TEST IS FAILING... need a CloseHealthCheck function + close them in cloud_sql_proxy.go - t.Errorf("GET /liveness is expected to, but does not return an error after Shutdown is complete.") - } - -}*/ +} \ No newline at end of file diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index fe0fd0e2d..6258c58a7 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -44,8 +44,8 @@ func TestBadStartup(t *testing.T) { if resp.StatusCode != 500 { t.Errorf("got status code %v instead of 500", resp.StatusCode) } - if resp.Status != "ok\n" { - t.Errorf("got status %v instead of \"ok\\n\"", resp.Status) + if resp.Status != "error\n" { + t.Errorf("got status %v instead of \"error\\n\"", resp.Status) } } From b75abed97a5ad50cba6314d8aca2d40c707e9933 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 1 Jul 2021 17:51:11 +0000 Subject: [PATCH 018/123] Change test for MaxConnections to be more complete --- healthcheck/healthcheck_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 75e03361e..27a2c6464 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -24,9 +24,6 @@ func TestLiveness(t *testing.T) { if resp.StatusCode != 200 { t.Errorf("got status code %v instead of 200", resp.StatusCode) } - if resp.Status != "200 OK" { - t.Errorf("got status \"%v\" instead of \"200 OK\"", resp.Status) - } } func TestUnexpectedTermination(t *testing.T) { @@ -44,9 +41,6 @@ func TestBadStartup(t *testing.T) { if resp.StatusCode != 500 { t.Errorf("got status code %v instead of 500", resp.StatusCode) } - if resp.Status != "500 Internal Server Error" { - t.Errorf("got status \"%v\" instead of \"500 Internal Server Error\"", resp.Status) - } } func TestSuccessfulStartup(t *testing.T) { @@ -61,9 +55,6 @@ func TestSuccessfulStartup(t *testing.T) { if resp.StatusCode != 200 { t.Errorf("got status code %v instead of 200", resp.StatusCode) } - if resp.Status != "200 OK" { - t.Errorf("got status \"%v\" instead of \"200 OK\"", resp.Status) - } } func TestMaxConnections(t *testing.T) { @@ -71,9 +62,18 @@ func TestMaxConnections(t *testing.T) { hc := InitHealthCheck(proxyClient) NotifyReadyForConnections(hc) + resp, err := http.Get("http://localhost:8080/readiness") + if err != nil { + t.Fatal(err) + } + + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } + proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections - resp, err := http.Get("http://localhost:8080/readiness") + resp, err = http.Get("http://localhost:8080/readiness") if err != nil { t.Fatal(err) } From 200a92b1a6a0603703b18afe6a6f044f1ecd3c0d Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 1 Jul 2021 20:53:30 +0000 Subject: [PATCH 019/123] Remove unwanted test --- healthcheck/healthcheck.go | 2 -- healthcheck/healthcheck_test.go | 4 ---- 2 files changed, 6 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index f10e0d6ef..aa794f79c 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -34,7 +34,6 @@ type HealthCheck struct { } func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { - hc := &HealthCheck{ live: true, ready: false, @@ -85,7 +84,6 @@ func livenessTest() bool { // readinessTest checks several criteria before determining the proxy is ready. func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { - // Wait until the 'Ready For Connections' log to mark the proxy client as ready. if !hc.started { return false diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 27a2c6464..de2dd3d88 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -26,10 +26,6 @@ func TestLiveness(t *testing.T) { } } -func TestUnexpectedTermination(t *testing.T) { - -} - func TestBadStartup(t *testing.T) { proxyClient := newClient(0) InitHealthCheck(proxyClient) From d832fb73f8402186cf3433913c5c9cf55ccbe3dc Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 1 Jul 2021 20:56:32 +0000 Subject: [PATCH 020/123] Add License information --- healthcheck/healthcheck_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index de2dd3d88..4af6fd43e 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1,3 +1,17 @@ +// Copyright 2015 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package healthcheck import ( @@ -77,4 +91,4 @@ func TestMaxConnections(t *testing.T) { if resp.StatusCode != 500 { t.Errorf("got status code %v instead of 500", resp.StatusCode) } -} \ No newline at end of file +} From 10033a5231d6657b17f80b99856723e04f095152 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:03:48 +0000 Subject: [PATCH 021/123] fix copyright year --- healthcheck/healthcheck.go | 2 +- healthcheck/healthcheck_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index aa794f79c..5f182b069 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -1,4 +1,4 @@ -// Copyright 2015 Google Inc. All Rights Reserved. +// Copyright 2021 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 4af6fd43e..23d4fe0a0 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1,4 +1,4 @@ -// Copyright 2015 Google Inc. All Rights Reserved. +// Copyright 2021 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From f14b4f24160ad92794cd9771b4693305b4fec2a6 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:14:05 +0000 Subject: [PATCH 022/123] use early returns instead of else --- healthcheck/healthcheck.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 5f182b069..a84cbcb48 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -17,6 +17,7 @@ package healthcheck import ( "net/http" + "sync" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -36,8 +37,6 @@ type HealthCheck struct { func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { hc := &HealthCheck{ live: true, - ready: false, - started: false, } // Handlers used to set up HTTP endpoint for communicating proxy health. @@ -46,10 +45,10 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { if hc.ready { w.WriteHeader(200) w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - w.Write([]byte("error\n")) + return } + w.WriteHeader(500) + w.Write([]byte("error\n")) }) http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { @@ -57,10 +56,10 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { if hc.live { w.WriteHeader(200) w.Write([]byte("ok\n")) - } else { - w.WriteHeader(500) - w.Write([]byte("error\n")) + return } + w.WriteHeader(500) + w.Write([]byte("error\n")) }) go func() { From 0b989c159eaea7e1efdb9151d0133369696f3516 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:16:17 +0000 Subject: [PATCH 023/123] use early return on error case --- healthcheck/healthcheck.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index a84cbcb48..dae983ce6 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -42,24 +42,24 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { // Handlers used to set up HTTP endpoint for communicating proxy health. http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { hc.ready = readinessTest(proxyClient, hc) - if hc.ready { - w.WriteHeader(200) - w.Write([]byte("ok\n")) + if !hc.ready { + w.WriteHeader(500) + w.Write([]byte("error\n")) return } - w.WriteHeader(500) - w.Write([]byte("error\n")) + w.WriteHeader(200) + w.Write([]byte("ok\n")) }) http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { hc.live = livenessTest() - if hc.live { - w.WriteHeader(200) - w.Write([]byte("ok\n")) + if !hc.live { + w.WriteHeader(500) + w.Write([]byte("error\n")) return } - w.WriteHeader(500) - w.Write([]byte("error\n")) + w.WriteHeader(200) + w.Write([]byte("ok\n")) }) go func() { From 234dd7a6daec4558432310e8cb4b9f3d2c1fb00e Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:41:42 +0000 Subject: [PATCH 024/123] sync reads and writes --- healthcheck/healthcheck.go | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index dae983ce6..e4ea01a70 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -18,11 +18,18 @@ package healthcheck import ( "net/http" "sync" + "sync/atomic" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +var ( + readinessMutex = &sync.Mutex{} + livenessMutex = &sync.Mutex{} + startupMutex = &sync.Mutex{} +) + // HealthCheck is a type used to implement health checks for the proxy. type HealthCheck struct { // live and ready correspond to liveness and readiness probing in Kubernetes @@ -36,28 +43,36 @@ type HealthCheck struct { func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { hc := &HealthCheck{ - live: true, + live: true, } // Handlers used to set up HTTP endpoint for communicating proxy health. http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { + readinessMutex.Lock() hc.ready = readinessTest(proxyClient, hc) if !hc.ready { + readinessMutex.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } + readinessMutex.Unlock() + w.WriteHeader(200) w.Write([]byte("ok\n")) }) http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { + livenessMutex.Lock() hc.live = livenessTest() if !hc.live { + livenessMutex.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } + livenessMutex.Unlock() + w.WriteHeader(200) w.Write([]byte("ok\n")) }) @@ -73,7 +88,9 @@ func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { } func NotifyReadyForConnections(hc *HealthCheck) { + startupMutex.Lock() hc.started = true + startupMutex.Unlock() } // livenessTest returns true as long as the proxy is running. @@ -84,14 +101,16 @@ func livenessTest() bool { // readinessTest checks several criteria before determining the proxy is ready. func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { // Wait until the 'Ready For Connections' log to mark the proxy client as ready. + startupMutex.Lock() if !hc.started { + startupMutex.Unlock() return false } // Mark not ready if the proxy client is at MaxConnections - if proxyClient.MaxConnections > 0 && proxyClient.ConnectionsCounter >= proxyClient.MaxConnections { + if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { return false } return true -} \ No newline at end of file +} From 599fd096cf7b971ed6d9fadbf918f42d6220f742 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:42:34 +0000 Subject: [PATCH 025/123] rename InitHealthCheck to NewHealthCheck --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- healthcheck/healthcheck.go | 2 +- healthcheck/healthcheck_test.go | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 490c2a7f6..2c37ffca7 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -594,7 +594,7 @@ func main() { var hc *healthcheck.HealthCheck if *useHealthCheck { - hc = healthcheck.InitHealthCheck(proxyClient) + hc = healthcheck.NewHealthCheck(proxyClient) } // Initialize a source of new connections to Cloud SQL instances. diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index e4ea01a70..e0d124868 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -41,7 +41,7 @@ type HealthCheck struct { started bool } -func InitHealthCheck(proxyClient *proxy.Client) *HealthCheck { +func NewHealthCheck(proxyClient *proxy.Client) *HealthCheck { hc := &HealthCheck{ live: true, } diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 23d4fe0a0..8b1bd5fc6 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -29,7 +29,7 @@ func newClient(mc uint64) *proxy.Client { func TestLiveness(t *testing.T) { proxyClient := newClient(0) - InitHealthCheck(proxyClient) + NewHealthCheck(proxyClient) resp, err := http.Get("http://localhost:8080/liveness") if err != nil { @@ -42,7 +42,7 @@ func TestLiveness(t *testing.T) { func TestBadStartup(t *testing.T) { proxyClient := newClient(0) - InitHealthCheck(proxyClient) + NewHealthCheck(proxyClient) resp, err := http.Get("http://localhost:8080/readiness") if err != nil { @@ -55,7 +55,7 @@ func TestBadStartup(t *testing.T) { func TestSuccessfulStartup(t *testing.T) { proxyClient := newClient(0) - hc := InitHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient) NotifyReadyForConnections(hc) resp, err := http.Get("http://localhost:8080/readiness") @@ -69,7 +69,7 @@ func TestSuccessfulStartup(t *testing.T) { func TestMaxConnections(t *testing.T) { proxyClient := newClient(10) // MaxConnections == 10 - hc := InitHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient) NotifyReadyForConnections(hc) resp, err := http.Get("http://localhost:8080/readiness") From ff2c39ba99822b1786c3ee46bdd0c81dc2db8fa4 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 22:46:49 +0000 Subject: [PATCH 026/123] forgot an unlock --- healthcheck/healthcheck.go | 1 + 1 file changed, 1 insertion(+) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index e0d124868..c43960437 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -106,6 +106,7 @@ func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { startupMutex.Unlock() return false } + startupMutex.Unlock() // Mark not ready if the proxy client is at MaxConnections if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { From 5835b2cd1277e0c8a81f850c5d4f707b0b2141c7 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 1 Jul 2021 23:03:37 +0000 Subject: [PATCH 027/123] add pointer receiver to NotifyReadyForConnections() --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 6 ++---- healthcheck/healthcheck.go | 18 ++++++++++-------- healthcheck/healthcheck_test.go | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 2c37ffca7..73b2eb52a 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -591,7 +591,7 @@ func main() { RefreshCfgBuffer: refreshCfgBuffer, } - var hc *healthcheck.HealthCheck + var hc *healthcheck.HC if *useHealthCheck { hc = healthcheck.NewHealthCheck(proxyClient) @@ -636,9 +636,7 @@ func main() { logging.Infof("Ready for new connections") - if *useHealthCheck { - healthcheck.NotifyReadyForConnections(hc) - } + hc.NotifyReadyForConnections() signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGTERM, syscall.SIGINT) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index c43960437..e4f4dd0f3 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -31,7 +31,7 @@ var ( ) // HealthCheck is a type used to implement health checks for the proxy. -type HealthCheck struct { +type HC struct { // live and ready correspond to liveness and readiness probing in Kubernetes // health checks live bool @@ -41,8 +41,8 @@ type HealthCheck struct { started bool } -func NewHealthCheck(proxyClient *proxy.Client) *HealthCheck { - hc := &HealthCheck{ +func NewHealthCheck(proxyClient *proxy.Client) *HC { + hc := &HC{ live: true, } @@ -87,10 +87,12 @@ func NewHealthCheck(proxyClient *proxy.Client) *HealthCheck { return hc } -func NotifyReadyForConnections(hc *HealthCheck) { - startupMutex.Lock() - hc.started = true - startupMutex.Unlock() +func (hc *HC) NotifyReadyForConnections() { + if hc != nil { + startupMutex.Lock() + hc.started = true + startupMutex.Unlock() + } } // livenessTest returns true as long as the proxy is running. @@ -99,7 +101,7 @@ func livenessTest() bool { } // readinessTest checks several criteria before determining the proxy is ready. -func readinessTest(proxyClient *proxy.Client, hc *HealthCheck) bool { +func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Wait until the 'Ready For Connections' log to mark the proxy client as ready. startupMutex.Lock() if !hc.started { diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 8b1bd5fc6..7130e1e07 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -56,7 +56,7 @@ func TestBadStartup(t *testing.T) { func TestSuccessfulStartup(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - NotifyReadyForConnections(hc) + hc.NotifyReadyForConnections() resp, err := http.Get("http://localhost:8080/readiness") if err != nil { @@ -70,7 +70,7 @@ func TestSuccessfulStartup(t *testing.T) { func TestMaxConnections(t *testing.T) { proxyClient := newClient(10) // MaxConnections == 10 hc := NewHealthCheck(proxyClient) - NotifyReadyForConnections(hc) + hc.NotifyReadyForConnections() resp, err := http.Get("http://localhost:8080/readiness") if err != nil { From 1f8e64ee5a7409fdda9288170c61247bac4d7a4d Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 6 Jul 2021 16:14:19 +0000 Subject: [PATCH 028/123] GoDoc for exported functions --- healthcheck/healthcheck.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index e4f4dd0f3..6d54061d5 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -41,6 +41,8 @@ type HC struct { started bool } +// NewHealthCheck initializes a HC object and exposes the appropriate HTTP endpoints +// for communicating proxy health. func NewHealthCheck(proxyClient *proxy.Client) *HC { hc := &HC{ live: true, @@ -87,6 +89,8 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { return hc } +// NotifyReadyForConnections changes the value of 'started' in a health +// check object to true, marking the proxy as done starting up. func (hc *HC) NotifyReadyForConnections() { if hc != nil { startupMutex.Lock() @@ -110,7 +114,7 @@ func readinessTest(proxyClient *proxy.Client, hc *HC) bool { } startupMutex.Unlock() - // Mark not ready if the proxy client is at MaxConnections + // Mark not ready if the proxy client is at MaxConnections. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { return false } From 0efecc024b8c916b9319acd452471b8e2ae166a9 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 6 Jul 2021 16:21:44 +0000 Subject: [PATCH 029/123] Keep reference to server --- healthcheck/healthcheck.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 6d54061d5..c431f7312 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -39,13 +39,20 @@ type HC struct { // started is used to support readiness probing and should not be confused // for relating to startup probing. started bool + // srv is a pointer to the HTTP server + srv *http.Server } // NewHealthCheck initializes a HC object and exposes the appropriate HTTP endpoints // for communicating proxy health. func NewHealthCheck(proxyClient *proxy.Client) *HC { + srv := &http.Server{ + Addr: ":8080", // TODO: Pick a good port. + } + hc := &HC{ live: true, + srv: srv, } // Handlers used to set up HTTP endpoint for communicating proxy health. @@ -80,8 +87,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { }) go func() { - err := http.ListenAndServe(":8080", nil) - if err != nil { + if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { logging.Errorf("Failed to start endpoint(s): %v", err) } }() From dfdfe3572113301532193a4665e6f3b431bcd397 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 6 Jul 2021 17:35:17 +0000 Subject: [PATCH 030/123] Define consts for URLs, paths, port numbers --- healthcheck/healthcheck.go | 10 +++++++--- healthcheck/healthcheck_test.go | 13 ++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index c431f7312..6b1b71b6d 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -24,6 +24,10 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +const livenessPath = "/liveness" +const readinessPath = "/readiness" +const portNum = ":8080" // TODO(monazhn): Think about a good port number. + var ( readinessMutex = &sync.Mutex{} livenessMutex = &sync.Mutex{} @@ -47,7 +51,7 @@ type HC struct { // for communicating proxy health. func NewHealthCheck(proxyClient *proxy.Client) *HC { srv := &http.Server{ - Addr: ":8080", // TODO: Pick a good port. + Addr: portNum, // TODO: Pick a good port. } hc := &HC{ @@ -56,7 +60,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { } // Handlers used to set up HTTP endpoint for communicating proxy health. - http.HandleFunc("/readiness", func(w http.ResponseWriter, _ *http.Request) { + http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { readinessMutex.Lock() hc.ready = readinessTest(proxyClient, hc) if !hc.ready { @@ -71,7 +75,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { w.Write([]byte("ok\n")) }) - http.HandleFunc("/liveness", func(w http.ResponseWriter, _ *http.Request) { + http.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { livenessMutex.Lock() hc.live = livenessTest() if !hc.live { diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 7130e1e07..ac1243ca7 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -21,6 +21,9 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +const livenessURL = "http://localhost:8080/liveness" +const readinessURL = "http://localhost:8080/readiness" + func newClient(mc uint64) *proxy.Client { return &proxy.Client{ MaxConnections: mc, @@ -31,7 +34,7 @@ func TestLiveness(t *testing.T) { proxyClient := newClient(0) NewHealthCheck(proxyClient) - resp, err := http.Get("http://localhost:8080/liveness") + resp, err := http.Get(livenessURL) if err != nil { t.Fatal(err) } @@ -44,7 +47,7 @@ func TestBadStartup(t *testing.T) { proxyClient := newClient(0) NewHealthCheck(proxyClient) - resp, err := http.Get("http://localhost:8080/readiness") + resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) } @@ -58,7 +61,7 @@ func TestSuccessfulStartup(t *testing.T) { hc := NewHealthCheck(proxyClient) hc.NotifyReadyForConnections() - resp, err := http.Get("http://localhost:8080/readiness") + resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) } @@ -72,7 +75,7 @@ func TestMaxConnections(t *testing.T) { hc := NewHealthCheck(proxyClient) hc.NotifyReadyForConnections() - resp, err := http.Get("http://localhost:8080/readiness") + resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) } @@ -83,7 +86,7 @@ func TestMaxConnections(t *testing.T) { proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections - resp, err = http.Get("http://localhost:8080/readiness") + resp, err = http.Get(readinessURL) if err != nil { t.Fatal(err) } From b3bc07005c07939325d35d1cc0c8e6b80fd870eb Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 6 Jul 2021 17:47:39 +0000 Subject: [PATCH 031/123] Close health check after each test --- healthcheck/healthcheck.go | 10 ++++++++++ healthcheck/healthcheck_test.go | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 6b1b71b6d..62aca1421 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -16,6 +16,7 @@ package healthcheck import ( + "context" "net/http" "sync" "sync/atomic" @@ -99,6 +100,15 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { return hc } +// CloseHealthCheck gracefully shuts down the HTTP server belonging to the HC object. +func (hc *HC) CloseHealthCheck() { + if hc != nil { + if err := hc.srv.Shutdown(context.Background()); err != nil { + logging.Errorf("Failed to shut down health check: ", err) + } + } +} + // NotifyReadyForConnections changes the value of 'started' in a health // check object to true, marking the proxy as done starting up. func (hc *HC) NotifyReadyForConnections() { diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index ac1243ca7..c75440bfb 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -32,7 +32,8 @@ func newClient(mc uint64) *proxy.Client { func TestLiveness(t *testing.T) { proxyClient := newClient(0) - NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient) + defer hc.CloseHealthCheck() resp, err := http.Get(livenessURL) if err != nil { @@ -45,7 +46,8 @@ func TestLiveness(t *testing.T) { func TestBadStartup(t *testing.T) { proxyClient := newClient(0) - NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient) + defer hc.CloseHealthCheck() resp, err := http.Get(readinessURL) if err != nil { @@ -59,6 +61,8 @@ func TestBadStartup(t *testing.T) { func TestSuccessfulStartup(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) + defer hc.CloseHealthCheck() + hc.NotifyReadyForConnections() resp, err := http.Get(readinessURL) @@ -73,6 +77,8 @@ func TestSuccessfulStartup(t *testing.T) { func TestMaxConnections(t *testing.T) { proxyClient := newClient(10) // MaxConnections == 10 hc := NewHealthCheck(proxyClient) + defer hc.CloseHealthCheck() + hc.NotifyReadyForConnections() resp, err := http.Get(readinessURL) @@ -95,3 +101,25 @@ func TestMaxConnections(t *testing.T) { t.Errorf("got status code %v instead of 500", resp.StatusCode) } } + +func TestCloseHealthCheck(t *testing.T) { + proxyClient := newClient(0) + hc := NewHealthCheck(proxyClient) + defer hc.CloseHealthCheck() + + resp, err := http.Get(livenessURL) + if err != nil { + t.Fatal(err) + } + + if resp.StatusCode != 200 { + t.Errorf("got status code %v instead of 200", resp.StatusCode) + } + + hc.CloseHealthCheck() + + _, err = http.Get(livenessURL) + if err == nil { // If NO error + t.Fatal(err) + } +} From e6f929f02d453de55f1b304b0c8bcf8ba911a3a2 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 6 Jul 2021 18:02:41 +0000 Subject: [PATCH 032/123] Fix for flaky tests. --- healthcheck/healthcheck.go | 12 ++++++------ healthcheck/healthcheck_test.go | 13 ++++++++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 62aca1421..2d1ab3d96 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -31,11 +31,11 @@ const portNum = ":8080" // TODO(monazhn): Think about a good port number. var ( readinessMutex = &sync.Mutex{} - livenessMutex = &sync.Mutex{} - startupMutex = &sync.Mutex{} + livenessMutex = &sync.Mutex{} + startupMutex = &sync.Mutex{} ) -// HealthCheck is a type used to implement health checks for the proxy. +// HC is a type used to implement health checks for the proxy. type HC struct { // live and ready correspond to liveness and readiness probing in Kubernetes // health checks @@ -86,7 +86,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { return } livenessMutex.Unlock() - + w.WriteHeader(200) w.Write([]byte("ok\n")) }) @@ -126,7 +126,7 @@ func livenessTest() bool { // readinessTest checks several criteria before determining the proxy is ready. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { - // Wait until the 'Ready For Connections' log to mark the proxy client as ready. + // Wait until the 'Ready For Connections' log to mark the proxy as ready. startupMutex.Lock() if !hc.started { startupMutex.Unlock() @@ -134,7 +134,7 @@ func readinessTest(proxyClient *proxy.Client, hc *HC) bool { } startupMutex.Unlock() - // Mark not ready if the proxy client is at MaxConnections. + // Mark not ready if the proxy is at MaxConnections. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { return false } diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index c75440bfb..54c6b4233 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -17,6 +17,7 @@ package healthcheck import ( "net/http" "testing" + "time" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) @@ -33,7 +34,9 @@ func newClient(mc uint64) *proxy.Client { func TestLiveness(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() + defer hc.CloseHealthCheck() // Close health check upon exiting the test. + + time.Sleep(100 * time.Millisecond) // Wait for ListenAndServe to begin to avoid flaky tests. resp, err := http.Get(livenessURL) if err != nil { @@ -49,6 +52,8 @@ func TestBadStartup(t *testing.T) { hc := NewHealthCheck(proxyClient) defer hc.CloseHealthCheck() + time.Sleep(100 * time.Millisecond) + resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) @@ -63,6 +68,8 @@ func TestSuccessfulStartup(t *testing.T) { hc := NewHealthCheck(proxyClient) defer hc.CloseHealthCheck() + time.Sleep(100 * time.Millisecond) + hc.NotifyReadyForConnections() resp, err := http.Get(readinessURL) @@ -79,6 +86,8 @@ func TestMaxConnections(t *testing.T) { hc := NewHealthCheck(proxyClient) defer hc.CloseHealthCheck() + time.Sleep(100 * time.Millisecond) + hc.NotifyReadyForConnections() resp, err := http.Get(readinessURL) @@ -107,6 +116,8 @@ func TestCloseHealthCheck(t *testing.T) { hc := NewHealthCheck(proxyClient) defer hc.CloseHealthCheck() + time.Sleep(100 * time.Millisecond) + resp, err := http.Get(livenessURL) if err != nil { t.Fatal(err) From 0dac6c5d6d6e72f4d2bc8c3b802b22c9862e332e Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 7 Jul 2021 13:34:30 +0000 Subject: [PATCH 033/123] "Factored" const declarations --- healthcheck/healthcheck.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 2d1ab3d96..efcd35bb3 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -25,9 +25,11 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) -const livenessPath = "/liveness" -const readinessPath = "/readiness" -const portNum = ":8080" // TODO(monazhn): Think about a good port number. +const ( + livenessPath = "/liveness" + readinessPath = "/readiness" + portNum = ":8080" // TODO(monazhn): Think about a good port number. +) var ( readinessMutex = &sync.Mutex{} @@ -52,7 +54,7 @@ type HC struct { // for communicating proxy health. func NewHealthCheck(proxyClient *proxy.Client) *HC { srv := &http.Server{ - Addr: portNum, // TODO: Pick a good port. + Addr: portNum, } hc := &HC{ From 633f95c092df67741cbcdf0b8f9448cde02e374b Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 7 Jul 2021 15:58:51 +0000 Subject: [PATCH 034/123] Add informative error logs when readiness fails Useful for client-side debugging. --- healthcheck/healthcheck.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index efcd35bb3..3b29b4014 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -132,12 +132,14 @@ func readinessTest(proxyClient *proxy.Client, hc *HC) bool { startupMutex.Lock() if !hc.started { startupMutex.Unlock() + logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } startupMutex.Unlock() - // Mark not ready if the proxy is at MaxConnections. + // Mark as not ready if the proxy is at the optional MaxConnections limit. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { + logging.Errorf("Readiness failed because proxy has reached the maximum connections limit (%d).", proxyClient.MaxConnections) return false } From 076329c5b20dc4fcfd4cc2d360afd12a9e5e8644 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 7 Jul 2021 16:58:30 +0000 Subject: [PATCH 035/123] styling --- healthcheck/healthcheck_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 54c6b4233..c00c35d58 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -21,9 +21,11 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) - -const livenessURL = "http://localhost:8080/liveness" -const readinessURL = "http://localhost:8080/readiness" + +const ( + livenessURL = "http://localhost:8080/liveness" + readinessURL = "http://localhost:8080/readiness" +) func newClient(mc uint64) *proxy.Client { return &proxy.Client{ @@ -94,7 +96,6 @@ func TestMaxConnections(t *testing.T) { if err != nil { t.Fatal(err) } - if resp.StatusCode != 200 { t.Errorf("got status code %v instead of 200", resp.StatusCode) } @@ -105,7 +106,6 @@ func TestMaxConnections(t *testing.T) { if err != nil { t.Fatal(err) } - if resp.StatusCode != 500 { t.Errorf("got status code %v instead of 500", resp.StatusCode) } @@ -122,7 +122,6 @@ func TestCloseHealthCheck(t *testing.T) { if err != nil { t.Fatal(err) } - if resp.StatusCode != 200 { t.Errorf("got status code %v instead of 200", resp.StatusCode) } From 5b1c3ab99f1130e4510b406d2c637cba21cbcd34 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 8 Jul 2021 15:25:57 +0000 Subject: [PATCH 036/123] Add comments --- healthcheck/healthcheck.go | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 3b29b4014..a198c17a3 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -39,19 +39,25 @@ var ( // HC is a type used to implement health checks for the proxy. type HC struct { - // live and ready correspond to liveness and readiness probing in Kubernetes - // health checks + // live being true means the proxy is running; in the case of the proxy + // being unexpectedly terminated, we should (re)start the proxy. + // live is related to Kubernetes liveness probing. live bool + // ready being true means the proxy is ready to serve new traffic; in the + // case that ready is false, we should wait to send new traffic to the + // proxy. The value of ready determines the success or failure of + // Kubernetes readiness probing. ready bool - // started is used to support readiness probing and should not be confused - // for relating to startup probing. + // started is a flag used to support readiness probing and should not be + // confused for affecting startup probing. When started becomes true, the + // proxy is done starting up. started bool - // srv is a pointer to the HTTP server + // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server } -// NewHealthCheck initializes a HC object and exposes the appropriate HTTP endpoints -// for communicating proxy health. +// NewHealthCheck initializes a HC object and exposes HTTP endpoints used to +// communicate proxy health. func NewHealthCheck(proxyClient *proxy.Client) *HC { srv := &http.Server{ Addr: portNum, @@ -62,7 +68,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { srv: srv, } - // Handlers used to set up HTTP endpoint for communicating proxy health. + // Handlers used to set up HTTP endpoints. http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { readinessMutex.Lock() hc.ready = readinessTest(proxyClient, hc) @@ -102,7 +108,8 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { return hc } -// CloseHealthCheck gracefully shuts down the HTTP server belonging to the HC object. +// CloseHealthCheck gracefully shuts down the HTTP server belonging to the HC +// object. func (hc *HC) CloseHealthCheck() { if hc != nil { if err := hc.srv.Shutdown(context.Background()); err != nil { @@ -111,8 +118,8 @@ func (hc *HC) CloseHealthCheck() { } } -// NotifyReadyForConnections changes the value of 'started' in a health -// check object to true, marking the proxy as done starting up. +// NotifyReadyForConnections changes the value of 'started' in the HC +// object to true, marking the proxy as done starting up. func (hc *HC) NotifyReadyForConnections() { if hc != nil { startupMutex.Lock() @@ -126,9 +133,10 @@ func livenessTest() bool { return true } -// readinessTest checks several criteria before determining the proxy is ready. +// readinessTest will check several criteria before determining the proxy is +// ready for new connections. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { - // Wait until the 'Ready For Connections' log to mark the proxy as ready. + // Mark as not ready until we reach the 'Ready for Connections' log. startupMutex.Lock() if !hc.started { startupMutex.Unlock() From a8da1d6b047ff7ff5a2266bbb6f88e957e9e8dcc Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 8 Jul 2021 17:14:43 +0000 Subject: [PATCH 037/123] renamed mutex variables, added comments --- healthcheck/healthcheck.go | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index a198c17a3..57a7d8a0c 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -31,10 +31,13 @@ const ( portNum = ":8080" // TODO(monazhn): Think about a good port number. ) +// Locks to synchronize reads and writes for each HC boolean. Separate locks +// for each boolean is ideal so more than one boolean can be read/written +// simultaneously. var ( - readinessMutex = &sync.Mutex{} - livenessMutex = &sync.Mutex{} - startupMutex = &sync.Mutex{} + readinessL = &sync.Mutex{} + livenessL = &sync.Mutex{} + startupL = &sync.Mutex{} ) // HC is a type used to implement health checks for the proxy. @@ -70,30 +73,30 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { // Handlers used to set up HTTP endpoints. http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - readinessMutex.Lock() + readinessL.Lock() hc.ready = readinessTest(proxyClient, hc) if !hc.ready { - readinessMutex.Unlock() + readinessL.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } - readinessMutex.Unlock() + readinessL.Unlock() w.WriteHeader(200) w.Write([]byte("ok\n")) }) http.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - livenessMutex.Lock() + livenessL.Lock() hc.live = livenessTest() if !hc.live { - livenessMutex.Unlock() + livenessL.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } - livenessMutex.Unlock() + livenessL.Unlock() w.WriteHeader(200) w.Write([]byte("ok\n")) @@ -118,13 +121,12 @@ func (hc *HC) CloseHealthCheck() { } } -// NotifyReadyForConnections changes the value of 'started' in the HC -// object to true, marking the proxy as done starting up. +// NotifyReadyForConnections indicates to the proxy's HC that has finished startup. func (hc *HC) NotifyReadyForConnections() { if hc != nil { - startupMutex.Lock() + startupL.Lock() hc.started = true - startupMutex.Unlock() + startupL.Unlock() } } @@ -137,13 +139,13 @@ func livenessTest() bool { // ready for new connections. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. - startupMutex.Lock() + startupL.Lock() if !hc.started { - startupMutex.Unlock() + startupL.Unlock() logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } - startupMutex.Unlock() + startupL.Unlock() // Mark as not ready if the proxy is at the optional MaxConnections limit. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { From 0ac05234479572e56d1e22a5fd3438e6c9f41192 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:08:54 +0000 Subject: [PATCH 038/123] chagne Inc. to LLC --- healthcheck/healthcheck.go | 2 +- healthcheck/healthcheck_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 57a7d8a0c..63f60e468 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -1,4 +1,4 @@ -// Copyright 2021 Google Inc. All Rights Reserved. +// Copyright 2021 Google LLC All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index c00c35d58..6a1be38d7 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 Google Inc. All Rights Reserved. +// Copyright 2021 Google LLC All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From e8b86b548b0ea7408246bf129dad60aaaae2b420 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:17:32 +0000 Subject: [PATCH 039/123] define locks in HC struct, replace usage of startupL with readinessL --- healthcheck/healthcheck.go | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 63f60e468..6f78e46cc 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -31,15 +31,6 @@ const ( portNum = ":8080" // TODO(monazhn): Think about a good port number. ) -// Locks to synchronize reads and writes for each HC boolean. Separate locks -// for each boolean is ideal so more than one boolean can be read/written -// simultaneously. -var ( - readinessL = &sync.Mutex{} - livenessL = &sync.Mutex{} - startupL = &sync.Mutex{} -) - // HC is a type used to implement health checks for the proxy. type HC struct { // live being true means the proxy is running; in the case of the proxy @@ -55,6 +46,9 @@ type HC struct { // confused for affecting startup probing. When started becomes true, the // proxy is done starting up. started bool + // locks to protect HC booleans from concurrent HTTP GETs. + readinessL sync.Mutex + livenessL sync.Mutex // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server } @@ -73,30 +67,30 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { // Handlers used to set up HTTP endpoints. http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - readinessL.Lock() + hc.readinessL.Lock() hc.ready = readinessTest(proxyClient, hc) if !hc.ready { - readinessL.Unlock() + hc.readinessL.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } - readinessL.Unlock() + hc.readinessL.Unlock() w.WriteHeader(200) w.Write([]byte("ok\n")) }) http.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - livenessL.Lock() + hc.livenessL.Lock() hc.live = livenessTest() if !hc.live { - livenessL.Unlock() + hc.livenessL.Unlock() w.WriteHeader(500) w.Write([]byte("error\n")) return } - livenessL.Unlock() + hc.livenessL.Unlock() w.WriteHeader(200) w.Write([]byte("ok\n")) @@ -124,9 +118,9 @@ func (hc *HC) CloseHealthCheck() { // NotifyReadyForConnections indicates to the proxy's HC that has finished startup. func (hc *HC) NotifyReadyForConnections() { if hc != nil { - startupL.Lock() + hc.readinessL.Lock() hc.started = true - startupL.Unlock() + hc.readinessL.Unlock() } } @@ -139,13 +133,10 @@ func livenessTest() bool { // ready for new connections. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. - startupL.Lock() if !hc.started { - startupL.Unlock() logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } - startupL.Unlock() // Mark as not ready if the proxy is at the optional MaxConnections limit. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { From e2c8126bc1adb70a9c59f564d1acfd4cfbe61202 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:21:00 +0000 Subject: [PATCH 040/123] change CloseHealthCheck to Close --- healthcheck/healthcheck.go | 2 +- healthcheck/healthcheck_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 6f78e46cc..cc73ad3d8 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -107,7 +107,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { // CloseHealthCheck gracefully shuts down the HTTP server belonging to the HC // object. -func (hc *HC) CloseHealthCheck() { +func (hc *HC) Close() { if hc != nil { if err := hc.srv.Shutdown(context.Background()); err != nil { logging.Errorf("Failed to shut down health check: ", err) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 6a1be38d7..30efa53ae 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -36,7 +36,7 @@ func newClient(mc uint64) *proxy.Client { func TestLiveness(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() // Close health check upon exiting the test. + defer hc.Close() // Close health check upon exiting the test. time.Sleep(100 * time.Millisecond) // Wait for ListenAndServe to begin to avoid flaky tests. @@ -52,7 +52,7 @@ func TestLiveness(t *testing.T) { func TestBadStartup(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() + defer hc.Close() time.Sleep(100 * time.Millisecond) @@ -68,7 +68,7 @@ func TestBadStartup(t *testing.T) { func TestSuccessfulStartup(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() + defer hc.Close() time.Sleep(100 * time.Millisecond) @@ -86,7 +86,7 @@ func TestSuccessfulStartup(t *testing.T) { func TestMaxConnections(t *testing.T) { proxyClient := newClient(10) // MaxConnections == 10 hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() + defer hc.Close() time.Sleep(100 * time.Millisecond) @@ -114,7 +114,7 @@ func TestMaxConnections(t *testing.T) { func TestCloseHealthCheck(t *testing.T) { proxyClient := newClient(0) hc := NewHealthCheck(proxyClient) - defer hc.CloseHealthCheck() + defer hc.Close() time.Sleep(100 * time.Millisecond) @@ -126,7 +126,7 @@ func TestCloseHealthCheck(t *testing.T) { t.Errorf("got status code %v instead of 200", resp.StatusCode) } - hc.CloseHealthCheck() + hc.Close() _, err = http.Get(livenessURL) if err == nil { // If NO error From 9ee6d991089a6a8d52a3fc0d7f574a4f3ede15d4 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:24:04 +0000 Subject: [PATCH 041/123] omit newHealthCheck function --- healthcheck/healthcheck_test.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 30efa53ae..19a44afa1 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -27,14 +27,8 @@ const ( readinessURL = "http://localhost:8080/readiness" ) -func newClient(mc uint64) *proxy.Client { - return &proxy.Client{ - MaxConnections: mc, - } -} - func TestLiveness(t *testing.T) { - proxyClient := newClient(0) + proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() // Close health check upon exiting the test. @@ -50,7 +44,7 @@ func TestLiveness(t *testing.T) { } func TestBadStartup(t *testing.T) { - proxyClient := newClient(0) + proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -66,7 +60,7 @@ func TestBadStartup(t *testing.T) { } func TestSuccessfulStartup(t *testing.T) { - proxyClient := newClient(0) + proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -84,7 +78,9 @@ func TestSuccessfulStartup(t *testing.T) { } func TestMaxConnections(t *testing.T) { - proxyClient := newClient(10) // MaxConnections == 10 + proxyClient := &proxy.Client{ + MaxConnections: 10, + } hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -112,7 +108,7 @@ func TestMaxConnections(t *testing.T) { } func TestCloseHealthCheck(t *testing.T) { - proxyClient := newClient(0) + proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() From b2cb5923e218a662931c35f5fd53b17242c48aea Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:25:36 +0000 Subject: [PATCH 042/123] edit test names --- healthcheck/healthcheck_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 19a44afa1..323c321dd 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -43,7 +43,7 @@ func TestLiveness(t *testing.T) { } } -func TestBadStartup(t *testing.T) { +func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -59,7 +59,7 @@ func TestBadStartup(t *testing.T) { } } -func TestSuccessfulStartup(t *testing.T) { +func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -77,7 +77,7 @@ func TestSuccessfulStartup(t *testing.T) { } } -func TestMaxConnections(t *testing.T) { +func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, } From 98bcf1ca07ec7c7b6075fd0d827390f9cb23f2d1 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:37:24 +0000 Subject: [PATCH 043/123] added comments describing each test --- healthcheck/healthcheck_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 323c321dd..5511d81f2 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -27,6 +27,7 @@ const ( readinessURL = "http://localhost:8080/readiness" ) +// Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) @@ -43,6 +44,7 @@ func TestLiveness(t *testing.T) { } } +// Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) @@ -59,6 +61,8 @@ func TestStartupFail(t *testing.T) { } } +// Test to verify that when startup has finished, and MaxConnections has not been reached, +// the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) @@ -66,6 +70,7 @@ func TestStartupPass(t *testing.T) { time.Sleep(100 * time.Millisecond) + // Simulate the proxy client completing startup. hc.NotifyReadyForConnections() resp, err := http.Get(readinessURL) @@ -77,6 +82,8 @@ func TestStartupPass(t *testing.T) { } } +// Test to verify that when startup has finished, but MaxConnections has been reached, +// the readiness endpoint writes 500. func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, @@ -87,18 +94,9 @@ func TestMaxConnectionsReached(t *testing.T) { time.Sleep(100 * time.Millisecond) hc.NotifyReadyForConnections() - - resp, err := http.Get(readinessURL) - if err != nil { - t.Fatal(err) - } - if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) - } - proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections - resp, err = http.Get(readinessURL) + resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) } @@ -107,6 +105,8 @@ func TestMaxConnectionsReached(t *testing.T) { } } +// Test to verify that after closing a healthcheck, its liveness endpoint serves +// an error func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) From f98f04d1b9d4a6dad0782db71cc787f62573b267 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 00:50:15 +0000 Subject: [PATCH 044/123] forgot a period --- healthcheck/healthcheck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 5511d81f2..8b7fdbada 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -106,7 +106,7 @@ func TestMaxConnectionsReached(t *testing.T) { } // Test to verify that after closing a healthcheck, its liveness endpoint serves -// an error +// an error. func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) From 93e0a1fe976f88181fa970c1ed0e8b6f1a96c9f1 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 15:11:16 +0000 Subject: [PATCH 045/123] removed variables storing the results of the health tests --- healthcheck/healthcheck.go | 37 +++++++++---------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index cc73ad3d8..c5b23aaec 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -33,22 +33,11 @@ const ( // HC is a type used to implement health checks for the proxy. type HC struct { - // live being true means the proxy is running; in the case of the proxy - // being unexpectedly terminated, we should (re)start the proxy. - // live is related to Kubernetes liveness probing. - live bool - // ready being true means the proxy is ready to serve new traffic; in the - // case that ready is false, we should wait to send new traffic to the - // proxy. The value of ready determines the success or failure of - // Kubernetes readiness probing. - ready bool // started is a flag used to support readiness probing and should not be // confused for affecting startup probing. When started becomes true, the - // proxy is done starting up. + // proxy is done starting up. started is protected by startedL. started bool - // locks to protect HC booleans from concurrent HTTP GETs. - readinessL sync.Mutex - livenessL sync.Mutex + startedL sync.Mutex // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server } @@ -61,37 +50,26 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { } hc := &HC{ - live: true, srv: srv, } // Handlers used to set up HTTP endpoints. http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - hc.readinessL.Lock() - hc.ready = readinessTest(proxyClient, hc) - if !hc.ready { - hc.readinessL.Unlock() + if !readinessTest(proxyClient, hc) { w.WriteHeader(500) w.Write([]byte("error\n")) return } - hc.readinessL.Unlock() - w.WriteHeader(200) w.Write([]byte("ok\n")) }) http.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - hc.livenessL.Lock() - hc.live = livenessTest() - if !hc.live { - hc.livenessL.Unlock() + if !livenessTest() { w.WriteHeader(500) w.Write([]byte("error\n")) return } - hc.livenessL.Unlock() - w.WriteHeader(200) w.Write([]byte("ok\n")) }) @@ -118,9 +96,9 @@ func (hc *HC) Close() { // NotifyReadyForConnections indicates to the proxy's HC that has finished startup. func (hc *HC) NotifyReadyForConnections() { if hc != nil { - hc.readinessL.Lock() + hc.startedL.Lock() hc.started = true - hc.readinessL.Unlock() + hc.startedL.Unlock() } } @@ -133,10 +111,13 @@ func livenessTest() bool { // ready for new connections. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. + hc.startedL.Lock() if !hc.started { + hc.startedL.Unlock() logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } + hc.startedL.Unlock() // Mark as not ready if the proxy is at the optional MaxConnections limit. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { From a5559bfe3d9c7ab88e0940def099a1cb876e01d7 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 15:15:03 +0000 Subject: [PATCH 046/123] typo --- healthcheck/healthcheck.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index c5b23aaec..66d10a086 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -93,7 +93,7 @@ func (hc *HC) Close() { } } -// NotifyReadyForConnections indicates to the proxy's HC that has finished startup. +// NotifyReadyForConnections indicates that the proxy has finished startup. func (hc *HC) NotifyReadyForConnections() { if hc != nil { hc.startedL.Lock() From f8c1f979f32b1ada23fa50895adf0db956cf76ac Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 9 Jul 2021 16:06:14 +0000 Subject: [PATCH 047/123] Derive URLs from constants --- healthcheck/healthcheck.go | 5 ++--- healthcheck/healthcheck_test.go | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 66d10a086..083487d0f 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -28,7 +28,7 @@ import ( const ( livenessPath = "/liveness" readinessPath = "/readiness" - portNum = ":8080" // TODO(monazhn): Think about a good port number. + portNum = ":8080" ) // HC is a type used to implement health checks for the proxy. @@ -53,7 +53,6 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { srv: srv, } - // Handlers used to set up HTTP endpoints. http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { if !readinessTest(proxyClient, hc) { w.WriteHeader(500) @@ -83,7 +82,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { return hc } -// CloseHealthCheck gracefully shuts down the HTTP server belonging to the HC +// Close gracefully shuts down the HTTP server belonging to the HC // object. func (hc *HC) Close() { if hc != nil { diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 8b7fdbada..bfcc69859 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -23,12 +23,13 @@ import ( ) const ( - livenessURL = "http://localhost:8080/liveness" - readinessURL = "http://localhost:8080/readiness" + livenessURL = "http://localhost" + portNum + livenessPath + readinessURL = "http://localhost" + portNum + readinessPath ) // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { + //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() // Close health check upon exiting the test. @@ -46,6 +47,7 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { + //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -64,6 +66,7 @@ func TestStartupFail(t *testing.T) { // Test to verify that when startup has finished, and MaxConnections has not been reached, // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { + //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() @@ -85,6 +88,7 @@ func TestStartupPass(t *testing.T) { // Test to verify that when startup has finished, but MaxConnections has been reached, // the readiness endpoint writes 500. func TestMaxConnectionsReached(t *testing.T) { + //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{ MaxConnections: 10, } @@ -129,3 +133,11 @@ func TestCloseHealthCheck(t *testing.T) { t.Fatal(err) } } + +/*func TestMain(m *testing.M) { + proxyClient := newClient(0) + hc := NewHealthCheck(proxyClient) + code := m.Run() + hc.Close() + os.Exit(code) +}*/ \ No newline at end of file From 2d62d1ead557dc8e57784d7596d058b7af78e4d4 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 9 Jul 2021 16:23:14 +0000 Subject: [PATCH 048/123] Separate ListenAndServe into Listen and Serve Fixes issue of test flakiness by having listener accept connections before the return. --- healthcheck/healthcheck.go | 10 ++++++++-- healthcheck/healthcheck_test.go | 23 ----------------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 083487d0f..5091e7124 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -17,6 +17,7 @@ package healthcheck import ( "context" + "net" "net/http" "sync" "sync/atomic" @@ -73,9 +74,14 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { w.Write([]byte("ok\n")) }) + ln, err := net.Listen("tcp", portNum) + if err != nil { + logging.Errorf("Failed to listen: %v", err) + } + go func() { - if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { - logging.Errorf("Failed to start endpoint(s): %v", err) + if err := srv.Serve(ln); err != nil && err != http.ErrServerClosed { + logging.Errorf("Failed to serve: %v", err) } }() diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index bfcc69859..42f033061 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -17,7 +17,6 @@ package healthcheck import ( "net/http" "testing" - "time" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) @@ -29,13 +28,10 @@ const ( // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { - //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() // Close health check upon exiting the test. - time.Sleep(100 * time.Millisecond) // Wait for ListenAndServe to begin to avoid flaky tests. - resp, err := http.Get(livenessURL) if err != nil { t.Fatal(err) @@ -47,13 +43,10 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { - //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() - time.Sleep(100 * time.Millisecond) - resp, err := http.Get(readinessURL) if err != nil { t.Fatal(err) @@ -66,13 +59,10 @@ func TestStartupFail(t *testing.T) { // Test to verify that when startup has finished, and MaxConnections has not been reached, // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { - //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient) defer hc.Close() - time.Sleep(100 * time.Millisecond) - // Simulate the proxy client completing startup. hc.NotifyReadyForConnections() @@ -88,15 +78,12 @@ func TestStartupPass(t *testing.T) { // Test to verify that when startup has finished, but MaxConnections has been reached, // the readiness endpoint writes 500. func TestMaxConnectionsReached(t *testing.T) { - //http.DefaultServeMux = new(http.ServeMux) proxyClient := &proxy.Client{ MaxConnections: 10, } hc := NewHealthCheck(proxyClient) defer hc.Close() - time.Sleep(100 * time.Millisecond) - hc.NotifyReadyForConnections() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections @@ -116,8 +103,6 @@ func TestCloseHealthCheck(t *testing.T) { hc := NewHealthCheck(proxyClient) defer hc.Close() - time.Sleep(100 * time.Millisecond) - resp, err := http.Get(livenessURL) if err != nil { t.Fatal(err) @@ -133,11 +118,3 @@ func TestCloseHealthCheck(t *testing.T) { t.Fatal(err) } } - -/*func TestMain(m *testing.M) { - proxyClient := newClient(0) - hc := NewHealthCheck(proxyClient) - code := m.Run() - hc.Close() - os.Exit(code) -}*/ \ No newline at end of file From 9c519f74c409aa134b4535f56a048ed3df2ddaea Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Fri, 9 Jul 2021 17:32:53 +0000 Subject: [PATCH 049/123] made port number a configurable argument from command line --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 4 ++-- healthcheck/healthcheck.go | 10 ++++++---- healthcheck/healthcheck_test.go | 27 +++++++++++--------------- 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 73b2eb52a..5715fdc80 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -136,6 +136,7 @@ unavailable.`, // Settings for healthcheck useHealthCheck = flag.Bool("use_health_check", false, "When set, periodically checks and communicates the health of the proxy client.") + hcPort = flag.String("hc_port", "8080", "Health checks will listen and serve this port. Defaults to 8080.") ) const ( @@ -592,9 +593,8 @@ func main() { } var hc *healthcheck.HC - if *useHealthCheck { - hc = healthcheck.NewHealthCheck(proxyClient) + hc = healthcheck.NewHealthCheck(proxyClient, *hcPort) } // Initialize a source of new connections to Cloud SQL instances. diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 5091e7124..aa27fec57 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -29,7 +29,6 @@ import ( const ( livenessPath = "/liveness" readinessPath = "/readiness" - portNum = ":8080" ) // HC is a type used to implement health checks for the proxy. @@ -39,18 +38,21 @@ type HC struct { // proxy is done starting up. started is protected by startedL. started bool startedL sync.Mutex + // port designates which port HC listens and serves. + port string // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server } // NewHealthCheck initializes a HC object and exposes HTTP endpoints used to // communicate proxy health. -func NewHealthCheck(proxyClient *proxy.Client) *HC { +func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { srv := &http.Server{ - Addr: portNum, + Addr: ":" + port, } hc := &HC{ + port: port, srv: srv, } @@ -74,7 +76,7 @@ func NewHealthCheck(proxyClient *proxy.Client) *HC { w.Write([]byte("ok\n")) }) - ln, err := net.Listen("tcp", portNum) + ln, err := net.Listen("tcp", srv.Addr) if err != nil { logging.Errorf("Failed to listen: %v", err) } diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 42f033061..1316acf9a 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -20,19 +20,14 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) - -const ( - livenessURL = "http://localhost" + portNum + livenessPath - readinessURL = "http://localhost" + portNum + readinessPath -) // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient, "8080") defer hc.Close() // Close health check upon exiting the test. - resp, err := http.Get(livenessURL) + resp, err := http.Get("http://localhost:" + hc.port + livenessPath) if err != nil { t.Fatal(err) } @@ -44,10 +39,10 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient, "8080") defer hc.Close() - resp, err := http.Get(readinessURL) + resp, err := http.Get("http://localhost:" + hc.port + readinessPath) if err != nil { t.Fatal(err) } @@ -60,13 +55,13 @@ func TestStartupFail(t *testing.T) { // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient, "8080") defer hc.Close() // Simulate the proxy client completing startup. hc.NotifyReadyForConnections() - resp, err := http.Get(readinessURL) + resp, err := http.Get("http://localhost:" + hc.port + readinessPath) if err != nil { t.Fatal(err) } @@ -81,13 +76,13 @@ func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, } - hc := NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient, "8080") defer hc.Close() hc.NotifyReadyForConnections() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections - resp, err := http.Get(readinessURL) + resp, err := http.Get("http://localhost:" + hc.port + readinessPath) if err != nil { t.Fatal(err) } @@ -100,10 +95,10 @@ func TestMaxConnectionsReached(t *testing.T) { // an error. func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient) + hc := NewHealthCheck(proxyClient, "8080") defer hc.Close() - resp, err := http.Get(livenessURL) + resp, err := http.Get("http://localhost:" + hc.port + livenessPath) if err != nil { t.Fatal(err) } @@ -113,7 +108,7 @@ func TestCloseHealthCheck(t *testing.T) { hc.Close() - _, err = http.Get(livenessURL) + _, err = http.Get("http://localhost:" + hc.port + livenessPath) if err == nil { // If NO error t.Fatal(err) } From 3a49c957774f38764d6907d89b0a01a920f4dbcf Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 9 Jul 2021 22:12:35 +0000 Subject: [PATCH 050/123] Use non-default ServeMux Fixes the 'multiple registrations' panic when running package tests (DefaultServeMux is a global state). --- healthcheck/healthcheck.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index aa27fec57..d86240cbb 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -47,8 +47,11 @@ type HC struct { // NewHealthCheck initializes a HC object and exposes HTTP endpoints used to // communicate proxy health. func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { + mux := http.NewServeMux() + srv := &http.Server{ Addr: ":" + port, + Handler: mux, } hc := &HC{ @@ -56,7 +59,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { srv: srv, } - http.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { + mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { if !readinessTest(proxyClient, hc) { w.WriteHeader(500) w.Write([]byte("error\n")) @@ -66,7 +69,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { w.Write([]byte("ok\n")) }) - http.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { + mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { if !livenessTest() { w.WriteHeader(500) w.Write([]byte("error\n")) From 6bf83ea415fd41abcb4e09b9952e77364a0722ea Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Sat, 10 Jul 2021 01:06:54 +0000 Subject: [PATCH 051/123] Iterate readiness criteria --- healthcheck/healthcheck.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index d86240cbb..5db496807 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -117,8 +117,10 @@ func livenessTest() bool { return true } -// readinessTest will check several criteria before determining the proxy is -// ready for new connections. +// readinessTest will check the following criteria before determining whether the +// proxy is ready for new connections. +// 1. Finished starting up / sent the 'Ready for Connections' log. +// 2. Not yet hit the MaxConnections limit, if applicable. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. hc.startedL.Lock() From 463dd2e25c3e8a0ef1a886f9ee01c032b05175c3 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Sat, 10 Jul 2021 01:31:41 +0000 Subject: [PATCH 052/123] Have Close function take a context as an argument --- healthcheck/healthcheck.go | 7 +++---- healthcheck/healthcheck_test.go | 13 +++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 5db496807..6e42607fa 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -93,11 +93,10 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { return hc } -// Close gracefully shuts down the HTTP server belonging to the HC -// object. -func (hc *HC) Close() { +// Close gracefully shuts down the HTTP server belonging to the HC object. +func (hc *HC) Close(ctx context.Context) { if hc != nil { - if err := hc.srv.Shutdown(context.Background()); err != nil { + if err := hc.srv.Shutdown(ctx); err != nil { logging.Errorf("Failed to shut down health check: ", err) } } diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 1316acf9a..a4cefaa2d 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -15,6 +15,7 @@ package healthcheck import ( + "context" "net/http" "testing" @@ -25,7 +26,7 @@ import ( func TestLiveness(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient, "8080") - defer hc.Close() // Close health check upon exiting the test. + defer hc.Close(context.Background()) // Close health check upon exiting the test. resp, err := http.Get("http://localhost:" + hc.port + livenessPath) if err != nil { @@ -40,7 +41,7 @@ func TestLiveness(t *testing.T) { func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient, "8080") - defer hc.Close() + defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + hc.port + readinessPath) if err != nil { @@ -56,7 +57,7 @@ func TestStartupFail(t *testing.T) { func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient, "8080") - defer hc.Close() + defer hc.Close(context.Background()) // Simulate the proxy client completing startup. hc.NotifyReadyForConnections() @@ -77,7 +78,7 @@ func TestMaxConnectionsReached(t *testing.T) { MaxConnections: 10, } hc := NewHealthCheck(proxyClient, "8080") - defer hc.Close() + defer hc.Close(context.Background()) hc.NotifyReadyForConnections() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections @@ -96,7 +97,7 @@ func TestMaxConnectionsReached(t *testing.T) { func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient, "8080") - defer hc.Close() + defer hc.Close(context.Background()) // TODO (monazhn): remove this Close? resp, err := http.Get("http://localhost:" + hc.port + livenessPath) if err != nil { @@ -106,7 +107,7 @@ func TestCloseHealthCheck(t *testing.T) { t.Errorf("got status code %v instead of 200", resp.StatusCode) } - hc.Close() + hc.Close(context.Background()) _, err = http.Get("http://localhost:" + hc.port + livenessPath) if err == nil { // If NO error From 46d226ffd85eb8b0cb0fecacb6fa7db2da0b698d Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Sat, 10 Jul 2021 01:40:05 +0000 Subject: [PATCH 053/123] Define a constant for testing port --- healthcheck/healthcheck_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index a4cefaa2d..1845f61ad 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -22,10 +22,12 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) +const testPort = "8080" + // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, "8080") + hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) // Close health check upon exiting the test. resp, err := http.Get("http://localhost:" + hc.port + livenessPath) @@ -40,7 +42,7 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, "8080") + hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + hc.port + readinessPath) @@ -56,7 +58,7 @@ func TestStartupFail(t *testing.T) { // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, "8080") + hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) // Simulate the proxy client completing startup. @@ -77,7 +79,7 @@ func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, } - hc := NewHealthCheck(proxyClient, "8080") + hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) hc.NotifyReadyForConnections() @@ -96,7 +98,7 @@ func TestMaxConnectionsReached(t *testing.T) { // an error. func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, "8080") + hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) // TODO (monazhn): remove this Close? resp, err := http.Get("http://localhost:" + hc.port + livenessPath) From 8cbd3df438f35957525c49b83a484e0267bd65d7 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 12 Jul 2021 16:32:36 +0000 Subject: [PATCH 054/123] Remove deferred call to Close() --- healthcheck/healthcheck.go | 2 +- healthcheck/healthcheck_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index 6e42607fa..fb584aa99 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -70,7 +70,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - if !livenessTest() { + if !livenessTest() { // Because livenessTest() returns true, this case should not be reached. w.WriteHeader(500) w.Write([]byte("error\n")) return diff --git a/healthcheck/healthcheck_test.go b/healthcheck/healthcheck_test.go index 1845f61ad..0720f70ca 100644 --- a/healthcheck/healthcheck_test.go +++ b/healthcheck/healthcheck_test.go @@ -99,7 +99,6 @@ func TestMaxConnectionsReached(t *testing.T) { func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} hc := NewHealthCheck(proxyClient, testPort) - defer hc.Close(context.Background()) // TODO (monazhn): remove this Close? resp, err := http.Get("http://localhost:" + hc.port + livenessPath) if err != nil { From b6e2be36bc79dd1444bf0dff08a26bae80546212 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Mon, 12 Jul 2021 22:11:09 +0000 Subject: [PATCH 055/123] rename flag to useHttpHealthCheck --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 5715fdc80..e13d6b44a 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -135,7 +135,7 @@ unavailable.`, ) // Settings for healthcheck - useHealthCheck = flag.Bool("use_health_check", false, "When set, periodically checks and communicates the health of the proxy client.") + useHttpHealthCheck = flag.Bool("use_health_check", false, "When set, periodically checks and communicates the health of the proxy client.") hcPort = flag.String("hc_port", "8080", "Health checks will listen and serve this port. Defaults to 8080.") ) @@ -593,7 +593,7 @@ func main() { } var hc *healthcheck.HC - if *useHealthCheck { + if *useHttpHealthCheck { hc = healthcheck.NewHealthCheck(proxyClient, *hcPort) } From 502ee01eb3b544e3374b70066a87c773b3361e08 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Mon, 12 Jul 2021 22:11:58 +0000 Subject: [PATCH 056/123] make caller ensure hc is not nil before NotifyReadyForConnections --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 4 +++- healthcheck/healthcheck.go | 8 +++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index e13d6b44a..db96d6529 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -636,7 +636,9 @@ func main() { logging.Infof("Ready for new connections") - hc.NotifyReadyForConnections() + if hc != nil { + hc.NotifyReadyForConnections() + } signals := make(chan os.Signal, 1) signal.Notify(signals, syscall.SIGTERM, syscall.SIGINT) diff --git a/healthcheck/healthcheck.go b/healthcheck/healthcheck.go index fb584aa99..d8e6c6494 100644 --- a/healthcheck/healthcheck.go +++ b/healthcheck/healthcheck.go @@ -104,11 +104,9 @@ func (hc *HC) Close(ctx context.Context) { // NotifyReadyForConnections indicates that the proxy has finished startup. func (hc *HC) NotifyReadyForConnections() { - if hc != nil { - hc.startedL.Lock() - hc.started = true - hc.startedL.Unlock() - } + hc.startedL.Lock() + hc.started = true + hc.startedL.Unlock() } // livenessTest returns true as long as the proxy is running. From 32ed5c1b8448217df161f887b6d6d3ec44834366 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 00:55:24 +0000 Subject: [PATCH 057/123] fix flag --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index db96d6529..a5f3dd6ef 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -135,7 +135,7 @@ unavailable.`, ) // Settings for healthcheck - useHttpHealthCheck = flag.Bool("use_health_check", false, "When set, periodically checks and communicates the health of the proxy client.") + useHttpHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") hcPort = flag.String("hc_port", "8080", "Health checks will listen and serve this port. Defaults to 8080.") ) From 306d070be8ed5c7ca3ed5bd68f1b3e07de7a5912 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 02:50:39 +0000 Subject: [PATCH 058/123] Internal health check package --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- healthcheck/README.md | 0 internal/healthcheck/README.md | 23 +++++++++++++++++++ .../healthcheck}/healthcheck.go | 0 .../healthcheck}/healthcheck_test.go | 0 5 files changed, 24 insertions(+), 1 deletion(-) delete mode 100644 healthcheck/README.md create mode 100644 internal/healthcheck/README.md rename {healthcheck => internal/healthcheck}/healthcheck.go (100%) rename {healthcheck => internal/healthcheck}/healthcheck_test.go (100%) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index a5f3dd6ef..a4f1f3390 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -34,7 +34,7 @@ import ( "syscall" "time" - "github.com/GoogleCloudPlatform/cloudsql-proxy/healthcheck" + "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/certs" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/fuse" diff --git a/healthcheck/README.md b/healthcheck/README.md deleted file mode 100644 index e69de29bb..000000000 diff --git a/internal/healthcheck/README.md b/internal/healthcheck/README.md new file mode 100644 index 000000000..1e55788c7 --- /dev/null +++ b/internal/healthcheck/README.md @@ -0,0 +1,23 @@ +# Using the Cloud SQL proxy with health checks + + +## Kubernetes users + + +-use_health_check flag +-hc_port= flag (defaults to 8080) + + + +Add liveness/readiness probes to Kubernetes YAML deployment. +- Sample Deployment (no) +- User Guide + +## Non-Kubernetes users + +-use_health_check flag +-hc_port= flag (defaults to 8080) + + + + diff --git a/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go similarity index 100% rename from healthcheck/healthcheck.go rename to internal/healthcheck/healthcheck.go diff --git a/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go similarity index 100% rename from healthcheck/healthcheck_test.go rename to internal/healthcheck/healthcheck_test.go From 6247e30270e45a7ede711b1ebee2f8e4f73cb0f3 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 12 Jul 2021 23:26:48 -0400 Subject: [PATCH 059/123] Remove incomplete README Will be included in the next PR. --- internal/healthcheck/README.md | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/internal/healthcheck/README.md b/internal/healthcheck/README.md index 1e55788c7..8b1378917 100644 --- a/internal/healthcheck/README.md +++ b/internal/healthcheck/README.md @@ -1,23 +1 @@ -# Using the Cloud SQL proxy with health checks - - -## Kubernetes users - - --use_health_check flag --hc_port= flag (defaults to 8080) - - - -Add liveness/readiness probes to Kubernetes YAML deployment. -- Sample Deployment (no) -- User Guide - -## Non-Kubernetes users - --use_health_check flag --hc_port= flag (defaults to 8080) - - - From 47088d636826bccd146a4fceccd98d0ae49bc7b6 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 03:33:05 +0000 Subject: [PATCH 060/123] Clean up comments --- internal/healthcheck/healthcheck.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index d8e6c6494..7e38bb8ef 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -38,7 +38,7 @@ type HC struct { // proxy is done starting up. started is protected by startedL. started bool startedL sync.Mutex - // port designates which port HC listens and serves. + // port designates the port number on which HC listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server @@ -116,7 +116,7 @@ func livenessTest() bool { // readinessTest will check the following criteria before determining whether the // proxy is ready for new connections. -// 1. Finished starting up / sent the 'Ready for Connections' log. +// 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. func readinessTest(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. From e629b6180afc21eb5072f695d7c4562bbf9270ce Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:31:22 +0000 Subject: [PATCH 061/123] change default port to 9090 --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- internal/healthcheck/healthcheck_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index a4f1f3390..6ea2cb120 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -136,7 +136,7 @@ unavailable.`, // Settings for healthcheck useHttpHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") - hcPort = flag.String("hc_port", "8080", "Health checks will listen and serve this port. Defaults to 8080.") + hcPort = flag.String("hc_port", "9090", "Health checks will listen and serve this port. Defaults to 9090.") ) const ( diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 0720f70ca..6efbfa867 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -22,7 +22,7 @@ import ( "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) -const testPort = "8080" +const testPort = "9090" // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { From ddf3faa675c30f5496350cb1b7861d9e6d594edd Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:35:10 +0000 Subject: [PATCH 062/123] place mutex before boolean --- internal/healthcheck/healthcheck.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 7e38bb8ef..a4132dcb7 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -33,11 +33,11 @@ const ( // HC is a type used to implement health checks for the proxy. type HC struct { - // started is a flag used to support readiness probing and should not be - // confused for affecting startup probing. When started becomes true, the - // proxy is done starting up. started is protected by startedL. - started bool + // startedL protects started, a flag that indicates whether the proxy is + // done starting up. started is used to support readiness probing and + // should not be confused for affecting startup probing. startedL sync.Mutex + started bool // port designates the port number on which HC listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. From 50c5156ca814c8b0bdfe609c227d4a83be8764b1 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:37:49 +0000 Subject: [PATCH 063/123] removed newlines --- internal/healthcheck/healthcheck.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index a4132dcb7..023b61bf9 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -62,21 +62,21 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { if !readinessTest(proxyClient, hc) { w.WriteHeader(500) - w.Write([]byte("error\n")) + w.Write([]byte("error")) return } w.WriteHeader(200) - w.Write([]byte("ok\n")) + w.Write([]byte("ok")) }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { if !livenessTest() { // Because livenessTest() returns true, this case should not be reached. w.WriteHeader(500) - w.Write([]byte("error\n")) + w.Write([]byte("error")) return } w.WriteHeader(200) - w.Write([]byte("ok\n")) + w.Write([]byte("ok")) }) ln, err := net.Listen("tcp", srv.Addr) From e0dae228a925d728a460df334e857dbd4b4dd939 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:38:43 +0000 Subject: [PATCH 064/123] rename readinessTest and livenessTest to isReady and isLive --- internal/healthcheck/healthcheck.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 023b61bf9..0cafa3bae 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -60,7 +60,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { } mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - if !readinessTest(proxyClient, hc) { + if !isReady(proxyClient, hc) { w.WriteHeader(500) w.Write([]byte("error")) return @@ -70,7 +70,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - if !livenessTest() { // Because livenessTest() returns true, this case should not be reached. + if !isLive() { // Because livenessTest() returns true, this case should not be reached. w.WriteHeader(500) w.Write([]byte("error")) return @@ -110,7 +110,7 @@ func (hc *HC) NotifyReadyForConnections() { } // livenessTest returns true as long as the proxy is running. -func livenessTest() bool { +func isLive() bool { return true } @@ -118,7 +118,7 @@ func livenessTest() bool { // proxy is ready for new connections. // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. -func readinessTest(proxyClient *proxy.Client, hc *HC) bool { +func isReady(proxyClient *proxy.Client, hc *HC) bool { // Mark as not ready until we reach the 'Ready for Connections' log. hc.startedL.Lock() if !hc.started { From 0bf5baf110aadfb268c0556a5b5137095377b598 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:39:41 +0000 Subject: [PATCH 065/123] rename NotifyReadyForConnections to NotifyStarted --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- internal/healthcheck/healthcheck.go | 2 +- internal/healthcheck/healthcheck_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 6ea2cb120..5e0db5293 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -637,7 +637,7 @@ func main() { logging.Infof("Ready for new connections") if hc != nil { - hc.NotifyReadyForConnections() + hc.NotifyStarted() } signals := make(chan os.Signal, 1) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 0cafa3bae..ac12850a8 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -103,7 +103,7 @@ func (hc *HC) Close(ctx context.Context) { } // NotifyReadyForConnections indicates that the proxy has finished startup. -func (hc *HC) NotifyReadyForConnections() { +func (hc *HC) NotifyStarted() { hc.startedL.Lock() hc.started = true hc.startedL.Unlock() diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 6efbfa867..4934270d0 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -62,7 +62,7 @@ func TestStartupPass(t *testing.T) { defer hc.Close(context.Background()) // Simulate the proxy client completing startup. - hc.NotifyReadyForConnections() + hc.NotifyStarted() resp, err := http.Get("http://localhost:" + hc.port + readinessPath) if err != nil { @@ -82,7 +82,7 @@ func TestMaxConnectionsReached(t *testing.T) { hc := NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) - hc.NotifyReadyForConnections() + hc.NotifyStarted() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections resp, err := http.Get("http://localhost:" + hc.port + readinessPath) From 0d7dc72b904ad3f300a02d39ebc11a949831dba7 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:42:54 +0000 Subject: [PATCH 066/123] fix function names in comments --- internal/healthcheck/healthcheck.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index ac12850a8..dce0cebbb 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -70,7 +70,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - if !isLive() { // Because livenessTest() returns true, this case should not be reached. + if !isLive() { // Because isLive() returns true, this case should not be reached. w.WriteHeader(500) w.Write([]byte("error")) return @@ -102,19 +102,19 @@ func (hc *HC) Close(ctx context.Context) { } } -// NotifyReadyForConnections indicates that the proxy has finished startup. +// NotifyStarted tells the HC that the proxy has finished startup. func (hc *HC) NotifyStarted() { hc.startedL.Lock() hc.started = true hc.startedL.Unlock() } -// livenessTest returns true as long as the proxy is running. +// isLive returns true as long as the proxy is running. func isLive() bool { return true } -// readinessTest will check the following criteria before determining whether the +// isReady will check the following criteria before determining whether the // proxy is ready for new connections. // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. From de3bd1c8ad774dc61374d10b1a05ea37e5bce71f Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 16:51:09 +0000 Subject: [PATCH 067/123] change package to healthcheck_test --- internal/healthcheck/healthcheck_test.go | 31 ++++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 4934270d0..07c855711 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package healthcheck +package healthcheck_test import ( "context" @@ -20,17 +20,22 @@ import ( "testing" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" + "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" ) -const testPort = "9090" +const ( + livenessPath = "/liveness" + readinessPath = "/readiness" + testPort = "9090" +) // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) // Close health check upon exiting the test. - resp, err := http.Get("http://localhost:" + hc.port + livenessPath) + resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { t.Fatal(err) } @@ -42,10 +47,10 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) - resp, err := http.Get("http://localhost:" + hc.port + readinessPath) + resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { t.Fatal(err) } @@ -58,13 +63,13 @@ func TestStartupFail(t *testing.T) { // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) // Simulate the proxy client completing startup. hc.NotifyStarted() - resp, err := http.Get("http://localhost:" + hc.port + readinessPath) + resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { t.Fatal(err) } @@ -79,13 +84,13 @@ func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, } - hc := NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(proxyClient, testPort) defer hc.Close(context.Background()) hc.NotifyStarted() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections - resp, err := http.Get("http://localhost:" + hc.port + readinessPath) + resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { t.Fatal(err) } @@ -98,9 +103,9 @@ func TestMaxConnectionsReached(t *testing.T) { // an error. func TestCloseHealthCheck(t *testing.T) { proxyClient := &proxy.Client{} - hc := NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(proxyClient, testPort) - resp, err := http.Get("http://localhost:" + hc.port + livenessPath) + resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { t.Fatal(err) } @@ -110,7 +115,7 @@ func TestCloseHealthCheck(t *testing.T) { hc.Close(context.Background()) - _, err = http.Get("http://localhost:" + hc.port + livenessPath) + _, err = http.Get("http://localhost:" + testPort + livenessPath) if err == nil { // If NO error t.Fatal(err) } From 331e6bcd67e3d011b7cdef7bba108ce24e58ddde Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:03:32 +0000 Subject: [PATCH 068/123] inline --- internal/healthcheck/healthcheck_test.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 07c855711..53464646e 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -31,9 +31,8 @@ const ( // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { - proxyClient := &proxy.Client{} - hc := healthcheck.NewHealthCheck(proxyClient, testPort) - defer hc.Close(context.Background()) // Close health check upon exiting the test. + hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { @@ -46,8 +45,7 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { - proxyClient := &proxy.Client{} - hc := healthcheck.NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + readinessPath) @@ -62,8 +60,7 @@ func TestStartupFail(t *testing.T) { // Test to verify that when startup has finished, and MaxConnections has not been reached, // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { - proxyClient := &proxy.Client{} - hc := healthcheck.NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) defer hc.Close(context.Background()) // Simulate the proxy client completing startup. @@ -102,8 +99,7 @@ func TestMaxConnectionsReached(t *testing.T) { // Test to verify that after closing a healthcheck, its liveness endpoint serves // an error. func TestCloseHealthCheck(t *testing.T) { - proxyClient := &proxy.Client{} - hc := healthcheck.NewHealthCheck(proxyClient, testPort) + hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { From 22729f371598ab12e7aba726a74512c1036db963 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:05:17 +0000 Subject: [PATCH 069/123] reduce startedL critical section is isReady --- internal/healthcheck/healthcheck.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index dce0cebbb..c5c666b11 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -119,16 +119,17 @@ func isLive() bool { // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. func isReady(proxyClient *proxy.Client, hc *HC) bool { - // Mark as not ready until we reach the 'Ready for Connections' log. + // Not ready until we reach the 'Ready for Connections' log. hc.startedL.Lock() - if !hc.started { - hc.startedL.Unlock() + started := hc.started + hc.startedL.Unlock() + + if !started { logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } - hc.startedL.Unlock() - // Mark as not ready if the proxy is at the optional MaxConnections limit. + // Not ready if the proxy is at the optional MaxConnections limit. if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { logging.Errorf("Readiness failed because proxy has reached the maximum connections limit (%d).", proxyClient.MaxConnections) return false From 8e2d75aa02909120d50b66b3cbc89ca2e709348c Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:06:48 +0000 Subject: [PATCH 070/123] rename proxyClient parameter to c --- internal/healthcheck/healthcheck.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index c5c666b11..9870bbdb8 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -46,7 +46,7 @@ type HC struct { // NewHealthCheck initializes a HC object and exposes HTTP endpoints used to // communicate proxy health. -func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { +func NewHealthCheck(c *proxy.Client, port string) *HC { mux := http.NewServeMux() srv := &http.Server{ @@ -60,7 +60,7 @@ func NewHealthCheck(proxyClient *proxy.Client, port string) *HC { } mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - if !isReady(proxyClient, hc) { + if !isReady(c, hc) { w.WriteHeader(500) w.Write([]byte("error")) return @@ -118,7 +118,7 @@ func isLive() bool { // proxy is ready for new connections. // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. -func isReady(proxyClient *proxy.Client, hc *HC) bool { +func isReady(c *proxy.Client, hc *HC) bool { // Not ready until we reach the 'Ready for Connections' log. hc.startedL.Lock() started := hc.started @@ -130,8 +130,8 @@ func isReady(proxyClient *proxy.Client, hc *HC) bool { } // Not ready if the proxy is at the optional MaxConnections limit. - if proxyClient.MaxConnections > 0 && atomic.LoadUint64(&proxyClient.ConnectionsCounter) >= proxyClient.MaxConnections { - logging.Errorf("Readiness failed because proxy has reached the maximum connections limit (%d).", proxyClient.MaxConnections) + if c.MaxConnections > 0 && atomic.LoadUint64(&c.ConnectionsCounter) >= c.MaxConnections { + logging.Errorf("Readiness failed because proxy has reached the maximum connections limit (%d).", c.MaxConnections) return false } From 2ca923071934799a971989b615fb6a69b94cb69c Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:28:25 +0000 Subject: [PATCH 071/123] make exported functions return errors instead of logging them --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 5 ++- internal/healthcheck/healthcheck.go | 15 +++---- internal/healthcheck/healthcheck_test.go | 52 ++++++++++++++++-------- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 5e0db5293..4675dee82 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -594,7 +594,10 @@ func main() { var hc *healthcheck.HC if *useHttpHealthCheck { - hc = healthcheck.NewHealthCheck(proxyClient, *hcPort) + hc, err = healthcheck.NewHealthCheck(proxyClient, *hcPort) + if err != nil { + logging.Errorf("Could not initialize health check: %v", err) + } } // Initialize a source of new connections to Cloud SQL instances. diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 9870bbdb8..2943fe8f7 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -46,7 +46,7 @@ type HC struct { // NewHealthCheck initializes a HC object and exposes HTTP endpoints used to // communicate proxy health. -func NewHealthCheck(c *proxy.Client, port string) *HC { +func NewHealthCheck(c *proxy.Client, port string) (*HC, error) { mux := http.NewServeMux() srv := &http.Server{ @@ -81,7 +81,7 @@ func NewHealthCheck(c *proxy.Client, port string) *HC { ln, err := net.Listen("tcp", srv.Addr) if err != nil { - logging.Errorf("Failed to listen: %v", err) + return nil, err } go func() { @@ -90,16 +90,13 @@ func NewHealthCheck(c *proxy.Client, port string) *HC { } }() - return hc + return hc, nil } // Close gracefully shuts down the HTTP server belonging to the HC object. -func (hc *HC) Close(ctx context.Context) { - if hc != nil { - if err := hc.srv.Shutdown(ctx); err != nil { - logging.Errorf("Failed to shut down health check: ", err) - } - } +func (hc *HC) Close(ctx context.Context) error { + err := hc.srv.Shutdown(ctx) + return err } // NotifyStarted tells the HC that the proxy has finished startup. diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 53464646e..5c1ac3e09 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -31,36 +31,45 @@ const ( // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { - hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { - t.Fatal(err) + t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) + t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) } } // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { - hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { - t.Fatal(err) + t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != 500 { - t.Errorf("got status code %v instead of 500", resp.StatusCode) + t.Errorf("Got status code %v instead of 500\n", resp.StatusCode) } } // Test to verify that when startup has finished, and MaxConnections has not been reached, // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { - hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } defer hc.Close(context.Background()) // Simulate the proxy client completing startup. @@ -68,10 +77,10 @@ func TestStartupPass(t *testing.T) { resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { - t.Fatal(err) + t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) + t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) } } @@ -81,7 +90,10 @@ func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 10, } - hc := healthcheck.NewHealthCheck(proxyClient, testPort) + hc, err := healthcheck.NewHealthCheck(proxyClient, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } defer hc.Close(context.Background()) hc.NotifyStarted() @@ -89,30 +101,36 @@ func TestMaxConnectionsReached(t *testing.T) { resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { - t.Fatal(err) + t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != 500 { - t.Errorf("got status code %v instead of 500", resp.StatusCode) + t.Errorf("Got status code %v instead of 500\n", resp.StatusCode) } } // Test to verify that after closing a healthcheck, its liveness endpoint serves // an error. func TestCloseHealthCheck(t *testing.T) { - hc := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { - t.Fatal(err) + t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != 200 { - t.Errorf("got status code %v instead of 200", resp.StatusCode) + t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) } - hc.Close(context.Background()) + err = hc.Close(context.Background()) + if err != nil { + t.Fatalf("Failed to close health check: %v\n", err) + } _, err = http.Get("http://localhost:" + testPort + livenessPath) if err == nil { // If NO error - t.Fatal(err) + t.Fatalf("HTTP GET did not fail after closing health check\n") } } From 856eb58dac01ddc2193c7ebae2385c1a81b9acfa Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:41:34 +0000 Subject: [PATCH 072/123] use errors.Is instead of basic equality check --- internal/healthcheck/healthcheck.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index 2943fe8f7..ad784c5b2 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -17,6 +17,7 @@ package healthcheck import ( "context" + "errors" "net" "net/http" "sync" @@ -83,9 +84,9 @@ func NewHealthCheck(c *proxy.Client, port string) (*HC, error) { if err != nil { return nil, err } - + go func() { - if err := srv.Serve(ln); err != nil && err != http.ErrServerClosed { + if err := srv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) { logging.Errorf("Failed to serve: %v", err) } }() From 237410ac5b8fa1b073ec0f78ad2409b7f77c772c Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:42:24 +0000 Subject: [PATCH 073/123] change MaxConnections from 10 to 1 when testing --- internal/healthcheck/healthcheck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 5c1ac3e09..56c3c3bed 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -88,7 +88,7 @@ func TestStartupPass(t *testing.T) { // the readiness endpoint writes 500. func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ - MaxConnections: 10, + MaxConnections: 1, } hc, err := healthcheck.NewHealthCheck(proxyClient, testPort) if err != nil { From e5a5e417d1cd4266f2d9a57fa2c94a995a4c3188 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 17:44:48 +0000 Subject: [PATCH 074/123] Delete README file --- internal/healthcheck/README.md | 1 - 1 file changed, 1 deletion(-) delete mode 100644 internal/healthcheck/README.md diff --git a/internal/healthcheck/README.md b/internal/healthcheck/README.md deleted file mode 100644 index 8b1378917..000000000 --- a/internal/healthcheck/README.md +++ /dev/null @@ -1 +0,0 @@ - From ae7b6aeb1c0add8d892b7b86ce7ea9c7ccafbcfb Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 17:47:22 +0000 Subject: [PATCH 075/123] rename flag from useHttpHealthCheck to useHTTPHealthCheck --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 4675dee82..a9e65063b 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -135,7 +135,7 @@ unavailable.`, ) // Settings for healthcheck - useHttpHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") + useHTTPHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") hcPort = flag.String("hc_port", "9090", "Health checks will listen and serve this port. Defaults to 9090.") ) @@ -593,7 +593,7 @@ func main() { } var hc *healthcheck.HC - if *useHttpHealthCheck { + if *useHTTPHealthCheck { hc, err = healthcheck.NewHealthCheck(proxyClient, *hcPort) if err != nil { logging.Errorf("Could not initialize health check: %v", err) From 2f19a54f91d2d398abe5ea8f321ffd48731f6152 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 17:57:58 +0000 Subject: [PATCH 076/123] Change port number for testing --- internal/healthcheck/healthcheck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 56c3c3bed..bbadda928 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -26,7 +26,7 @@ import ( const ( livenessPath = "/liveness" readinessPath = "/readiness" - testPort = "9090" + testPort = "8090" ) // Test to verify that when the proxy client is up, the liveness endpoint writes 200. From 9043485520d08eaea0fe564bc9c6b45076c73115 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 18:10:08 +0000 Subject: [PATCH 077/123] rename hcPort flag to healthCheckPort --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index a9e65063b..2a143cba3 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -136,7 +136,7 @@ unavailable.`, // Settings for healthcheck useHTTPHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") - hcPort = flag.String("hc_port", "9090", "Health checks will listen and serve this port. Defaults to 9090.") + healthCheckPort = flag.String("health_check_port", "8090", "Health checks will listen and serve this port. Defaults to 8090.") ) const ( @@ -594,7 +594,7 @@ func main() { var hc *healthcheck.HC if *useHTTPHealthCheck { - hc, err = healthcheck.NewHealthCheck(proxyClient, *hcPort) + hc, err = healthcheck.NewHealthCheck(proxyClient, *healthCheckPort) if err != nil { logging.Errorf("Could not initialize health check: %v", err) } From 115c2014f533676e100839be2e71b2a427a14e87 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 18:16:31 +0000 Subject: [PATCH 078/123] Add GoDoc for started flag --- internal/healthcheck/healthcheck.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index ad784c5b2..e544e383c 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -34,10 +34,11 @@ const ( // HC is a type used to implement health checks for the proxy. type HC struct { - // startedL protects started, a flag that indicates whether the proxy is + // startedL protects the started flag + startedL sync.Mutex + // started is a flag that indicates whether the proxy is // done starting up. started is used to support readiness probing and // should not be confused for affecting startup probing. - startedL sync.Mutex started bool // port designates the port number on which HC listens and serves. port string From 4734ed6126656fb567f0ebad7a80489402f9c96c Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 18:22:57 +0000 Subject: [PATCH 079/123] Add back deferred Close() --- internal/healthcheck/healthcheck_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index bbadda928..89b4f91ac 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -115,6 +115,7 @@ func TestCloseHealthCheck(t *testing.T) { if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } + defer hc.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { From edb2a08068283667d381644f0cac3e8594446dac Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 18:36:20 +0000 Subject: [PATCH 080/123] Rename HC to Server --- internal/healthcheck/healthcheck.go | 36 ++++++++++++------------ internal/healthcheck/healthcheck_test.go | 32 ++++++++++----------- 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/internal/healthcheck/healthcheck.go b/internal/healthcheck/healthcheck.go index e544e383c..08839a429 100644 --- a/internal/healthcheck/healthcheck.go +++ b/internal/healthcheck/healthcheck.go @@ -32,23 +32,23 @@ const ( readinessPath = "/readiness" ) -// HC is a type used to implement health checks for the proxy. -type HC struct { +// Server is a type used to implement health checks for the proxy. +type Server struct { // startedL protects the started flag startedL sync.Mutex // started is a flag that indicates whether the proxy is // done starting up. started is used to support readiness probing and // should not be confused for affecting startup probing. started bool - // port designates the port number on which HC listens and serves. + // port designates the port number on which Server listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. srv *http.Server } -// NewHealthCheck initializes a HC object and exposes HTTP endpoints used to +// NewServer initializes a Server object and exposes HTTP endpoints used to // communicate proxy health. -func NewHealthCheck(c *proxy.Client, port string) (*HC, error) { +func NewServer(c *proxy.Client, port string) (*Server, error) { mux := http.NewServeMux() srv := &http.Server{ @@ -56,13 +56,13 @@ func NewHealthCheck(c *proxy.Client, port string) (*HC, error) { Handler: mux, } - hc := &HC{ + s := &Server{ port: port, srv: srv, } mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - if !isReady(c, hc) { + if !isReady(c, s) { w.WriteHeader(500) w.Write([]byte("error")) return @@ -92,20 +92,20 @@ func NewHealthCheck(c *proxy.Client, port string) (*HC, error) { } }() - return hc, nil + return s, nil } // Close gracefully shuts down the HTTP server belonging to the HC object. -func (hc *HC) Close(ctx context.Context) error { - err := hc.srv.Shutdown(ctx) +func (s *Server) Close(ctx context.Context) error { + err := s.srv.Shutdown(ctx) return err } // NotifyStarted tells the HC that the proxy has finished startup. -func (hc *HC) NotifyStarted() { - hc.startedL.Lock() - hc.started = true - hc.startedL.Unlock() +func (s *Server) NotifyStarted() { + s.startedL.Lock() + s.started = true + s.startedL.Unlock() } // isLive returns true as long as the proxy is running. @@ -117,11 +117,11 @@ func isLive() bool { // proxy is ready for new connections. // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. -func isReady(c *proxy.Client, hc *HC) bool { +func isReady(c *proxy.Client, s *Server) bool { // Not ready until we reach the 'Ready for Connections' log. - hc.startedL.Lock() - started := hc.started - hc.startedL.Unlock() + s.startedL.Lock() + started := s.started + s.startedL.Unlock() if !started { logging.Errorf("Readiness failed because proxy has not finished starting up.") diff --git a/internal/healthcheck/healthcheck_test.go b/internal/healthcheck/healthcheck_test.go index 89b4f91ac..154976bfe 100644 --- a/internal/healthcheck/healthcheck_test.go +++ b/internal/healthcheck/healthcheck_test.go @@ -19,23 +19,23 @@ import ( "net/http" "testing" - "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" + "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) const ( - livenessPath = "/liveness" + livenessPath = "/liveness" readinessPath = "/readiness" - testPort = "8090" + testPort = "8090" ) // Test to verify that when the proxy client is up, the liveness endpoint writes 200. func TestLiveness(t *testing.T) { - hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } - defer hc.Close(context.Background()) + defer s.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { @@ -48,11 +48,11 @@ func TestLiveness(t *testing.T) { // Test to verify that when startup has not finished, the readiness endpoint writes 500. func TestStartupFail(t *testing.T) { - hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } - defer hc.Close(context.Background()) + defer s.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { @@ -66,14 +66,14 @@ func TestStartupFail(t *testing.T) { // Test to verify that when startup has finished, and MaxConnections has not been reached, // the readiness endpoint writes 200. func TestStartupPass(t *testing.T) { - hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } - defer hc.Close(context.Background()) + defer s.Close(context.Background()) // Simulate the proxy client completing startup. - hc.NotifyStarted() + s.NotifyStarted() resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { @@ -90,13 +90,13 @@ func TestMaxConnectionsReached(t *testing.T) { proxyClient := &proxy.Client{ MaxConnections: 1, } - hc, err := healthcheck.NewHealthCheck(proxyClient, testPort) + s, err := healthcheck.NewServer(proxyClient, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } - defer hc.Close(context.Background()) + defer s.Close(context.Background()) - hc.NotifyStarted() + s.NotifyStarted() proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections resp, err := http.Get("http://localhost:" + testPort + readinessPath) @@ -111,11 +111,11 @@ func TestMaxConnectionsReached(t *testing.T) { // Test to verify that after closing a healthcheck, its liveness endpoint serves // an error. func TestCloseHealthCheck(t *testing.T) { - hc, err := healthcheck.NewHealthCheck(&proxy.Client{}, testPort) + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v\n", err) } - defer hc.Close(context.Background()) + defer s.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { @@ -125,7 +125,7 @@ func TestCloseHealthCheck(t *testing.T) { t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) } - err = hc.Close(context.Background()) + err = s.Close(context.Background()) if err != nil { t.Fatalf("Failed to close health check: %v\n", err) } From c4cc7ec619a1f24651a913352eda752fdb4d00ab Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 19:04:17 +0000 Subject: [PATCH 081/123] Move internal pkg to cmd/cloud_sql_proxy/internal --- .../cloud_sql_proxy/internal}/healthcheck/healthcheck.go | 0 .../cloud_sql_proxy/internal}/healthcheck/healthcheck_test.go | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename {internal => cmd/cloud_sql_proxy/internal}/healthcheck/healthcheck.go (100%) rename {internal => cmd/cloud_sql_proxy/internal}/healthcheck/healthcheck_test.go (100%) diff --git a/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go similarity index 100% rename from internal/healthcheck/healthcheck.go rename to cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go diff --git a/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go similarity index 100% rename from internal/healthcheck/healthcheck_test.go rename to cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go From 362ce6ebbd19ab08cf6e3ce2c3338f27d6665d94 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 19:12:34 +0000 Subject: [PATCH 082/123] fix imports and name changes from HC to Server --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 8 ++++---- .../internal/healthcheck/healthcheck_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 2a143cba3..2615f3cc0 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -34,7 +34,7 @@ import ( "syscall" "time" - "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" + "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/certs" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/fuse" @@ -592,11 +592,11 @@ func main() { RefreshCfgBuffer: refreshCfgBuffer, } - var hc *healthcheck.HC + var hc *healthcheck.Server if *useHTTPHealthCheck { - hc, err = healthcheck.NewHealthCheck(proxyClient, *healthCheckPort) + hc, err = healthcheck.NewServer(proxyClient, *healthCheckPort) if err != nil { - logging.Errorf("Could not initialize health check: %v", err) + logging.Errorf("Could not initialize health check server: %v", err) } } diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 154976bfe..6e68c51f3 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -19,7 +19,7 @@ import ( "net/http" "testing" - "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" + "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) From 8c58cc39ee0a79d6738a0c8e71b56e68b961758a Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 19:17:06 +0000 Subject: [PATCH 083/123] keep startedL and started together --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 08839a429..50a6800b7 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -34,11 +34,10 @@ const ( // Server is a type used to implement health checks for the proxy. type Server struct { - // startedL protects the started flag + // started is a flag that indicates whether the proxy is done starting up. + // started is used to support readiness probing and should not be confused + // for affecting startup probing. startedL protects started. startedL sync.Mutex - // started is a flag that indicates whether the proxy is - // done starting up. started is used to support readiness probing and - // should not be confused for affecting startup probing. started bool // port designates the port number on which Server listens and serves. port string From 07c3295bbe3e209a9653077d07ce816ebfcc0b35 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 19:49:44 +0000 Subject: [PATCH 084/123] Rename health check port --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index a9e65063b..bedffef3f 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -34,7 +34,7 @@ import ( "syscall" "time" - "github.com/GoogleCloudPlatform/cloudsql-proxy/internal/healthcheck" + "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/certs" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/fuse" @@ -135,8 +135,8 @@ unavailable.`, ) // Settings for healthcheck - useHTTPHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an http server that checks and communicates the health of the proxy client.") - hcPort = flag.String("hc_port", "9090", "Health checks will listen and serve this port. Defaults to 9090.") + useHTTPHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an HTTP server that checks and communicates the health of the proxy client.") + healthCheckPort = flag.String("health_check_port", "8090", "When applicable, health checks take place on this port number. Defaults to 8090.") ) const ( @@ -592,11 +592,11 @@ func main() { RefreshCfgBuffer: refreshCfgBuffer, } - var hc *healthcheck.HC + var s *healthcheck.Server if *useHTTPHealthCheck { - hc, err = healthcheck.NewHealthCheck(proxyClient, *hcPort) + s, err = healthcheck.NewServer(proxyClient, *healthCheckPort) if err != nil { - logging.Errorf("Could not initialize health check: %v", err) + logging.Errorf("Could not initialize Server for health checks: %v", err) } } @@ -639,8 +639,8 @@ func main() { logging.Infof("Ready for new connections") - if hc != nil { - hc.NotifyStarted() + if s != nil { + s.NotifyStarted() } signals := make(chan os.Signal, 1) From c795f3ecd89a36b97729253f0abe8abacd237da7 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 20:14:10 +0000 Subject: [PATCH 085/123] rename variables for clarity --- .../internal/healthcheck/healthcheck.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 50a6800b7..78f249025 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -55,13 +55,13 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { Handler: mux, } - s := &Server{ + hcServer := &Server{ port: port, srv: srv, } mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { - if !isReady(c, s) { + if !isReady(c, hcServer) { w.WriteHeader(500) w.Write([]byte("error")) return @@ -91,16 +91,16 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { } }() - return s, nil + return hcServer, nil } -// Close gracefully shuts down the HTTP server belonging to the HC object. +// Close gracefully shuts down the HTTP server belonging to the Server object. func (s *Server) Close(ctx context.Context) error { err := s.srv.Shutdown(ctx) return err } -// NotifyStarted tells the HC that the proxy has finished startup. +// NotifyStarted tells the Server that the proxy has finished startup. func (s *Server) NotifyStarted() { s.startedL.Lock() s.started = true From 6bcddc52b2d5f8b377e3ac3449859c115b991541 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Tue, 13 Jul 2021 22:05:00 +0000 Subject: [PATCH 086/123] Check for concrete error type --- .../internal/healthcheck/healthcheck_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 6e68c51f3..fdef82c28 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -16,8 +16,10 @@ package healthcheck_test import ( "context" + "errors" "net/http" "testing" + "syscall" "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -131,7 +133,7 @@ func TestCloseHealthCheck(t *testing.T) { } _, err = http.Get("http://localhost:" + testPort + livenessPath) - if err == nil { // If NO error - t.Fatalf("HTTP GET did not fail after closing health check\n") + if !errors.Is(err, syscall.ECONNREFUSED) { + t.Fatalf("HTTP GET did not give a 'connection refused' error after closing health check\n") } } From 6f05d65247089df553479dc1a71859ffa7659a15 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 13 Jul 2021 22:09:15 +0000 Subject: [PATCH 087/123] add AvailableConn function --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 3 +-- proxy/proxy/client.go | 5 +++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 78f249025..3ec8181a2 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -21,7 +21,6 @@ import ( "net" "net/http" "sync" - "sync/atomic" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -128,7 +127,7 @@ func isReady(c *proxy.Client, s *Server) bool { } // Not ready if the proxy is at the optional MaxConnections limit. - if c.MaxConnections > 0 && atomic.LoadUint64(&c.ConnectionsCounter) >= c.MaxConnections { + if !c.AvailableConn() { logging.Errorf("Readiness failed because proxy has reached the maximum connections limit (%d).", c.MaxConnections) return false } diff --git a/proxy/proxy/client.go b/proxy/proxy/client.go index 9216357f4..a0d519ad8 100644 --- a/proxy/proxy/client.go +++ b/proxy/proxy/client.go @@ -489,6 +489,11 @@ func (c *Client) InstanceVersionContext(ctx context.Context, instance string) (s return version, nil } +// AvailableConn returns false if MaxConnections is set and has been reached, true otherwise. +func (c *Client) AvailableConn() bool { + return c.MaxConnections == 0 || atomic.LoadUint64(&c.ConnectionsCounter) < c.MaxConnections +} + // Shutdown waits up to a given amount of time for all active connections to // close. Returns an error if there are still active connections after waiting // for the whole length of the timeout. From 3426cc9597923fdcdc79d400bc89f60a3ee571b1 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 14 Jul 2021 15:33:30 +0000 Subject: [PATCH 088/123] Table-driven tests --- .../internal/healthcheck/healthcheck_test.go | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index fdef82c28..72026f2b7 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -48,41 +48,44 @@ func TestLiveness(t *testing.T) { } } -// Test to verify that when startup has not finished, the readiness endpoint writes 500. -func TestStartupFail(t *testing.T) { - s, err := healthcheck.NewServer(&proxy.Client{}, testPort) - if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) - } - defer s.Close(context.Background()) - - resp, err := http.Get("http://localhost:" + testPort + readinessPath) - if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) - } - if resp.StatusCode != 500 { - t.Errorf("Got status code %v instead of 500\n", resp.StatusCode) - } -} - -// Test to verify that when startup has finished, and MaxConnections has not been reached, -// the readiness endpoint writes 200. -func TestStartupPass(t *testing.T) { - s, err := healthcheck.NewServer(&proxy.Client{}, testPort) - if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) - } - defer s.Close(context.Background()) - - // Simulate the proxy client completing startup. - s.NotifyStarted() - - resp, err := http.Get("http://localhost:" + testPort + readinessPath) - if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) - } - if resp.StatusCode != 200 { - t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) +// Test to verify that 1. when startup has NOT finished, the readiness endpoint writes 500. +// 2. when startup HAS finished (and MaxConnections limit not specified), the readiness +// endpoint writes 200. +func TestStartup(t *testing.T) { + cases := []struct { + finishedStartup bool + statusCode int + }{ + { + finishedStartup: false, + statusCode: 500, + }, + { + finishedStartup: true, + statusCode: 200, + }, + } + + for _, c := range cases { + func() { + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } + defer s.Close(context.Background()) + + if c.finishedStartup == true { + s.NotifyStarted() // Simulate the proxy client completing startup. + } + + resp, err := http.Get("http://localhost:" + testPort + readinessPath) + if err != nil { + t.Fatalf("HTTP GET failed: %v\n", err) + } + if resp.StatusCode != c.statusCode { + t.Errorf("Got status code %v instead of %v\n", resp.StatusCode, c.statusCode) + } + }() } } From b6ce6fe2e2383e57878c03b9831deb2574ac90be Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 14 Jul 2021 16:20:35 +0000 Subject: [PATCH 089/123] Remove unnecessary newline characters --- .../internal/healthcheck/healthcheck_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 72026f2b7..f59660d63 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -35,16 +35,16 @@ const ( func TestLiveness(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) + t.Fatalf("Could not initialize health check: %v", err) } defer s.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) + t.Fatalf("HTTP GET failed: %v", err) } if resp.StatusCode != 200 { - t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) + t.Errorf("Got status code %v instead of 200", resp.StatusCode) } } @@ -70,7 +70,7 @@ func TestStartup(t *testing.T) { func() { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) + t.Fatalf("Could not initialize health check: %v", err) } defer s.Close(context.Background()) @@ -80,10 +80,10 @@ func TestStartup(t *testing.T) { resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) + t.Fatalf("HTTP GET failed: %v", err) } if resp.StatusCode != c.statusCode { - t.Errorf("Got status code %v instead of %v\n", resp.StatusCode, c.statusCode) + t.Errorf("Got status code %v instead of %v", resp.StatusCode, c.statusCode) } }() } @@ -97,7 +97,7 @@ func TestMaxConnectionsReached(t *testing.T) { } s, err := healthcheck.NewServer(proxyClient, testPort) if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) + t.Fatalf("Could not initialize health check: %v", err) } defer s.Close(context.Background()) @@ -106,10 +106,10 @@ func TestMaxConnectionsReached(t *testing.T) { resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) + t.Fatalf("HTTP GET failed: %v", err) } if resp.StatusCode != 500 { - t.Errorf("Got status code %v instead of 500\n", resp.StatusCode) + t.Errorf("Got status code %v instead of 500", resp.StatusCode) } } @@ -118,25 +118,25 @@ func TestMaxConnectionsReached(t *testing.T) { func TestCloseHealthCheck(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { - t.Fatalf("Could not initialize health check: %v\n", err) + t.Fatalf("Could not initialize health check: %v", err) } defer s.Close(context.Background()) resp, err := http.Get("http://localhost:" + testPort + livenessPath) if err != nil { - t.Fatalf("HTTP GET failed: %v\n", err) + t.Fatalf("HTTP GET failed: %v", err) } if resp.StatusCode != 200 { - t.Errorf("Got status code %v instead of 200\n", resp.StatusCode) + t.Errorf("Got status code %v instead of 200", resp.StatusCode) } err = s.Close(context.Background()) if err != nil { - t.Fatalf("Failed to close health check: %v\n", err) + t.Fatalf("Failed to close health check: %v", err) } _, err = http.Get("http://localhost:" + testPort + livenessPath) if !errors.Is(err, syscall.ECONNREFUSED) { - t.Fatalf("HTTP GET did not give a 'connection refused' error after closing health check\n") + t.Fatalf("HTTP GET did not give a 'connection refused' error after closing health check") } } From ac78dd0d8349f6bf4ea66683d9059699c4f0e27c Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 14 Jul 2021 16:23:57 +0000 Subject: [PATCH 090/123] Rename proxyClient to c --- .../internal/healthcheck/healthcheck_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index f59660d63..01607a517 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -92,17 +92,17 @@ func TestStartup(t *testing.T) { // Test to verify that when startup has finished, but MaxConnections has been reached, // the readiness endpoint writes 500. func TestMaxConnectionsReached(t *testing.T) { - proxyClient := &proxy.Client{ + c := &proxy.Client{ MaxConnections: 1, } - s, err := healthcheck.NewServer(proxyClient, testPort) + s, err := healthcheck.NewServer(c, testPort) if err != nil { t.Fatalf("Could not initialize health check: %v", err) } defer s.Close(context.Background()) s.NotifyStarted() - proxyClient.ConnectionsCounter = proxyClient.MaxConnections // Simulate reaching the limit for maximum number of connections + c.ConnectionsCounter = c.MaxConnections // Simulate reaching the limit for maximum number of connections resp, err := http.Get("http://localhost:" + testPort + readinessPath) if err != nil { From 3c84d6e6d19767bd764e578ff85ad31f78cd28a9 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 16:54:54 +0000 Subject: [PATCH 091/123] move started docstring --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 3ec8181a2..d890c70c5 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -33,10 +33,11 @@ const ( // Server is a type used to implement health checks for the proxy. type Server struct { + // startedL protects started. + startedL sync.Mutex // started is a flag that indicates whether the proxy is done starting up. // started is used to support readiness probing and should not be confused - // for affecting startup probing. startedL protects started. - startedL sync.Mutex + // for affecting startup probing. started bool // port designates the port number on which Server listens and serves. port string From 714ec3ecfd6e4876ae1db1fa3b851f885981e49b Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 16:56:44 +0000 Subject: [PATCH 092/123] make Serve's log message more explicit --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index d890c70c5..f740bf66c 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -87,7 +87,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { go func() { if err := srv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) { - logging.Errorf("Failed to serve: %v", err) + logging.Errorf("Failed to started health check HTTP server: %v", err) } }() From 9533088ce2f8ad36f9f6293fc810ac8988131e79 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 16:57:49 +0000 Subject: [PATCH 093/123] inline Close --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index f740bf66c..f54408ba6 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -96,8 +96,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { // Close gracefully shuts down the HTTP server belonging to the Server object. func (s *Server) Close(ctx context.Context) error { - err := s.srv.Shutdown(ctx) - return err + return s.srv.Shutdown(ctx) } // NotifyStarted tells the Server that the proxy has finished startup. From 84c0c856106d515fcc57a9493d49a59de6f02763 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 17:02:48 +0000 Subject: [PATCH 094/123] make proxy exit if health check cannot be initialized --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 56c69815b..a3f958f2d 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -597,7 +597,9 @@ func main() { hc, err = healthcheck.NewServer(proxyClient, *healthCheckPort) if err != nil { logging.Errorf("Could not initialize health check server: %v", err) + os.Exit(1) } + defer hc.Close(context.Background()) } // Initialize a source of new connections to Cloud SQL instances. From ec6b64947460098c5f4575e77772be746a61055a Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 14 Jul 2021 17:38:03 +0000 Subject: [PATCH 095/123] Use pre-declared ctx variable --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index a3f958f2d..380515810 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -599,7 +599,7 @@ func main() { logging.Errorf("Could not initialize health check server: %v", err) os.Exit(1) } - defer hc.Close(context.Background()) + defer hc.Close(ctx) } // Initialize a source of new connections to Cloud SQL instances. From fc9f4da3ffc3b4745a42279ee254afdf7a8bc92a Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 21:17:42 +0000 Subject: [PATCH 096/123] use http package constants instead of 500 and 200 --- .../internal/healthcheck/healthcheck.go | 8 +++--- .../internal/healthcheck/healthcheck_test.go | 25 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index f54408ba6..8242b52ed 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -62,21 +62,21 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { if !isReady(c, hcServer) { - w.WriteHeader(500) + w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte("error")) return } - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write([]byte("ok")) }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { if !isLive() { // Because isLive() returns true, this case should not be reached. - w.WriteHeader(500) + w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte("error")) return } - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Write([]byte("ok")) }) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 01607a517..0af76ed2a 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -31,7 +31,7 @@ const ( testPort = "8090" ) -// Test to verify that when the proxy client is up, the liveness endpoint writes 200. +// Test to verify that when the proxy client is up, the liveness endpoint writes http.StatusOK. func TestLiveness(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { @@ -43,14 +43,15 @@ func TestLiveness(t *testing.T) { if err != nil { t.Fatalf("HTTP GET failed: %v", err) } - if resp.StatusCode != 200 { - t.Errorf("Got status code %v instead of 200", resp.StatusCode) + if resp.StatusCode != http.StatusOK { + t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusOK) } } -// Test to verify that 1. when startup has NOT finished, the readiness endpoint writes 500. +// Test to verify that +// 1. when startup has NOT finished, the readiness endpoint writes http.StatusServiceUnavailable. // 2. when startup HAS finished (and MaxConnections limit not specified), the readiness -// endpoint writes 200. +// endpoint writes http.StatusOK. func TestStartup(t *testing.T) { cases := []struct { finishedStartup bool @@ -58,11 +59,11 @@ func TestStartup(t *testing.T) { }{ { finishedStartup: false, - statusCode: 500, + statusCode: http.StatusServiceUnavailable, }, { finishedStartup: true, - statusCode: 200, + statusCode: http.StatusOK, }, } @@ -90,7 +91,7 @@ func TestStartup(t *testing.T) { } // Test to verify that when startup has finished, but MaxConnections has been reached, -// the readiness endpoint writes 500. +// the readiness endpoint writes http.StatusServiceUnavailable. func TestMaxConnectionsReached(t *testing.T) { c := &proxy.Client{ MaxConnections: 1, @@ -108,8 +109,8 @@ func TestMaxConnectionsReached(t *testing.T) { if err != nil { t.Fatalf("HTTP GET failed: %v", err) } - if resp.StatusCode != 500 { - t.Errorf("Got status code %v instead of 500", resp.StatusCode) + if resp.StatusCode != http.StatusServiceUnavailable { + t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusServiceUnavailable) } } @@ -126,8 +127,8 @@ func TestCloseHealthCheck(t *testing.T) { if err != nil { t.Fatalf("HTTP GET failed: %v", err) } - if resp.StatusCode != 200 { - t.Errorf("Got status code %v instead of 200", resp.StatusCode) + if resp.StatusCode != http.StatusOK { + t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusOK) } err = s.Close(context.Background()) From fe19c6b305bfb6e577b1f9bf5b088a0a404c77f4 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 21:20:50 +0000 Subject: [PATCH 097/123] add comment to AvailableConn --- proxy/proxy/client.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/proxy/proxy/client.go b/proxy/proxy/client.go index a0d519ad8..9820d8ccb 100644 --- a/proxy/proxy/client.go +++ b/proxy/proxy/client.go @@ -489,7 +489,8 @@ func (c *Client) InstanceVersionContext(ctx context.Context, instance string) (s return version, nil } -// AvailableConn returns false if MaxConnections is set and has been reached, true otherwise. +// AvailableConn returns false if MaxConnections has been reached, true otherwise. +// When MaxConnections is 0, there is no limit. func (c *Client) AvailableConn() bool { return c.MaxConnections == 0 || atomic.LoadUint64(&c.ConnectionsCounter) < c.MaxConnections } From 3bc693c8987b4c0a01351cc439fbd1a542589b9c Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 22:20:51 +0000 Subject: [PATCH 098/123] use a channel for started instead of a bool --- .../internal/healthcheck/healthcheck.go | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 8242b52ed..97e7b95ef 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -20,7 +20,6 @@ import ( "errors" "net" "net/http" - "sync" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -33,12 +32,10 @@ const ( // Server is a type used to implement health checks for the proxy. type Server struct { - // startedL protects started. - startedL sync.Mutex - // started is a flag that indicates whether the proxy is done starting up. - // started is used to support readiness probing and should not be confused - // for affecting startup probing. - started bool + // started is a channel used to indicate whether the proxy has finished + // starting up. The channel is open when startup is in progress and + // closed when startup is complete. + started chan bool // port designates the port number on which Server listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. @@ -56,6 +53,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { } hcServer := &Server{ + started: make(chan bool), port: port, srv: srv, } @@ -101,9 +99,7 @@ func (s *Server) Close(ctx context.Context) error { // NotifyStarted tells the Server that the proxy has finished startup. func (s *Server) NotifyStarted() { - s.startedL.Lock() - s.started = true - s.startedL.Unlock() + close(s.started) } // isLive returns true as long as the proxy is running. @@ -116,12 +112,10 @@ func isLive() bool { // 1. Finished starting up / been sent the 'Ready for Connections' log. // 2. Not yet hit the MaxConnections limit, if applicable. func isReady(c *proxy.Client, s *Server) bool { - // Not ready until we reach the 'Ready for Connections' log. - s.startedL.Lock() - started := s.started - s.startedL.Unlock() - - if !started { + // Not ready until we reach the 'Ready for Connections' log + select { + case <- s.started: // When the channel is closed, the proxy has finished starting up. Do nothing. + default: logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } From 90002bf1c3a783e67ef84619012f2f15621e68a0 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 14 Jul 2021 22:26:42 +0000 Subject: [PATCH 099/123] change started channel type --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 97e7b95ef..df9dbb4f6 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -35,7 +35,7 @@ type Server struct { // started is a channel used to indicate whether the proxy has finished // starting up. The channel is open when startup is in progress and // closed when startup is complete. - started chan bool + started chan struct{} // port designates the port number on which Server listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. @@ -53,7 +53,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { } hcServer := &Server{ - started: make(chan bool), + started: make(chan struct{}), port: port, srv: srv, } From f40df79cf54d698e5a1eceeec6f8d47cb91f61ec Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 14 Jul 2021 23:05:41 +0000 Subject: [PATCH 100/123] Revert back to separate tests See context: https://github.com/monazhn/cloudsql-proxy/pull/1#discussion_r669750556 --- .../internal/healthcheck/healthcheck_test.go | 75 +++++++++---------- 1 file changed, 36 insertions(+), 39 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 0af76ed2a..6a1760e44 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -48,45 +48,42 @@ func TestLiveness(t *testing.T) { } } -// Test to verify that -// 1. when startup has NOT finished, the readiness endpoint writes http.StatusServiceUnavailable. -// 2. when startup HAS finished (and MaxConnections limit not specified), the readiness -// endpoint writes http.StatusOK. -func TestStartup(t *testing.T) { - cases := []struct { - finishedStartup bool - statusCode int - }{ - { - finishedStartup: false, - statusCode: http.StatusServiceUnavailable, - }, - { - finishedStartup: true, - statusCode: http.StatusOK, - }, - } - - for _, c := range cases { - func() { - s, err := healthcheck.NewServer(&proxy.Client{}, testPort) - if err != nil { - t.Fatalf("Could not initialize health check: %v", err) - } - defer s.Close(context.Background()) - - if c.finishedStartup == true { - s.NotifyStarted() // Simulate the proxy client completing startup. - } - - resp, err := http.Get("http://localhost:" + testPort + readinessPath) - if err != nil { - t.Fatalf("HTTP GET failed: %v", err) - } - if resp.StatusCode != c.statusCode { - t.Errorf("Got status code %v instead of %v", resp.StatusCode, c.statusCode) - } - }() +// Test to verify that when startup has NOT finished, the readiness endpoint writes +// http.StatusServiceUnavailable. +func TestStartupFail(t *testing.T) { + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } + defer s.Close(context.Background()) + + resp, err := http.Get("http://localhost:" + testPort + readinessPath) + if err != nil { + t.Fatalf("HTTP GET failed: %v\n", err) + } + if resp.StatusCode != http.StatusServiceUnavailable { + t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusServiceUnavailable) + } +} + +// Test to verify that when startup HAS finished (and MaxConnections limit not specified), +// the readiness endpoint writes http.StatusOK. +func TestStartupPass(t *testing.T) { + s, err := healthcheck.NewServer(&proxy.Client{}, testPort) + if err != nil { + t.Fatalf("Could not initialize health check: %v\n", err) + } + defer s.Close(context.Background()) + + // Simulate the proxy client completing startup. + s.NotifyStarted() + + resp, err := http.Get("http://localhost:" + testPort + readinessPath) + if err != nil { + t.Fatalf("HTTP GET failed: %v\n", err) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusOK) } } From ffa7280c19b21870f5907e000e7d88722851c2b9 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 15 Jul 2021 17:07:31 +0000 Subject: [PATCH 101/123] use Once to ensure that started can only close once --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index df9dbb4f6..27b907e04 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -20,6 +20,7 @@ import ( "errors" "net" "net/http" + "sync" "github.com/GoogleCloudPlatform/cloudsql-proxy/logging" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" @@ -36,6 +37,8 @@ type Server struct { // starting up. The channel is open when startup is in progress and // closed when startup is complete. started chan struct{} + // once ensures that started can only closed once. + once sync.Once // port designates the port number on which Server listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. @@ -99,7 +102,7 @@ func (s *Server) Close(ctx context.Context) error { // NotifyStarted tells the Server that the proxy has finished startup. func (s *Server) NotifyStarted() { - close(s.started) + s.once.Do(func() { close(s.started) }) } // isLive returns true as long as the proxy is running. From 4ba3cfeb70069d76ac53a2863676631472062c4a Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Thu, 15 Jul 2021 17:59:13 +0000 Subject: [PATCH 102/123] make once a pointer --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 27b907e04..631dfa71c 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -38,7 +38,7 @@ type Server struct { // closed when startup is complete. started chan struct{} // once ensures that started can only closed once. - once sync.Once + once *sync.Once // port designates the port number on which Server listens and serves. port string // srv is a pointer to the HTTP server used to communicated proxy health. @@ -57,6 +57,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { hcServer := &Server{ started: make(chan struct{}), + once: &sync.Once{}, port: port, srv: srv, } From aec9362f65a3fe43ae8d5dde869145956c96198d Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Mon, 19 Jul 2021 22:41:03 +0000 Subject: [PATCH 103/123] add startup probe --- .../internal/healthcheck/healthcheck.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 631dfa71c..c025716a4 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -27,6 +27,7 @@ import ( ) const ( + startupPath = "/startup" livenessPath = "/liveness" readinessPath = "/readiness" ) @@ -62,6 +63,17 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { srv: srv, } + mux.HandleFunc(startupPath, func(w http.ResponseWriter, _ *http.Request) { + select { + case <- hcServer.started: // When the channel is closed, the proxy has finished starting up. + w.WriteHeader(http.StatusOK) + w.Write([]byte("ok")) + default: + w.WriteHeader(http.StatusServiceUnavailable) + w.Write([]byte("error")) + } + }) + mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { if !isReady(c, hcServer) { w.WriteHeader(http.StatusServiceUnavailable) From 2048a33baa286b9e486e1cfeebe0794c3de61c88 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Mon, 19 Jul 2021 22:50:44 +0000 Subject: [PATCH 104/123] add tests against /startup --- .../internal/healthcheck/healthcheck.go | 2 +- .../internal/healthcheck/healthcheck_test.go | 25 ++++++++++++++++--- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index c025716a4..7f419e446 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -85,7 +85,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { }) mux.HandleFunc(livenessPath, func(w http.ResponseWriter, _ *http.Request) { - if !isLive() { // Because isLive() returns true, this case should not be reached. + if !isLive() { // Because isLive() always returns true, this case should not be reached. w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte("error")) return diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 6a1760e44..5c8ab2e3a 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -26,6 +26,7 @@ import ( ) const ( + startupPath = "/startup" livenessPath = "/liveness" readinessPath = "/readiness" testPort = "8090" @@ -57,12 +58,20 @@ func TestStartupFail(t *testing.T) { } defer s.Close(context.Background()) - resp, err := http.Get("http://localhost:" + testPort + readinessPath) + resp, err := http.Get("http://localhost:" + testPort + startupPath) if err != nil { t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != http.StatusServiceUnavailable { - t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusServiceUnavailable) + t.Errorf("%v returned status code %v instead of %v", startupPath, resp.StatusCode, http.StatusServiceUnavailable) + } + + resp, err = http.Get("http://localhost:" + testPort + readinessPath) + if err != nil { + t.Fatalf("HTTP GET failed: %v\n", err) + } + if resp.StatusCode != http.StatusServiceUnavailable { + t.Errorf("%v returned status code %v instead of %v", readinessPath, resp.StatusCode, http.StatusServiceUnavailable) } } @@ -78,12 +87,20 @@ func TestStartupPass(t *testing.T) { // Simulate the proxy client completing startup. s.NotifyStarted() - resp, err := http.Get("http://localhost:" + testPort + readinessPath) + resp, err := http.Get("http://localhost:" + testPort + startupPath) if err != nil { t.Fatalf("HTTP GET failed: %v\n", err) } if resp.StatusCode != http.StatusOK { - t.Errorf("Got status code %v instead of %v", resp.StatusCode, http.StatusOK) + t.Errorf("%v returned status code %v instead of %v", startupPath, resp.StatusCode, http.StatusOK) + } + + resp, err = http.Get("http://localhost:" + testPort + readinessPath) + if err != nil { + t.Fatalf("HTTP GET failed: %v\n", err) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("%v returned status code %v instead of %v", readinessPath, resp.StatusCode, http.StatusOK) } } From ef65faed5039281513f8c167aaffb7022533b564 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Mon, 19 Jul 2021 22:52:21 +0000 Subject: [PATCH 105/123] fix comments --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 5c8ab2e3a..63bece873 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -49,7 +49,7 @@ func TestLiveness(t *testing.T) { } } -// Test to verify that when startup has NOT finished, the readiness endpoint writes +// Test to verify that when startup has NOT finished, the startup and readiness endpoints write // http.StatusServiceUnavailable. func TestStartupFail(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) @@ -76,7 +76,7 @@ func TestStartupFail(t *testing.T) { } // Test to verify that when startup HAS finished (and MaxConnections limit not specified), -// the readiness endpoint writes http.StatusOK. +// the startup and readiness endpoints write http.StatusOK. func TestStartupPass(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) if err != nil { From 14e6f04487168874ea941a4edc2cc562389c1ea6 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 20 Jul 2021 18:44:08 +0000 Subject: [PATCH 106/123] add helper function that checks the status of the started channel --- .../internal/healthcheck/healthcheck.go | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 7f419e446..2ebeb32d8 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -64,14 +64,12 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { } mux.HandleFunc(startupPath, func(w http.ResponseWriter, _ *http.Request) { - select { - case <- hcServer.started: // When the channel is closed, the proxy has finished starting up. - w.WriteHeader(http.StatusOK) - w.Write([]byte("ok")) - default: + if !hcServer.proxyStarted() { w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte("error")) } + w.WriteHeader(http.StatusOK) + w.Write([]byte("ok")) }) mux.HandleFunc(readinessPath, func(w http.ResponseWriter, _ *http.Request) { @@ -118,6 +116,16 @@ func (s *Server) NotifyStarted() { s.once.Do(func() { close(s.started) }) } +// proxyStarted returns true if started is closed, false otherwise +func (s *Server) proxyStarted() bool { + select { + case <- s.started: + return true + default: + return false + } +} + // isLive returns true as long as the proxy is running. func isLive() bool { return true @@ -129,9 +137,7 @@ func isLive() bool { // 2. Not yet hit the MaxConnections limit, if applicable. func isReady(c *proxy.Client, s *Server) bool { // Not ready until we reach the 'Ready for Connections' log - select { - case <- s.started: // When the channel is closed, the proxy has finished starting up. Do nothing. - default: + if !s.proxyStarted() { logging.Errorf("Readiness failed because proxy has not finished starting up.") return false } From 6751476c2f40fbf356e24d00732035818df19120 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 20 Jul 2021 18:48:16 +0000 Subject: [PATCH 107/123] fix logging message typo --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 2ebeb32d8..a60159c68 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -99,7 +99,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { go func() { if err := srv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) { - logging.Errorf("Failed to started health check HTTP server: %v", err) + logging.Errorf("Failed to start health check HTTP server: %v", err) } }() From 0055c1567c8665456edc48034ba761ffa4577373 Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Tue, 20 Jul 2021 19:03:33 +0000 Subject: [PATCH 108/123] forgot a return --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index a60159c68..4952dc508 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -67,6 +67,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { if !hcServer.proxyStarted() { w.WriteHeader(http.StatusServiceUnavailable) w.Write([]byte("error")) + return } w.WriteHeader(http.StatusOK) w.Write([]byte("ok")) From c5709e64715ddbaf3e91bf09a63317a8768a5fba Mon Sep 17 00:00:00 2001 From: Tiffany Xiang Date: Wed, 21 Jul 2021 22:19:29 +0000 Subject: [PATCH 109/123] comments --- .../internal/healthcheck/healthcheck.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 4952dc508..c7cece73e 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -34,11 +34,11 @@ const ( // Server is a type used to implement health checks for the proxy. type Server struct { - // started is a channel used to indicate whether the proxy has finished - // starting up. The channel is open when startup is in progress and - // closed when startup is complete. + // started is used to indicate whether the proxy has finished starting up. + // If started is open, startup has not finished. If started is closed, + // startup is complete. started chan struct{} - // once ensures that started can only closed once. + // once ensures that started can only be closed once. once *sync.Once // port designates the port number on which Server listens and serves. port string @@ -117,7 +117,7 @@ func (s *Server) NotifyStarted() { s.once.Do(func() { close(s.started) }) } -// proxyStarted returns true if started is closed, false otherwise +// proxyStarted returns true if started is closed, false otherwise. func (s *Server) proxyStarted() bool { select { case <- s.started: From 11655a3dc7c2f73394526a108737d16a27342f86 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 28 Jul 2021 18:03:38 -0400 Subject: [PATCH 110/123] Add health check flags to README --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 5f665093f..d6ffb3924 100644 --- a/README.md +++ b/README.md @@ -284,6 +284,14 @@ msg. For example, the startup message looks like: gcloud's active project: [my-project-id]"} ``` +#### `-use_http_health_check` + +Enables HTTP health checks on /startup, /liveness, and /readiness endpoints. If +deployed on Kubernetes, the configuration file should specify the probes in use. + +#### `-health_check_port=8090` + +Specifies the port that the health check server listens and serves on. Defaults to 8090. ## Running as a Kubernetes Sidecar From dd22c2c2b38721a27305c57699283fc222148c58 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Wed, 28 Jul 2021 18:43:48 -0400 Subject: [PATCH 111/123] Clarify wording --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d6ffb3924..b8e42da7f 100644 --- a/README.md +++ b/README.md @@ -286,8 +286,8 @@ gcloud's active project: [my-project-id]"} ``` #### `-use_http_health_check` -Enables HTTP health checks on /startup, /liveness, and /readiness endpoints. If -deployed on Kubernetes, the configuration file should specify the probes in use. +Enables HTTP health checks for the proxy, including startup, liveness, and readiness probing. +Requires that you configure the Kubernetes container with HTTP probes. #### `-health_check_port=8090` From c64a93c6a0caf2ef0bddc8ac0fc52b43ab2dddf6 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Thu, 29 Jul 2021 15:19:36 -0400 Subject: [PATCH 112/123] Move changes into separate branch branch name: instructions --- README.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/README.md b/README.md index b8e42da7f..5f665093f 100644 --- a/README.md +++ b/README.md @@ -284,14 +284,6 @@ msg. For example, the startup message looks like: gcloud's active project: [my-project-id]"} ``` -#### `-use_http_health_check` - -Enables HTTP health checks for the proxy, including startup, liveness, and readiness probing. -Requires that you configure the Kubernetes container with HTTP probes. - -#### `-health_check_port=8090` - -Specifies the port that the health check server listens and serves on. Defaults to 8090. ## Running as a Kubernetes Sidecar From ea94208ef97c1393992a7e28ced04ba0c9ab0b6c Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 30 Jul 2021 17:25:42 +0000 Subject: [PATCH 113/123] Add instructions for using the new health check feature (#3) Co-authored-by: Tiffany Xiang --- README.md | 9 ++ .../proxy_with_http_health_check.yaml | 116 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 examples/k8s-health-check/proxy_with_http_health_check.yaml diff --git a/README.md b/README.md index 5f665093f..556877678 100644 --- a/README.md +++ b/README.md @@ -285,6 +285,15 @@ gcloud's active project: [my-project-id]"} ``` +#### `-use_http_health_check` + +Enables HTTP health checks for the proxy, including startup, liveness, and readiness probing. +Requires that you configure the Kubernetes container with HTTP probes ([sample](https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/main/examples/k8s-health-check/proxy_with_http_health_check.yaml)). + +#### `-health_check_port=8090` + +Specifies the port that the health check server listens and serves on. Defaults to 8090. + ## Running as a Kubernetes Sidecar See the [example here][sidecar-example] as well as [Connecting from Google diff --git a/examples/k8s-health-check/proxy_with_http_health_check.yaml b/examples/k8s-health-check/proxy_with_http_health_check.yaml new file mode 100644 index 000000000..6c959d24d --- /dev/null +++ b/examples/k8s-health-check/proxy_with_http_health_check.yaml @@ -0,0 +1,116 @@ +# You must configure probes in your deployment to use health checks in Kubernetes. +# This sample configuration for HTTP probes is adapted from proxy_with_workload_identity.yaml. +apiVersion: apps/v1 +kind: Deployment +metadata: + name: +spec: + selector: + matchLabels: + app: + template: + metadata: + labels: + app: + spec: + containers: + - name: + # ... other container configuration + env: + - name: DB_USER + valueFrom: + secretKeyRef: + name: + key: username + - name: DB_PASS + valueFrom: + secretKeyRef: + name: + key: password + - name: DB_NAME + valueFrom: + secretKeyRef: + name: + key: database + - name: cloud-sql-proxy + # It is recommended to use the latest version of the Cloud SQL proxy + # Make sure to update on a regular schedule! + image: gcr.io/cloudsql-docker/gce-proxy:1.17 + command: + - "/cloud_sql_proxy" + + # If connecting from a VPC-native GKE cluster, you can use the + # following flag to have the proxy connect over private IP + # - "-ip_address_types=PRIVATE" + + # Replace DB_PORT with the port the proxy should listen on + # Defaults: MySQL: 3306, Postgres: 5432, SQLServer: 1433 + - "-instances==tcp:" + # Enable HTTP health checks on the default port (8090). + - "-use_http_health_check" + # [START cloud_sql_proxy_k8s_volume_mount] + # This flag specifies where the service account key can be found + - "-credential_file=/secrets/service_account.json" + securityContext: + # The default Cloud SQL proxy image runs as the + # "nonroot" user and group (uid: 65532) by default. + runAsNonRoot: true + volumeMounts: + - name: + mountPath: /secrets/ + readOnly: true + # [END cloud_sql_proxy_k8s_volume_mount] + # Resource configuration depends on an application's requirements. You + # should adjust the following values based on what your application + # needs. For details, see https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + resources: + requests: + # The proxy's memory use scales linearly with the number of active + # connections. Fewer open connections will use less memory. Adjust + # this value based on your application's requirements. + memory: "2Gi" + # The proxy's CPU use scales linearly with the amount of IO between + # the database and the application. Adjust this value based on your + # application's requirements. + cpu: "1" + # Recommended configurations for health check probes. + # Probe parameters can be adjusted to best fit the requirements of your application. + # For details, see https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ + livenessProbe: + httpGet: + path: /liveness + port: 8090 + # Number of seconds after the container has started before the first probe is scheduled. Defaults to 0. + # Not necessary when the startup probe is in use. + initialDelaySeconds: 0 + # Frequency of the probe. Defaults to 10. + periodSeconds: 10 + # Number of seconds after which the probe times out. Defaults to 1. + timeoutSeconds: 5 + # Number of times the probe is allowed to fail before the transition from healthy to failure state. + # Defaults to 3. + failureThreshold: 1 + readinessProbe: + httpGet: + path: /liveness + port: 8090 + initialDelaySeconds: 0 + periodSeconds: 10 + timeoutSeconds: 5 + # Number of times the probe must report success to transition from failure to healthy state. + # Defaults to 1 for readiness probe. + successThreshold: 1 + failureThreshold: 1 + startupProbe: + httpGet: + path: /startup + port: 8090 + periodSeconds: 1 + timeoutSeconds: 5 + failureThreshold: 20 + # [START cloud_sql_proxy_k8s_volume_secret] + volumes: + - name: + secret: + secretName: + # [END cloud_sql_proxy_k8s_volume_secret] From 4aaff8dc9d3eb4d7548ab3d9de5e2bc490c3b081 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 30 Jul 2021 21:29:48 +0000 Subject: [PATCH 114/123] Fix lint --- cmd/cloud_sql_proxy/cloud_sql_proxy.go | 2 +- .../internal/healthcheck/healthcheck.go | 18 +++++++++--------- .../internal/healthcheck/healthcheck_test.go | 6 +++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/cmd/cloud_sql_proxy/cloud_sql_proxy.go b/cmd/cloud_sql_proxy/cloud_sql_proxy.go index 1bf7545ff..7ad55fb8f 100644 --- a/cmd/cloud_sql_proxy/cloud_sql_proxy.go +++ b/cmd/cloud_sql_proxy/cloud_sql_proxy.go @@ -135,7 +135,7 @@ unavailable.`, // Settings for healthcheck useHTTPHealthCheck = flag.Bool("use_http_health_check", false, "When set, creates an HTTP server that checks and communicates the health of the proxy client.") - healthCheckPort = flag.String("health_check_port", "8090", "When applicable, health checks take place on this port number. Defaults to 8090.") + healthCheckPort = flag.String("health_check_port", "8090", "When applicable, health checks take place on this port number. Defaults to 8090.") ) const ( diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index c7cece73e..4fa16c589 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -27,14 +27,14 @@ import ( ) const ( - startupPath = "/startup" - livenessPath = "/liveness" + startupPath = "/startup" + livenessPath = "/liveness" readinessPath = "/readiness" ) // Server is a type used to implement health checks for the proxy. type Server struct { - // started is used to indicate whether the proxy has finished starting up. + // started is used to indicate whether the proxy has finished starting up. // If started is open, startup has not finished. If started is closed, // startup is complete. started chan struct{} @@ -52,15 +52,15 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { mux := http.NewServeMux() srv := &http.Server{ - Addr: ":" + port, + Addr: ":" + port, Handler: mux, } hcServer := &Server{ started: make(chan struct{}), - once: &sync.Once{}, - port: port, - srv: srv, + once: &sync.Once{}, + port: port, + srv: srv, } mux.HandleFunc(startupPath, func(w http.ResponseWriter, _ *http.Request) { @@ -97,7 +97,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { if err != nil { return nil, err } - + go func() { if err := srv.Serve(ln); err != nil && !errors.Is(err, http.ErrServerClosed) { logging.Errorf("Failed to start health check HTTP server: %v", err) @@ -120,7 +120,7 @@ func (s *Server) NotifyStarted() { // proxyStarted returns true if started is closed, false otherwise. func (s *Server) proxyStarted() bool { select { - case <- s.started: + case <-s.started: return true default: return false diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 63bece873..d2cb87c43 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -18,15 +18,15 @@ import ( "context" "errors" "net/http" - "testing" "syscall" + "testing" "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" "github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy" ) const ( - startupPath = "/startup" + startupPath = "/startup" livenessPath = "/liveness" readinessPath = "/readiness" testPort = "8090" @@ -75,7 +75,7 @@ func TestStartupFail(t *testing.T) { } } -// Test to verify that when startup HAS finished (and MaxConnections limit not specified), +// Test to verify that when startup HAS finished (and MaxConnections limit not specified), // the startup and readiness endpoints write http.StatusOK. func TestStartupPass(t *testing.T) { s, err := healthcheck.NewServer(&proxy.Client{}, testPort) From 14c553cf8a04e5a8e2ca51136a9b3b7ebe63ddab Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Fri, 30 Jul 2021 22:15:23 +0000 Subject: [PATCH 115/123] Improve error message --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index d2cb87c43..3a38c690a 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -152,6 +152,6 @@ func TestCloseHealthCheck(t *testing.T) { _, err = http.Get("http://localhost:" + testPort + livenessPath) if !errors.Is(err, syscall.ECONNREFUSED) { - t.Fatalf("HTTP GET did not give a 'connection refused' error after closing health check") + t.Fatalf("Got '%v' instead of 'connected fused' error after closing health check server.", err) } } From 3e98064c36f170345052fadc53d9acf66d831e3d Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Sat, 31 Jul 2021 00:05:49 +0000 Subject: [PATCH 116/123] Modify error condition Fixes failing test on Windows. --- .../internal/healthcheck/healthcheck_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go index 3a38c690a..f0b2a8d3f 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck_test.go @@ -16,9 +16,7 @@ package healthcheck_test import ( "context" - "errors" "net/http" - "syscall" "testing" "github.com/GoogleCloudPlatform/cloudsql-proxy/cmd/cloud_sql_proxy/internal/healthcheck" @@ -151,7 +149,7 @@ func TestCloseHealthCheck(t *testing.T) { } _, err = http.Get("http://localhost:" + testPort + livenessPath) - if !errors.Is(err, syscall.ECONNREFUSED) { - t.Fatalf("Got '%v' instead of 'connected fused' error after closing health check server.", err) + if err == nil { + t.Fatalf("HTTP GET did not return error after closing health check server.") } } From e1e8dfb2fd6eec19ef8ab0aad42713d15a573376 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 09:24:38 -0400 Subject: [PATCH 117/123] Remove region tags --- examples/k8s-health-check/proxy_with_http_health_check.yaml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/examples/k8s-health-check/proxy_with_http_health_check.yaml b/examples/k8s-health-check/proxy_with_http_health_check.yaml index 6c959d24d..b35052827 100644 --- a/examples/k8s-health-check/proxy_with_http_health_check.yaml +++ b/examples/k8s-health-check/proxy_with_http_health_check.yaml @@ -48,7 +48,6 @@ spec: - "-instances==tcp:" # Enable HTTP health checks on the default port (8090). - "-use_http_health_check" - # [START cloud_sql_proxy_k8s_volume_mount] # This flag specifies where the service account key can be found - "-credential_file=/secrets/service_account.json" securityContext: @@ -59,7 +58,6 @@ spec: - name: mountPath: /secrets/ readOnly: true - # [END cloud_sql_proxy_k8s_volume_mount] # Resource configuration depends on an application's requirements. You # should adjust the following values based on what your application # needs. For details, see https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ @@ -108,9 +106,7 @@ spec: periodSeconds: 1 timeoutSeconds: 5 failureThreshold: 20 - # [START cloud_sql_proxy_k8s_volume_secret] volumes: - name: secret: secretName: - # [END cloud_sql_proxy_k8s_volume_secret] From f18f54942fd6bb5e197da392e02febe2647932a7 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 11:38:13 -0400 Subject: [PATCH 118/123] Use footer link --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 556877678..581afac9b 100644 --- a/README.md +++ b/README.md @@ -288,7 +288,7 @@ gcloud's active project: [my-project-id]"} #### `-use_http_health_check` Enables HTTP health checks for the proxy, including startup, liveness, and readiness probing. -Requires that you configure the Kubernetes container with HTTP probes ([sample](https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/main/examples/k8s-health-check/proxy_with_http_health_check.yaml)). +Requires that you configure the Kubernetes container with HTTP probes ([example][health-check-example]). #### `-health_check_port=8090` @@ -345,6 +345,7 @@ Install via Nuget, follow these [connect-to-k8s]: https://cloud.google.com/sql/docs/mysql/connect-kubernetes-engine [connection-overview]: https://cloud.google.com/sql/docs/mysql/connect-overview [contributing]: CONTRIBUTING.md +[health-check-example]: https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/master/examples/k8s-health-check/proxy_with_http_health_check.yaml [iam-auth]: https://cloud.google.com/sql/docs/postgres/authentication [pkg-badge]: https://pkg.go.dev/badge/github.com/GoogleCloudPlatform/cloudsql-proxy.svg [pkg-docs]: https://pkg.go.dev/github.com/GoogleCloudPlatform/cloudsql-proxy From 1ab9c5ceec2799cafd494df74843ba579b366b5d Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 12:00:06 -0400 Subject: [PATCH 119/123] Clean up docstrings --- cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go index 4fa16c589..3bb956af8 100644 --- a/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go +++ b/cmd/cloud_sql_proxy/internal/healthcheck/healthcheck.go @@ -42,11 +42,11 @@ type Server struct { once *sync.Once // port designates the port number on which Server listens and serves. port string - // srv is a pointer to the HTTP server used to communicated proxy health. + // srv is a pointer to the HTTP server used to communicate proxy health. srv *http.Server } -// NewServer initializes a Server object and exposes HTTP endpoints used to +// NewServer initializes a Server and exposes HTTP endpoints used to // communicate proxy health. func NewServer(c *proxy.Client, port string) (*Server, error) { mux := http.NewServeMux() @@ -107,7 +107,7 @@ func NewServer(c *proxy.Client, port string) (*Server, error) { return hcServer, nil } -// Close gracefully shuts down the HTTP server belonging to the Server object. +// Close gracefully shuts down the HTTP server belonging to the Server. func (s *Server) Close(ctx context.Context) error { return s.srv.Shutdown(ctx) } From edef210526a21e25e23eb575ab9d8a241b2d8769 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 15:50:44 -0400 Subject: [PATCH 120/123] Clean up --- examples/k8s-health-check/proxy_with_http_health_check.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/examples/k8s-health-check/proxy_with_http_health_check.yaml b/examples/k8s-health-check/proxy_with_http_health_check.yaml index b35052827..537771256 100644 --- a/examples/k8s-health-check/proxy_with_http_health_check.yaml +++ b/examples/k8s-health-check/proxy_with_http_health_check.yaml @@ -46,8 +46,11 @@ spec: # Replace DB_PORT with the port the proxy should listen on # Defaults: MySQL: 3306, Postgres: 5432, SQLServer: 1433 - "-instances==tcp:" - # Enable HTTP health checks on the default port (8090). + # Enables HTTP health checks. - "-use_http_health_check" + # Specifies the health check server port. + # Defaults to 8090. + - "-health_check_port=" # This flag specifies where the service account key can be found - "-credential_file=/secrets/service_account.json" securityContext: From e97040e58fabef2b543cd64d6b9a5c46e874ee49 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 15:51:30 -0400 Subject: [PATCH 121/123] Add instructions for using health checks --- examples/k8s-health-check/README.md | 70 +++++++++++++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 examples/k8s-health-check/README.md diff --git a/examples/k8s-health-check/README.md b/examples/k8s-health-check/README.md new file mode 100644 index 000000000..a78d1c0d6 --- /dev/null +++ b/examples/k8s-health-check/README.md @@ -0,0 +1,70 @@ +# Cloud SQL proxy health checks + +Kubernetes supports three types of health checks. +1. Liveness probes determine whether a container is healthy. When this probe is unsuccessful, the container is restarted. +2. Readiness probes determine whether a container can serve new traffic. When this probe fails, Kubernetes will wait to send requests to the container. +3. Startup probes determine whether a container is done starting up. As soon as this probe succeeds, Kubernetes switches over to using liveness and readiness probing. + +## Running Cloud SQL proxy with health checks in Kubernetes +1. Configure your Cloud SQL proxy container to include health check probes. + > [proxy_with_http_health_check.yaml](proxy_with_http_health_check.yaml#L77-L111) + ```yaml + # Recommended configurations for health check probes. + # Probe parameters can be adjusted to best fit the requirements of your application. + # For details, see https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ + livenessProbe: + httpGet: + path: /liveness + port: 8090 + # Number of seconds after the container has started before the first probe is scheduled. Defaults to 0. + # Not necessary when the startup probe is in use. + initialDelaySeconds: 0 + # Frequency of the probe. Defaults to 10. + periodSeconds: 10 + # Number of seconds after which the probe times out. Defaults to 1. + timeoutSeconds: 5 + # Number of times the probe is allowed to fail before the transition from healthy to failure state. + # Defaults to 3. + failureThreshold: 1 + readinessProbe: + httpGet: + path: /liveness + port: 8090 + initialDelaySeconds: 0 + periodSeconds: 10 + timeoutSeconds: 5 + # Number of times the probe must report success to transition from failure to healthy state. + # Defaults to 1 for readiness probe. + successThreshold: 1 + failureThreshold: 1 + startupProbe: + httpGet: + path: /startup + port: 8090 + periodSeconds: 1 + timeoutSeconds: 5 + failureThreshold: 20 + ``` + +2. Add `-use_http_health_check` and `-health-check-port` (optional) to your proxy container configuration under `command: `. + > [proxy_with_http_health_check.yaml](proxy_with_http_health_check.yaml#L39-L55) + ```yaml + command: + - "/cloud_sql_proxy" + + # If connecting from a VPC-native GKE cluster, you can use the + # following flag to have the proxy connect over private IP + # - "-ip_address_types=PRIVATE" + + # Replace DB_PORT with the port the proxy should listen on + # Defaults: MySQL: 3306, Postgres: 5432, SQLServer: 1433 + - "-instances==tcp:" + # Enables HTTP health checks. + - "-use_http_health_check" + # Specifies the health check server port. + # Defaults to 8090. + - "-health_check_port=" + # This flag specifies where the service account key can be found + - "-credential_file=/secrets/service_account.json" + ``` + From 6b3e7dcc11c3bd9c70618660bff0547c540daab8 Mon Sep 17 00:00:00 2001 From: tifftoff <48542947+tifftoff@users.noreply.github.com> Date: Mon, 2 Aug 2021 12:58:51 -0700 Subject: [PATCH 122/123] Update README.md --- examples/k8s-health-check/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/k8s-health-check/README.md b/examples/k8s-health-check/README.md index a78d1c0d6..a8fd01889 100644 --- a/examples/k8s-health-check/README.md +++ b/examples/k8s-health-check/README.md @@ -1,9 +1,9 @@ # Cloud SQL proxy health checks -Kubernetes supports three types of health checks. -1. Liveness probes determine whether a container is healthy. When this probe is unsuccessful, the container is restarted. -2. Readiness probes determine whether a container can serve new traffic. When this probe fails, Kubernetes will wait to send requests to the container. -3. Startup probes determine whether a container is done starting up. As soon as this probe succeeds, Kubernetes switches over to using liveness and readiness probing. +Kubernetes supports three types of health checks. +1. Startup probes determine whether a container is done starting up. As soon as this probe succeeds, Kubernetes switches over to using liveness and readiness probing. +2. Liveness probes determine whether a container is healthy. When this probe is unsuccessful, the container is restarted. +3. Readiness probes determine whether a container can serve new traffic. When this probe fails, Kubernetes will wait to send requests to the container. ## Running Cloud SQL proxy with health checks in Kubernetes 1. Configure your Cloud SQL proxy container to include health check probes. From 1339f8c606f05f37a1dc2f0872a70690177a46f7 Mon Sep 17 00:00:00 2001 From: Mona Zhang Date: Mon, 2 Aug 2021 17:22:39 -0400 Subject: [PATCH 123/123] Link to README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 7af8e8a91..6d2e7ce39 100644 --- a/README.md +++ b/README.md @@ -286,7 +286,7 @@ message and optionally stacktrace. For example, the startup message looks like: #### `-use_http_health_check` Enables HTTP health checks for the proxy, including startup, liveness, and readiness probing. -Requires that you configure the Kubernetes container with HTTP probes ([example][health-check-example]). +Requires that you configure the Kubernetes container with HTTP probes ([instructions][health-check-example]). #### `-health_check_port=8090` @@ -343,7 +343,7 @@ Install via Nuget, follow these [connect-to-k8s]: https://cloud.google.com/sql/docs/mysql/connect-kubernetes-engine [connection-overview]: https://cloud.google.com/sql/docs/mysql/connect-overview [contributing]: CONTRIBUTING.md -[health-check-example]: https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/master/examples/k8s-health-check/proxy_with_http_health_check.yaml +[health-check-example]: https://github.com/GoogleCloudPlatform/cloudsql-proxy/tree/main/examples/k8s-health-check#cloud-sql-proxy-health-checks [iam-auth]: https://cloud.google.com/sql/docs/postgres/authentication [pkg-badge]: https://pkg.go.dev/badge/github.com/GoogleCloudPlatform/cloudsql-proxy.svg [pkg-docs]: https://pkg.go.dev/github.com/GoogleCloudPlatform/cloudsql-proxy