Merge pull request #4426 from MichaelEischer/consistent-forget-policy

forget: replace `--keep-* -1` with `--keep-* unlimited`
This commit is contained in:
Michael Eischer 2023-07-28 19:06:00 +00:00 committed by GitHub
commit 95050117eb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 128 additions and 62 deletions

View file

@ -1,8 +1,9 @@
Bugfix: Restic forget --keep-* options now interpret "-1" as "forever"
Bugfix: Support "unlimited" in `forget --keep-*` options
Restic would forget snapshots that should have been kept when "-1" was
used as a value for --keep-* options. It now interprets "-1" as forever,
e.g. an option like --keep-monthly -1 will keep all monthly snapshots.
Restic would forget snapshots that should have been kept when a negative value
was passed to the `--keep-*` options. Negative values are now forbidden. To
keep all snapshots, the special value `unlimited` is now supported. For
example, `--keep-monthly unlimited` will keep all monthly snapshots.
https://github.com/restic/restic/issues/2565
https://github.com/restic/restic/pull/4234

View file

@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"io"
"strconv"
"github.com/restic/restic/internal/errors"
"github.com/restic/restic/internal/restic"
@ -36,14 +37,49 @@ Exit status is 0 if the command was successful, and non-zero if there was any er
},
}
type ForgetPolicyCount int
var ErrNegativePolicyCount = errors.New("negative values not allowed, use 'unlimited' instead")
func (c *ForgetPolicyCount) Set(s string) error {
switch s {
case "unlimited":
*c = -1
default:
val, err := strconv.ParseInt(s, 10, 0)
if err != nil {
return err
}
if val < 0 {
return ErrNegativePolicyCount
}
*c = ForgetPolicyCount(val)
}
return nil
}
func (c *ForgetPolicyCount) String() string {
switch *c {
case -1:
return "unlimited"
default:
return strconv.FormatInt(int64(*c), 10)
}
}
func (c *ForgetPolicyCount) Type() string {
return "n"
}
// ForgetOptions collects all options for the forget command.
type ForgetOptions struct {
Last int
Hourly int
Daily int
Weekly int
Monthly int
Yearly int
Last ForgetPolicyCount
Hourly ForgetPolicyCount
Daily ForgetPolicyCount
Weekly ForgetPolicyCount
Monthly ForgetPolicyCount
Yearly ForgetPolicyCount
Within restic.Duration
WithinHourly restic.Duration
WithinDaily restic.Duration
@ -67,12 +103,12 @@ func init() {
cmdRoot.AddCommand(cmdForget)
f := cmdForget.Flags()
f.IntVarP(&forgetOptions.Last, "keep-last", "l", 0, "keep the last `n` snapshots (use '-1' to keep all snapshots)")
f.IntVarP(&forgetOptions.Hourly, "keep-hourly", "H", 0, "keep the last `n` hourly snapshots (use '-1' to keep all hourly snapshots)")
f.IntVarP(&forgetOptions.Daily, "keep-daily", "d", 0, "keep the last `n` daily snapshots (use '-1' to keep all daily snapshots)")
f.IntVarP(&forgetOptions.Weekly, "keep-weekly", "w", 0, "keep the last `n` weekly snapshots (use '-1' to keep all weekly snapshots)")
f.IntVarP(&forgetOptions.Monthly, "keep-monthly", "m", 0, "keep the last `n` monthly snapshots (use '-1' to keep all monthly snapshots)")
f.IntVarP(&forgetOptions.Yearly, "keep-yearly", "y", 0, "keep the last `n` yearly snapshots (use '-1' to keep all yearly snapshots)")
f.VarP(&forgetOptions.Last, "keep-last", "l", "keep the last `n` snapshots (use 'unlimited' to keep all snapshots)")
f.VarP(&forgetOptions.Hourly, "keep-hourly", "H", "keep the last `n` hourly snapshots (use 'unlimited' to keep all hourly snapshots)")
f.VarP(&forgetOptions.Daily, "keep-daily", "d", "keep the last `n` daily snapshots (use 'unlimited' to keep all daily snapshots)")
f.VarP(&forgetOptions.Weekly, "keep-weekly", "w", "keep the last `n` weekly snapshots (use 'unlimited' to keep all weekly snapshots)")
f.VarP(&forgetOptions.Monthly, "keep-monthly", "m", "keep the last `n` monthly snapshots (use 'unlimited' to keep all monthly snapshots)")
f.VarP(&forgetOptions.Yearly, "keep-yearly", "y", "keep the last `n` yearly snapshots (use 'unlimited' to keep all yearly snapshots)")
f.VarP(&forgetOptions.Within, "keep-within", "", "keep snapshots that are newer than `duration` (eg. 1y5m7d2h) relative to the latest snapshot")
f.VarP(&forgetOptions.WithinHourly, "keep-within-hourly", "", "keep hourly snapshots that are newer than `duration` (eg. 1y5m7d2h) relative to the latest snapshot")
f.VarP(&forgetOptions.WithinDaily, "keep-within-daily", "", "keep daily snapshots that are newer than `duration` (eg. 1y5m7d2h) relative to the latest snapshot")
@ -165,12 +201,12 @@ func runForget(ctx context.Context, opts ForgetOptions, gopts GlobalOptions, arg
}
policy := restic.ExpirePolicy{
Last: opts.Last,
Hourly: opts.Hourly,
Daily: opts.Daily,
Weekly: opts.Weekly,
Monthly: opts.Monthly,
Yearly: opts.Yearly,
Last: int(opts.Last),
Hourly: int(opts.Hourly),
Daily: int(opts.Daily),
Weekly: int(opts.Weekly),
Monthly: int(opts.Monthly),
Yearly: int(opts.Yearly),
Within: opts.Within,
WithinHourly: opts.WithinHourly,
WithinDaily: opts.WithinDaily,

View file

@ -7,55 +7,84 @@ import (
rtest "github.com/restic/restic/internal/test"
)
func TestForgetPolicyValues(t *testing.T) {
testCases := []struct {
input string
value ForgetPolicyCount
err string
}{
{"0", ForgetPolicyCount(0), ""},
{"1", ForgetPolicyCount(1), ""},
{"unlimited", ForgetPolicyCount(-1), ""},
{"", ForgetPolicyCount(0), "strconv.ParseInt: parsing \"\": invalid syntax"},
{"-1", ForgetPolicyCount(0), ErrNegativePolicyCount.Error()},
{"abc", ForgetPolicyCount(0), "strconv.ParseInt: parsing \"abc\": invalid syntax"},
}
for _, testCase := range testCases {
t.Run("", func(t *testing.T) {
var count ForgetPolicyCount
err := count.Set(testCase.input)
if testCase.err != "" {
rtest.Assert(t, err != nil, "should have returned error for input %+v", testCase.input)
rtest.Equals(t, testCase.err, err.Error())
} else {
rtest.Assert(t, err == nil, "expected no error for input %+v, got %v", testCase.input, err)
rtest.Equals(t, testCase.value, count)
rtest.Equals(t, testCase.input, count.String())
}
})
}
}
func TestForgetOptionValues(t *testing.T) {
const negValErrorMsg = "Fatal: negative values other than -1 are not allowed for --keep-*"
const negDurationValErrorMsg = "Fatal: durations containing negative values are not allowed for --keep-within*"
testCases := []struct {
input ForgetOptions
expectsError bool
errorMsg string
input ForgetOptions
errorMsg string
}{
{ForgetOptions{Last: 1}, false, ""},
{ForgetOptions{Hourly: 1}, false, ""},
{ForgetOptions{Daily: 1}, false, ""},
{ForgetOptions{Weekly: 1}, false, ""},
{ForgetOptions{Monthly: 1}, false, ""},
{ForgetOptions{Yearly: 1}, false, ""},
{ForgetOptions{Last: 0}, false, ""},
{ForgetOptions{Hourly: 0}, false, ""},
{ForgetOptions{Daily: 0}, false, ""},
{ForgetOptions{Weekly: 0}, false, ""},
{ForgetOptions{Monthly: 0}, false, ""},
{ForgetOptions{Yearly: 0}, false, ""},
{ForgetOptions{Last: -1}, false, ""},
{ForgetOptions{Hourly: -1}, false, ""},
{ForgetOptions{Daily: -1}, false, ""},
{ForgetOptions{Weekly: -1}, false, ""},
{ForgetOptions{Monthly: -1}, false, ""},
{ForgetOptions{Yearly: -1}, false, ""},
{ForgetOptions{Last: -2}, true, negValErrorMsg},
{ForgetOptions{Hourly: -2}, true, negValErrorMsg},
{ForgetOptions{Daily: -2}, true, negValErrorMsg},
{ForgetOptions{Weekly: -2}, true, negValErrorMsg},
{ForgetOptions{Monthly: -2}, true, negValErrorMsg},
{ForgetOptions{Yearly: -2}, true, negValErrorMsg},
{ForgetOptions{Within: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""},
{ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""},
{ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""},
{ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d3h")}, false, ""},
{ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("2y4m6d8h")}, false, ""},
{ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y4m6d8h")}, false, ""},
{ForgetOptions{Within: restic.ParseDurationOrPanic("-1y2m3d3h")}, true, negDurationValErrorMsg},
{ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y-2m3d3h")}, true, negDurationValErrorMsg},
{ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m-3d3h")}, true, negDurationValErrorMsg},
{ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d-3h")}, true, negDurationValErrorMsg},
{ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("-2y4m6d8h")}, true, negDurationValErrorMsg},
{ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y-4m6d8h")}, true, negDurationValErrorMsg},
{ForgetOptions{Last: 1}, ""},
{ForgetOptions{Hourly: 1}, ""},
{ForgetOptions{Daily: 1}, ""},
{ForgetOptions{Weekly: 1}, ""},
{ForgetOptions{Monthly: 1}, ""},
{ForgetOptions{Yearly: 1}, ""},
{ForgetOptions{Last: 0}, ""},
{ForgetOptions{Hourly: 0}, ""},
{ForgetOptions{Daily: 0}, ""},
{ForgetOptions{Weekly: 0}, ""},
{ForgetOptions{Monthly: 0}, ""},
{ForgetOptions{Yearly: 0}, ""},
{ForgetOptions{Last: -1}, ""},
{ForgetOptions{Hourly: -1}, ""},
{ForgetOptions{Daily: -1}, ""},
{ForgetOptions{Weekly: -1}, ""},
{ForgetOptions{Monthly: -1}, ""},
{ForgetOptions{Yearly: -1}, ""},
{ForgetOptions{Last: -2}, negValErrorMsg},
{ForgetOptions{Hourly: -2}, negValErrorMsg},
{ForgetOptions{Daily: -2}, negValErrorMsg},
{ForgetOptions{Weekly: -2}, negValErrorMsg},
{ForgetOptions{Monthly: -2}, negValErrorMsg},
{ForgetOptions{Yearly: -2}, negValErrorMsg},
{ForgetOptions{Within: restic.ParseDurationOrPanic("1y2m3d3h")}, ""},
{ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y2m3d3h")}, ""},
{ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m3d3h")}, ""},
{ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d3h")}, ""},
{ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("2y4m6d8h")}, ""},
{ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y4m6d8h")}, ""},
{ForgetOptions{Within: restic.ParseDurationOrPanic("-1y2m3d3h")}, negDurationValErrorMsg},
{ForgetOptions{WithinHourly: restic.ParseDurationOrPanic("1y-2m3d3h")}, negDurationValErrorMsg},
{ForgetOptions{WithinDaily: restic.ParseDurationOrPanic("1y2m-3d3h")}, negDurationValErrorMsg},
{ForgetOptions{WithinWeekly: restic.ParseDurationOrPanic("1y2m3d-3h")}, negDurationValErrorMsg},
{ForgetOptions{WithinMonthly: restic.ParseDurationOrPanic("-2y4m6d8h")}, negDurationValErrorMsg},
{ForgetOptions{WithinYearly: restic.ParseDurationOrPanic("2y-4m6d8h")}, negDurationValErrorMsg},
}
for _, testCase := range testCases {
err := verifyForgetOptions(&testCase.input)
if testCase.expectsError {
if testCase.errorMsg != "" {
rtest.Assert(t, err != nil, "should have returned error for input %+v", testCase.input)
rtest.Equals(t, testCase.errorMsg, err.Error())
} else {