Merge pull request #154 from fluxcd/small-refactors

This commit is contained in:
Hidde Beydals 2020-09-22 12:42:37 +02:00 committed by GitHub
commit c1fb9b7fa5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 58 additions and 54 deletions

View File

@ -67,10 +67,13 @@ jobs:
kubectl -n source-system wait helmchart/mariadb-git --for=condition=ready --timeout=5m
kubectl -n source-system delete -f ./config/testdata/helmchart-valuesfile
- name: Setup Minio
env:
MINIO_VER: ${{ 'v6.3.1' }}
run: |
kubectl create ns minio
helm repo add minio https://helm.min.io/
helm upgrade --wait -i minio minio/minio \
--version $MINIO_VER \
--namespace minio \
--set accessKey=myaccesskey \
--set secretKey=mysecretkey \

View File

@ -49,6 +49,15 @@ type Artifact struct {
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"`
}
// HasRevision returns true if the given revision matches the current
// Revision of the Artifact.
func (in *Artifact) HasRevision(revision string) bool {
if in == nil {
return false
}
return in.Revision == revision
}
// ArtifactDir returns the artifact dir path in the form of
// <source-kind>/<source-namespace>/<source-name>.
func ArtifactDir(kind, namespace, name string) string {

View File

@ -211,7 +211,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket
// return early on unchanged revision
artifact := r.Storage.NewArtifactFor(bucket.Kind, bucket.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", revision))
if bucket.GetArtifact() != nil && bucket.GetArtifact().Revision == revision {
if bucket.GetArtifact().HasRevision(artifact.Revision) {
if artifact.URL != bucket.GetArtifact().URL {
r.Storage.SetArtifactURL(bucket.GetArtifact())
bucket.Status.URL = r.Storage.SetHostname(bucket.Status.URL)

View File

@ -198,7 +198,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
// return early on unchanged revision
artifact := r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), revision, fmt.Sprintf("%s.tar.gz", commit.Hash.String()))
if repository.GetArtifact() != nil && repository.GetArtifact().Revision == revision {
if repository.GetArtifact().HasRevision(artifact.Revision) {
if artifact.URL != repository.GetArtifact().URL {
r.Storage.SetArtifactURL(repository.GetArtifact())
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)

View File

@ -238,12 +238,12 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
}
// Return early if the revision is still the same as the current artifact
artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
if !force && repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version {
if artifact.URL != repository.GetArtifact().URL {
if !force && repository.GetArtifact().HasRevision(newArtifact.Revision) {
if newArtifact.URL != repository.GetArtifact().URL {
r.Storage.SetArtifactURL(chart.GetArtifact())
repository.Status.URL = r.Storage.SetHostname(chart.Status.URL)
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
}
return chart, nil
}
@ -296,21 +296,19 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
err = fmt.Errorf("auth options error: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
}
if cleanup != nil {
defer cleanup()
}
defer cleanup()
clientOpts = opts
}
// Ensure artifact directory exists
err = r.Storage.MkdirAll(artifact)
err = r.Storage.MkdirAll(newArtifact)
if err != nil {
err = fmt.Errorf("unable to create chart directory: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
// Acquire a lock for the artifact
unlock, err := r.Storage.Lock(artifact)
unlock, err := r.Storage.Lock(newArtifact)
if err != nil {
err = fmt.Errorf("unable to acquire lock: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
@ -328,7 +326,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
// or write the chart directly to storage.
var (
readyReason = sourcev1.ChartPullSucceededReason
readyMessage = fmt.Sprintf("Fetched revision: %s", artifact.Revision)
readyMessage = fmt.Sprintf("Fetched revision: %s", newArtifact.Revision)
)
switch {
case chart.Spec.ValuesFile != "" && chart.Spec.ValuesFile != chartutil.ValuesfileName:
@ -362,36 +360,29 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
}
// Copy the packaged chart to the artifact path
cf, err := os.Open(pkgPath)
if err != nil {
err = fmt.Errorf("failed to open chart package: %w", err)
if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
err = fmt.Errorf("failed to write chart package to storage: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
if err := r.Storage.Copy(&artifact, cf); err != nil {
cf.Close()
err = fmt.Errorf("failed to copy chart package to storage: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
cf.Close()
readyMessage = fmt.Sprintf("Fetched and packaged revision: %s", artifact.Revision)
readyMessage = fmt.Sprintf("Fetched and packaged revision: %s", newArtifact.Revision)
readyReason = sourcev1.ChartPackageSucceededReason
default:
// Write artifact to storage
if err := r.Storage.AtomicWriteFile(&artifact, res, 0644); err != nil {
if err := r.Storage.AtomicWriteFile(&newArtifact, res, 0644); err != nil {
err = fmt.Errorf("unable to write chart file: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
}
// Update symlink
chartUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", cv.Name))
chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", cv.Name))
if err != nil {
err = fmt.Errorf("storage error: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
return sourcev1.HelmChartReady(chart, artifact, chartUrl, readyReason, readyMessage), nil
return sourcev1.HelmChartReady(chart, newArtifact, chartUrl, readyReason, readyMessage), nil
}
func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
@ -432,32 +423,30 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
}
// Return early if the revision is still the same as the current chart artifact
chartArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version,
fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version))
if !force && chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version {
if chartArtifact.URL != artifact.URL {
r.Storage.SetArtifactURL(&chartArtifact)
if !force && chart.GetArtifact().HasRevision(newArtifact.Revision) {
if newArtifact.URL != artifact.URL {
r.Storage.SetArtifactURL(&newArtifact)
chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
}
return chart, nil
}
// Overwrite default values if instructed to
if chart.Spec.ValuesFile != "" {
if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
}
// Overwrite default values if configured
if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
}
// Ensure artifact directory exists
err = r.Storage.MkdirAll(chartArtifact)
err = r.Storage.MkdirAll(newArtifact)
if err != nil {
err = fmt.Errorf("unable to create artifact directory: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
// Acquire a lock for the artifact
unlock, err := r.Storage.Lock(chartArtifact)
unlock, err := r.Storage.Lock(newArtifact)
if err != nil {
err = fmt.Errorf("unable to acquire lock: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
@ -475,27 +464,20 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
}
// Copy the packaged chart to the artifact path
cf, err := os.Open(pkgPath)
if err != nil {
err = fmt.Errorf("failed to open chart package: %w", err)
if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
err = fmt.Errorf("failed to write chart package to storage: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
if err := r.Storage.Copy(&chartArtifact, cf); err != nil {
cf.Close()
err = fmt.Errorf("failed to copy chart package to storage: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
cf.Close()
// Update symlink
cUrl, err := r.Storage.Symlink(chartArtifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name))
cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name))
if err != nil {
err = fmt.Errorf("storage error: %w", err)
return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
}
message := fmt.Sprintf("Fetched and packaged revision: %s", chartArtifact.Revision)
return sourcev1.HelmChartReady(chart, chartArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
message := fmt.Sprintf("Fetched and packaged revision: %s", newArtifact.Revision)
return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
}
// resetStatus returns a modified v1alpha1.HelmChart and a boolean indicating

View File

@ -195,9 +195,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
err = fmt.Errorf("auth options error: %w", err)
return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
}
if cleanup != nil {
defer cleanup()
}
defer cleanup()
clientOpts = opts
}
@ -219,7 +217,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
// return early on unchanged generation
artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano),
fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano))))
if repository.GetArtifact() != nil && repository.GetArtifact().Revision == i.Generated.Format(time.RFC3339Nano) {
if repository.GetArtifact().HasRevision(artifact.Revision) {
if artifact.URL != repository.GetArtifact().URL {
r.Storage.SetArtifactURL(repository.GetArtifact())
repository.Status.URL = r.Storage.SetHostname(repository.Status.URL)

View File

@ -339,6 +339,18 @@ func (s *Storage) Copy(artifact *sourcev1.Artifact, reader io.Reader) (err error
return nil
}
// CopyFromPath atomically copies the contents of the given path to the path of
// the v1alpha1.Artifact. If successful, the checksum and last update time on the
// artifact is set.
func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err error) {
f, err := os.Open(path)
if err != nil {
return err
}
defer f.Close()
return s.Copy(artifact, f)
}
// Symlink creates or updates a symbolic link for the given v1alpha1.Artifact
// and returns the URL for the symlink.
func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, error) {

View File

@ -28,7 +28,7 @@ import (
// OverwriteChartDefaultValues overwrites the chart default values file in the
// given chartPath with the contents of the given valuesFile.
func OverwriteChartDefaultValues(chartPath, valuesFile string) error {
if valuesFile == chartutil.ValuesfileName {
if valuesFile == "" || valuesFile == chartutil.ValuesfileName {
return nil
}
srcPath := path.Join(chartPath, valuesFile)

View File

@ -60,7 +60,7 @@ func TLSClientConfigFromSecret(secret corev1.Secret) (getter.Option, func(), err
certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"]
switch {
case len(certBytes)+len(keyBytes)+len(caBytes) == 0:
return nil, nil, nil
return nil, func() {}, nil
case (len(certBytes) > 0 && len(keyBytes) == 0) || (len(keyBytes) > 0 && len(certBytes) == 0):
return nil, nil, fmt.Errorf("invalid '%s' secret data: fields 'certFile' and 'keyFile' require each other's presence",
secret.Name)