[MM-61280] disable init data if providing external database #870
Conversation
cmd/ltctl/main.go
Outdated
| @@ -34,7 +34,7 @@ func RunCreateCmdF(cmd *cobra.Command, args []string) error { | |||
| return fmt.Errorf("failed to create SSH agent: %w", err) | |||
| } | |||
|
|
|||
| initData := config.DBDumpURI == "" | |||
| initData := config.DBDumpURI == "" || config.ExternalDBSettings.DataSource != "" | |||
There was a problem hiding this comment.
Wondering if this should be an invalid case at the config level. Does having both defined make sense?
There was a problem hiding this comment.
No, because it will try to upload the dump to the database using the TerraformDBSettings credentials and getting the database URL from terraform's output DBWriter.
There was a problem hiding this comment.
I don't understand this either. With this change, if both DBDumpURI and DataSource are non-empty, initData is true, right? Shouldn't this be
| initData := config.DBDumpURI == "" || config.ExternalDBSettings.DataSource != "" | |
| initData := config.DBDumpURI == "" && config.ExternalDBSettings.DataSource == "" |
There was a problem hiding this comment.
That looks better 👍
There was a problem hiding this comment.
I'd still welcome an addition to func (c *Config) IsValid() if having both set is a clear invalid case to avoid unexpected side effects. Not blocking though.
There was a problem hiding this comment.
Rather do it now than forget later: 9aa44f4
Co-authored-by: Alejandro García Montoro <[email protected]>
…t-load-test-ng into fix/initdb-external
deployment/config.go
Outdated
| @@ -398,6 +398,10 @@ func (c *Config) IsValid() error { | |||
| return fmt.Errorf("load-test download url is not in correct format: %q", c.LoadTestDownloadURL) | |||
| } | |||
|
|
|||
| if c.ExternalDBSettings.DataSource != "" && c.DBDumpURI != "" { | |||
| return errors.New("both ExternalDBSettings.DataSource and DbdumpURI are set, only one can be set") | |||
There was a problem hiding this comment.
Minor typo
| return errors.New("both ExternalDBSettings.DataSource and DbdumpURI are set, only one can be set") | |
| return errors.New("both ExternalDBSettings.DataSource and DBDumpURI are set, only one can be set") |
Summary
Avoid running the database initialization commands if providing an external database since the
IngestDumponly works for terraform created databases. External databases are usually already populated and ready to use as they come from backups.Ticket Link
https://mattermost.atlassian.net/browse/MM-61280