Merge pull request #410 from thaJeztah/19.03_backport_fix_buildkit_prunegc_filter_config

[19.03 backport] daemon/config: fix filter type in BuildKit GC config
Upstream-commit: e2e3abec71f30e533a01e8f1f9669e55b1361fc1
Component: engine

Author: andrewhsu

components/engine/api/types/filters/parse.go

@@ -36,6 +36,15 @@ func NewArgs(initialArgs ...KeyValuePair) Args {
 	return args
 }
 
+// Keys returns all the keys in list of Args
+func (args Args) Keys() []string {
+	keys := make([]string, 0, len(args.fields))
+	for k := range args.fields {
+		keys = append(keys, k)
+	}
+	return keys
+}
+
 // MarshalJSON returns a JSON byte representation of the Args
 func (args Args) MarshalJSON() ([]byte, error) {
 	if len(args.fields) == 0 {

components/engine/builder/builder-next/controller.go

@@ -8,6 +8,7 @@ import (
 	"github.com/containerd/containerd/content/local"
 	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/types"
+	"github.com/docker/docker/api/types/filters"
 	"github.com/docker/docker/builder/builder-next/adapters/containerimage"
 	"github.com/docker/docker/builder/builder-next/adapters/localinlinecache"
 	"github.com/docker/docker/builder/builder-next/adapters/snapshot"
@@ -232,7 +233,7 @@ func getGCPolicy(conf config.BuilderConfig, root string) ([]client.PruneInfo, er
 				gcPolicy[i], err = toBuildkitPruneInfo(types.BuildCachePruneOptions{
 					All:         p.All,
 					KeepStorage: b,
-					Filters:     p.Filter,
+					Filters:     filters.Args(p.Filter),
 				})
 				if err != nil {
 					return nil, err

components/engine/daemon/config/builder.go

@@ -1,12 +1,57 @@
 package config
 
-import "github.com/docker/docker/api/types/filters"
+import (
+	"encoding/json"
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/docker/docker/api/types/filters"
+)
 
 // BuilderGCRule represents a GC rule for buildkit cache
 type BuilderGCRule struct {
-	All         bool         `json:",omitempty"`
-	Filter      filters.Args `json:",omitempty"`
-	KeepStorage string       `json:",omitempty"`
+	All         bool            `json:",omitempty"`
+	Filter      BuilderGCFilter `json:",omitempty"`
+	KeepStorage string          `json:",omitempty"`
+}
+
+// BuilderGCFilter contains garbage-collection filter rules for a BuildKit builder
+type BuilderGCFilter filters.Args
+
+// MarshalJSON returns a JSON byte representation of the BuilderGCFilter
+func (x *BuilderGCFilter) MarshalJSON() ([]byte, error) {
+	f := filters.Args(*x)
+	keys := f.Keys()
+	sort.Strings(keys)
+	arr := make([]string, 0, len(keys))
+	for _, k := range keys {
+		values := f.Get(k)
+		for _, v := range values {
+			arr = append(arr, fmt.Sprintf("%s=%s", k, v))
+		}
+	}
+	return json.Marshal(arr)
+}
+
+// UnmarshalJSON fills the BuilderGCFilter values structure from JSON input
+func (x *BuilderGCFilter) UnmarshalJSON(data []byte) error {
+	var arr []string
+	f := filters.NewArgs()
+	if err := json.Unmarshal(data, &arr); err != nil {
+		// backwards compat for deprecated buggy form
+		err := json.Unmarshal(data, &f)
+		*x = BuilderGCFilter(f)
+		return err
+	}
+	for _, s := range arr {
+		fields := strings.SplitN(s, "=", 2)
+		name := strings.ToLower(strings.TrimSpace(fields[0]))
+		value := strings.TrimSpace(fields[1])
+		f.Add(name, value)
+	}
+	*x = BuilderGCFilter(f)
+	return nil
 }
 
 // BuilderGCConfig contains GC config for a buildkit builder

components/engine/daemon/config/builder_test.go

@@ -0,0 +1,44 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/docker/docker/api/types/filters"
+	"github.com/google/go-cmp/cmp"
+	"gotest.tools/assert"
+	"gotest.tools/fs"
+)
+
+func TestBuilderGC(t *testing.T) {
+	tempFile := fs.NewFile(t, "config", fs.WithContent(`{
+  "builder": {
+    "gc": {
+      "enabled": true,
+      "policy": [
+        {"keepStorage": "10GB", "filter": ["unused-for=2200h"]},
+        {"keepStorage": "50GB", "filter": {"unused-for": {"3300h": true}}},
+        {"keepStorage": "100GB", "all": true}
+      ]
+    }
+  }
+}`))
+	defer tempFile.Remove()
+	configFile := tempFile.Path()
+
+	cfg, err := MergeDaemonConfigurations(&Config{}, nil, configFile)
+	assert.NilError(t, err)
+	assert.Assert(t, cfg.Builder.GC.Enabled)
+	f1 := filters.NewArgs()
+	f1.Add("unused-for", "2200h")
+	f2 := filters.NewArgs()
+	f2.Add("unused-for", "3300h")
+	expectedPolicy := []BuilderGCRule{
+		{KeepStorage: "10GB", Filter: BuilderGCFilter(f1)},
+		{KeepStorage: "50GB", Filter: BuilderGCFilter(f2)}, /* parsed from deprecated form */
+		{KeepStorage: "100GB", All: true},
+	}
+	assert.DeepEqual(t, cfg.Builder.GC.Policy, expectedPolicy, cmp.AllowUnexported(BuilderGCFilter{}))
+	// double check to please the skeptics
+	assert.Assert(t, filters.Args(cfg.Builder.GC.Policy[0].Filter).UniqueExactMatch("unused-for", "2200h"))
+	assert.Assert(t, filters.Args(cfg.Builder.GC.Policy[1].Filter).UniqueExactMatch("unused-for", "3300h"))
+}