Skip to content

Commit

Permalink
Make etag checks optional
Browse files Browse the repository at this point in the history
This isn't perfect but I think it moves us closer in the right
direction. Previously, we had a process-wide global etag cache that
would prevent sending multipl HEAD requests to check the etag value of
any URLs we fetched (keys and APKINDEX, mostly).

This is great for a local tool or even the terraform provider, but it's
awful as a library used by a service because the lifetime of the process
is liable to outlive the validity of the cach.

This hoists that global etag caching mechanism out into a little
apk.Cache struct, which is where I'm hoping to put even more things that
I'm ashamed of going forward.

Using the same apk.NewCache(true) instance across multiple builds will
skip HEAD requests to the same URL and just assume that the tag hasn't
changed. This was the previous default behavior.

Using the same apk.NewCache(false) instance across multiple builds (the
default) will send a HEAD request prior to every key or index fetch in
order to see if they've changed.

We still have some global caching going on for actually parsing the
results, which keys off of etag values, so we shouldn't have any
progressions re: the recent CPU savings changes I've made.

Another thing this introduces (because we dropped the more powerful
etag-based caching) is a some singleflight instances for these outgoing
requests. We don't want to cache the results indefinitely, but we do
want to avoid sending redundant requests in every case, so
apk.NewCache(false) will still coalesce multiple concurrent requests for
the same URL, so there's still benefit to using this cache object even
with the etag caching disabled.

One thing that's missing here is a distinction between ~request-scoped
caching if e.g. I want to use an etag cache for any outgoing URLs for a
single build, but I also want to use a global cache for coalescing
requests. I'll probably split the singleflight stuff off from this
struct at some point, but I want to measure first to see if that's really
necessary.

Signed-off-by: Jon Johnson <[email protected]>
  • Loading branch information
jonjohnsonjr committed Oct 2, 2024
1 parent b485c09 commit 5b7ad9c
Show file tree
Hide file tree
Showing 14 changed files with 360 additions and 277 deletions.
3 changes: 2 additions & 1 deletion internal/cli/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (

"github.com/chainguard-dev/clog"

"chainguard.dev/apko/pkg/apk/apk"
"chainguard.dev/apko/pkg/build"
"chainguard.dev/apko/pkg/build/oci"
"chainguard.dev/apko/pkg/build/types"
Expand Down Expand Up @@ -108,7 +109,7 @@ Along the image, apko will generate SBOMs (software bill of materials) describin
build.WithTags(args[1]),
build.WithVCS(withVCS),
build.WithAnnotations(annotations),
build.WithCacheDir(cacheDir, offline),
build.WithCache(cacheDir, offline, apk.NewCache(true)),
build.WithLockFile(lockfile),
build.WithTempDir(tmp),
build.WithIncludePaths(includePaths),
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/dot.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ apko dot --web -S example.yaml
build.WithExtraKeys(extraKeys),
build.WithExtraBuildRepos(extraBuildRepos),
build.WithExtraRuntimeRepos(extraRuntimeRepos),
build.WithCacheDir(cacheDir, offline),
build.WithCache(cacheDir, offline, apk.NewCache(true)),
)
},
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (

"github.com/chainguard-dev/clog"

"chainguard.dev/apko/pkg/apk/apk"
"chainguard.dev/apko/pkg/build"
"chainguard.dev/apko/pkg/build/oci"
"chainguard.dev/apko/pkg/build/types"
Expand Down Expand Up @@ -116,7 +117,7 @@ in a keychain.`,
build.WithTags(args[1:]...),
build.WithVCS(withVCS),
build.WithAnnotations(annotations),
build.WithCacheDir(cacheDir, offline),
build.WithCache(cacheDir, offline, apk.NewCache(true)),
build.WithLockFile(lockfile),
build.WithTempDir(tmp),
build.WithIgnoreSignatures(ignoreSignatures),
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/show-config.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/spf13/cobra"
"gopkg.in/yaml.v3"

"chainguard.dev/apko/pkg/apk/apk"
apkfs "chainguard.dev/apko/pkg/apk/fs"
"chainguard.dev/apko/pkg/build"
)
Expand All @@ -49,7 +50,7 @@ The derived configuration is rendered in YAML.
build.WithExtraKeys(extraKeys),
build.WithExtraBuildRepos(extraBuildRepos),
build.WithExtraRuntimeRepos(extraRuntimeRepos),
build.WithCacheDir(cacheDir, offline),
build.WithCache(cacheDir, offline, apk.NewCache(true)),
)
},
}
Expand Down
3 changes: 2 additions & 1 deletion internal/cli/show-packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/chainguard-dev/clog"

"chainguard.dev/apko/pkg/apk/apk"
"chainguard.dev/apko/pkg/build"
"chainguard.dev/apko/pkg/build/types"
)
Expand Down Expand Up @@ -108,7 +109,7 @@ packagelock and packagelock-source are particularly useful for inserting back in
build.WithExtraKeys(extraKeys),
build.WithExtraBuildRepos(extraBuildRepos),
build.WithExtraRuntimeRepos(extraRuntimeRepos),
build.WithCacheDir(cacheDir, offline),
build.WithCache(cacheDir, offline, apk.NewCache(true)),
)
},
}
Expand Down
Loading

0 comments on commit 5b7ad9c

Please sign in to comment.