update default and basic gc control to use free and max storage

tonistiigi · tonistiigi · commit a73e1b09c635 · 2024-10-04T17:21:50.000-07:00
Update default policy to include maximum and free storage controls.

New default policy is combination of all three controls.

Minimum reserved storage: 10GB / 10% (10% was old default)
Maintain free storage: 20%
Maximum allowed storage: 100GB / 80%

Signed-off-by: Tonis Tiigi &lt;tonistiigi@gmail.com&gt;
diff --git a/api/services/control/control.pb.go b/api/services/control/control.pb.go
diff --git a/api/services/control/control.proto b/api/services/control/control.proto
@@ -29,8 +29,8 @@ message PruneRequest {
 	bool all = 2;
 	int64 keepDuration = 3;
 
-	int64 minStorage = 5;
-	int64 maxStorage = 4;
+	int64 minStorage = 4;
+	int64 maxStorage = 5;
 	int64 free = 6;
 }
 
diff --git a/api/types/worker.pb.go b/api/types/worker.pb.go
diff --git a/api/types/worker.proto b/api/types/worker.proto
@@ -19,9 +19,9 @@ message GCPolicy {
 	int64 keepDuration = 2;
 	repeated string filters = 4;
 
-	int64 minStorage = 5;
-	// maxStorage was renamed from freeBytes
-	int64 maxStorage = 3;
+	// minStorage was renamed from freeBytes
+	int64 minStorage = 3;
+	int64 maxStorage = 5;
 	int64 free = 6;
 }
 
diff --git a/cache/manager.go b/cache/manager.go
@@ -1058,14 +1058,12 @@ func (cm *cacheManager) pruneOnce(ctx context.Context, ch chan client.UsageInfo,
 		}
 	}
 
-	dstat, err := disk.GetDiskStat(cm.root)
-	if err != nil {
-		if opt.Free != 0 {
-			// if we are pruning based on disk space, failing to get info on it
-			// is fatal
+	var dstat disk.DiskStat
+	if opt.Free != 0 {
+		dstat, err = disk.GetDiskStat(cm.root)
+		if err != nil {
 			return err
 		}
-		bklog.L.Warnf("failed to get disk size: %v", err)
 	}
 
 	return cm.prune(ctx, ch, pruneOpt{
diff --git a/cmd/buildctl/debug/workers.go b/cmd/buildctl/debug/workers.go
@@ -89,16 +89,16 @@ func printWorkersVerbose(tw *tabwriter.Writer, winfo []*client.WorkerInfo) {
 				fmt.Fprintf(tw, "\tFilters:\t%s\n", strings.Join(rule.Filter, " "))
 			}
 			if rule.KeepDuration > 0 {
-				fmt.Fprintf(tw, "\tKeep Duration:\t%v\n", rule.KeepDuration.String())
-			}
-			if rule.MaxStorage > 0 {
-				fmt.Fprintf(tw, "\tKeep Bytes:\t%g\n", units.Bytes(rule.MaxStorage))
+				fmt.Fprintf(tw, "\tKeep duration:\t%v\n", rule.KeepDuration.String())
 			}
 			if rule.MinStorage > 0 {
-				fmt.Fprintf(tw, "\tKeep Bytes (min):\t%g\n", units.Bytes(rule.MinStorage))
+				fmt.Fprintf(tw, "\tReserved storage:\t%g\n", units.Bytes(rule.MinStorage))
 			}
 			if rule.Free > 0 {
-				fmt.Fprintf(tw, "\tFree Bytes:\t%g\n", units.Bytes(rule.MinStorage))
+				fmt.Fprintf(tw, "\tMaintain free storage:\t%g\n", units.Bytes(rule.Free))
+			}
+			if rule.MaxStorage > 0 {
+				fmt.Fprintf(tw, "\tMaximum storage:\t%g\n", units.Bytes(rule.MaxStorage))
 			}
 		}
 		fmt.Fprintf(tw, "\n")
diff --git a/cmd/buildkitd/config/config.go b/cmd/buildkitd/config/config.go
@@ -75,8 +75,12 @@ type OTELConfig struct {
 }
 
 type GCConfig struct {
-	GC            *bool      `toml:"gc"`
+	GC *bool `toml:"gc"`
+	// Deprecated: use GCMinStorage instead
 	GCKeepStorage DiskSpace  `toml:"gckeepstorage"`
+	GCMaxStorage  DiskSpace  `toml:"gcmaxstorage"`
+	GCMinStorage  DiskSpace  `toml:"gcminstorage"`
+	GCFreeStorage DiskSpace  `toml:"gcfreestorage"`
 	GCPolicy      []GCPolicy `toml:"gcpolicy"`
 }
 
diff --git a/cmd/buildkitd/config/gcpolicy.go b/cmd/buildkitd/config/gcpolicy.go
@@ -7,7 +7,6 @@ import (
 	"time"
 
 	"github.com/docker/go-units"
-	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/disk"
 	"github.com/pkg/errors"
 )
@@ -69,9 +68,12 @@ func (d *DiskSpace) UnmarshalText(textb []byte) error {
 
 const defaultCap int64 = 2e9 // 2GB
 
-func DefaultGCPolicy(keep DiskSpace) []GCPolicy {
-	if keep == (DiskSpace{}) {
-		keep = DetectDefaultGCCap()
+func DefaultGCPolicy(cfg GCConfig, dstat disk.DiskStat) []GCPolicy {
+	if cfg.IsUnset() {
+		cfg.GCMinStorage = cfg.GCKeepStorage
+	}
+	if cfg.IsUnset() {
+		cfg = DetectDefaultGCCap(dstat)
 	}
 	return []GCPolicy{
 		// if build cache uses more than 512MB delete the most easily reproducible data after it has not been used for 2 days
@@ -83,16 +85,22 @@ func DefaultGCPolicy(keep DiskSpace) []GCPolicy {
 		// remove any data not used for 60 days
 		{
 			KeepDuration: Duration{Duration: time.Duration(60) * 24 * time.Hour}, // 60d
-			MaxStorage:   keep,
+			Free:         cfg.GCFreeStorage,
+			MinStorage:   cfg.GCMinStorage,
+			MaxStorage:   cfg.GCMaxStorage,
 		},
 		// keep the unshared build cache under cap
 		{
-			MaxStorage: keep,
+			Free:       cfg.GCFreeStorage,
+			MinStorage: cfg.GCMinStorage,
+			MaxStorage: cfg.GCMaxStorage,
 		},
 		// if previous policies were insufficient start deleting internal data to keep build cache under cap
 		{
 			All:        true,
-			MaxStorage: keep,
+			Free:       cfg.GCFreeStorage,
+			MinStorage: cfg.GCMinStorage,
+			MaxStorage: cfg.GCMaxStorage,
 		},
 	}
 }
@@ -107,24 +115,39 @@ func stripQuotes(s string) string {
 	return s
 }
 
-func DetectDefaultGCCap() DiskSpace {
-	return DiskSpace{Percentage: DiskSpacePercentage}
+func DetectDefaultGCCap(dstat disk.DiskStat) GCConfig {
+	reserve := DiskSpace{Percentage: DiskSpaceReservePercentage}
+	if reserve.AsBytes(dstat) > DiskSpaceReserveBytes {
+		reserve = DiskSpace{Bytes: DiskSpaceReserveBytes}
+	}
+	max := DiskSpace{Percentage: DiskSpaceMaxPercentage}
+	if max.AsBytes(dstat) > DiskSpaceMaxBytes {
+		max = DiskSpace{Bytes: DiskSpaceMaxBytes}
+	}
+	return GCConfig{
+		GCMinStorage:  reserve,
+		GCMaxStorage:  max,
+		GCFreeStorage: DiskSpace{Percentage: DiskSpaceFreePercentage},
+	}
 }
 
-func (d DiskSpace) AsBytes(root string) int64 {
+func (d DiskSpace) AsBytes(dstat disk.DiskStat) int64 {
 	if d.Bytes != 0 {
 		return d.Bytes
 	}
 	if d.Percentage == 0 {
 		return 0
 	}
 
-	dstat, err := disk.GetDiskStat(root)
-	if err != nil {
-		bklog.L.Warnf("failed to get disk size: %v", err)
+	if dstat.Total == 0 {
 		return defaultCap
 	}
+
 	avail := dstat.Total * d.Percentage / 100
 	rounded := (avail/(1<<30) + 1) * 1e9 // round up
 	return rounded
 }
+
+func (cfg *GCConfig) IsUnset() bool {
+	return cfg.GCMinStorage == DiskSpace{} && cfg.GCMaxStorage == DiskSpace{} && cfg.GCFreeStorage == DiskSpace{}
+}
diff --git a/cmd/buildkitd/config/gcpolicy_unix.go b/cmd/buildkitd/config/gcpolicy_unix.go
@@ -3,4 +3,10 @@
 
 package config
 
-var DiskSpacePercentage int64 = 10
+const (
+	DiskSpaceReservePercentage int64 = 10
+	DiskSpaceReserveBytes      int64 = 10 * 1e9 // 10GB
+	DiskSpaceFreePercentage    int64 = 20
+	DiskSpaceMaxPercentage     int64 = 80
+	DiskSpaceMaxBytes          int64 = 100 * 1e9 // 100GB
+)
diff --git a/cmd/buildkitd/config/gcpolicy_windows.go b/cmd/buildkitd/config/gcpolicy_windows.go
@@ -3,6 +3,12 @@
 
 package config
 
-// set as double that for Linux since
-// Windows images are generally larger.
-var DiskSpacePercentage int64 = 20
+const (
+	// Windows images are generally larger.
+	// set as double that for Linux since
+	DiskSpaceReservePercentage int64 = 20
+	DiskSpaceReserveBytes      int64 = 10 * 1e9 // 10GB
+	DiskSpaceFreePercentage    int64 = 20
+	DiskSpaceMaxPercentage     int64 = 80
+	DiskSpaceMaxBytes          int64 = 100 * 1e9 // 100GB
+)
diff --git a/cmd/buildkitd/main.go b/cmd/buildkitd/main.go
@@ -47,6 +47,7 @@ import (
 	"github.com/moby/buildkit/util/archutil"
 	"github.com/moby/buildkit/util/bklog"
 	"github.com/moby/buildkit/util/db/boltutil"
+	"github.com/moby/buildkit/util/disk"
 	"github.com/moby/buildkit/util/grpcerrors"
 	"github.com/moby/buildkit/util/profiler"
 	"github.com/moby/buildkit/util/resolver"
@@ -920,22 +921,23 @@ func getGCPolicy(cfg config.GCConfig, root string) []client.PruneInfo {
 	if cfg.GC != nil && !*cfg.GC {
 		return nil
 	}
+	dstat, _ := disk.GetDiskStat(root)
 	if len(cfg.GCPolicy) == 0 {
-		cfg.GCPolicy = config.DefaultGCPolicy(cfg.GCKeepStorage)
+		cfg.GCPolicy = config.DefaultGCPolicy(cfg, dstat)
 	}
 	out := make([]client.PruneInfo, 0, len(cfg.GCPolicy))
 	for _, rule := range cfg.GCPolicy {
 		//nolint:staticcheck
-		if rule.KeepBytes != (config.DiskSpace{}) {
-			rule.MaxStorage = rule.KeepBytes
+		if rule.MinStorage == (config.DiskSpace{}) && rule.KeepBytes != (config.DiskSpace{}) {
+			rule.MinStorage = rule.KeepBytes
 		}
 		out = append(out, client.PruneInfo{
 			Filter:       rule.Filters,
 			All:          rule.All,
 			KeepDuration: rule.KeepDuration.Duration,
-			MinStorage:   rule.MinStorage.AsBytes(root),
-			MaxStorage:   rule.MaxStorage.AsBytes(root),
-			Free:         rule.Free.AsBytes(root),
+			MinStorage:   rule.MinStorage.AsBytes(dstat),
+			MaxStorage:   rule.MaxStorage.AsBytes(dstat),
+			Free:         rule.Free.AsBytes(dstat),
 		})
 	}
 	return out
diff --git a/cmd/buildkitd/main_containerd_worker.go b/cmd/buildkitd/main_containerd_worker.go
@@ -13,6 +13,7 @@ import (
 	"github.com/containerd/containerd/defaults"
 	"github.com/moby/buildkit/cmd/buildkitd/config"
 	"github.com/moby/buildkit/util/bklog"
+	"github.com/moby/buildkit/util/disk"
 	"github.com/moby/buildkit/util/network/cniprovider"
 	"github.com/moby/buildkit/util/network/netproviders"
 	"github.com/moby/buildkit/worker"
@@ -149,15 +150,13 @@ func init() {
 			Usage: "Enable automatic garbage collection on worker",
 		})
 	}
-	flags = append(flags, cli.Int64Flag{
+	flags = append(flags, cli.StringFlag{
 		Name:  "containerd-worker-gc-keepstorage",
-		Usage: "Amount of storage GC keep locally (MB)",
-		Value: func() int64 {
-			keep := defaultConf.Workers.Containerd.GCKeepStorage.AsBytes(defaultConf.Root)
-			if keep == 0 {
-				keep = config.DetectDefaultGCCap().AsBytes(defaultConf.Root)
-			}
-			return keep / 1e6
+		Usage: "Amount of storage GC keep locally, format \"Reserve[,Free[,Maximum]]\" (MB)",
+		Value: func() string {
+			cfg := defaultConf.Workers.Containerd.GCConfig
+			dstat, _ := disk.GetDiskStat(defaultConf.Root)
+			return gcConfigToString(cfg, dstat)
 		}(),
 		Hidden: len(defaultConf.Workers.Containerd.GCPolicy) != 0,
 	})
@@ -229,7 +228,13 @@ func applyContainerdFlags(c *cli.Context, cfg *config.Config) error {
 	}
 
 	if c.GlobalIsSet("containerd-worker-gc-keepstorage") {
-		cfg.Workers.Containerd.GCKeepStorage = config.DiskSpace{Bytes: c.GlobalInt64("containerd-worker-gc-keepstorage") * 1e6}
+		gc, err := stringToGCConfig(c.GlobalString("containerd-worker-gc-keepstorage"))
+		if err != nil {
+			return err
+		}
+		cfg.Workers.Containerd.GCMinStorage = gc.GCMinStorage
+		cfg.Workers.Containerd.GCFreeStorage = gc.GCFreeStorage
+		cfg.Workers.Containerd.GCMaxStorage = gc.GCMaxStorage
 	}
 
 	if c.GlobalIsSet("containerd-worker-net") {
diff --git a/cmd/buildkitd/main_oci_worker.go b/cmd/buildkitd/main_oci_worker.go
@@ -33,6 +33,7 @@ import (
 	"github.com/moby/buildkit/executor/oci"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/util/bklog"
+	"github.com/moby/buildkit/util/disk"
 	"github.com/moby/buildkit/util/network/cniprovider"
 	"github.com/moby/buildkit/util/network/netproviders"
 	"github.com/moby/buildkit/util/resolver"
@@ -149,15 +150,13 @@ func init() {
 			Usage: "Enable automatic garbage collection on worker",
 		})
 	}
-	flags = append(flags, cli.Int64Flag{
+	flags = append(flags, cli.StringFlag{
 		Name:  "oci-worker-gc-keepstorage",
-		Usage: "Amount of storage GC keep locally (MB)",
-		Value: func() int64 {
-			keep := defaultConf.Workers.OCI.GCKeepStorage.AsBytes(defaultConf.Root)
-			if keep == 0 {
-				keep = config.DetectDefaultGCCap().AsBytes(defaultConf.Root)
-			}
-			return keep / 1e6
+		Usage: "Amount of storage GC keep locally, format \"Reserve[,Free[,Maximum]]\" (MB)",
+		Value: func() string {
+			cfg := defaultConf.Workers.OCI.GCConfig
+			dstat, _ := disk.GetDiskStat(defaultConf.Root)
+			return gcConfigToString(cfg, dstat)
 		}(),
 		Hidden: len(defaultConf.Workers.OCI.GCPolicy) != 0,
 	})
@@ -222,7 +221,13 @@ func applyOCIFlags(c *cli.Context, cfg *config.Config) error {
 	}
 
 	if c.GlobalIsSet("oci-worker-gc-keepstorage") {
-		cfg.Workers.OCI.GCKeepStorage = config.DiskSpace{Bytes: c.GlobalInt64("oci-worker-gc-keepstorage") * 1e6}
+		gc, err := stringToGCConfig(c.GlobalString("oci-worker-gc-keepstorage"))
+		if err != nil {
+			return err
+		}
+		cfg.Workers.OCI.GCMinStorage = gc.GCMinStorage
+		cfg.Workers.OCI.GCFreeStorage = gc.GCFreeStorage
+		cfg.Workers.OCI.GCMaxStorage = gc.GCMaxStorage
 	}
 
 	if c.GlobalIsSet("oci-worker-net") {
diff --git a/cmd/buildkitd/util.go b/cmd/buildkitd/util.go

Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,8 @@ message PruneRequest {`
`29`	`29`	`bool all = 2;`
`30`	`30`	`int64 keepDuration = 3;`
`31`	`31`
`32`		`- int64 minStorage = 5;`
`33`		`- int64 maxStorage = 4;`
	`32`	`+ int64 minStorage = 4;`
	`33`	`+ int64 maxStorage = 5;`
`34`	`34`	`int64 free = 6;`
`35`	`35`	`}`
`36`	`36`
Original file line number	Diff line number	Diff line change
`@@ -1058,14 +1058,12 @@ func (cm *cacheManager) pruneOnce(ctx context.Context, ch chan client.UsageInfo,`
`1058`	`1058`	`}`
`1059`	`1059`	`}`
`1060`	`1060`
`1061`		`- dstat, err := disk.GetDiskStat(cm.root)`
`1062`		`- if err != nil {`
`1063`		`- if opt.Free != 0 {`
`1064`		`- // if we are pruning based on disk space, failing to get info on it`
`1065`		`- // is fatal`
	`1061`	`+ var dstat disk.DiskStat`
	`1062`	`+ if opt.Free != 0 {`
	`1063`	`+ dstat, err = disk.GetDiskStat(cm.root)`
	`1064`	`+ if err != nil {`
`1066`	`1065`	`return err`
`1067`	`1066`	`}`
`1068`		`- bklog.L.Warnf("failed to get disk size: %v", err)`
`1069`	`1067`	`}`
`1070`	`1068`
`1071`	`1069`	`return cm.prune(ctx, ch, pruneOpt{`
Original file line number	Diff line number	Diff line change
`@@ -89,16 +89,16 @@ func printWorkersVerbose(tw tabwriter.Writer, winfo []client.WorkerInfo) {`
`89`	`89`	`fmt.Fprintf(tw, "\tFilters:\t%s\n", strings.Join(rule.Filter, " "))`
`90`	`90`	`}`
`91`	`91`	`if rule.KeepDuration > 0 {`
`92`		`- fmt.Fprintf(tw, "\tKeep Duration:\t%v\n", rule.KeepDuration.String())`
`93`		`- }`
`94`		`- if rule.MaxStorage > 0 {`
`95`		`- fmt.Fprintf(tw, "\tKeep Bytes:\t%g\n", units.Bytes(rule.MaxStorage))`
	`92`	`+ fmt.Fprintf(tw, "\tKeep duration:\t%v\n", rule.KeepDuration.String())`
`96`	`93`	`}`
`97`	`94`	`if rule.MinStorage > 0 {`
`98`		`- fmt.Fprintf(tw, "\tKeep Bytes (min):\t%g\n", units.Bytes(rule.MinStorage))`
	`95`	`+ fmt.Fprintf(tw, "\tReserved storage:\t%g\n", units.Bytes(rule.MinStorage))`
`99`	`96`	`}`
`100`	`97`	`if rule.Free > 0 {`
`101`		`- fmt.Fprintf(tw, "\tFree Bytes:\t%g\n", units.Bytes(rule.MinStorage))`
	`98`	`+ fmt.Fprintf(tw, "\tMaintain free storage:\t%g\n", units.Bytes(rule.Free))`
	`99`	`+ }`
	`100`	`+ if rule.MaxStorage > 0 {`
	`101`	`+ fmt.Fprintf(tw, "\tMaximum storage:\t%g\n", units.Bytes(rule.MaxStorage))`
`102`	`102`	`}`
`103`	`103`	`}`
`104`	`104`	`fmt.Fprintf(tw, "\n")`
Original file line number	Diff line number	Diff line change
`@@ -75,8 +75,12 @@ type OTELConfig struct {`
`75`	`75`	`}`
`76`	`76`
`77`	`77`	`type GCConfig struct {`
`78`		- GC *bool `toml:"gc"`
	`78`	+ GC *bool `toml:"gc"`
	`79`	`+ // Deprecated: use GCMinStorage instead`
`79`	`80`	GCKeepStorage DiskSpace `toml:"gckeepstorage"`
	`81`	+ GCMaxStorage DiskSpace `toml:"gcmaxstorage"`
	`82`	+ GCMinStorage DiskSpace `toml:"gcminstorage"`
	`83`	+ GCFreeStorage DiskSpace `toml:"gcfreestorage"`
`80`	`84`	GCPolicy []GCPolicy `toml:"gcpolicy"`
`81`	`85`	`}`
`82`	`86`