Skip to content

Commit 4243fb5

Browse files
djshow832djshow832
authored
config, cmd: fix that the proxy configs are overwritten (#691)
1 parent 4471d26 commit 4243fb5

File tree

10 files changed

+38
-37
lines changed

10 files changed

+38
-37
lines changed

‎cmd/tiproxy/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func main() {
3232
rootCmd.PersistentFlags().StringVar(&sctx.ConfigFile, "config", "", "proxy config file path")
3333
rootCmd.PersistentFlags().StringVar(&deprecatedStr, "log_encoder", "", "deprecated and will be removed")
3434
rootCmd.PersistentFlags().StringVar(&deprecatedStr, "log_level", "", "deprecated and will be removed")
35-
rootCmd.PersistentFlags().StringVar(&sctx.Overlay.Proxy.AdvertiseAddr, "advertise-addr", "", "advertise address")
35+
rootCmd.PersistentFlags().StringVar(&sctx.AdvertiseAddr, "advertise-addr", "", "advertise address")
3636

3737
metrics.MaxProcsGauge.Set(float64(runtime.GOMAXPROCS(0)))
3838

‎pkg/manager/config/config.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,9 @@ func (e *ConfigManager) SetTOMLConfig(data []byte) (err error) {
5252
return errors.WithStack(err)
5353
}
5454

55-
if err = toml.Unmarshal(e.overlay, base); err != nil {
56-
return errors.WithStack(err)
55+
// Overwrite the config with command line args.
56+
if len(e.advertiseAddr) > 0 {
57+
base.Proxy.AdvertiseAddr = e.advertiseAddr
5758
}
5859

5960
if err = base.Check(); err != nil {

‎pkg/manager/config/config_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ func TestConfigReload(t *testing.T) {
2222
require.NoError(t, err)
2323
require.NoError(t, f.Close())
2424

25-
cfgmgr1, _, _ := testConfigManager(t, tmpcfg)
25+
cfgmgr1, _, _ := testConfigManager(t, tmpcfg, "addr")
2626

27-
cfgmgr2, _, _ := testConfigManager(t, "")
27+
cfgmgr2, _, _ := testConfigManager(t, "", "addr")
2828

2929
cases := []struct {
3030
name string
@@ -41,7 +41,7 @@ func TestConfigReload(t *testing.T) {
4141
},
4242
postcfg: `proxy.pd-addrs = ""`,
4343
postcheck: func(c *config.Config) bool {
44-
return c.Proxy.PDAddrs == ""
44+
return c.Proxy.PDAddrs == "" && c.Proxy.AdvertiseAddr == "addr"
4545
},
4646
},
4747
{
@@ -135,7 +135,7 @@ func TestConfigRemove(t *testing.T) {
135135
require.NoError(t, err)
136136
require.NoError(t, f.Close())
137137

138-
cfgmgr, _, _ := testConfigManager(t, tmpcfg)
138+
cfgmgr, _, _ := testConfigManager(t, tmpcfg, "")
139139

140140
// remove and recreate the file in a very short time
141141
require.NoError(t, os.Remove(tmpcfg))
@@ -271,7 +271,7 @@ func TestFilePath(t *testing.T) {
271271
require.NoError(t, f.Close())
272272
}
273273

274-
cfgmgr, _, _ = testConfigManager(t, test.filename)
274+
cfgmgr, _, _ = testConfigManager(t, test.filename, "")
275275
require.Equal(t, pdAddr1, cfgmgr.GetConfig().Proxy.PDAddrs)
276276

277277
// Test write.
@@ -298,7 +298,7 @@ func TestFilePath(t *testing.T) {
298298
}
299299

300300
func TestChecksum(t *testing.T) {
301-
cfgmgr, _, _ := testConfigManager(t, "")
301+
cfgmgr, _, _ := testConfigManager(t, "", "")
302302
c1 := cfgmgr.GetConfigChecksum()
303303
require.NoError(t, cfgmgr.SetTOMLConfig([]byte(`proxy.addr = "gg"`)))
304304
c2 := cfgmgr.GetConfigChecksum()

‎pkg/manager/config/manager.go‎

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ type KVValue struct {
3434
}
3535

3636
type ConfigManager struct {
37-
wg waitgroup.WaitGroup
38-
cancel context.CancelFunc
39-
logger *zap.Logger
37+
wg waitgroup.WaitGroup
38+
cancel context.CancelFunc
39+
logger *zap.Logger
40+
advertiseAddr string
4041

4142
kv *btree.BTreeG[KVValue]
4243

4344
checkFileInterval time.Duration
4445
fileContent []byte
45-
overlay []byte
4646
sts struct {
4747
sync.Mutex
4848
listeners []chan<- *config.Config
@@ -57,26 +57,18 @@ func NewConfigManager() *ConfigManager {
5757
}
5858
}
5959

60-
func (e *ConfigManager) Init(ctx context.Context, logger *zap.Logger, configFile string, overlay *config.Config) error {
61-
var err error
60+
func (e *ConfigManager) Init(ctx context.Context, logger *zap.Logger, configFile string, advertiseAddr string) error {
6261
var nctx context.Context
6362
nctx, e.cancel = context.WithCancel(ctx)
6463

6564
e.logger = logger
65+
e.advertiseAddr = advertiseAddr
6666

6767
// for namespace persistence
6868
e.kv = btree.NewBTreeG(func(a, b KVValue) bool {
6969
return a.Key < b.Key
7070
})
7171

72-
// for config watch
73-
if overlay != nil {
74-
e.overlay, err = overlay.ToBytes()
75-
if err != nil {
76-
return err
77-
}
78-
}
79-
8072
if configFile != "" {
8173
if err := e.reloadConfigFile(configFile); err != nil {
8274
return err

‎pkg/manager/config/manager_test.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,11 @@ import (
99
"testing"
1010
"time"
1111

12-
"github.com/pingcap/tiproxy/lib/config"
1312
"github.com/pingcap/tiproxy/lib/util/logger"
1413
"github.com/stretchr/testify/require"
1514
)
1615

17-
func testConfigManager(t *testing.T, configFile string, overlays ...*config.Config) (*ConfigManager, fmt.Stringer, context.Context) {
16+
func testConfigManager(t *testing.T, configFile string, advertiseAddr string) (*ConfigManager, fmt.Stringer, context.Context) {
1817
logger, text := logger.CreateLoggerForTest(t)
1918

2019
ctx, cancel := context.WithCancel(context.Background())
@@ -24,7 +23,7 @@ func testConfigManager(t *testing.T, configFile string, overlays ...*config.Conf
2423

2524
cfgmgr := NewConfigManager()
2625
cfgmgr.checkFileInterval = 20 * time.Millisecond
27-
require.NoError(t, cfgmgr.Init(ctx, logger, configFile, nil))
26+
require.NoError(t, cfgmgr.Init(ctx, logger, configFile, advertiseAddr))
2827

2928
t.Cleanup(func() {
3029
require.NoError(t, cfgmgr.Close())

‎pkg/manager/config/namespace_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
)
1515

1616
func TestBase(t *testing.T) {
17-
cfgmgr, _, ctx := testConfigManager(t, "")
17+
cfgmgr, _, ctx := testConfigManager(t, "", "")
1818

1919
nsNum := 10
2020
valNum := 30
@@ -74,7 +74,7 @@ func TestBase(t *testing.T) {
7474
}
7575

7676
func TestBaseConcurrency(t *testing.T) {
77-
cfgmgr, _, ctx := testConfigManager(t, "")
77+
cfgmgr, _, ctx := testConfigManager(t, "", "")
7878

7979
var wg waitgroup.WaitGroup
8080
batchNum := 16

‎pkg/sctx/context.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,13 @@ package sctx
55

66
import (
77
"github.com/gin-gonic/gin"
8-
"github.com/pingcap/tiproxy/lib/config"
98
"github.com/pingcap/tiproxy/pkg/proxy/backend"
109
)
1110

1211
type Context struct {
13-
Overlay config.Config
14-
ConfigFile string
15-
Handler ServerHandler
12+
AdvertiseAddr string
13+
ConfigFile string
14+
Handler ServerHandler
1615
}
1716

1817
type ServerHandler interface {

‎pkg/server/api/server_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func createServer(t *testing.T) (*Server, func(t *testing.T, method string, path
3030
lg, _ := logger.CreateLoggerForTest(t)
3131
ready := atomic.NewBool(true)
3232
cfgmgr := mgrcfg.NewConfigManager()
33-
require.NoError(t, cfgmgr.Init(context.Background(), lg, "", nil))
33+
require.NoError(t, cfgmgr.Init(context.Background(), lg, "", ""))
3434
crtmgr := mgrcrt.NewCertManager()
3535
require.NoError(t, crtmgr.Init(cfgmgr.GetConfig(), lg, cfgmgr.WatchConfig()))
3636
srv, err := NewServer(config.API{

‎pkg/server/server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,13 @@ func NewServer(ctx context.Context, sctx *sctx.Context) (srv *Server, err error)
6868

6969
// set up logger
7070
var lg *zap.Logger
71-
if srv.loggerManager, lg, err = logger.NewLoggerManager(&sctx.Overlay.Log); err != nil {
71+
if srv.loggerManager, lg, err = logger.NewLoggerManager(nil); err != nil {
7272
return
7373
}
7474
srv.loggerManager.Init(srv.configManager.WatchConfig())
7575

7676
// setup config manager
77-
if err = srv.configManager.Init(ctx, lg.Named("config"), sctx.ConfigFile, &sctx.Overlay); err != nil {
77+
if err = srv.configManager.Init(ctx, lg.Named("config"), sctx.ConfigFile, sctx.AdvertiseAddr); err != nil {
7878
return
7979
}
8080
cfg := srv.configManager.GetConfig()

‎pkg/server/server_test.go‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,32 @@ package server
55

66
import (
77
"context"
8+
"os"
89
"testing"
910

11+
"github.com/pelletier/go-toml/v2"
1012
"github.com/pingcap/tiproxy/lib/util/logger"
1113
"github.com/pingcap/tiproxy/pkg/sctx"
1214
"github.com/pingcap/tiproxy/pkg/util/etcd"
1315
"github.com/stretchr/testify/require"
1416
)
1517

1618
func TestServer(t *testing.T) {
19+
dir := t.TempDir()
1720
lg, _ := logger.CreateLoggerForTest(t)
18-
etcdServer, err := etcd.CreateEtcdServer("0.0.0.0:0", t.TempDir(), lg)
21+
etcdServer, err := etcd.CreateEtcdServer("0.0.0.0:0", dir, lg)
1922
require.NoError(t, err)
23+
configFile := dir + "/config.toml"
2024
endpoint := etcdServer.Clients[0].Addr().String()
2125
cfg := etcd.ConfigForEtcdTest(endpoint)
26+
b, err := toml.Marshal(cfg)
27+
require.NoError(t, err)
28+
require.NoError(t, os.WriteFile(configFile, b, 0o644))
2229

23-
server, err := NewServer(context.Background(), &sctx.Context{Overlay: *cfg})
30+
server, err := NewServer(context.Background(), &sctx.Context{
31+
ConfigFile: configFile,
32+
})
2433
require.NoError(t, err)
2534
require.NoError(t, server.Close())
35+
etcdServer.Close()
2636
}

0 commit comments

Comments
 (0)