Merge pull request #841 from shomron/issue-598-helm-caching

Helm client caches chart configuration to avoid multiple back-to-back downloads

pkg/helm/caching_client.go

+// Copyright 2018 The ksonnet authors
+//
+//
+//    Licensed under the Apache License, Version 2.0 (the "License");
+//    you may not use this file except in compliance with the License.
+//    You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//    Unless required by applicable law or agreed to in writing, software
+//    distributed under the License is distributed on an "AS IS" BASIS,
+//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//    See the License for the specific language governing permissions and
+//    limitations under the License.
+
+package helm
+
+type nameVersion struct {
+	name    string
+	version string
+}
+
+// CachingClient is a caching wrapper over a Helm HTTP client
+type CachingClient struct {
+	RepositoryClient
+	cache map[nameVersion]*RepositoryChart // not thread-safe
+}
+
+func NewCachingClient(c RepositoryClient) *CachingClient {
+	return &CachingClient{
+		RepositoryClient: c,
+		cache:            make(map[nameVersion]*RepositoryChart),
+	}
+}
+
+// Chart returns a Chart with a given name and version. If the version is blank, it returns
+// the latest version.
+func (c *CachingClient) Chart(name, version string) (*RepositoryChart, error) {
+	tuple := nameVersion{name, version}
+	if chart, ok := c.cache[tuple]; ok {
+		return chart, nil
+	}
+
+	chart, err := c.RepositoryClient.Chart(name, version)
+	if err != nil {
+		return nil, err
+	}
+
+	if c.cache == nil {
+		c.cache = make(map[nameVersion]*RepositoryChart)
+	}
+	c.cache[tuple] = chart
+
+	return chart, nil
+}

pkg/helm/caching_client_test.go

+// Copyright 2018 The ksonnet authors
+//
+//
+//    Licensed under the Apache License, Version 2.0 (the "License");
+//    you may not use this file except in compliance with the License.
+//    You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//    Unless required by applicable law or agreed to in writing, software
+//    distributed under the License is distributed on an "AS IS" BASIS,
+//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//    See the License for the specific language governing permissions and
+//    limitations under the License.
+
+package helm
+
+import (
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type fakeRepositoryClient struct {
+	entries    *Repository
+	entriesErr error
+
+	chart    *RepositoryChart
+	chartErr error
+
+	fetchReader io.ReadCloser
+	fetchErr    error
+}
+
+func (hrc *fakeRepositoryClient) Repository() (*Repository, error) {
+	return hrc.entries, hrc.entriesErr
+}
+
+func (hrc *fakeRepositoryClient) Chart(string, string) (*RepositoryChart, error) {
+	if hrc.chart == nil {
+		return nil, hrc.chartErr
+	}
+	chartCopy := *hrc.chart
+	return &chartCopy, hrc.chartErr
+}
+
+func (hrc *fakeRepositoryClient) Fetch(s string) (io.ReadCloser, error) {
+	return hrc.fetchReader, hrc.fetchErr
+}
+
+func TestCachingClient_Chart(t *testing.T) {
+	bareClient := &fakeRepositoryClient{
+		chart: &RepositoryChart{
+			Name:        "app-a",
+			Version:     "0.1.0",
+			Description: "description",
+			URLs:        []string{"http://example.com/archive"},
+		},
+	}
+	cc := &CachingClient{
+		RepositoryClient: bareClient,
+	}
+
+	// Show that bare client returns different chart each time
+	var name = "app-a"
+	var version = "0.1.0"
+	var prev *RepositoryChart
+	for i := 0; i < 2; i++ {
+		chart, err := bareClient.Chart(name, version)
+		require.NoError(t, err)
+		assert.False(t, prev == chart)
+		prev = chart
+	}
+
+	// Show that caching client does return same instance when name/version is the same
+	var err error
+	prev, err = cc.Chart(name, version)
+	require.NoError(t, err)
+	for i := 0; i < 2; i++ {
+		chart, err := cc.Chart(name, version)
+		require.NoError(t, err)
+		assert.True(t, prev == chart)
+	}
+
+	// Show that it changes when name or version change
+	name = "app-b"
+	chart, err := cc.Chart(name, version)
+	require.NoError(t, err)
+	assert.False(t, prev == chart)
+	prev = chart
+
+	chart, err = cc.Chart(name, version)
+	require.NoError(t, err)
+	assert.True(t, prev == chart)
+
+	version = "0.2.0"
+	chart, err = cc.Chart(name, version)
+	require.NoError(t, err)
+	assert.False(t, prev == chart)
+}

pkg/registry/add.go

@@ -48,7 +48,8 @@ func Add(a app.App, protocol Protocol, name string, uri string, isOverride bool,
 		if err != nil {
 			return nil, errors.Wrap(err, "initializing helm HTTP client")
 		}
-		r, err = helmFactory(a, initSpec, hc)
+		cc := helm.NewCachingClient(hc)
+		r, err = helmFactory(a, initSpec, cc)
 	default:
 		return nil, errors.Errorf("invalid registry protocol %q", protocol)
 	}

pkg/registry/add_test.go

@@ -111,9 +111,10 @@ func TestAdd_Helm(t *testing.T) {
 
 		httpClient, err := helm.NewHTTPClient("http://example.com", getterMock)
 		require.NoError(t, err)
+		cc := helm.NewCachingClient(httpClient)
 
-		helmFactory = func(a app.App, registryConfig *app.RegistryConfig, _ *helm.HTTPClient) (*Helm, error) {
-			return NewHelm(a, registryConfig, httpClient, nil)
+		helmFactory = func(a app.App, registryConfig *app.RegistryConfig, _ helm.RepositoryClient) (*Helm, error) {
+			return NewHelm(a, registryConfig, cc, nil)
 		}
 
 		spec, err := Add(appMock, ProtocolHelm, "new", "http://example.com", false, nil)

pkg/registry/helm.go

@@ -33,12 +33,12 @@ import (
 
 var (
 	// helmFactory creates Helm registry instances.
-	helmFactory = func(a app.App, registryConfig *app.RegistryConfig, httpClient *helm.HTTPClient) (*Helm, error) {
+	helmFactory = func(a app.App, registryConfig *app.RegistryConfig, httpClient helm.RepositoryClient) (*Helm, error) {
 		return NewHelm(a, registryConfig, httpClient, nil)
 	}
 )
 
-type helmFactoryFn func(a app.App, registryConfig *app.RegistryConfig, httpClient *helm.HTTPClient) (*Helm, error)
+type helmFactoryFn func(a app.App, registryConfig *app.RegistryConfig, httpClient helm.RepositoryClient) (*Helm, error)
 
 // Helm is a Helm repository.
 type Helm struct {
@@ -114,14 +114,12 @@ func (h *Helm) MakeRegistryConfig() *app.RegistryConfig {
 	return h.spec
 }
 
-// ResolveLibrarySpec returns a resolved spec for a part.
-func (h *Helm) ResolveLibrarySpec(partName, version string) (*parts.Spec, error) {
-	chart, err := h.repositoryClient.Chart(partName, version)
-	if err != nil {
-		return nil, errors.Wrapf(err, "retrieving chart %s-%s", partName, version)
+// makeChartSpec constructs a parts.Spec for a helm chart.
+func makeChartSpec(chart *helm.RepositoryChart) *parts.Spec {
+	if chart == nil {
+		return nil
 	}
-
-	part := &parts.Spec{
+	return &parts.Spec{
 		APIVersion: parts.DefaultAPIVersion,
 		Kind:       parts.DefaultKind,
 
@@ -129,17 +127,20 @@ func (h *Helm) ResolveLibrarySpec(partName, version string) (*parts.Spec, error)
 		Version:     chart.Version,
 		Description: chart.Description,
 	}
-
-	return part, nil
 }
 
-// ResolveLibrary fetches the part and creates a parts spec and library ref spec.
-func (h *Helm) ResolveLibrary(partName string, partAlias string, version string, onFile ResolveFile, onDir ResolveDirectory) (*parts.Spec, *app.LibraryConfig, error) {
-	part, err := h.ResolveLibrarySpec(partName, version)
+// ResolveLibrarySpec returns a resolved spec for a part.
+func (h *Helm) ResolveLibrarySpec(partName, version string) (*parts.Spec, error) {
+	chart, err := h.repositoryClient.Chart(partName, version)
 	if err != nil {
-		return nil, nil, err
+		return nil, errors.Wrapf(err, "retrieving chart %s-%s", partName, version)
 	}
 
+	return makeChartSpec(chart), nil
+}
+
+// ResolveLibrary fetches the part and creates a parts spec and library ref spec.
+func (h *Helm) ResolveLibrary(partName string, partAlias string, version string, onFile ResolveFile, onDir ResolveDirectory) (*parts.Spec, *app.LibraryConfig, error) {
 	chart, err := h.repositoryClient.Chart(partName, version)
 	if err != nil {
 		return nil, nil, errors.Wrapf(err, "retrieving chart %s-%s", partName, version)
@@ -177,6 +178,8 @@ func (h *Helm) ResolveLibrary(partName string, partAlias string, version string,
 		}
 	}
 
+	part := makeChartSpec(chart)
+
 	refSpec := &app.LibraryConfig{
 		Name:     partAlias,
 		Registry: h.Name(),

pkg/registry/manager.go

@@ -38,7 +38,7 @@ func Locate(a app.App, spec *app.RegistryConfig, httpClient *http.Client) (Regis
 		if err != nil {
 			return nil, err
 		}
-		return NewHelm(a, spec, client, nil)
+		return NewHelm(a, spec, helm.NewCachingClient(client), nil)
 	default:
 		return nil, errors.Errorf("invalid registry protocol %q", spec.Protocol)
 	}