Skip to content

Commit

Permalink
controllers: implement internal Helm pkg changes
Browse files Browse the repository at this point in the history 
Signed-off-by: Hidde Beydals <hello@hidde.co>
  • Loading branch information
@hiddeco
hiddeco committed Nov 1, 2021
1 parent 9924347 commit eefae20
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 52 deletions.
44 changes: 12 additions & 32 deletions controllers/helmchart_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
}

// Initialize the chart repository and load the index file
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
indexPath := r.Storage.LocalPath(*repository.GetArtifact())
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, indexPath, r.Getters, clientOpts)
if err != nil {
switch err.(type) {
case *url.Error:
Expand All @@ -331,15 +332,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
}
indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
b, err := io.ReadAll(indexFile)
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
if err = chartRepo.LoadIndex(b); err != nil {
if err := chartRepo.LoadFromCache(); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}

Expand All @@ -348,6 +341,9 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
// Unload as soon as we have the chart version, as we will not require it
// anymore from here on
chartRepo.UnloadIndex()

// Return early if the revision is still the same as the current artifact
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), chartVer.Version,
Expand Down Expand Up @@ -653,8 +649,12 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
clientOpts = append(clientOpts, opts...)
}

// Initialize the chart repository and load the index file
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
// Initialize the chart repository
var cachedIndex string
if artifact := repository.GetArtifact(); artifact != nil {
cachedIndex = r.Storage.LocalPath(*artifact)
}
chartRepo, err := helm.NewChartRepository(repository.Spec.URL, cachedIndex, r.Getters, clientOpts)
if err != nil {
switch err.(type) {
case *url.Error:
Expand All @@ -663,26 +663,6 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
}
if repository.Status.Artifact != nil {
indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
b, err := io.ReadAll(indexFile)
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
if err = chartRepo.LoadIndex(b); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
} else {
// Download index
err = chartRepo.DownloadIndex()
if err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
}
}

dwr = append(dwr, &helm.DependencyWithRepository{
Dependency: dep,
Repository: chartRepo,
Expand Down
32 changes: 20 additions & 12 deletions controllers/helmrepository_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.
}

// Reconcile the artifact to storage
if result, err := r.reconcileArtifact(ctx, obj, artifact, index); err != nil || result.IsZero() {
if result, err := r.reconcileArtifact(ctx, obj, artifact, &index); err != nil || result.IsZero() {
return result, err
}

Expand Down Expand Up @@ -305,7 +305,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
}

// Construct Helm chart repository with options and download index
newIndex, err := helm.NewChartRepository(obj.Spec.URL, r.Getters, clientOpts)
newIndex, err := helm.NewChartRepository(obj.Spec.URL, "", r.Getters, clientOpts)
if err != nil {
switch err.(type) {
case *url.Error:
Expand All @@ -320,7 +320,9 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
return ctrl.Result{}, nil
}
}
if err := newIndex.DownloadIndex(); err != nil {

checksum, err := newIndex.CacheIndex()
if err != nil {
conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FailedReason,
"Failed to download Helm repository index: %s", err.Error())
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.FetchFailedCondition,
Expand All @@ -332,16 +334,16 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
conditions.Delete(obj, sourcev1.FetchFailedCondition)

// Mark observations about the revision on the object
if !obj.GetArtifact().HasRevision(index.Checksum) {
if !obj.GetArtifact().HasRevision(checksum) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision",
"New index revision '%s'", index.Checksum)
"New index revision '%s'", checksum)
}

// Create potential new artifact
*artifact = r.Storage.NewArtifactFor(obj.Kind,
obj.ObjectMeta.GetObjectMeta(),
index.Checksum,
fmt.Sprintf("index-%s.yaml", index.Checksum))
checksum,
fmt.Sprintf("index-%s.yaml", checksum))
return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

Expand All @@ -355,7 +357,7 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
// updated to its path.
//
// The caller should assume a failure if an error is returned, or the Result is zero.
func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, index helm.ChartRepository) (ctrl.Result, error) {
func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, index *helm.ChartRepository) (ctrl.Result, error) {
// Always restore the Ready condition in case it got removed due to a transient error
defer func() {
if obj.GetArtifact() != nil {
Expand All @@ -374,10 +376,11 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
}

// Confirm the wrapper contains a loaded index
if index.Index == nil {
err := fmt.Errorf("failed to reconcile artifact: no repository index provided")
return ctrl.Result{}, err
// Attempt to load the index, this ensures it passes validation
if !index.HasIndex() {
if err := index.LoadFromCache(); err != nil {
return ctrl.Result{}, fmt.Errorf("failed to reconcile artifact: %w", err)
}
}

// Ensure artifact directory exists and acquire lock
Expand All @@ -399,6 +402,11 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s
"Failed to marshal downloaded index: %s", err)
return ctrl.Result{}, err
}

// Unload index now that we have the validated bytes
index.UnloadIndex()

// Write it to storage
if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(b), 0644); err != nil {
r.Eventf(ctx, obj, events.EventSeverityError, sourcev1.StorageOperationFailedReason,
"Failed to write Helm repository index to storage: %s", err.Error())
Expand Down
15 changes: 7 additions & 8 deletions controllers/helmrepository_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (
"strings"
"testing"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/helmtestserver"
"github.com/fluxcd/pkg/runtime/conditions"
"github.com/go-logr/logr"
. "github.com/onsi/gomega"
"helm.sh/helm/v3/pkg/getter"
Expand All @@ -38,10 +41,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/yaml"

"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/helmtestserver"
"github.com/fluxcd/pkg/runtime/conditions"

sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
"github.com/fluxcd/source-controller/internal/helm"
)
Expand Down Expand Up @@ -545,7 +544,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
{
name: "Index is empty",
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, index *helm.ChartRepository) {
*index = helm.ChartRepository{}
index.Index = nil
},
wantErr: true,
},
Expand Down Expand Up @@ -579,10 +578,10 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {

dirIB, err := yaml.Marshal(dirI)
g.Expect(err).NotTo(HaveOccurred())
i, err := helm.NewChartRepository("https://example.com", testGetters, nil)
i, err := helm.NewChartRepository("https://example.com", "", testGetters, nil)
g.Expect(err).NotTo(HaveOccurred())
g.Expect(i).ToNot(BeNil())
g.Expect(i.LoadIndex(dirIB)).To(Succeed())
g.Expect(i.LoadIndexFromBytes(dirIB)).To(Succeed())

artifact := testStorage.NewArtifactFor(obj.Kind, obj, "existing", "foo.tar.gz")
artifact.Checksum = i.Checksum
Expand All @@ -591,7 +590,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
tt.beforeFunc(g, obj, artifact, i)
}

got, err := r.reconcileArtifact(logr.NewContext(ctx, log.NullLogger{}), obj, artifact, *i)
got, err := r.reconcileArtifact(logr.NewContext(ctx, log.NullLogger{}), obj, artifact, i)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))

Expand Down

0 comments on commit eefae20

Please sign in to comment.