Skip to content

Commit 9964dea

Browse files
kubernetes-prow[bot]kubernetes-prow[bot]
authored
Merge pull request #18764 from hakman/file-hash-cache
assets: cache file hashes instead of nodeup assets
2 parents f2cacd4 + b0131cd commit 9964dea

6 files changed

Lines changed: 240 additions & 16 deletions

File tree

‎pkg/assets/builder.go‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ import (
3838
"k8s.io/kops/util/pkg/vfs"
3939
)
4040

41+
// downloadedFileHashes caches hashes read from checksum files, keyed by resolved URL so
42+
// canonical and mirrored assets do not share entries. Commands can create multiple
43+
// AssetBuilders, but each builder must still remap and register its own assets.
44+
var downloadedFileHashes sync.Map // resolved URL -> *hashing.Hash
45+
4146
// ImageDigestResolver looks up the manifest digest for an image, returning it in the form
4247
// "sha256:...".
4348
type ImageDigestResolver func(image string) (string, error)
@@ -386,6 +391,11 @@ func (a *AssetBuilder) findHash(file *FileAsset) (*hashing.Hash, error) {
386391
return knownHash, nil
387392
}
388393

394+
if cachedHash, found := downloadedFileHashes.Load(u.String()); found {
395+
klog.V(8).Infof("using cached hash for %q", u)
396+
return cachedHash.(*hashing.Hash), nil
397+
}
398+
389399
klog.V(2).Infof("asset %q is not well-known, downloading hash", file.CanonicalURL)
390400

391401
// We now prefer sha256 hashes
@@ -418,7 +428,14 @@ func (a *AssetBuilder) findHash(file *FileAsset) (*hashing.Hash, error) {
418428
klog.Infof("Hash file was empty %q", hashURL)
419429
continue
420430
}
421-
return hashing.FromString(fields[0])
431+
hash, err := hashing.FromString(fields[0])
432+
if err != nil {
433+
return nil, err
434+
}
435+
436+
downloadedFileHashes.Store(u.String(), hash)
437+
438+
return hash, nil
422439
}
423440
if ext == ".sha256" {
424441
klog.V(2).Infof("Unable to read new sha256 hash file (is this an older/unsupported kubernetes release?)")

‎pkg/assets/builder_test.go‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,22 @@ package assets
1818

1919
import (
2020
"fmt"
21+
"net/http"
22+
"net/http/httptest"
2123
"net/url"
2224
"os"
25+
"path"
2326
"path/filepath"
2427
"strings"
2528
"sync"
29+
"sync/atomic"
2630
"testing"
2731

2832
"k8s.io/kops/pkg/apis/kops"
2933
"k8s.io/kops/pkg/featureflag"
3034
"k8s.io/kops/pkg/testutils/golden"
3135
"k8s.io/kops/util/pkg/hashing"
36+
"k8s.io/kops/util/pkg/vfs"
3237
)
3338

3439
func buildAssetBuilder(t *testing.T) *AssetBuilder {
@@ -339,3 +344,131 @@ func TestAssetBuilderConcurrentCollection(t *testing.T) {
339344
}
340345
}
341346
}
347+
348+
func resetDownloadedFileHashes(t *testing.T) {
349+
t.Helper()
350+
351+
downloadedFileHashes.Clear()
352+
t.Cleanup(downloadedFileHashes.Clear)
353+
}
354+
355+
func hashHandler(assetPath string, hash string, requests *atomic.Int64) http.HandlerFunc {
356+
return func(w http.ResponseWriter, r *http.Request) {
357+
if r.URL.Path != assetPath+".sha256" {
358+
// The VFS retries 404 and 5xx responses.
359+
http.Error(w, "not found", http.StatusForbidden)
360+
return
361+
}
362+
requests.Add(1)
363+
fmt.Fprintf(w, "%s %s\n", hash, path.Base(assetPath))
364+
}
365+
}
366+
367+
func newHashServer(t *testing.T, assetPath string, hash string, requests *atomic.Int64) *httptest.Server {
368+
t.Helper()
369+
370+
server := httptest.NewServer(hashHandler(assetPath, hash, requests))
371+
t.Cleanup(server.Close)
372+
373+
return server
374+
}
375+
376+
func TestFindHashCachesDownloadedHashesByResolvedURL(t *testing.T) {
377+
resetDownloadedFileHashes(t)
378+
379+
const assetPath = "/binaries/example/linux/amd64/example"
380+
const canonicalHash = "2222222222222222222222222222222222222222222222222222222222222222"
381+
const mirroredHash = "3333333333333333333333333333333333333333333333333333333333333333"
382+
383+
var canonicalRequests atomic.Int64
384+
canonicalServer := newHashServer(t, assetPath, canonicalHash, &canonicalRequests)
385+
386+
var mirroredRequests atomic.Int64
387+
mirroredServer := newHashServer(t, assetPath, mirroredHash, &mirroredRequests)
388+
389+
assetURL, err := url.Parse(canonicalServer.URL + assetPath)
390+
if err != nil {
391+
t.Fatalf("error parsing asset url: %v", err)
392+
}
393+
394+
vfsContext := vfs.NewVFSContext()
395+
396+
// Each builder registers the asset, but only the first downloads its checksum.
397+
for i := 0; i < 3; i++ {
398+
builder := NewAssetBuilder(vfsContext, &kops.AssetsSpec{}, false)
399+
400+
asset, err := builder.RemapFile(assetURL, nil)
401+
if err != nil {
402+
t.Fatalf("error remapping file with builder %d: %v", i, err)
403+
}
404+
if actual := asset.SHAValue.Hex(); actual != canonicalHash {
405+
t.Errorf("unexpected hash from builder %d: actual %q, expected %q", i, actual, canonicalHash)
406+
}
407+
if actual := len(builder.FileAssets()); actual != 1 {
408+
t.Errorf("expected builder %d to register 1 file asset, got %d", i, actual)
409+
}
410+
}
411+
412+
// The mirror must not reuse the canonical URL's cached hash.
413+
fileRepository := mirroredServer.URL
414+
mirroredBuilder := NewAssetBuilder(vfsContext, &kops.AssetsSpec{FileRepository: &fileRepository}, false)
415+
mirroredAsset, err := mirroredBuilder.RemapFile(assetURL, nil)
416+
if err != nil {
417+
t.Fatalf("error remapping mirrored file: %v", err)
418+
}
419+
if actual := mirroredAsset.SHAValue.Hex(); actual != mirroredHash {
420+
t.Errorf("unexpected mirrored hash: actual %q, expected %q", actual, mirroredHash)
421+
}
422+
423+
if actual := canonicalRequests.Load(); actual != 1 {
424+
t.Errorf("expected 1 canonical checksum request, got %d", actual)
425+
}
426+
if actual := mirroredRequests.Load(); actual != 1 {
427+
t.Errorf("expected 1 mirrored checksum request, got %d", actual)
428+
}
429+
}
430+
431+
func TestFindHashDoesNotCacheFailures(t *testing.T) {
432+
resetDownloadedFileHashes(t)
433+
434+
const assetPath = "/binaries/example/linux/amd64/example"
435+
const hash = "4444444444444444444444444444444444444444444444444444444444444444"
436+
437+
var published atomic.Bool
438+
var requests atomic.Int64
439+
handler := hashHandler(assetPath, hash, &requests)
440+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
441+
if !published.Load() {
442+
// The VFS retries 404 and 5xx responses.
443+
http.Error(w, "not found", http.StatusForbidden)
444+
return
445+
}
446+
handler.ServeHTTP(w, r)
447+
}))
448+
t.Cleanup(server.Close)
449+
450+
assetURL, err := url.Parse(server.URL + assetPath)
451+
if err != nil {
452+
t.Fatalf("error parsing asset url: %v", err)
453+
}
454+
455+
vfsContext := vfs.NewVFSContext()
456+
builder := NewAssetBuilder(vfsContext, &kops.AssetsSpec{}, false)
457+
458+
if _, err := builder.RemapFile(assetURL, nil); err == nil {
459+
t.Fatal("expected an error while the checksum file is unavailable")
460+
}
461+
462+
published.Store(true)
463+
464+
asset, err := builder.RemapFile(assetURL, nil)
465+
if err != nil {
466+
t.Fatalf("error remapping file after the checksum file was published: %v", err)
467+
}
468+
if actual := asset.SHAValue.Hex(); actual != hash {
469+
t.Errorf("unexpected hash: actual %q, expected %q", actual, hash)
470+
}
471+
if actual := requests.Load(); actual != 1 {
472+
t.Errorf("expected 1 successful checksum request, got %d", actual)
473+
}
474+
}

‎pkg/nodemodel/wellknownassets/kopsassets.go‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,6 @@ const (
3434

3535
var kopsBaseURL *url.URL
3636

37-
// nodeUpAsset caches the nodeup binary download url/hash
38-
var nodeUpAsset map[architectures.Architecture]*assets.MirroredAsset
39-
4037
// BaseURL returns the base url for the distribution of kops - in particular for nodeup & docker images
4138
func BaseURL() (*url.URL, error) {
4239
// returning cached value
@@ -82,17 +79,9 @@ func copyBaseURL(base *url.URL) (*url.URL, error) {
8279
return u, nil
8380
}
8481

85-
// NodeUpAsset returns the asset for where nodeup should be downloaded
82+
// NodeUpAsset returns nodeup after registering it with assetsBuilder. The result is not cached
83+
// because its locations depend on the builder's repository mapping. Hashes are cached separately.
8684
func NodeUpAsset(assetsBuilder *assets.AssetBuilder, arch architectures.Architecture) (*assets.MirroredAsset, error) {
87-
if nodeUpAsset == nil {
88-
nodeUpAsset = make(map[architectures.Architecture]*assets.MirroredAsset)
89-
}
90-
if nodeUpAsset[arch] != nil {
91-
// Avoid repeated logging
92-
klog.V(8).Infof("Using cached nodeup location for %s: %v", arch, nodeUpAsset[arch].Locations)
93-
return nodeUpAsset[arch], nil
94-
}
95-
9685
asset, err := KopsFileURL(fmt.Sprintf("linux/%s/nodeup", arch), assetsBuilder)
9786
if err != nil {
9887
return nil, err
@@ -104,10 +93,9 @@ func NodeUpAsset(assetsBuilder *assets.AssetBuilder, arch architectures.Architec
10493
return nil, err
10594
}
10695
}
107-
nodeUpAsset[arch] = assets.BuildMirroredAsset(asset)
10896
klog.V(8).Infof("Using default nodeup location for %s: %q", arch, asset.DownloadURL.String())
10997

110-
return nodeUpAsset[arch], nil
98+
return assets.BuildMirroredAsset(asset), nil
11199
}
112100

113101
// KopsFileURL returns the base url for the distribution of kops - in particular for nodeup & docker images

‎pkg/nodemodel/wellknownassets/kopsassets_test.go‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@ package wellknownassets
1818

1919
import (
2020
"fmt"
21+
"net/http"
22+
"net/http/httptest"
2123
"net/url"
2224
"reflect"
25+
"sync/atomic"
2326
"testing"
2427

2528
"k8s.io/kops"
29+
kopsapi "k8s.io/kops/pkg/apis/kops"
2630
"k8s.io/kops/pkg/assets"
31+
"k8s.io/kops/util/pkg/architectures"
2732
"k8s.io/kops/util/pkg/hashing"
33+
"k8s.io/kops/util/pkg/vfs"
2834
)
2935

3036
func TestBaseURL_OverridesVersionFromKopsBaseURL(t *testing.T) {
@@ -130,3 +136,71 @@ func Test_BuildMirroredAsset(t *testing.T) {
130136
})
131137
}
132138
}
139+
140+
func TestNodeUpAssetRegistersWithEveryAssetBuilder(t *testing.T) {
141+
hashes := map[architectures.Architecture]string{
142+
architectures.ArchitectureAmd64: "5555555555555555555555555555555555555555555555555555555555555555",
143+
architectures.ArchitectureArm64: "6666666666666666666666666666666666666666666666666666666666666666",
144+
}
145+
146+
var requests atomic.Int64
147+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
148+
for arch, hash := range hashes {
149+
if r.URL.Path == fmt.Sprintf("/kops/%s/linux/%s/nodeup.sha256", kops.Version, arch) {
150+
requests.Add(1)
151+
fmt.Fprintf(w, "%s nodeup\n", hash)
152+
return
153+
}
154+
}
155+
// The VFS retries 404 and 5xx responses.
156+
http.Error(w, "not found", http.StatusForbidden)
157+
}))
158+
t.Cleanup(server.Close)
159+
160+
kopsBaseURL = nil
161+
t.Cleanup(func() {
162+
kopsBaseURL = nil
163+
})
164+
// Keep kops.Version unchanged when BaseURL parses KOPS_BASE_URL.
165+
t.Setenv("KOPS_BASE_URL", fmt.Sprintf("%s/kops/%s", server.URL, kops.Version))
166+
167+
vfsContext := vfs.NewVFSContext()
168+
169+
// Both update and get-assets builders must register nodeup.
170+
for _, getAssets := range []bool{false, true} {
171+
t.Run(fmt.Sprintf("getAssets=%t", getAssets), func(t *testing.T) {
172+
assetBuilder := assets.NewAssetBuilder(vfsContext, &kopsapi.AssetsSpec{}, getAssets)
173+
174+
for _, arch := range []architectures.Architecture{architectures.ArchitectureAmd64, architectures.ArchitectureArm64} {
175+
asset, err := NodeUpAsset(assetBuilder, arch)
176+
if err != nil {
177+
t.Fatalf("NodeUpAsset(%s) error: %v", arch, err)
178+
}
179+
expectedLocation := fmt.Sprintf("%s/kops/%s/linux/%s/nodeup", server.URL, kops.Version, arch)
180+
if !reflect.DeepEqual(asset.Locations, []string{expectedLocation}) {
181+
t.Errorf("unexpected nodeup locations for %s: %v", arch, asset.Locations)
182+
}
183+
if asset.Hash.Hex() != hashes[arch] {
184+
t.Errorf("unexpected nodeup hash for %s: actual %q, expected %q", arch, asset.Hash.Hex(), hashes[arch])
185+
}
186+
}
187+
188+
var registered []string
189+
for _, fileAsset := range assetBuilder.FileAssets() {
190+
registered = append(registered, fileAsset.CanonicalURL.String())
191+
}
192+
expected := []string{
193+
fmt.Sprintf("%s/kops/%s/linux/amd64/nodeup", server.URL, kops.Version),
194+
fmt.Sprintf("%s/kops/%s/linux/arm64/nodeup", server.URL, kops.Version),
195+
}
196+
if !reflect.DeepEqual(registered, expected) {
197+
t.Errorf("unexpected registered file assets:\nActual: %v\nExpect: %v", registered, expected)
198+
}
199+
})
200+
}
201+
202+
// Each architecture's checksum is downloaded once across both builders.
203+
if actual := requests.Load(); actual != int64(len(hashes)) {
204+
t.Errorf("expected %d checksum requests, got %d", len(hashes), actual)
205+
}
206+
}

‎tests/integration/update_cluster/containerd/assets.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ files:
55
- canonical: https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.31.7/linux/arm64/ecr-credential-provider-linux-arm64
66
download: https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.31.7/linux/arm64/ecr-credential-provider-linux-arm64
77
sha: 1980e3a038cb16da48a137743b31fb81de6c0b59fa06c206c2bc20ce0a52f849
8+
- canonical: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/amd64/nodeup
9+
download: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/amd64/nodeup
10+
sha: c86e072f622b91546b7b3f3cb1a0f8a131e48b966ad018a0ac1520ceedf37725
11+
- canonical: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/arm64/nodeup
12+
download: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/arm64/nodeup
13+
sha: 64a9a9510538a449e85d05e13e3cd98b80377d68a673447c26821d40f00f0075
814
- canonical: https://dl.k8s.io/release/v1.32.0/bin/linux/amd64/kubectl
915
download: https://dl.k8s.io/release/v1.32.0/bin/linux/amd64/kubectl
1016
sha: 646d58f6d98ee670a71d9cdffbf6625aeea2849d567f214bc43a35f8ccb7bf70

‎tests/integration/update_cluster/privatedns1/assets.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ files:
55
- canonical: https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.31.7/linux/arm64/ecr-credential-provider-linux-arm64
66
download: https://artifacts.k8s.io/binaries/cloud-provider-aws/v1.31.7/linux/arm64/ecr-credential-provider-linux-arm64
77
sha: 1980e3a038cb16da48a137743b31fb81de6c0b59fa06c206c2bc20ce0a52f849
8+
- canonical: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/amd64/nodeup
9+
download: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/amd64/nodeup
10+
sha: c86e072f622b91546b7b3f3cb1a0f8a131e48b966ad018a0ac1520ceedf37725
11+
- canonical: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/arm64/nodeup
12+
download: https://artifacts.k8s.io/binaries/kops/1.34.0-beta.1/linux/arm64/nodeup
13+
sha: 64a9a9510538a449e85d05e13e3cd98b80377d68a673447c26821d40f00f0075
814
- canonical: https://dl.k8s.io/release/v1.34.0/bin/linux/amd64/kubectl
915
download: https://dl.k8s.io/release/v1.34.0/bin/linux/amd64/kubectl
1016
sha: cfda68cba5848bc3b6c6135ae2f20ba2c78de20059f68789c090166d6abc3e2c

0 commit comments

Comments
 (0)