fix(cli): Convert --run-missed from bool to Enum (#3337)

This commit is contained in:
PhracturedBlue
2023-09-22 10:18:19 -07:00
committed by GitHub
parent 062e3277f6
commit bcb07da5f3
9 changed files with 110 additions and 95 deletions

View File

@@ -16,14 +16,14 @@ type policySchedulingFlags struct {
policySetTimesOfDay []string
policySetCron string
policySetManual bool
policySetRunMissed bool
policySetRunMissed string
}
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("run-missed", "Run missed time-of-day snapshots (has no effect on interval snapshots)").BoolVar(&c.policySetRunMissed)
cmd.Flag("run-missed", "Run missed time-of-day snapshots ('true', 'false', 'inherit')").EnumVar(&c.policySetRunMissed, booleanEnumValues...)
cmd.Flag("manual", "Only create snapshots manually").BoolVar(&c.policySetManual)
}
@@ -93,7 +93,9 @@ func (c *policySchedulingFlags) setScheduleFromFlags(ctx context.Context, sp *po
}
}
c.setRunMissedFromFlags(ctx, sp, changeCount)
if err := c.setRunMissedFromFlags(ctx, sp, changeCount); err != nil {
return errors.Wrap(err, "invalid run-missed value")
}
if sp.Manual {
*changeCount++
@@ -107,18 +109,12 @@ func (c *policySchedulingFlags) setScheduleFromFlags(ctx context.Context, sp *po
}
// Update RunMissed policy flag if changed.
func (c *policySchedulingFlags) setRunMissedFromFlags(ctx context.Context, sp *policy.SchedulingPolicy, changeCount *int) {
if (c.policySetRunMissed && !sp.RunMissed) || (!c.policySetRunMissed && sp.RunMissed) {
*changeCount++
sp.RunMissed = c.policySetRunMissed
if sp.RunMissed {
log(ctx).Infof(" - missed time-of-day snapshots will run immediately\n")
} else {
log(ctx).Infof(" - missed time-of-day snapshots will run at next scheduled time\n")
}
func (c *policySchedulingFlags) setRunMissedFromFlags(ctx context.Context, sp *policy.SchedulingPolicy, changeCount *int) error {
if err := applyPolicyBoolPtr(ctx, "run missed snapshots", &sp.RunMissed, c.policySetRunMissed, changeCount); err != nil {
return errors.Wrap(err, "invalid scheduling policy")
}
return nil
}
// splitCronExpressions splits the provided string into a list of cron expressions.

View File

@@ -28,14 +28,14 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
{
name: "No values provided as command line arguments",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
fileArg: "",
dirArg: "",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
expChangeCount: 0,
},
@@ -57,7 +57,7 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
fileArg: "true",
dirArg: "some-malformed-arg",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: nil,
},
expErr: true,
@@ -80,77 +80,77 @@ func TestSetErrorHandlingPolicyFromFlags(t *testing.T) {
fileArg: "true",
dirArg: "true",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
expChangeCount: 2,
},
{
name: "Set to false",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
fileArg: "false",
dirArg: "false",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: policy.NewOptionalBool(false),
IgnoreDirectoryErrors: policy.NewOptionalBool(false),
},
expChangeCount: 2,
},
{
name: "File false, dir true",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(false),
},
fileArg: "false",
dirArg: "true",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(false),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
expChangeCount: 2,
},
{
name: "File true, dir false",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(false),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
fileArg: "true",
dirArg: "false",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(false),
},
expChangeCount: 2,
},
{
name: "File inherit, dir true",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(false),
},
fileArg: "inherit",
dirArg: "true",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: nil,
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
expChangeCount: 2,
},
{
name: "File true, dir inherit",
startingPolicy: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(false),
IgnoreDirectoryErrors: policy.NewOptionalBool(true),
},
fileArg: "true",
dirArg: "inherit",
expResult: &policy.ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreFileErrors: policy.NewOptionalBool(true),
IgnoreDirectoryErrors: nil,
},
expChangeCount: 2,
@@ -182,7 +182,7 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
timesOfDayArg []string
cronArg string
manualArg bool
runMissedArg bool
runMissedArg string
expResult *policy.SchedulingPolicy
expErrMsg string
expChangeCount int
@@ -419,10 +419,10 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
startingPolicy: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
},
runMissedArg: true,
runMissedArg: "true",
expResult: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
expChangeCount: 1,
},
@@ -430,25 +430,25 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
name: "Clear RunMissed",
startingPolicy: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
expResult: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
RunMissed: false,
RunMissed: policy.NewOptionalBool(false),
},
runMissedArg: "false",
expChangeCount: 1,
},
{
name: "RunMissed unchanged",
startingPolicy: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
expResult: &policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{Hour: 12, Minute: 0}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
runMissedArg: true,
expChangeCount: 0,
},
} {
@@ -476,7 +476,3 @@ func TestSetSchedulingPolicyFromFlags(t *testing.T) {
})
}
}
func newOptionalBool(b policy.OptionalBool) *policy.OptionalBool {
return &b
}

View File

@@ -292,8 +292,17 @@ func appendSchedulingPolicyRows(rows []policyTableRow, p *policy.Policy, def *po
}
if len(p.SchedulingPolicy.TimesOfDay) > 0 {
rows = append(rows, policyTableRow{" Run missed snapshots:", boolToString(p.SchedulingPolicy.RunMissed), definitionPointToString(p.Target(), def.SchedulingPolicy.RunMissed)},
policyTableRow{" Snapshot times:", "", definitionPointToString(p.Target(), def.SchedulingPolicy.TimesOfDay)})
rows = append(rows,
policyTableRow{
" Run missed snapshots:",
boolToString(p.SchedulingPolicy.RunMissed.OrDefault(false)),
definitionPointToString(p.Target(), def.SchedulingPolicy.RunMissed),
},
policyTableRow{
" Snapshot times:",
"",
definitionPointToString(p.Target(), def.SchedulingPolicy.TimesOfDay),
})
for _, tod := range p.SchedulingPolicy.TimesOfDay {
rows = append(rows, policyTableRow{" " + tod.String(), "", ""})

View File

@@ -123,6 +123,7 @@ func TestSourceRefreshesAfterPolicy(t *testing.T) {
TimesOfDay: []policy.TimeOfDay{
{Hour: (currentHour + 2) % 24, Minute: 33},
},
RunMissed: policy.NewOptionalBool(false),
},
})
@@ -136,6 +137,7 @@ func TestSourceRefreshesAfterPolicy(t *testing.T) {
TimesOfDay: []policy.TimeOfDay{
{Hour: (currentHour + 2) % 24, Minute: 55},
},
RunMissed: policy.NewOptionalBool(false),
},
})

View File

@@ -48,13 +48,13 @@ type args struct {
},
args: args{
src: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
},
},
expResult: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
},
},
{
@@ -65,47 +65,47 @@ type args struct {
},
args: args{
src: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
expResult: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
{
name: "Starting policy already has a value set at false - expect no change from merged policy",
fields: fields{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
},
args: args{
src: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
expResult: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
},
},
{
name: "Policy being merged has a value set at true - expect no change from merged policy",
fields: fields{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
args: args{
src: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
},
},
expResult: ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(true),
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
{
@@ -117,12 +117,12 @@ type args struct {
args: args{
src: ErrorHandlingPolicy{
IgnoreFileErrors: nil,
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
expResult: ErrorHandlingPolicy{
IgnoreFileErrors: nil,
IgnoreDirectoryErrors: newOptionalBool(true),
IgnoreDirectoryErrors: NewOptionalBool(true),
},
},
} {

View File

@@ -12,7 +12,8 @@ func (b *OptionalBool) OrDefault(def bool) bool {
return bool(*b)
}
func newOptionalBool(b OptionalBool) *OptionalBool {
// NewOptionalBool provides an OptionalBool pointer.
func NewOptionalBool(b OptionalBool) *OptionalBool {
return &b
}

View File

@@ -15,9 +15,9 @@
// defaultErrorHandlingPolicy is the default error handling policy.
defaultErrorHandlingPolicy = ErrorHandlingPolicy{
IgnoreFileErrors: newOptionalBool(false),
IgnoreDirectoryErrors: newOptionalBool(false),
IgnoreUnknownTypes: newOptionalBool(true),
IgnoreFileErrors: NewOptionalBool(false),
IgnoreDirectoryErrors: NewOptionalBool(false),
IgnoreUnknownTypes: NewOptionalBool(true),
}
// defaultFilesPolicy is the default file ignore policy.
@@ -46,10 +46,12 @@
KeepWeekly: newOptionalInt(defaultKeepWeekly),
KeepMonthly: newOptionalInt(defaultKeepMonthly),
KeepAnnual: newOptionalInt(defaultKeepAnnual),
IgnoreIdenticalSnapshots: newOptionalBool(defaultIgnoreIdenticalSnapshots),
IgnoreIdenticalSnapshots: NewOptionalBool(defaultIgnoreIdenticalSnapshots),
}
defaultSchedulingPolicy = SchedulingPolicy{}
defaultSchedulingPolicy = SchedulingPolicy{
RunMissed: NewOptionalBool(defaultRunMissed),
}
defaultUploadPolicy = UploadPolicy{
MaxParallelSnapshots: newOptionalInt(1),

View File

@@ -57,12 +57,12 @@ func SortAndDedupeTimesOfDay(tod []TimeOfDay) []TimeOfDay {
// SchedulingPolicy describes policy for scheduling snapshots.
type SchedulingPolicy struct {
IntervalSeconds int64 `json:"intervalSeconds,omitempty"`
TimesOfDay []TimeOfDay `json:"timeOfDay,omitempty"`
NoParentTimesOfDay bool `json:"noParentTimeOfDay,omitempty"`
Manual bool `json:"manual,omitempty"`
Cron []string `json:"cron,omitempty"`
RunMissed bool `json:"runMissed,omitempty"`
IntervalSeconds int64 `json:"intervalSeconds,omitempty"`
TimesOfDay []TimeOfDay `json:"timeOfDay,omitempty"`
NoParentTimesOfDay bool `json:"noParentTimeOfDay,omitempty"`
Manual bool `json:"manual,omitempty"`
Cron []string `json:"cron,omitempty"`
RunMissed *OptionalBool `json:"runMissed,omitempty"`
}
// SchedulingPolicyDefinition specifies which policy definition provided the value of a particular field.
@@ -74,6 +74,9 @@ type SchedulingPolicyDefinition struct {
RunMissed snapshot.SourceInfo `json:"runMissed,omitempty"`
}
// defaultRunMissed is the value for RunMissed.
const defaultRunMissed = false
// Interval returns the snapshot interval or zero if not specified.
func (p *SchedulingPolicy) Interval() time.Duration {
return time.Duration(p.IntervalSeconds) * time.Second
@@ -162,7 +165,11 @@ func (p *SchedulingPolicy) checkMissedSnapshot(now, previousSnapshotTime, nextSn
const halfhour = 30 * time.Minute
return (len(p.TimesOfDay) > 0 || len(p.Cron) > 0) && p.RunMissed && previousSnapshotTime.Add(oneDay-halfhour).Before(now) && nextSnapshotTime.After(now.Add(halfhour))
if !p.RunMissed.OrDefault(false) {
return false
}
return (len(p.TimesOfDay) > 0 || len(p.Cron) > 0) && previousSnapshotTime.Add(oneDay-halfhour).Before(now) && nextSnapshotTime.After(now.Add(halfhour))
}
// Merge applies default values from the provided policy.
@@ -185,7 +192,7 @@ func (p *SchedulingPolicy) Merge(src SchedulingPolicy, def *SchedulingPolicyDefi
}
mergeBool(&p.Manual, src.Manual, &def.Manual, si)
mergeBool(&p.RunMissed, src.RunMissed, &def.RunMissed, si)
mergeOptionalBool(&p.RunMissed, src.RunMissed, &def.RunMissed, si)
}
// IsManualSnapshot returns the SchedulingPolicy manual value from the given policy tree.

View File

@@ -192,7 +192,8 @@ func TestNextSnapshotTime(t *testing.T) {
{
name: "Cron policy using minute and hour rules",
pol: policy.SchedulingPolicy{
Cron: []string{"0 23 * * *"},
Cron: []string{"0 23 * * *"},
RunMissed: policy.NewOptionalBool(false),
},
now: time.Date(2020, time.January, 1, 10, 0, 0, 0, time.Local),
// matches 23:00
@@ -202,7 +203,8 @@ func TestNextSnapshotTime(t *testing.T) {
{
name: "Cron policy using minute, hour, month, and day rules",
pol: policy.SchedulingPolicy{
Cron: []string{"5 3 * Feb Thu"},
Cron: []string{"5 3 * Feb Thu"},
RunMissed: policy.NewOptionalBool(false),
},
now: time.Date(2020, time.January, 1, 1, 0, 0, 0, time.Local),
// matches next Thursday in February, 3:05
@@ -213,7 +215,7 @@ func TestNextSnapshotTime(t *testing.T) {
name: "Run immediately since last run was missed and RunMissed is set",
pol: policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{11, 55}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
now: time.Date(2020, time.January, 2, 11, 55, 30, 0, time.Local),
previousSnapshotTime: time.Date(2020, time.January, 1, 11, 55, 0, 0, time.Local),
@@ -224,7 +226,7 @@ func TestNextSnapshotTime(t *testing.T) {
name: "Don't run immediately even though RunMissed is set, because next run is upcoming",
pol: policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{11, 55}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
now: time.Date(2020, time.January, 3, 11, 30, 0, 0, time.Local),
previousSnapshotTime: time.Date(2020, time.January, 1, 11, 55, 0, 0, time.Local),
@@ -235,7 +237,7 @@ func TestNextSnapshotTime(t *testing.T) {
name: "Don't run immediately even though RunMissed is set because last run was not missed",
pol: policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{11, 55}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
now: time.Date(2020, time.January, 2, 11, 30, 0, 0, time.Local),
previousSnapshotTime: time.Date(2020, time.January, 1, 11, 55, 0, 0, time.Local),
@@ -246,7 +248,7 @@ func TestNextSnapshotTime(t *testing.T) {
name: "Don't run immediately even though RunMissed is set because last run was not missed",
pol: policy.SchedulingPolicy{
TimesOfDay: []policy.TimeOfDay{{10, 0}},
RunMissed: true,
RunMissed: policy.NewOptionalBool(true),
},
now: time.Date(2020, time.January, 2, 11, 0, 0, 0, time.Local),
previousSnapshotTime: time.Date(2020, time.January, 2, 10, 0, 0, 0, time.Local),