From 275bbcd3001452bd4cf518f0e517926af0a2daa3 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 19 Dec 2024 18:29:31 +0100 Subject: [PATCH] builder: don't fall back to defaultKeepStorage when set to zero commit b08ff8120466cb646637767cccb70ccce7a06b4b updated this code to only produce an error if an invalid value was set by the user, and to avoid errors on empty values. However, the intent of this code was to allow `0` as a valid value for cases where gc is to be handled through other properties / filters. This patch only sets the default if no value was set by the user, but doesn't set the default if a value is set by the user, but zero. Signed-off-by: Sebastiaan van Stijn --- builder/builder-next/controller.go | 35 +++++++++++++++--------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/builder/builder-next/controller.go b/builder/builder-next/controller.go index c05b882ba2..ab29e719b3 100644 --- a/builder/builder-next/controller.go +++ b/builder/builder-next/controller.go @@ -429,38 +429,37 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt func getGCPolicy(conf config.BuilderConfig, root string) ([]client.PruneInfo, error) { var gcPolicy []client.PruneInfo if conf.GC.Enabled { - var ( - defaultKeepStorage int64 - err error - ) - - if conf.GC.DefaultKeepStorage != "" { - defaultKeepStorage, err = units.RAMInBytes(conf.GC.DefaultKeepStorage) - if err != nil { - return nil, errors.Wrapf(err, "failed to parse defaultKeepStorage") - } - } - if conf.GC.Policy == nil { + var defaultKeepStorage int64 + if conf.GC.DefaultKeepStorage != "" { + b, err := units.RAMInBytes(conf.GC.DefaultKeepStorage) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse defaultKeepStorage") + } + defaultKeepStorage = b + } gcPolicy = mobyworker.DefaultGCPolicy(root, defaultKeepStorage) } else { gcPolicy = make([]client.PruneInfo, len(conf.GC.Policy)) for i, p := range conf.GC.Policy { - var b int64 + var keepStorage int64 if p.KeepStorage != "" { - b, err = units.RAMInBytes(p.KeepStorage) + b, err := units.RAMInBytes(p.KeepStorage) if err != nil { return nil, errors.Wrapf(err, "failed to parse keepStorage") } - } - if b == 0 { - b = defaultKeepStorage + // don't set a default here, zero is a valid value when + // specified by the user, as the gc-policy may be determined + // through other filters; + // https://github.com/moby/moby/pull/49062#issuecomment-2554981829 + keepStorage = b } // FIXME(thaJeztah): wire up new options https://github.com/moby/moby/issues/48639 + var err error gcPolicy[i], err = toBuildkitPruneInfo(types.BuildCachePruneOptions{ All: p.All, - KeepStorage: b, + KeepStorage: keepStorage, Filters: filters.Args(p.Filter), }) if err != nil {