HelmChartReconciler refactor

Signed-off-by: Hidde Beydals <hello@hidde.co>

api/v1beta2/condition_types.go

@@ -32,6 +32,10 @@ const (
 	// Source may be outdated.
 	// This is a "negative polarity" or "abnormal-true" type, and is only present on the resource if it is True.
 	FetchFailedCondition string = "FetchFailed"
+
+	// BuildFailedCondition indicates a transient or persistent build failure of a Source's Artifact.
+	// If True, the Source can be in an ArtifactOutdatedCondition
+	BuildFailedCondition string = "BuildFailed"
 )
 
 const (

api/v1beta2/helmchart_types.go

@@ -19,12 +19,10 @@ package v1beta2
 import (
 	"time"
 
-	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/fluxcd/pkg/apis/acl"
 	"github.com/fluxcd/pkg/apis/meta"
-	"github.com/fluxcd/pkg/runtime/conditions"
 )
 
 // HelmChartKind is the string representation of a HelmChart.
@@ -115,6 +113,16 @@ type HelmChartStatus struct {
 	// +optional
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 
+	// ObservedSourceArtifactRevision is the last observed Artifact.Revision
+	// of the Source reference.
+	// +optional
+	ObservedSourceArtifactRevision string `json:"observedSourceArtifactRevision,omitempty"`
+
+	// ObservedChartName is the last observed chart name as defined by the
+	// resolved chart reference.
+	// +optional
+	ObservedChartName string `json:"observedChartName,omitempty"`
+
 	// Conditions holds the conditions for the HelmChart.
 	// +optional
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
@@ -148,46 +156,6 @@ const (
 	ChartPackageSucceededReason string = "ChartPackageSucceeded"
 )
 
-// HelmChartProgressing resets the conditions of the HelmChart to meta.Condition
-// of type meta.ReadyCondition with status 'Unknown' and meta.ProgressingReason
-// reason and message. It returns the modified HelmChart.
-func HelmChartProgressing(chart HelmChart) HelmChart {
-	chart.Status.ObservedGeneration = chart.Generation
-	chart.Status.URL = ""
-	chart.Status.Conditions = []metav1.Condition{}
-	conditions.MarkUnknown(&chart, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress")
-	return chart
-}
-
-// HelmChartReady sets the given Artifact and URL on the HelmChart and sets the
-// meta.ReadyCondition to 'True', with the given reason and message. It returns
-// the modified HelmChart.
-func HelmChartReady(chart HelmChart, artifact Artifact, url, reason, message string) HelmChart {
-	chart.Status.Artifact = &artifact
-	chart.Status.URL = url
-	conditions.MarkTrue(&chart, meta.ReadyCondition, reason, message)
-	return chart
-}
-
-// HelmChartNotReady sets the meta.ReadyCondition on the given HelmChart to
-// 'False', with the given reason and message. It returns the modified
-// HelmChart.
-func HelmChartNotReady(chart HelmChart, reason, message string) HelmChart {
-	conditions.MarkFalse(&chart, meta.ReadyCondition, reason, message)
-	return chart
-}
-
-// HelmChartReadyMessage returns the message of the meta.ReadyCondition with
-// status 'True', or an empty string.
-func HelmChartReadyMessage(chart HelmChart) string {
-	if c := apimeta.FindStatusCondition(chart.Status.Conditions, meta.ReadyCondition); c != nil {
-		if c.Status == metav1.ConditionTrue {
-			return c.Message
-		}
-	}
-	return ""
-}
-
 // GetConditions returns the status conditions of the object.
 func (in HelmChart) GetConditions() []metav1.Condition {
 	return in.Status.Conditions

config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml

@@ -517,10 +517,18 @@ spec:
                   reconcile request value, so a change of the annotation value can
                   be detected.
                 type: string
+              observedChartName:
+                description: ObservedChartName is the last observed chart name as
+                  defined by the resolved chart reference.
+                type: string
               observedGeneration:
                 description: ObservedGeneration is the last observed generation.
                 format: int64
                 type: integer
+              observedSourceArtifactRevision:
+                description: ObservedSourceArtifactRevision is the last observed Artifact.Revision
+                  of the Source reference.
+                type: string
               url:
                 description: URL is the download link for the last chart pulled.
                 type: string

controllers/helmchart_controller_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/gittestserver"
 	"github.com/fluxcd/pkg/helmtestserver"
+	"github.com/fluxcd/pkg/runtime/conditions"
 	"github.com/go-git/go-billy/v5/memfs"
 	"github.com/go-git/go-git/v5"
 	"github.com/go-git/go-git/v5/config"
@@ -49,7 +50,7 @@ import (
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
 )
 
-var _ = Describe("HelmChartReconciler", func() {
+var _ = FDescribe("HelmChartReconciler", func() {
 
 	const (
 		timeout       = time.Second * 30
@@ -270,7 +271,7 @@ var _ = Describe("HelmChartReconciler", func() {
 				got := &sourcev1.HelmChart{}
 				Eventually(func() bool {
 					_ = k8sClient.Get(context.Background(), key, got)
-					return got.Status.ObservedGeneration > updated.Status.ObservedGeneration &&
+					return got.Status.ObservedGeneration > updated.Status.ObservedGeneration && got.GetArtifact() != nil &&
 						ginkgoTestStorage.ArtifactExist(*got.Status.Artifact)
 				}, timeout, interval).Should(BeTrue())
 				f, err := os.Stat(ginkgoTestStorage.LocalPath(*got.Status.Artifact))
@@ -292,6 +293,9 @@ var _ = Describe("HelmChartReconciler", func() {
 			Eventually(func() bool {
 				_ = k8sClient.Get(context.Background(), key, updated)
 				for _, c := range updated.Status.Conditions {
+					fmt.Fprintf(GinkgoWriter, "condition type: %s\n", c.Type)
+					fmt.Fprintf(GinkgoWriter, "condition reason: %s\n", c.Reason)
+					fmt.Fprintf(GinkgoWriter, "condition message: %s\n", c.Message)
 					if c.Reason == sourcev1.ChartPullFailedReason &&
 						strings.Contains(c.Message, "failed to retrieve source") {
 						return true
@@ -394,12 +398,8 @@ var _ = Describe("HelmChartReconciler", func() {
 			Expect(k8sClient.Update(context.Background(), chart)).Should(Succeed())
 			Eventually(func() bool {
 				_ = k8sClient.Get(context.Background(), key, chart)
-				for _, c := range chart.Status.Conditions {
-					if c.Reason == sourcev1.ChartPullFailedReason {
-						return true
-					}
-				}
-				return false
+				return conditions.GetReason(chart, sourcev1.FetchFailedCondition) == "InvalidChartReference" &&
+					conditions.IsStalled(chart)
 			}, timeout, interval).Should(BeTrue())
 			Expect(chart.GetArtifact()).NotTo(BeNil())
 			Expect(chart.Status.Artifact.Revision).Should(Equal("0.1.1"))
@@ -495,13 +495,7 @@ var _ = Describe("HelmChartReconciler", func() {
 			got := &sourcev1.HelmChart{}
 			Eventually(func() bool {
 				_ = k8sClient.Get(context.Background(), key, got)
-				for _, c := range got.Status.Conditions {
-					if c.Reason == sourcev1.AuthenticationFailedReason &&
-						strings.Contains(c.Message, "auth secret error") {
-						return true
-					}
-				}
-				return false
+				return conditions.GetReason(got, sourcev1.FetchFailedCondition) == sourcev1.AuthenticationFailedReason
 			}, timeout, interval).Should(BeTrue())
 
 			By("Applying secret with missing keys")
@@ -515,7 +509,7 @@ var _ = Describe("HelmChartReconciler", func() {
 				got := &sourcev1.HelmChart{}
 				_ = k8sClient.Get(context.Background(), key, got)
 				for _, c := range got.Status.Conditions {
-					if c.Reason == sourcev1.ChartPullFailedReason &&
+					if c.Reason == "ChartPullError" &&
 						strings.Contains(c.Message, "401 Unauthorized") {
 						return true
 					}
@@ -833,7 +827,7 @@ var _ = Describe("HelmChartReconciler", func() {
 					// if the artifact was changed due to the current update.
 					// Use status condition to be sure.
 					for _, condn := range got.Status.Conditions {
-						if strings.Contains(condn.Message, "with merged values files [./testdata/charts/helmchart/override.yaml]") &&
+						if strings.Contains(condn.Message, "merged values files [./testdata/charts/helmchart/override.yaml]") &&
 							ginkgoTestStorage.ArtifactExist(*got.Status.Artifact) {
 							return true
 						}

controllers/legacy_suite_test.go

@@ -137,9 +137,9 @@ var _ = BeforeSuite(func() {
 	Expect(err).ToNot(HaveOccurred(), "failed to setup HelmRepositoryReconciler")
 
 	err = (&HelmChartReconciler{
-		Client:  k8sManager.GetClient(),
-		Scheme:  scheme.Scheme,
-		Storage: ginkgoTestStorage,
+		Client:        k8sManager.GetClient(),
+		Storage:       ginkgoTestStorage,
+		EventRecorder: record.NewFakeRecorder(32),
 		Getters: getter.Providers{getter.Provider{
 			Schemes: []string{"http", "https"},
 			New:     getter.NewHTTPGetter,

internal/helm/chart/builder.go

@@ -115,15 +115,16 @@ func (o BuildOptions) GetValuesFiles() []string {
 	return o.ValuesFiles
 }
 
-// Build contains the Builder.Build result, including specific
+// Build contains the (partial) Builder.Build result, including specific
 // information about the built chart like ResolvedDependencies.
 type Build struct {
-	// Path is the absolute path to the packaged chart.
-	Path string
-	// Name of the packaged chart.
+	// Name of the chart.
 	Name string
-	// Version of the packaged chart.
+	// Version of the chart.
 	Version string
+	// Path is the absolute path to the packaged chart.
+	// Can be empty, in which case a failure should be assumed.
+	Path string
 	// ValuesFiles is the list of files used to compose the chart's
 	// default "values.yaml".
 	ValuesFiles []string
@@ -138,30 +139,45 @@ type Build struct {
 
 // Summary returns a human-readable summary of the Build.
 func (b *Build) Summary() string {
-	if b == nil || b.Name == "" || b.Version == "" {
-		return "No chart build."
+	if !b.HasMetadata() {
+		return "No chart build"
 	}
 
 	var s strings.Builder
 
-	var action = "Pulled"
-	if b.Packaged {
-		action = "Packaged"
+	var action = "New"
+	if b.Path != "" {
+		action = "Pulled"
+		if b.Packaged {
+			action = "Packaged"
+		}
 	}
 	s.WriteString(fmt.Sprintf("%s '%s' chart with version '%s'", action, b.Name, b.Version))
 
-	if b.Packaged && len(b.ValuesFiles) > 0 {
-		s.WriteString(fmt.Sprintf(", with merged values files %v", b.ValuesFiles))
+	if len(b.ValuesFiles) > 0 {
+		s.WriteString(fmt.Sprintf(" and merged values files %v", b.ValuesFiles))
 	}
 
-	if b.Packaged && b.ResolvedDependencies > 0 {
-		s.WriteString(fmt.Sprintf(", resolving %d dependencies before packaging", b.ResolvedDependencies))
-	}
-
-	s.WriteString(".")
 	return s.String()
 }
 
+// HasMetadata returns if the Build contains chart metadata.
+//
+// NOTE: This may return True while the build did not Complete successfully.
+// Which means it was able to successfully collect the metadata from the chart,
+// but failed further into the process.
+func (b *Build) HasMetadata() bool {
+	if b == nil {
+		return false
+	}
+	return b.Name != "" && b.Version != ""
+}
+
+// Complete returns if the Build completed successfully.
+func (b *Build) Complete() bool {
+	return b.HasMetadata() && b.Path != ""
+}
+
 // String returns the Path of the Build.
 func (b *Build) String() string {
 	if b == nil {

internal/helm/chart/builder_local.go

@@ -101,6 +101,9 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 		result.Version = ver.String()
 	}
 
+	isChartDir := pathIsDir(localRef.Path)
+	requiresPackaging := isChartDir || opts.VersionMetadata != "" || len(opts.GetValuesFiles()) != 0
+
 	// If all the following is true, we do not need to package the chart:
 	// - Chart name from cached chart matches resolved name
 	// - Chart version from cached chart matches calculated version
@@ -112,7 +115,9 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 			if err = curMeta.Validate(); err == nil {
 				if result.Name == curMeta.Name && result.Version == curMeta.Version {
 					result.Path = opts.CachedChart
-					result.ValuesFiles = opts.ValuesFiles
+					result.ValuesFiles = opts.GetValuesFiles()
+					result.Packaged = requiresPackaging
+
 					return result, nil
 				}
 			}
@@ -121,10 +126,9 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 
 	// If the chart at the path is already packaged and no custom values files
 	// options are set, we can copy the chart without making modifications
-	isChartDir := pathIsDir(localRef.Path)
-	if !isChartDir && len(opts.GetValuesFiles()) == 0 {
+	if !requiresPackaging {
 		if err = copyFileToPath(localRef.Path, p); err != nil {
-			return nil, &BuildError{Reason: ErrChartPull, Err: err}
+			return result, &BuildError{Reason: ErrChartPull, Err: err}
 		}
 		result.Path = p
 		return result, nil
@@ -134,7 +138,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 	var mergedValues map[string]interface{}
 	if len(opts.GetValuesFiles()) > 0 {
 		if mergedValues, err = mergeFileValues(localRef.WorkDir, opts.ValuesFiles); err != nil {
-			return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
+			return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
 		}
 	}
 
@@ -143,7 +147,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 	// or because we have merged values and need to repackage
 	chart, err := loader.Load(localRef.Path)
 	if err != nil {
-		return nil, &BuildError{Reason: ErrChartPackage, Err: err}
+		return result, &BuildError{Reason: ErrChartPackage, Err: err}
 	}
 	// Set earlier resolved version (with metadata)
 	chart.Metadata.Version = result.Version
@@ -151,7 +155,7 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 	// Overwrite default values with merged values, if any
 	if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil {
 		if err != nil {
-			return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
+			return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
 		}
 		result.ValuesFiles = opts.GetValuesFiles()
 	}
@@ -160,19 +164,19 @@ func (b *localChartBuilder) Build(ctx context.Context, ref Reference, p string,
 	if isChartDir {
 		if b.dm == nil {
 			err = fmt.Errorf("local chart builder requires dependency manager for unpackaged charts")
-			return nil, &BuildError{Reason: ErrDependencyBuild, Err: err}
+			return result, &BuildError{Reason: ErrDependencyBuild, Err: err}
 		}
 		if result.ResolvedDependencies, err = b.dm.Build(ctx, ref, chart); err != nil {
-			return nil, &BuildError{Reason: ErrDependencyBuild, Err: err}
+			return result, &BuildError{Reason: ErrDependencyBuild, Err: err}
 		}
 	}
 
 	// Package the chart
 	if err = packageToPath(chart, p); err != nil {
-		return nil, &BuildError{Reason: ErrChartPackage, Err: err}
+		return result, &BuildError{Reason: ErrChartPackage, Err: err}
 	}
 	result.Path = p
-	result.Packaged = true
+	result.Packaged = requiresPackaging
 	return result, nil
 }
 

internal/helm/chart/builder_remote.go

@@ -82,12 +82,13 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	cv, err := b.remote.Get(remoteRef.Name, remoteRef.Version)
 	if err != nil {
 		err = fmt.Errorf("failed to get chart version for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
+		return nil, &BuildError{Reason: ErrChartReference, Err: err}
 	}
 
 	result := &Build{}
 	result.Name = cv.Name
 	result.Version = cv.Version
+
 	// Set build specific metadata if instructed
 	if opts.VersionMetadata != "" {
 		ver, err := semver.NewVersion(result.Version)
@@ -102,6 +103,8 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 		result.Version = ver.String()
 	}
 
+	requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != ""
+
 	// If all the following is true, we do not need to download and/or build the chart:
 	// - Chart name from cached chart matches resolved name
 	// - Chart version from cached chart matches calculated version
@@ -114,6 +117,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 				if result.Name == curMeta.Name && result.Version == curMeta.Version {
 					result.Path = opts.CachedChart
 					result.ValuesFiles = opts.GetValuesFiles()
+					result.Packaged = requiresPackaging
 					return result, nil
 				}
 			}
@@ -124,12 +128,12 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	res, err := b.remote.DownloadChart(cv)
 	if err != nil {
 		err = fmt.Errorf("failed to download chart for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
+		return result, &BuildError{Reason: ErrChartPull, Err: err}
 	}
 
 	// Use literal chart copy from remote if no custom values files options are
-	// set or build option version metadata isn't set.
-	if len(opts.GetValuesFiles()) == 0 && opts.VersionMetadata == "" {
+	// set or version metadata isn't set.
+	if !requiresPackaging {
 		if err = validatePackageAndWriteToPath(res, p); err != nil {
 			return nil, &BuildError{Reason: ErrChartPull, Err: err}
 		}
@@ -141,14 +145,14 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	var chart *helmchart.Chart
 	if chart, err = loader.LoadArchive(res); err != nil {
 		err = fmt.Errorf("failed to load downloaded chart: %w", err)
-		return nil, &BuildError{Reason: ErrChartPackage, Err: err}
+		return result, &BuildError{Reason: ErrChartPackage, Err: err}
 	}
 	chart.Metadata.Version = result.Version
 
 	mergedValues, err := mergeChartValues(chart, opts.ValuesFiles)
 	if err != nil {
 		err = fmt.Errorf("failed to merge chart values: %w", err)
-		return nil, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
+		return result, &BuildError{Reason: ErrValuesFilesMerge, Err: err}
 	}
 	// Overwrite default values with merged values, if any
 	if ok, err = OverwriteChartDefaultValues(chart, mergedValues); ok || err != nil {

internal/helm/chart/builder_test.go

@@ -138,12 +138,32 @@ func TestChartBuildResult_Summary(t *testing.T) {
 		want  string
 	}{
 		{
-			name: "Simple",
+			name: "Build with metadata",
 			build: &Build{
 				Name:    "chart",
 				Version: "1.2.3-rc.1+bd6bf40",
 			},
-			want: "Pulled 'chart' chart with version '1.2.3-rc.1+bd6bf40'.",
+			want: "New 'chart' chart with version '1.2.3-rc.1+bd6bf40'",
+		},
+		{
+			name: "Pulled chart",
+			build: &Build{
+				Name:    "chart",
+				Version: "1.2.3-rc.1+bd6bf40",
+				Path:    "chart.tgz",
+			},
+			want: "Pulled 'chart' chart with version '1.2.3-rc.1+bd6bf40'",
+		},
+		{
+			name: "Packaged chart",
+			build: &Build{
+				Name:        "chart",
+				Version:     "arbitrary-version",
+				Packaged:    true,
+				ValuesFiles: []string{"a.yaml", "b.yaml"},
+				Path:        "chart.tgz",
+			},
+			want: "Packaged 'chart' chart with version 'arbitrary-version' and merged values files [a.yaml b.yaml]",
 		},
 		{
 			name: "With values files",
@@ -152,28 +172,19 @@ func TestChartBuildResult_Summary(t *testing.T) {
 				Version:     "arbitrary-version",
 				Packaged:    true,
 				ValuesFiles: []string{"a.yaml", "b.yaml"},
+				Path:        "chart.tgz",
 			},
-			want: "Packaged 'chart' chart with version 'arbitrary-version', with merged values files [a.yaml b.yaml].",
-		},
-		{
-			name: "With dependencies",
-			build: &Build{
-				Name:                 "chart",
-				Version:              "arbitrary-version",
-				Packaged:             true,
-				ResolvedDependencies: 5,
-			},
-			want: "Packaged 'chart' chart with version 'arbitrary-version', resolving 5 dependencies before packaging.",
+			want: "Packaged 'chart' chart with version 'arbitrary-version' and merged values files [a.yaml b.yaml]",
 		},
 		{
 			name:  "Empty build",
 			build: &Build{},
-			want:  "No chart build.",
+			want:  "No chart build",
 		},
 		{
 			name:  "Nil build",
 			build: nil,
-			want:  "No chart build.",
+			want:  "No chart build",
 		},
 	}
 	for _, tt := range tests {

internal/helm/chart/errors.go

@@ -22,22 +22,29 @@ import (
 )
 
 // BuildErrorReason is the descriptive reason for a BuildError.
-type BuildErrorReason string
+type BuildErrorReason struct {
+	// Reason is the programmatic build error reason in CamelCase.
+	Reason string
+
+	// Summary is the human build error reason, used to provide
+	// the Error string, and further context to the BuildError.
+	Summary string
+}
 
 // Error returns the string representation of BuildErrorReason.
 func (e BuildErrorReason) Error() string {
-	return string(e)
+	return e.Summary
 }
 
 // BuildError contains a wrapped Err and a Reason indicating why it occurred.
 type BuildError struct {
-	Reason error
+	Reason BuildErrorReason
 	Err    error
 }
 
 // Error returns Err as a string, prefixed with the Reason to provide context.
 func (e *BuildError) Error() string {
-	if e.Reason == nil {
+	if e.Reason.Error() == "" {
 		return e.Err.Error()
 	}
 	return fmt.Sprintf("%s: %s", e.Reason.Error(), e.Err.Error())
@@ -49,7 +56,7 @@ func (e *BuildError) Error() string {
 //  err := &BuildError{Reason: ErrChartPull, Err: errors.New("arbitrary transport error")}
 //  errors.Is(err, ErrChartPull)
 func (e *BuildError) Is(target error) bool {
-	if e.Reason != nil && e.Reason == target {
+	if e.Reason == target {
 		return true
 	}
 	return errors.Is(e.Err, target)
@@ -60,11 +67,21 @@ func (e *BuildError) Unwrap() error {
 	return e.Err
 }
 
+func IsPersistentBuildErrorReason(err error) bool {
+	switch err {
+	case ErrChartReference, ErrChartMetadataPatch, ErrValuesFilesMerge:
+		return true
+	default:
+		return false
+	}
+}
+
 var (
-	ErrChartReference     = BuildErrorReason("chart reference error")
-	ErrChartPull          = BuildErrorReason("chart pull error")
-	ErrChartMetadataPatch = BuildErrorReason("chart metadata patch error")
-	ErrValuesFilesMerge   = BuildErrorReason("values files merge error")
-	ErrDependencyBuild    = BuildErrorReason("dependency build error")
-	ErrChartPackage       = BuildErrorReason("chart package error")
+	ErrChartReference     = BuildErrorReason{Reason: "InvalidChartReference", Summary: "invalid chart reference"}
+	ErrChartPull          = BuildErrorReason{Reason: "ChartPullError", Summary: "chart pull error"}
+	ErrChartMetadataPatch = BuildErrorReason{Reason: "MetadataPatchError", Summary: "chart metadata patch error"}
+	ErrValuesFilesMerge   = BuildErrorReason{Reason: "ValuesFilesError", Summary: "values files merge error"}
+	ErrDependencyBuild    = BuildErrorReason{Reason: "DependencyBuildError", Summary: "dependency build error"}
+	ErrChartPackage       = BuildErrorReason{Reason: "ChartPackageError", Summary: "chart package error"}
+	ErrUnknown            = BuildErrorReason{Reason: "Unknown", Summary: "unknown build error"}
 )

internal/helm/chart/errors_test.go

@@ -26,7 +26,7 @@ import (
 func TestBuildErrorReason_Error(t *testing.T) {
 	g := NewWithT(t)
 
-	err := BuildErrorReason("reason")
+	err := BuildErrorReason{"Reason", "reason"}
 	g.Expect(err.Error()).To(Equal("reason"))
 }
 
@@ -39,7 +39,7 @@ func TestBuildError_Error(t *testing.T) {
 		{
 			name: "with reason",
 			err: &BuildError{
-				Reason: BuildErrorReason("reason"),
+				Reason: BuildErrorReason{"Reason", "reason"},
 				Err:    errors.New("error"),
 			},
 			want: "reason: error",

main.go

@@ -189,12 +189,11 @@ func main() {
 		os.Exit(1)
 	}
 	if err = (&controllers.HelmChartReconciler{
-		Client:          mgr.GetClient(),
-		Scheme:          mgr.GetScheme(),
-		Storage:         storage,
-		Getters:         getters,
-		EventRecorder:   eventRecorder,
-		MetricsRecorder: metricsH.MetricsRecorder,
+		Client:        mgr.GetClient(),
+		Storage:       storage,
+		Getters:       getters,
+		EventRecorder: eventRecorder,
+		Metrics:       metricsH,
 	}).SetupWithManagerAndOptions(mgr, controllers.HelmChartReconcilerOptions{
 		MaxConcurrentReconciles: concurrent,
 	}); err != nil {