Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve VTOrc config handling to support dynamic variables #17218

Merged
merged 23 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b3e4aec
feat: remove deprecated flag
GuptaManan100 Nov 12, 2024
85913d1
feat: remove config file flag and start using viper instead for insta…
GuptaManan100 Nov 12, 2024
0a27ba8
feat: move prevent cross cell promotion as well to viper
GuptaManan100 Nov 12, 2024
c6c89fb
feat: add sqldatafile, snapshot time and reasonable replication log t…
GuptaManan100 Nov 12, 2024
7b86cb3
feat: add audit file location to viper
GuptaManan100 Nov 12, 2024
1ab5cc3
feat: move remaining audit flags to viper
GuptaManan100 Nov 12, 2024
ec2ef87
feat: move remaining configs to viper
GuptaManan100 Nov 12, 2024
2121e16
feat: add api to read config and add a test for dynamic configuration
GuptaManan100 Nov 12, 2024
0420ce5
feat: add summary changes
GuptaManan100 Nov 12, 2024
897ab10
feat: fix vtorc config in local example
GuptaManan100 Nov 13, 2024
d20979b
feat: change way we use config in the e2e tests
GuptaManan100 Nov 21, 2024
d5923cd
feat: add two more fields to viper
GuptaManan100 Nov 21, 2024
7ef3b3b
Merge remote-tracking branch 'upstream/main' into vtorc-config-reload…
GuptaManan100 Nov 21, 2024
6822752
feat: fix flag in config
GuptaManan100 Nov 22, 2024
8b1e52b
test: start adding dynamic config tests in e2e fashion
GuptaManan100 Nov 24, 2024
3a422fa
test: add all the remaining configs to the test
GuptaManan100 Nov 24, 2024
ed96f72
feat: change the keys of the viper configs to match the flag names
GuptaManan100 Nov 25, 2024
9f14d8b
feat: add logging for configuration changes
GuptaManan100 Nov 25, 2024
b63d4f8
feat: fix config in examples and update summary
GuptaManan100 Nov 26, 2024
a939288
Merge remote-tracking branch 'upstream/main' into vtorc-config-reload…
GuptaManan100 Nov 27, 2024
cb16b34
Merge remote-tracking branch 'upstream/main' into vtorc-config-reload…
GuptaManan100 Dec 3, 2024
a9761f5
feat: address review comments
GuptaManan100 Dec 3, 2024
85fbc91
feat: don't use deleted flag in vtorc
GuptaManan100 Dec 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: add audit file location to viper
Signed-off-by: Manan Gupta <manan@planetscale.com>
  • Loading branch information
GuptaManan100 committed Nov 12, 2024
commit 7b86cb3f9e5363e530868ce9d794110ce42f4eec
27 changes: 22 additions & 5 deletions go/vt/vtorc/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const (

var (
instancePollTime = viperutil.Configure(
"InstancePollTime",
"instance-pPollTime",
viperutil.Options[time.Duration]{
FlagName: "instance-poll-time",
Default: 5 * time.Second,
Expand Down Expand Up @@ -89,10 +89,18 @@ var (
Dynamic: true,
},
)

auditFileLocation = viperutil.Configure(
"AuditFileLocation",
viperutil.Options[string]{
FlagName: "audit-file-location",
Default: "",
Dynamic: false,
},
)
)

var (
auditFileLocation = ""
auditToBackend = false
auditToSyslog = false
auditPurgeDuration = 7 * 24 * time.Hour // Equivalent of 7 days
Expand All @@ -114,7 +122,7 @@ func registerFlags(fs *pflag.FlagSet) {
fs.Duration("instance-poll-time", instancePollTime.Default(), "Timer duration on which VTOrc refreshes MySQL information")
fs.Duration("snapshot-topology-interval", snapshotTopologyInterval.Default(), "Timer duration on which VTOrc takes a snapshot of the current MySQL information it has in the database. Should be in multiple of hours")
fs.Duration("reasonable-replication-lag", reasonableReplicationLag.Default(), "Maximum replication lag on replicas which is deemed to be acceptable")
fs.StringVar(&auditFileLocation, "audit-file-location", auditFileLocation, "File location where the audit logs are to be stored")
fs.String("audit-file-location", auditFileLocation.Default(), "File location where the audit logs are to be stored")
fs.BoolVar(&auditToBackend, "audit-to-backend", auditToBackend, "Whether to store the audit log in the VTOrc database")
fs.BoolVar(&auditToSyslog, "audit-to-syslog", auditToSyslog, "Whether to store the audit log in the syslog")
fs.DurationVar(&auditPurgeDuration, "audit-purge-duration", auditPurgeDuration, "Duration for which audit logs are held before being purged. Should be in multiples of days")
Expand All @@ -132,6 +140,7 @@ func registerFlags(fs *pflag.FlagSet) {
sqliteDataFile,
snapshotTopologyInterval,
reasonableReplicationLag,
auditFileLocation,
)
}

Expand Down Expand Up @@ -194,10 +203,19 @@ func GetSnapshotTopologyInterval() time.Duration {
return snapshotTopologyInterval.Get()
}

// GetAuditFileLocation is a getter function.
func GetAuditFileLocation() string {
return auditFileLocation.Get()
}

// SetAuditFileLocation is a setter function.
func SetAuditFileLocation(v string) {
auditFileLocation.Set(v)
}

// UpdateConfigValuesFromFlags is used to update the config values from the flags defined.
// This is done before we read any configuration files from the user. So the config files take precedence.
func UpdateConfigValuesFromFlags() {
Config.AuditLogFile = auditFileLocation
Config.AuditToBackendDB = auditToBackend
Config.AuditToSyslog = auditToSyslog
Config.AuditPurgeDays = uint(auditPurgeDuration / (time.Hour * 24))
Expand Down Expand Up @@ -234,7 +252,6 @@ func LogConfigValues() {

func newConfiguration() *Configuration {
return &Configuration{
AuditLogFile: "",
AuditToSyslog: false,
AuditToBackendDB: false,
AuditPurgeDays: 7,
Expand Down
15 changes: 0 additions & 15 deletions go/vt/vtorc/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,21 +52,6 @@ func TestUpdateConfigValuesFromFlags(t *testing.T) {
require.Equal(t, testConfig, Config)
})

t.Run("override auditFileLocation", func(t *testing.T) {
oldAuditFileLocation := auditFileLocation
auditFileLocation = "newFile"
// Restore the changes we make
defer func() {
Config = newConfiguration()
auditFileLocation = oldAuditFileLocation
}()

testConfig := newConfiguration()
testConfig.AuditLogFile = "newFile"
UpdateConfigValuesFromFlags()
require.Equal(t, testConfig, Config)
})

t.Run("override auditToBackend", func(t *testing.T) {
oldAuditToBackend := auditToBackend
auditToBackend = true
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtorc/inst/audit_dao.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ func AuditOperation(auditType string, tabletAlias string, message string) error
}

auditWrittenToFile := false
if config.Config.AuditLogFile != "" {
if config.GetAuditFileLocation() != "" {
auditWrittenToFile = true
go func() {
f, err := os.OpenFile(config.Config.AuditLogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0640)
f, err := os.OpenFile(config.GetAuditFileLocation(), os.O_RDWR|os.O_CREATE|os.O_APPEND, 0640)
if err != nil {
log.Error(err)
return
Expand Down
8 changes: 4 additions & 4 deletions go/vt/vtorc/inst/audit_dao_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ import (
func TestAuditOperation(t *testing.T) {
// Restore original configurations
originalAuditSysLog := config.Config.AuditToSyslog
originalAuditLogFile := config.Config.AuditLogFile
originalAuditLogFile := config.GetAuditFileLocation()
originalAuditBackend := config.Config.AuditToBackendDB
defer func() {
config.Config.AuditToSyslog = originalAuditSysLog
config.Config.AuditLogFile = originalAuditLogFile
config.SetAuditFileLocation(originalAuditLogFile)
config.Config.AuditToBackendDB = originalAuditBackend
}()

Expand Down Expand Up @@ -78,7 +78,7 @@ func TestAuditOperation(t *testing.T) {
message := "test-message"

t.Run("audit to backend", func(t *testing.T) {
config.Config.AuditLogFile = ""
config.SetAuditFileLocation("")
config.Config.AuditToSyslog = false
config.Config.AuditToBackendDB = true

Expand Down Expand Up @@ -112,7 +112,7 @@ func TestAuditOperation(t *testing.T) {
file, err := os.CreateTemp("", "test-auditing-*")
require.NoError(t, err)
defer os.Remove(file.Name())
config.Config.AuditLogFile = file.Name()
config.SetAuditFileLocation(file.Name())

err = AuditOperation(auditType, tab100Alias, message)
require.NoError(t, err)
Expand Down