Z10 Z12 trim without day or time refused, keep count 0 removes nothing

A trim schedule of weekly or monthly with no trim-day or trim-time is one refused value naming it; a keep count of 0 is that kind's none: the existing scheduled ZFS snapshots are kept and a warning names them.

Builds: design 42 zfs-owns-zfs §20, §21, §22, A-33, A-35

modules/zfs/cmd/zfs-tools/config.go

@@ -263,6 +263,18 @@ func ParseConfig(text []byte, memMiB uint64) (*Config, []Refusal) {
 		return oneOf(s, "Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun")
 	})
 	c.TrimTime, _ = p.str("trim-time", clockForm)
+	// A-33: a trim schedule of weekly or monthly with no day or no time runs no trim and is said, as one refused value.
+	if c.TrimSchedule == "weekly" || c.TrimSchedule == "monthly" {
+		var missing []string
+		for _, need := range []struct{ key, have string }{{"trim-day", c.TrimDay}, {"trim-time", c.TrimTime}} {
+			if need.have == "" {
+				missing = append(missing, need.key)
+			}
+		}
+		if len(missing) > 0 {
+			p.refuse("trim-schedule", p.raw["trim-schedule"], "needs "+strings.Join(missing, " and ")+" set: without it no trim runs")
+		}
+	}
 	c.Zed, _ = p.str("zed", func(s string) string { return oneOf(s, "on", "off") })
 	if h, ok := p.str("hostid", func(s string) string {
 		switch {

modules/zfs/cmd/zfs-tools/config_test.go

@@ -31,6 +31,8 @@ func TestEachValueHasItsFormAndABlankOneIsNotManaged(t *testing.T) {
 		{"trim schedule", bad("trim-schedule", `"daily"`), "none, weekly, monthly"},
 		{"trim day", bad("trim-day", `"Monday"`), "Mon"},
 		{"trim time", bad("trim-time", `"24:00"`), "HH:MM"},
+		{"trim schedule without day or time", `{"trim-schedule":"weekly","trim-time":"03:00"}`, "needs trim-day"},
+		{"trim schedule without time", `{"trim-schedule":"monthly","trim-day":"Sun"}`, "needs trim-time"},
 		{"cache device with a slash", bad("cache-devices", `["a/b"]`), "under /dev/disk/by-id"},
 		{"cache device with a dollar", bad("cache-devices", `["a$b"]`), "under /dev/disk/by-id"},
 		{"cache device with a quote", bad("cache-devices", `["a\"b"]`), "under /dev/disk/by-id"},

modules/zfs/cmd/zfs-tools/snapshots_prune.go

@@ -120,6 +120,16 @@ func (m *Machine) PruneScheduled(ctx context.Context, c *Config, person bool, by
 	}
 	for _, k := range sortedKeys(groups) {
 		g := groups[k]
+		if c.Snapshots.Of(g[0].Kind) == 0 {
+			// A-35: a keep count of 0 is that kind's none: its scheduled snapshots are kept and a warning names them.
+			names := make([]string, 0, len(g))
+			for _, e := range g {
+				names = append(names, e.Dataset+"@"+e.Name)
+			}
+			sort.Strings(names)
+			m.Warnings = append(m.Warnings, fmt.Sprintf("the keep count of %s is 0, which removes nothing: kept %s", g[0].Kind, strings.Join(names, ", ")))
+			continue
+		}
 		for _, e := range surplus(g, c.Snapshots.Of(g[0].Kind)) {
 			full := e.Dataset + "@" + e.Name
 			why, err := m.refuseRemoval(ctx, c.Pool, e)

modules/zfs/cmd/zfs-tools/snapshots_prune_test.go

@@ -27,6 +27,7 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin
 		setup   func(st memStates)
 		removed []string
 		left    int
+		warns   string // a warning the run must have kept
 		wantErr bool
 	}{
 		{name: "every surplus in one run, oldest first", keeps: `{"hourly":2}`, l: &ledgerCalls{entries: four},
@@ -42,6 +43,8 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin
 		{name: "a name that does not fit is left", keeps: `{"hourly":1}`, l: &ledgerCalls{entries: []LedgerEntry{
 			{"tank", "before-move", "hourly", at(1)}, entry("tank", 2)}}, left: 1},
 		{name: "none removes nothing", keeps: `{"none":true}`, l: &ledgerCalls{entries: four}},
+		{name: "a keep count of 0 is none: nothing removed, a warning names them (A-35)", keeps: `{"hourly":0,"daily":1}`,
+			l: &ledgerCalls{entries: four}, warns: "keep count of hourly is 0"},
 		{name: "paused removes nothing", keeps: `{"hourly":1}`, l: &ledgerCalls{entries: four},
 			setup: func(st memStates) { st.Put(StateSchedule, KeyPaused, map[string]string{}) }},
 		{name: "no read, no removal", keeps: `{"hourly":1}`, l: &ledgerCalls{read: errors.New("no bus")}},
@@ -81,6 +84,9 @@ func TestPruningRemovesEverySurplusByItsRecordAndNothingThatDoesNotFit(t *testin
 		}
 		f.on(line("zfs", SnapshotsArgv()), list.String()+"tank@by-hand\t1791400000\t1\t2\n")
 		said, err := m.PruneScheduled(context.Background(), keepsOfJSON(t, c.keeps), false, "")
+		if c.warns != "" && !strings.Contains(strings.Join(m.Warnings, "|"), c.warns) {
+			t.Errorf("%s: warnings %v, want %q", c.name, m.Warnings, c.warns)
+		}
 		if (err != nil) != c.wantErr {
 			t.Errorf("%s: error %v", c.name, err)
 		}