From a77165f48adb0e2c87aa18eb2fbbbd48788580cb Mon Sep 17 00:00:00 2001 From: Anna Fang Date: Fri, 1 Sep 2023 15:32:34 +0000 Subject: [PATCH] fix: let proxy continue to run with inactive instance(s) (#1854) Replaced fmt.Errorf with logger.Errorf for instance failure Fail the inactive instance with unix path Fixed lint error Added inactive instance test with unix path Replace getNetworkType with networkType Fixed the error path indentation Make the unix path failure test a seperate func Fail the proxy binary if the tcp socket argument is wrong --- internal/proxy/proxy.go | 32 ++++++++---- internal/proxy/proxy_test.go | 97 ++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 9 deletions(-) diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index fd5a68fbc..bd1b9eebd 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -510,19 +510,25 @@ func NewClient(ctx context.Context, d cloudsql.Dialer, l cloudsql.Logger, conf * for _, inst := range conf.Instances { m, err := c.newSocketMount(ctx, conf, pc, inst) if err != nil { - for _, m := range mnts { - mErr := m.Close() - if mErr != nil { - l.Errorf("failed to close mount: %v", mErr) - } + c.logger.Errorf("[%v] Unable to mount socket: %v", inst.Name, err) + if networkType(conf, inst) == "unix" { + continue + } + switch err.(type) { + // this is not the inactive instance case. Fail the proxy + // so that the user could fix the proxy command error. + case *net.OpError: + return nil, fmt.Errorf("[%v] Unable to mount socket: %v", inst.Name, err) } - return nil, fmt.Errorf("[%v] Unable to mount socket: %v", inst.Name, err) } l.Infof("[%s] Listening on %s", inst.Name, m.Addr()) mnts = append(mnts, m) } c.mnts = mnts + if len(mnts) == 0 { + return nil, fmt.Errorf("No active instance to mount") + } return c, nil } @@ -747,6 +753,14 @@ type socketMount struct { dialOpts []cloudsqlconn.DialOption } +func networkType(conf *Config, inst InstanceConnConfig) string { + if (conf.UnixSocket == "" && inst.UnixSocket == "" && inst.UnixSocketPath == "") || + (inst.Addr != "" || inst.Port != 0) { + return "tcp" + } + return "unix" +} + func (c *Client) newSocketMount(ctx context.Context, conf *Config, pc *portConfig, inst InstanceConnConfig) (*socketMount, error) { var ( // network is one of "tcp" or "unix" @@ -765,8 +779,7 @@ func (c *Client) newSocketMount(ctx context.Context, conf *Config, pc *portConfi // instance) // use a TCP listener. // Otherwise, use a Unix socket. - if (conf.UnixSocket == "" && inst.UnixSocket == "" && inst.UnixSocketPath == "") || - (inst.Addr != "" || inst.Port != 0) { + if networkType(conf, inst) == "tcp" { network = "tcp" a := conf.Addr @@ -784,7 +797,6 @@ func (c *Client) newSocketMount(ctx context.Context, conf *Config, pc *portConfi version, err := c.dialer.EngineVersion(ctx, inst.Name) if err != nil { c.logger.Errorf("could not resolve version for %q: %v", inst.Name, err) - return nil, err } np = pc.nextDBPort(version) } @@ -801,6 +813,8 @@ func (c *Client) newSocketMount(ctx context.Context, conf *Config, pc *portConfi address, err = newUnixSocketMount(inst, conf.UnixSocket, strings.HasPrefix(version, "POSTGRES")) if err != nil { + c.logger.Errorf("could not mount unix socket %q for %q: %v", + conf.UnixSocket, inst.Name, err) return nil, err } } diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 8d509295e..db0b3b504 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -17,6 +17,7 @@ package proxy_test import ( "context" "errors" + "fmt" "io" "net" "os" @@ -83,6 +84,8 @@ func (f *fakeDialer) EngineVersion(_ context.Context, inst string) (string, erro return "MYSQL_8_0", nil case strings.Contains(inst, "sqlserver"): return "SQLSERVER_2019_STANDARD", nil + case strings.Contains(inst, "fakeserver"): + return "", fmt.Errorf("non existing server") default: return "POSTGRES_14", nil } @@ -306,6 +309,25 @@ func TestClientInitialization(t *testing.T) { filepath.Join(testUnixSocketPathPg), }, }, + { + desc: "without TCP port or unix socket for non functional instance", + in: &proxy.Config{ + Instances: []proxy.InstanceConnConfig{ + {Name: "proj:region:fakeserver"}, + }, + }, + }, + { + desc: "with TCP port for non functional instance", + in: &proxy.Config{ + Instances: []proxy.InstanceConnConfig{ + {Name: "proj:region:fakeserver", Port: 50000}, + }, + }, + wantTCPAddrs: []string{ + "127.0.0.1:50000", + }, + }, } for _, tc := range tcs { @@ -711,3 +733,78 @@ func TestRunConnectionCheck(t *testing.T) { } } + +func TestProxyInitializationWithFailedUnixSocket(t *testing.T) { + ctx := context.Background() + testDir, _ := createTempDir(t) + testUnixSocketPath := path.Join(testDir, "db") + + tcs := []struct { + desc string + in *proxy.Config + }{ + { + desc: "with unix socket for non functional instance", + in: &proxy.Config{ + Instances: []proxy.InstanceConnConfig{ + { + Name: "proj:region:fakeserver", + UnixSocketPath: testUnixSocketPath, + }, + }, + }, + }, + } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + _, err := proxy.NewClient(ctx, &fakeDialer{}, testLogger, tc.in) + if err == nil { + t.Fatalf("want non nil error, got = %v", err) + } + }) + } +} + +func TestProxyMultiInstances(t *testing.T) { + ctx := context.Background() + testDir, _ := createTempDir(t) + testUnixSocketPath := path.Join(testDir, "db") + + tcs := []struct { + desc string + in *proxy.Config + wantSuccess bool + }{ + { + desc: "with tcp socket and unix for non functional instance", + in: &proxy.Config{ + Instances: []proxy.InstanceConnConfig{ + { + Name: "proj:region:fakeserver", + UnixSocketPath: testUnixSocketPath, + }, + {Name: mysql, Port: 3306}, + }, + }, + wantSuccess: true, + }, + { + desc: "with two tcp socket instances", + in: &proxy.Config{ + Instances: []proxy.InstanceConnConfig{ + {Name: "proj:region:fakeserver", Port: 60000}, + {Name: mysql, Port: 60000}, + }, + }, + wantSuccess: false, + }, + } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + _, err := proxy.NewClient(ctx, &fakeDialer{}, testLogger, tc.in) + if tc.wantSuccess != (err == nil) { + t.Fatalf("want return = %v, got = %v", tc.wantSuccess, err == nil) + } + }) + } +}