Skip to content

Commit

Permalink
Rewrite HelmRepositoryReconciler to new standards
Browse files Browse the repository at this point in the history
This commit rewrites the `HelmRepositoryReconciler` to new standards,
while implementing the newly introduced Condition types, and trying to
adhere better to Kubernetes API conventions.

More specifically it introduces:

- Implementation of more explicit Condition types to highlight
  abnormalities.
- Extensive usage of the `conditions` subpackage from `runtime`.
- Better and more conflict-resilient (status)patching of reconciled
  objects using the `patch` subpackage from runtime.
- Proper implementation of kstatus' `Reconciling` and `Stalled`
  conditions.
- Refactoring of some Helm elements to make them easier to use within
  the new reconciler logic.
- Integration tests that solely rely on `testenv` and do not
  use Ginkgo.

There are a couple of TODOs marked in-code, these are suggestions for
the future and should be non-blocking.
In addition to the TODOs, more complex and/or edge-case test scenarios
may be added as well.

Signed-off-by: Hidde Beydals <hello@hidde.co>
  • Loading branch information
hiddeco committed Aug 12, 2021
1 parent 04c125f commit 4963963
Show file tree
Hide file tree
Showing 12 changed files with 965 additions and 681 deletions.
43 changes: 0 additions & 43 deletions api/v1beta1/helmrepository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import (

"github.com/fluxcd/pkg/apis/acl"
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/runtime/conditions"
apimeta "k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand Down Expand Up @@ -107,47 +105,6 @@ const (
IndexationSucceededReason string = "IndexationSucceed"
)

// HelmRepositoryProgressing resets the conditions of the HelmRepository to
// metav1.Condition of type meta.ReadyCondition with status 'Unknown' and
// meta.ProgressingReason reason and message. It returns the modified
// HelmRepository.
func HelmRepositoryProgressing(repository HelmRepository) HelmRepository {
repository.Status.ObservedGeneration = repository.Generation
repository.Status.URL = ""
repository.Status.Conditions = []metav1.Condition{}
conditions.MarkUnknown(&repository, meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress")
return repository
}

// HelmRepositoryReady sets the given Artifact and URL on the HelmRepository and
// sets the meta.ReadyCondition to 'True', with the given reason and message. It
// returns the modified HelmRepository.
func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reason, message string) HelmRepository {
repository.Status.Artifact = &artifact
repository.Status.URL = url
conditions.MarkTrue(&repository, meta.ReadyCondition, reason, message)
return repository
}

// HelmRepositoryNotReady sets the meta.ReadyCondition on the given
// HelmRepository to 'False', with the given reason and message. It returns the
// modified HelmRepository.
func HelmRepositoryNotReady(repository HelmRepository, reason, message string) HelmRepository {
conditions.MarkFalse(&repository, meta.ReadyCondition, reason, message)
return repository
}

// HelmRepositoryReadyMessage returns the message of the metav1.Condition of type
// meta.ReadyCondition with status 'True' if present, or an empty string.
func HelmRepositoryReadyMessage(repository HelmRepository) string {
if c := apimeta.FindStatusCondition(repository.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 HelmRepository) GetConditions() []metav1.Condition {
return in.Status.Conditions
Expand Down
18 changes: 14 additions & 4 deletions controllers/helmchart_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,17 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
if secret, err := r.getHelmRepositorySecret(ctx, &repository); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
} else if secret != nil {
opts, cleanup, err := helm.ClientOptionsFromSecret(*secret)
tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-source-", chart.Name, chart.Namespace))
if err != nil {
err = fmt.Errorf("failed to create temporary directory for auth: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
}
defer os.RemoveAll(tmpDir)
opts, err := helm.ClientOptionsFromSecret(*secret, tmpDir)
if err != nil {
err = fmt.Errorf("auth options error: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
}
defer cleanup()
clientOpts = append(clientOpts, opts...)
}

Expand Down Expand Up @@ -634,12 +639,17 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
if secret, err := r.getHelmRepositorySecret(ctx, repository); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
} else if secret != nil {
opts, cleanup, err := helm.ClientOptionsFromSecret(*secret)
tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-source-", chart.Name, chart.Namespace))
if err != nil {
err = fmt.Errorf("failed to create temporary directory for auth: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
}
defer os.RemoveAll(tmpDir)
opts, err := helm.ClientOptionsFromSecret(*secret, tmpDir)
if err != nil {
err = fmt.Errorf("auth options error: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
}
defer cleanup()
clientOpts = append(clientOpts, opts...)
}

Expand Down
Loading

0 comments on commit 4963963

Please sign in to comment.