Skip to content

Commit

Permalink
fix: let proxy continue to run with inactive instance(s) (#1854)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
annafang-google committed Sep 6, 2023
1 parent 0d96f2f commit 30238d6
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 9 deletions.
26 changes: 17 additions & 9 deletions internal/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -510,19 +510,19 @@ 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
}
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
}

Expand Down Expand Up @@ -747,6 +747,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"
Expand All @@ -765,8 +773,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
Expand All @@ -784,7 +791,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)
}
Expand All @@ -801,6 +807,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
}
}
Expand Down
53 changes: 53 additions & 0 deletions internal/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package proxy_test
import (
"context"
"errors"
"fmt"
"io"
"net"
"os"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -711,3 +733,34 @@ 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)
}
})
}
}

0 comments on commit 30238d6

Please sign in to comment.