diff --git a/cmd/praefect/subcmd_dataloss.go b/cmd/praefect/subcmd_dataloss.go index 5eb88f4e686df91a4210cd4064dbc506e77835e2..bc654d998fa9c6b8847ab5c662f739fc4ecb0a9a 100644 --- a/cmd/praefect/subcmd_dataloss.go +++ b/cmd/praefect/subcmd_dataloss.go @@ -2,6 +2,7 @@ package main import ( "context" + "errors" "flag" "fmt" "io" @@ -16,9 +17,17 @@ import ( const datalossCmdName = "dataloss" +var errDatalossFlagsConflict = errors.New("fully-unavailable option conflicts with partially-unavailable option") + type datalossSubcommand struct { - output io.Writer - virtualStorage string + output io.Writer + virtualStorage string + onlyIncludeFullyUnAvailable bool + /* + includePartiallyAvailable is just a variable for holding a deprecated partially-unavailable + flag's parsed value. + This variable is not read, and will be removed in the future. + */ includePartiallyAvailable bool } @@ -28,11 +37,18 @@ func newDatalossSubcommand() *datalossSubcommand { func (cmd *datalossSubcommand) FlagSet() *flag.FlagSet { fs := flag.NewFlagSet(datalossCmdName, flag.ContinueOnError) + fs.Usage = func() { + printfErr("Description:\n" + + " Displays unavailable repositories in the specified virtual storage.\n" + + " By default, also shows all partially-unavailable repositories. Partially-unavailable repositories\n" + + " have some unavailable and unreplicated nodes.") + fs.PrintDefaults() + } fs.StringVar(&cmd.virtualStorage, "virtual-storage", "", "virtual storage to check for data loss") + fs.BoolVar(&cmd.onlyIncludeFullyUnAvailable, "fully-unavailable", false, strings.TrimSpace(` +Display only fully-unavailable repositories. Partially-unavailable repositories are not displayed.`)) fs.BoolVar(&cmd.includePartiallyAvailable, "partially-unavailable", false, strings.TrimSpace(` -Additionally include repositories which are available but some assigned replicas -are unavailable. Such repositories are available but are not fully replicated. This -increases the chance of data loss on primary failure`)) +Display both fully-unavailable repositories and partially-unavailable repositories. Default behavior of command.`)) return fs } @@ -46,6 +62,10 @@ func (cmd *datalossSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) erro if flags.NArg() > 0 { return unexpectedPositionalArgsError{Command: flags.Name()} } + if cmd.onlyIncludeFullyUnAvailable && cmd.includePartiallyAvailable { + return errDatalossFlagsConflict + } + includePartiallyAvailable := !cmd.onlyIncludeFullyUnAvailable virtualStorages := []string{cmd.virtualStorage} if cmd.virtualStorage == "" { @@ -77,7 +97,7 @@ func (cmd *datalossSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) erro for _, vs := range virtualStorages { resp, err := client.DatalossCheck(ctx, &gitalypb.DatalossCheckRequest{ VirtualStorage: vs, - IncludePartiallyReplicated: cmd.includePartiallyAvailable, + IncludePartiallyReplicated: includePartiallyAvailable, }) if err != nil { return fmt.Errorf("error checking: %v", err) @@ -86,7 +106,7 @@ func (cmd *datalossSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) erro cmd.println(0, "Virtual storage: %s", vs) if len(resp.Repositories) == 0 { msg := "All repositories are available!" - if cmd.includePartiallyAvailable { + if includePartiallyAvailable { msg = "All repositories are fully available on all assigned storages!" } diff --git a/cmd/praefect/subcmd_dataloss_test.go b/cmd/praefect/subcmd_dataloss_test.go index 949c7919b06988c047be845b57d15665c589ed4b..2e8f3747adc944adbdcd75905682feefa46116d2 100644 --- a/cmd/praefect/subcmd_dataloss_test.go +++ b/cmd/praefect/subcmd_dataloss_test.go @@ -93,9 +93,35 @@ func TestDatalossSubcommand(t *testing.T) { args: []string{"-virtual-storage=virtual-storage-1", "positional-arg"}, error: unexpectedPositionalArgsError{Command: "dataloss"}, }, + { + desc: "the fully-available flag is not permitted to appear with a partially-unavailable flag", + args: []string{"-virtual-storage=virtual-storage-1", "-fully-unavailable", "-partially-unavailable"}, + error: errDatalossFlagsConflict, + }, + { + desc: "check deprecated partially-unavailable flag could work for backward compatibility", + args: []string{"-virtual-storage=virtual-storage-1", "-partially-unavailable"}, + output: `Virtual storage: virtual-storage-1 + Repositories: + repository-1: + Primary: gitaly-1 + In-Sync Storages: + gitaly-1, assigned host + Outdated Storages: + gitaly-2 is behind by 1 change or less, assigned host, unhealthy + gitaly-3 is behind by 1 change or less + repository-2 (unavailable): + Primary: gitaly-3 + In-Sync Storages: + gitaly-2, unhealthy + Outdated Storages: + gitaly-1 is behind by 2 changes or less, assigned host + gitaly-3 is behind by 1 change or less, assigned host +`, + }, { desc: "data loss with unavailable repositories", - args: []string{"-virtual-storage=virtual-storage-1"}, + args: []string{"-virtual-storage=virtual-storage-1", "-fully-unavailable"}, output: `Virtual storage: virtual-storage-1 Repositories: repository-2 (unavailable): @@ -109,7 +135,7 @@ func TestDatalossSubcommand(t *testing.T) { }, { desc: "data loss with partially unavailable repositories", - args: []string{"-virtual-storage=virtual-storage-1", "-partially-unavailable"}, + args: []string{"-virtual-storage=virtual-storage-1"}, output: `Virtual storage: virtual-storage-1 Repositories: repository-1: @@ -131,6 +157,7 @@ func TestDatalossSubcommand(t *testing.T) { { desc: "multiple virtual storages with unavailable repositories", virtualStorages: []*config.VirtualStorage{{Name: "virtual-storage-2"}, {Name: "virtual-storage-1"}}, + args: []string{"-fully-unavailable"}, output: `Virtual storage: virtual-storage-1 Repositories: repository-2 (unavailable): @@ -146,7 +173,6 @@ Virtual storage: virtual-storage-2 }, { desc: "multiple virtual storages with partially unavailable repositories", - args: []string{"-partially-unavailable"}, virtualStorages: []*config.VirtualStorage{{Name: "virtual-storage-2"}, {Name: "virtual-storage-1"}}, output: `Virtual storage: virtual-storage-1 Repositories: