Skip to content

Commit ffb1d89

Browse files
problameproblame
committed
config: support zrepl's day and week units for snapshotting.interval
Originally, I had a patch that would replace all usages of time.Duration in package config with the new config.Duration types, but: 1. these are all timeouts/retry intervals that have default values. Most users don't touch them, and if they do, they don't need day or week units. 2. go-yaml's error reporting for yaml.Unmarshaler is inferior to built-in types (line numbers are missing, so the error would not have sufficient context) fixes #486
1 parent a967986 commit ffb1d89

File tree

4 files changed

+129
-51
lines changed

4 files changed

+129
-51
lines changed

‎config/config.go‎

Lines changed: 5 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"log/syslog"
77
"os"
88
"reflect"
9-
"regexp"
10-
"strconv"
119
"time"
1210

1311
"github.com/pkg/errors"
@@ -190,13 +188,10 @@ func (i *PositiveDurationOrManual) UnmarshalYAML(u func(interface{}, bool) error
190188
return fmt.Errorf("value must not be empty")
191189
default:
192190
i.Manual = false
193-
i.Interval, err = time.ParseDuration(s)
191+
i.Interval, err = parsePositiveDuration(s)
194192
if err != nil {
195193
return err
196194
}
197-
if i.Interval <= 0 {
198-
return fmt.Errorf("value must be a positive duration, got %q", s)
199-
}
200195
}
201196
return nil
202197
}
@@ -228,10 +223,10 @@ type SnapshottingEnum struct {
228223
}
229224

230225
type SnapshottingPeriodic struct {
231-
Type string `yaml:"type"`
232-
Prefix string `yaml:"prefix"`
233-
Interval time.Duration `yaml:"interval,positive"`
234-
Hooks HookList `yaml:"hooks,optional"`
226+
Type string `yaml:"type"`
227+
Prefix string `yaml:"prefix"`
228+
Interval *PositiveDuration `yaml:"interval"`
229+
Hooks HookList `yaml:"hooks,optional"`
235230
}
236231

237232
type CronSpec struct {
@@ -715,41 +710,3 @@ func ParseConfigBytes(bytes []byte) (*Config, error) {
715710
}
716711
return c, nil
717712
}
718-
719-
var durationStringRegex *regexp.Regexp = regexp.MustCompile(`^\s*(\d+)\s*(s|m|h|d|w)\s*$`)
720-
721-
func parsePositiveDuration(e string) (d time.Duration, err error) {
722-
comps := durationStringRegex.FindStringSubmatch(e)
723-
if len(comps) != 3 {
724-
err = fmt.Errorf("does not match regex: %s %#v", e, comps)
725-
return
726-
}
727-
728-
durationFactor, err := strconv.ParseInt(comps[1], 10, 64)
729-
if err != nil {
730-
return 0, err
731-
}
732-
if durationFactor <= 0 {
733-
return 0, errors.New("duration must be positive integer")
734-
}
735-
736-
var durationUnit time.Duration
737-
switch comps[2] {
738-
case "s":
739-
durationUnit = time.Second
740-
case "m":
741-
durationUnit = time.Minute
742-
case "h":
743-
durationUnit = time.Hour
744-
case "d":
745-
durationUnit = 24 * time.Hour
746-
case "w":
747-
durationUnit = 24 * 7 * time.Hour
748-
default:
749-
err = fmt.Errorf("contains unknown time unit '%s'", comps[2])
750-
return
751-
}
752-
753-
d = time.Duration(durationFactor) * durationUnit
754-
return
755-
}

‎config/config_duration.go‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
package config
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"regexp"
7+
"strconv"
8+
"time"
9+
10+
"github.com/kr/pretty"
11+
"github.com/zrepl/yaml-config"
12+
)
13+
14+
type Duration struct{ d time.Duration }
15+
16+
func (d Duration) Duration() time.Duration { return d.d }
17+
18+
var _ yaml.Unmarshaler = &Duration{}
19+
20+
func (d *Duration) UnmarshalYAML(unmarshal func(v interface{}, not_strict bool) error) error {
21+
var s string
22+
err := unmarshal(&s, false)
23+
if err != nil {
24+
return err
25+
}
26+
d.d, err = parseDuration(s)
27+
if err != nil {
28+
d.d = 0
29+
return &yaml.TypeError{Errors: []string{fmt.Sprintf("cannot parse value %q: %s", s, err)}}
30+
}
31+
return nil
32+
}
33+
34+
type PositiveDuration struct{ d Duration }
35+
36+
var _ yaml.Unmarshaler = &PositiveDuration{}
37+
38+
func (d PositiveDuration) Duration() time.Duration { return d.d.Duration() }
39+
40+
func (d *PositiveDuration) UnmarshalYAML(unmarshal func(v interface{}, not_strict bool) error) error {
41+
err := d.d.UnmarshalYAML(unmarshal)
42+
if err != nil {
43+
return err
44+
}
45+
if d.d.Duration() <= 0 {
46+
return fmt.Errorf("duration must be positive, got %s", d.d.Duration())
47+
}
48+
return nil
49+
}
50+
51+
func parsePositiveDuration(e string) (time.Duration, error) {
52+
d, err := parseDuration(e)
53+
if err != nil {
54+
return d, err
55+
}
56+
if d <= 0 {
57+
return 0, errors.New("duration must be positive integer")
58+
}
59+
return d, err
60+
}
61+
62+
var durationStringRegex *regexp.Regexp = regexp.MustCompile(`^\s*([\+-]?\d+)\s*(|s|m|h|d|w)\s*$`)
63+
64+
func parseDuration(e string) (d time.Duration, err error) {
65+
comps := durationStringRegex.FindStringSubmatch(e)
66+
if comps == nil {
67+
err = fmt.Errorf("must match %s", durationStringRegex)
68+
return
69+
}
70+
if len(comps) != 3 {
71+
panic(pretty.Sprint(comps))
72+
}
73+
74+
durationFactor, err := strconv.ParseInt(comps[1], 10, 64)
75+
if err != nil {
76+
return 0, err
77+
}
78+
79+
var durationUnit time.Duration
80+
switch comps[2] {
81+
case "":
82+
if durationFactor != 0 {
83+
err = fmt.Errorf("missing time unit")
84+
return
85+
} else {
86+
// It's the case where user specified '0'.
87+
// We want to allow this, just like time.ParseDuration.
88+
}
89+
case "s":
90+
durationUnit = time.Second
91+
case "m":
92+
durationUnit = time.Minute
93+
case "h":
94+
durationUnit = time.Hour
95+
case "d":
96+
durationUnit = 24 * time.Hour
97+
case "w":
98+
durationUnit = 24 * 7 * time.Hour
99+
default:
100+
err = fmt.Errorf("contains unknown time unit '%s'", comps[2])
101+
return
102+
}
103+
104+
d = time.Duration(durationFactor) * durationUnit
105+
return
106+
}

‎config/config_snapshotting_test.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ jobs:
3838
interval: 10m
3939
`
4040

41+
periodicDaily := `
42+
snapshotting:
43+
type: periodic
44+
prefix: zrepl_
45+
interval: 1d
46+
`
47+
4148
hooks := `
4249
snapshotting:
4350
type: periodic
@@ -74,7 +81,15 @@ jobs:
7481
c = testValidConfig(t, fillSnapshotting(periodic))
7582
snp := c.Jobs[0].Ret.(*PushJob).Snapshotting.Ret.(*SnapshottingPeriodic)
7683
assert.Equal(t, "periodic", snp.Type)
77-
assert.Equal(t, 10*time.Minute, snp.Interval)
84+
assert.Equal(t, 10*time.Minute, snp.Interval.Duration())
85+
assert.Equal(t, "zrepl_", snp.Prefix)
86+
})
87+
88+
t.Run("periodicDaily", func(t *testing.T) {
89+
c = testValidConfig(t, fillSnapshotting(periodicDaily))
90+
snp := c.Jobs[0].Ret.(*PushJob).Snapshotting.Ret.(*SnapshottingPeriodic)
91+
assert.Equal(t, "periodic", snp.Type)
92+
assert.Equal(t, 24*time.Hour, snp.Interval.Duration())
7893
assert.Equal(t, "zrepl_", snp.Prefix)
7994
})
8095

‎daemon/snapper/periodic.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ func periodicFromConfig(g *config.Global, fsf zfs.DatasetFilter, in *config.Snap
2222
if in.Prefix == "" {
2323
return nil, errors.New("prefix must not be empty")
2424
}
25-
if in.Interval <= 0 {
25+
if in.Interval.Duration() <= 0 {
2626
return nil, errors.New("interval must be positive")
2727
}
2828

@@ -32,7 +32,7 @@ func periodicFromConfig(g *config.Global, fsf zfs.DatasetFilter, in *config.Snap
3232
}
3333

3434
args := periodicArgs{
35-
interval: in.Interval,
35+
interval: in.Interval.Duration(),
3636
fsf: fsf,
3737
planArgs: planArgs{
3838
prefix: in.Prefix,

0 commit comments

Comments
 (0)