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.

Sign up for GitHub

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

Jump to bottom

feat(snapshots): added ability to use cron expressions to schedule snapshots #3149

Merged
merged 3 commits into from
Jul 20, 2023
Merged

feat(snapshots): added ability to use cron expressions to schedule snapshots #3149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b953f2a
feat(snapshots): added ability to use cron expressions to schedule sn…
jkowalski Jul 9, 2023
dc1ad2d
upgrade ui
jkowalski Jul 16, 2023
1a77e60
pr feedback
jkowalski Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 52 additions & 1 deletion cli/command_policy_set_scheduling.go
View file
Open in desktop
Expand Up @@ -14,12 +14,14 @@
type policySchedulingFlags struct {
policySetInterval []time.Duration // not a list, just optional duration
policySetTimesOfDay []string
policySetCron string
policySetManual bool
}

func (c *policySchedulingFlags) setup(cmd *kingpin.CmdClause) {
cmd.Flag("snapshot-interval", "Interval between snapshots").DurationListVar(&c.policySetInterval)
cmd.Flag("snapshot-time", "Comma-separated times of day when to take snapshot (HH:mm,HH:mm,...) or 'inherit' to remove override").StringsVar(&c.policySetTimesOfDay)
cmd.Flag("snapshot-time-crontab", "Semicolon-separated crontab-compatible expressions (or 'inherit')").StringVar(&c.policySetCron)
cmd.Flag("manual", "Only create snapshots manually").BoolVar(&c.policySetManual)
}

Expand Down Expand Up @@ -71,6 +73,24 @@
}
}

if c.policySetCron != "" {
ce := splitCronExpressions(c.policySetCron)

if ce == nil {
log(ctx).Infof(" - resetting cron snapshot times to default")
} else {
log(ctx).Infof(" - setting cron snapshot times to %v", ce)
}

*changeCount++

sp.Cron = ce

if err := policy.ValidateSchedulingPolicy(*sp); err != nil {
return errors.Wrap(err, "invalid scheduling policy")
}
}

if sp.Manual {
*changeCount++

Expand All @@ -82,9 +102,32 @@
return nil
}

// splitCronExpressions splits the provided string into a list of cron expressions.
// Individual items are separated by semi-colons. As a special case, the string "inherit"
// returns a nil slice.
func splitCronExpressions(expr string) []string {
if expr == inheritPolicyString || expr == defaultPolicyString {
return nil
}

var result []string

parts := strings.Split(expr, ";")
for _, part := range parts {
part = strings.TrimSpace(part)
if part == "" {
continue
}

result = append(result, part)
}

return result
}

func (c *policySchedulingFlags) setManualFromFlags(ctx context.Context, sp *policy.SchedulingPolicy, changeCount *int) error {
// Cannot set both schedule and manual setting
if len(c.policySetInterval) > 0 || len(c.policySetTimesOfDay) > 0 {
if len(c.policySetInterval) > 0 || len(c.policySetTimesOfDay) > 0 || len(c.policySetCron) > 0 {
return errors.New("cannot set manual field when scheduling snapshots")
}

Expand All @@ -105,6 +148,14 @@
log(ctx).Infof(" - resetting snapshot times of day to default\n")
}

if len(sp.Cron) > 0 {
*changeCount++

sp.Cron = nil

log(ctx).Infof(" - resetting cron snapshot times to default\n")
}

Check warning on line 157 in cli/command_policy_set_scheduling.go

View check run for this annotation

Codecov / codecov/patch

cli/command_policy_set_scheduling.go#L152-L157 

Added lines #L152 - L157 were not covered by tests

*changeCount++

sp.Manual = c.policySetManual
Expand Down
128 changes: 84 additions & 44 deletions cli/command_policy_set_test.go
View file
Open in desktop
Expand Up @@ -5,6 +5,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/kopia/kopia/internal/testlogging"
"github.com/kopia/kopia/snapshot/policy"
)
Expand Down Expand Up @@ -35,7 +37,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
},
expErr: false,
expChangeCount: 0,
},
{
Expand Down Expand Up @@ -71,7 +72,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: nil,
IgnoreDirectoryErrors: nil,
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -83,7 +83,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -98,7 +97,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -113,7 +111,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(true),
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -128,7 +125,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(false),
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -143,7 +139,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: nil,
IgnoreDirectoryErrors: newOptionalBool(true),
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -158,7 +153,6 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: nil,
},
expErr: false,
expChangeCount: 2,
},
} {
Expand Down Expand Up @@ -187,16 +181,16 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
startingPolicy *policy.SchedulingPolicy
intervalArg []time.Duration
timesOfDayArg []string
cronArg string
manualArg bool
expResult *policy.SchedulingPolicy
expErr bool
expErrMsg string
expChangeCount int
}{
{
name: "No flags provided, no starting policy",
startingPolicy: &policy.SchedulingPolicy{},
expResult: &policy.SchedulingPolicy{},
expErr: false,
expChangeCount: 0,
},
{
Expand All @@ -206,7 +200,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
Manual: true,
},
expErr: false,
expChangeCount: 1,
},
{
Expand All @@ -218,7 +211,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
IntervalSeconds: 3600,
},
expErr: false,
expChangeCount: 1,
},
{
Expand All @@ -235,7 +227,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
},
},
},
expErr: false,
expChangeCount: 1,
},
{
Expand All @@ -246,7 +237,7 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
},
manualArg: true,
expResult: &policy.SchedulingPolicy{},
expErr: true,
expErrMsg: "cannot set manual field when scheduling snapshots",
expChangeCount: 0,
},
{
Expand All @@ -257,7 +248,16 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
},
manualArg: true,
expResult: &policy.SchedulingPolicy{},
expErr: true,
expErrMsg: "cannot set manual field when scheduling snapshots",
expChangeCount: 0,
},
{
name: "Manual and cron set, no starting policy",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: "* * * * *",
manualArg: true,
expResult: &policy.SchedulingPolicy{},
expErrMsg: "cannot set manual field when scheduling snapshots",
expChangeCount: 0,
},
{
Expand All @@ -269,7 +269,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
Manual: true,
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -286,7 +285,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
Manual: true,
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -304,7 +302,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
Manual: true,
},
expErr: false,
expChangeCount: 3,
},
{
Expand All @@ -318,7 +315,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
IntervalSeconds: 3600,
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -337,7 +333,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
},
},
},
expErr: false,
expChangeCount: 2,
},
{
Expand All @@ -352,7 +347,6 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
{Hour: 14, Minute: 0},
},
},
expErr: false,
expChangeCount: 1,
},
{
Expand All @@ -364,36 +358,82 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
expResult: &policy.SchedulingPolicy{
TimesOfDay: nil,
},
expErr: false,
expChangeCount: 1,
},
{
name: "Set single cron expression",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: "1 2 * * *",
expResult: &policy.SchedulingPolicy{
Cron: []string{"1 2 * * *"},
},
expChangeCount: 1,
},
{
name: "Set single cron expression with comment",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: "1 2 * * * # some comment",
expResult: &policy.SchedulingPolicy{
Cron: []string{"1 2 * * * # some comment"},
},
expChangeCount: 1,
},
{
name: "Support comment-only cron expression",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: "# some comment;1 2 * * * ",
expResult: &policy.SchedulingPolicy{
Cron: []string{"# some comment", "1 2 * * *"},
},
expChangeCount: 1,
},
{
name: "Set multiple cron expressions",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: ";1 2 * * *;;;2 1 * * *;",
expResult: &policy.SchedulingPolicy{
Cron: []string{"1 2 * * *", "2 1 * * *"},
},
expChangeCount: 1,
},
{
name: "Set invalid cron expression",
startingPolicy: &policy.SchedulingPolicy{},
cronArg: "aa bb * * *",
expErrMsg: "invalid cron expression",
expChangeCount: 1,
},
{
name: "Inherit cron expressions",
startingPolicy: &policy.SchedulingPolicy{
Cron: []string{"1 2 * * *", "2 1 * * *"},
},
cronArg: "inherit",
expResult: &policy.SchedulingPolicy{
Cron: nil,
},
expChangeCount: 1,
},
} {
t.Log(tc.name)
tc := tc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why not move the declaration for psf here, inside the closure, so it is a new variable every time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

t.Run(tc.name, func(t *testing.T) {
changeCount := 0

changeCount := 0

psf.policySetInterval = tc.intervalArg
psf.policySetTimesOfDay = tc.timesOfDayArg
psf.policySetManual = tc.manualArg
psf.policySetInterval = tc.intervalArg
psf.policySetTimesOfDay = tc.timesOfDayArg
psf.policySetManual = tc.manualArg
psf.policySetCron = tc.cronArg

err := psf.setSchedulingPolicyFromFlags(ctx, tc.startingPolicy, &changeCount)
if tc.expErr {
if err == nil {
t.Errorf("Expected error but got none")
}
} else {
if err != nil {
t.Errorf("Expected none but got err: %v", err)
err := psf.setSchedulingPolicyFromFlags(ctx, tc.startingPolicy, &changeCount)
if tc.expErrMsg != "" {
require.ErrorContains(t, err, tc.expErrMsg)
return
}
}

if !reflect.DeepEqual(tc.startingPolicy, tc.expResult) {
t.Errorf("Did not get expected output: (actual) %v != %v (expected)", tc.startingPolicy, tc.expResult)
}

if !reflect.DeepEqual(tc.expChangeCount, changeCount) {
t.Errorf("Did not get expected output: (actual) %v != %v (expected)", tc.expChangeCount, changeCount)
}
require.NoError(t, err)
require.Equal(t, tc.expResult, tc.startingPolicy)
require.Equal(t, tc.expChangeCount, changeCount)
})
}
}

Expand Down