diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 87a6e721..50d3474c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -40,7 +40,7 @@ jobs: username: fluxcdbot password: ${{ secrets.DOCKER_FLUXCD_PASSWORD }} - name: Publish amd64 image - uses: docker/build-push-action@v2-build-push + uses: docker/build-push-action@v2 with: push: ${{ github.event_name != 'pull_request' }} builder: ${{ steps.buildx.outputs.name }} @@ -51,7 +51,7 @@ jobs: ghcr.io/fluxcd/source-controller:${{ steps.get_version.outputs.VERSION }} docker.io/fluxcd/source-controller:${{ steps.get_version.outputs.VERSION }} - name: Publish arm64 image - uses: docker/build-push-action@v2-build-push + uses: docker/build-push-action@v2 with: push: ${{ github.event_name != 'pull_request' }} builder: ${{ steps.buildx.outputs.name }} diff --git a/api/v1alpha1/gitrepository_types.go b/api/v1alpha1/gitrepository_types.go index 2be87e6b..c795765e 100644 --- a/api/v1alpha1/gitrepository_types.go +++ b/api/v1alpha1/gitrepository_types.go @@ -99,6 +99,11 @@ type GitRepositoryVerification struct { // GitRepositoryStatus defines the observed state of a Git repository. type GitRepositoryStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the GitRepository. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -122,62 +127,46 @@ const ( GitOperationFailedReason string = "GitOperationFailed" ) -// GitRepositoryReady sets the given artifact and url on the -// GitRepository and resets the conditions to SourceCondition of -// type Ready with status true and the given reason and message. -// It returns the modified GitRepository. -func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason, message string) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionTrue, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } - repository.Status.URL = url - - if repository.Status.Artifact != nil { - if repository.Status.Artifact.Path != artifact.Path { - repository.Status.Artifact = &artifact - } - } else { - repository.Status.Artifact = &artifact - } - - return repository -} - // GitRepositoryProgressing resets the conditions of the GitRepository // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified GitRepository. func GitRepositoryProgressing(repository GitRepository) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionUnknown, - LastTransitionTime: metav1.Now(), - Reason: ProgressingReason, - Message: "reconciliation in progress", - }, - } + repository.Status.ObservedGeneration = repository.Generation + repository.Status.URL = "" + repository.Status.Artifact = nil + repository.Status.Conditions = []SourceCondition{} + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionUnknown, ProgressingReason, "reconciliation in progress") return repository } -// GitRepositoryNotReady resets the conditions of the GitRepository -// to SourceCondition of type Ready with status false and the given -// reason and message. It returns the modified GitRepository. +// SetGitRepositoryCondition sets the given condition with the given status, reason and message +// on the GitRepository. +func SetGitRepositoryCondition(repository *GitRepository, condition string, status corev1.ConditionStatus, reason, message string) { + repository.Status.Conditions = filterOutSourceCondition(repository.Status.Conditions, condition) + repository.Status.Conditions = append(repository.Status.Conditions, SourceCondition{ + Type: condition, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + }) +} + +// GitRepositoryReady sets the given artifact and url on the GitRepository +// and sets the ReadyCondition to True, with the given reason and +// message. It returns the modified GitRepository. +func GitRepositoryReady(repository GitRepository, artifact Artifact, url, reason, message string) GitRepository { + repository.Status.Artifact = &artifact + repository.Status.URL = url + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionTrue, reason, message) + return repository +} + +// GitRepositoryNotReady sets the ReadyCondition on the given GitRepository +// to False, with the given reason and message. It returns the modified +// GitRepository. func GitRepositoryNotReady(repository GitRepository, reason, message string) GitRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionFalse, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } + SetGitRepositoryCondition(&repository, ReadyCondition, corev1.ConditionFalse, reason, message) return repository } diff --git a/api/v1alpha1/helmchart_types.go b/api/v1alpha1/helmchart_types.go index f58f9be3..b398fbfd 100644 --- a/api/v1alpha1/helmchart_types.go +++ b/api/v1alpha1/helmchart_types.go @@ -62,6 +62,11 @@ type LocalHelmChartSourceReference struct { // HelmChartStatus defines the observed state of the HelmChart. type HelmChartStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the HelmChart. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -92,9 +97,10 @@ const ( ChartPackageSucceededReason string = "ChartPackageSucceeded" ) -// HelmReleaseProgressing resets any failures and registers progress toward reconciling the given HelmRelease +// HelmChartProgressing resets any failures and registers progress toward reconciling the given HelmChart // by setting the ReadyCondition to ConditionUnknown for ProgressingReason. func HelmChartProgressing(chart HelmChart) HelmChart { + chart.Status.ObservedGeneration = chart.Generation chart.Status.URL = "" chart.Status.Artifact = nil chart.Status.Conditions = []SourceCondition{} diff --git a/api/v1alpha1/helmrepository_types.go b/api/v1alpha1/helmrepository_types.go index b34f6b63..1870ff32 100644 --- a/api/v1alpha1/helmrepository_types.go +++ b/api/v1alpha1/helmrepository_types.go @@ -54,6 +54,11 @@ type HelmRepositorySpec struct { // HelmRepositoryStatus defines the observed state of the HelmRepository. type HelmRepositoryStatus struct { + // ObservedGeneration is the last observed generation. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // Conditions holds the conditions for the HelmRepository. // +optional Conditions []SourceCondition `json:"conditions,omitempty"` @@ -76,62 +81,46 @@ const ( IndexationSucceededReason string = "IndexationSucceed" ) -// HelmRepositoryReady sets the given artifact and url on the -// HelmRepository and resets the conditions to SourceCondition of -// type Ready with status true and the given reason and message. -// It returns the modified HelmRepository. -func HelmRepositoryReady(repository HelmRepository, artifact Artifact, url, reason, message string) HelmRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionTrue, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } - repository.Status.URL = url - - if repository.Status.Artifact != nil { - if repository.Status.Artifact.Path != artifact.Path { - repository.Status.Artifact = &artifact - } - } else { - repository.Status.Artifact = &artifact - } - - return repository -} - // HelmRepositoryProgressing resets the conditions of the HelmRepository // to SourceCondition of type Ready with status unknown and // progressing reason and message. It returns the modified HelmRepository. func HelmRepositoryProgressing(repository HelmRepository) HelmRepository { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionUnknown, - LastTransitionTime: metav1.Now(), - Reason: ProgressingReason, - Message: "reconciliation in progress", - }, - } + repository.Status.ObservedGeneration = repository.Generation + repository.Status.URL = "" + repository.Status.Artifact = nil + repository.Status.Conditions = []SourceCondition{} + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionUnknown, ProgressingReason, "reconciliation in progress") return repository } -// HelmRepositoryNotReady resets the conditions of the HelmRepository -// to SourceCondition of type Ready with status false and the given -// reason and message. It returns the modified HelmRepository. +// SetHelmRepositoryCondition sets the given condition with the given status, +// reason and message on the HelmRepository. +func SetHelmRepositoryCondition(repository *HelmRepository, condition string, status corev1.ConditionStatus, reason, message string) { + repository.Status.Conditions = filterOutSourceCondition(repository.Status.Conditions, condition) + repository.Status.Conditions = append(repository.Status.Conditions, SourceCondition{ + Type: condition, + Status: status, + LastTransitionTime: metav1.Now(), + Reason: reason, + Message: message, + }) +} + +// HelmRepositoryReady sets the given artifact and url on the HelmRepository +// and sets the 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 + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionTrue, reason, message) + return repository +} + +// HelmRepositoryNotReady sets the 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 { - repository.Status.Conditions = []SourceCondition{ - { - Type: ReadyCondition, - Status: corev1.ConditionFalse, - LastTransitionTime: metav1.Now(), - Reason: reason, - Message: message, - }, - } + SetHelmRepositoryCondition(&repository, ReadyCondition, corev1.ConditionFalse, reason, message) return repository } diff --git a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml index e60d5852..81a2249b 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_gitrepositories.yaml @@ -153,6 +153,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the GitRepository. items: description: SourceCondition contains condition information for a source. @@ -182,6 +183,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the artifact output of the last repository sync. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml index d1125575..b0a22796 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmcharts.yaml @@ -125,6 +125,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the HelmChart. items: description: SourceCondition contains condition information for a source. @@ -154,6 +155,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the last chart pulled. type: string diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index 8f7f675e..25016356 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -105,6 +105,7 @@ spec: - url type: object conditions: + description: Conditions holds the conditions for the HelmRepository. items: description: SourceCondition contains condition information for a source. @@ -134,6 +135,10 @@ spec: - type type: object type: array + observedGeneration: + description: ObservedGeneration is the last observed generation. + format: int64 + type: integer url: description: URL is the download link for the last index fetched. type: string diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index d88f02b1..d62603bf 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -28,7 +28,6 @@ import ( "github.com/go-git/go-git/v5/plumbing/transport" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -100,13 +99,8 @@ func (r *GitRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, erro } // set initial status - if reset, status := r.shouldResetStatus(repository); reset { - repository.Status = status - if err := r.Status().Update(ctx, &repository); err != nil { - log.Error(err, "unable to update status") - return ctrl.Result{Requeue: true}, err - } - } else { + if repository.Generation != repository.Status.ObservedGeneration || + repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.GitRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") @@ -202,6 +196,16 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour return sourcev1.GitRepositoryNotReady(repository, sourcev1.GitOperationFailedReason, err.Error()), err } + // 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 artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } + return repository, nil + } + // verify PGP signature if repository.Spec.Verification != nil { err := r.verify(ctx, types.NamespacedName{ @@ -213,11 +217,6 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour } } - // TODO(hidde): implement checksum when https://github.com/fluxcd/source-controller/pull/133 - // has been merged. - artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("%s.tar.gz", commit.Hash.String()), revision, "") - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -234,7 +233,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour defer unlock() // archive artifact and check integrity - if err := r.Storage.Archive(artifact, tmpGit, repository.Spec); err != nil { + if err := r.Storage.Archive(&artifact, tmpGit, repository.Spec); err != nil { err = fmt.Errorf("storage archive error: %w", err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -250,32 +249,6 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour return sourcev1.GitRepositoryReady(repository, artifact, url, sourcev1.GitOperationSucceedReason, message), nil } -// shouldResetStatus returns a boolean indicating if the status of the -// given repository should be reset. -func (r *GitRepositoryReconciler) shouldResetStatus(repository sourcev1.GitRepository) (bool, sourcev1.GitRepositoryStatus) { - resetStatus := false - if repository.Status.Artifact != nil { - if !r.Storage.ArtifactExist(*repository.Status.Artifact) { - resetStatus = true - } - } - - if len(repository.Status.Conditions) == 0 || resetStatus { - resetStatus = true - } - - return resetStatus, sourcev1.GitRepositoryStatus{ - Conditions: []sourcev1.SourceCondition{ - { - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionUnknown, - Reason: sourcev1.InitializingReason, - LastTransitionTime: metav1.Now(), - }, - }, - } -} - // verify returns an error if the PGP signature can't be verified func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret types.NamespacedName, commit *object.Commit) error { if commit.PGPSignature == "" { @@ -304,10 +277,10 @@ func (r *GitRepositoryReconciler) verify(ctx context.Context, publicKeySecret ty // the given repository. func (r *GitRepositoryReconciler) gc(repository sourcev1.GitRepository, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "")) } - if repository.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact) + if repository.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*repository.GetArtifact()) } return nil } diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index c6e7b720..9ebe88d7 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -23,7 +23,6 @@ import ( "net/url" "os" "path" - "path/filepath" "strings" "time" @@ -105,7 +104,8 @@ func (r *HelmChartReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) { } // set initial status - if chart.Generation == 0 || chart.GetArtifact() != nil && !r.Storage.ArtifactExist(*chart.GetArtifact()) { + if chart.Generation != chart.Status.ObservedGeneration || + chart.GetArtifact() != nil && !r.Storage.ArtifactExist(*chart.GetArtifact()) { chart = sourcev1.HelmChartProgressing(chart) if err := r.Status().Update(ctx, &chart); err != nil { log.Error(err, "unable to update status") @@ -195,6 +195,16 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } + // return early on unchanged chart version + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version, fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version)) + if repository.GetArtifact() != nil && repository.GetArtifact().Revision == cv.Version { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } + return chart, nil + } + // TODO(hidde): according to the Helm source the first item is not // always the correct one to pick, check for updates once in awhile. // Ref: https://github.com/helm/helm/blob/v3.3.0/pkg/downloader/chart_downloader.go#L241 @@ -255,15 +265,6 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err } - chartBytes, err := ioutil.ReadAll(res) - if err != nil { - return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err - } - - sum := r.Storage.Checksum(chartBytes) - artifact := r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(), - fmt.Sprintf("%s-%s-%s.tgz", cv.Name, cv.Version, sum), cv.Version, sum) - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -280,8 +281,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context, defer unlock() // save artifact to storage - err = r.Storage.WriteFile(artifact, chartBytes) - if err != nil { + if err := r.Storage.AtomicWriteFile(&artifact, res, 0644); err != nil { err = fmt.Errorf("unable to write chart file: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -317,7 +317,7 @@ func (r *HelmChartReconciler) getChartRepositoryWithArtifact(ctx context.Context return repository, err } - if repository.Status.Artifact == nil { + if repository.GetArtifact() == nil { err = fmt.Errorf("no repository index artifact found in HelmRepository '%s'", repository.Name) } @@ -362,15 +362,15 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, } // return early on unchanged chart version - if chart.Status.Artifact != nil && chartMetadata.Version == chart.Status.Artifact.Revision { + artifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), chartMetadata.Version, fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version)) + if chart.GetArtifact() != nil && chart.GetArtifact().Revision == chartMetadata.Version { + if artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } return chart, nil } - // TODO(hidde): implement checksum when https://github.com/fluxcd/source-controller/pull/133 - // has been merged. - artifact := r.Storage.ArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("%s-%s.tgz", chartMetadata.Name, chartMetadata.Version), chartMetadata.Version, "") - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -388,22 +388,35 @@ func (r *HelmChartReconciler) reconcileFromGitRepository(ctx context.Context, // package chart pkg := action.NewPackage() - pkg.Destination = filepath.Dir(r.Storage.LocalPath(artifact)) - _, err = pkg.Run(chartPath, nil) + pkg.Destination = tmpDir + src, err := pkg.Run(chartPath, nil) if err != nil { err = fmt.Errorf("chart package error: %w", err) return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err } + // copy chart package + cf, err := os.Open(src) + if err != nil { + err = fmt.Errorf("failed to open chart package: %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() + // update symlink - chartUrl, err := r.Storage.Symlink(artifact, fmt.Sprintf("%s-latest.tgz", chartMetadata.Name)) + cUrl, err := r.Storage.Symlink(artifact, 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", artifact.Revision) - return sourcev1.HelmChartReady(chart, artifact, chartUrl, sourcev1.ChartPackageSucceededReason, message), nil + return sourcev1.HelmChartReady(chart, artifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil } // getGitRepositoryWithArtifact attempts to get the GitRepository for the given @@ -426,7 +439,7 @@ func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, return repository, err } - if repository.Status.Artifact == nil { + if repository.GetArtifact() == nil { err = fmt.Errorf("no artifact found for GitRepository '%s'", repository.Name) } @@ -437,10 +450,10 @@ func (r *HelmChartReconciler) getGitRepositoryWithArtifact(ctx context.Context, // the given chart. func (r *HelmChartReconciler) gc(chart sourcev1.HelmChart, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(chart.Kind, chart.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), "", "")) } - if chart.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*chart.Status.Artifact) + if chart.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*chart.GetArtifact()) } return nil } diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 5e082381..a368ac40 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -17,6 +17,7 @@ limitations under the License. package controllers import ( + "bytes" "context" "fmt" "io/ioutil" @@ -29,7 +30,6 @@ import ( "helm.sh/helm/v3/pkg/getter" "helm.sh/helm/v3/pkg/repo" corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" @@ -41,6 +41,7 @@ import ( "github.com/fluxcd/pkg/recorder" "github.com/fluxcd/pkg/runtime/predicates" + sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1" "github.com/fluxcd/source-controller/internal/helm" ) @@ -103,13 +104,8 @@ func (r *HelmRepositoryReconciler) Reconcile(req ctrl.Request) (ctrl.Result, err } // set initial status - if reset, status := r.shouldResetStatus(repository); reset { - repository.Status = status - if err := r.Status().Update(ctx, &repository); err != nil { - log.Error(err, "unable to update status") - return ctrl.Result{Requeue: true}, err - } - } else { + if repository.Generation != repository.Status.ObservedGeneration || + repository.GetArtifact() != nil && !r.Storage.ArtifactExist(*repository.GetArtifact()) { repository = sourcev1.HelmRepositoryProgressing(repository) if err := r.Status().Update(ctx, &repository); err != nil { log.Error(err, "unable to update status") @@ -206,32 +202,37 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou } clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout())) - res, err := c.Get(u.String(), clientOpts...) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - data, err := ioutil.ReadAll(res) + b, err := ioutil.ReadAll(res) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - - i := &repo.IndexFile{} - if err := yaml.Unmarshal(data, i); err != nil { + i := repo.IndexFile{} + if err := yaml.Unmarshal(b, &i); err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } + + // 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 artifact.URL != repository.GetArtifact().URL { + r.Storage.SetArtifactURL(repository.GetArtifact()) + repository.Status.URL = r.Storage.SetHostname(repository.Status.URL) + } + return repository, nil + } + i.SortEntries() - - index, err := yaml.Marshal(i) + b, err = yaml.Marshal(&i) if err != nil { return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err } - sum := r.Storage.Checksum(index) - artifact := r.Storage.ArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), - fmt.Sprintf("index-%s.yaml", sum), i.Generated.Format(time.RFC3339Nano), sum) - // create artifact dir err = r.Storage.MkdirAll(artifact) if err != nil { @@ -248,8 +249,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou defer unlock() // save artifact to storage - err = r.Storage.WriteFile(artifact, index) - if err != nil { + if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(b), 0644); err != nil { err = fmt.Errorf("unable to write repository index file: %w", err) return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err } @@ -265,41 +265,14 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou return sourcev1.HelmRepositoryReady(repository, artifact, indexURL, sourcev1.IndexationSucceededReason, message), nil } -// shouldResetStatus returns a boolean indicating if the status of the -// given repository should be reset. -func (r *HelmRepositoryReconciler) shouldResetStatus(repository sourcev1.HelmRepository) (bool, sourcev1.HelmRepositoryStatus) { - resetStatus := false - if repository.Status.Artifact != nil { - if !r.Storage.ArtifactExist(*repository.Status.Artifact) { - resetStatus = true - } - } - - // set initial status - if len(repository.Status.Conditions) == 0 { - resetStatus = true - } - - return resetStatus, sourcev1.HelmRepositoryStatus{ - Conditions: []sourcev1.SourceCondition{ - { - Type: sourcev1.ReadyCondition, - Status: corev1.ConditionUnknown, - Reason: sourcev1.InitializingReason, - LastTransitionTime: metav1.Now(), - }, - }, - } -} - // gc performs a garbage collection on all but current artifacts of // the given repository. func (r *HelmRepositoryReconciler) gc(repository sourcev1.HelmRepository, all bool) error { if all { - return r.Storage.RemoveAll(r.Storage.ArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "", "")) + return r.Storage.RemoveAll(r.Storage.NewArtifactFor(repository.Kind, repository.GetObjectMeta(), "", "")) } - if repository.Status.Artifact != nil { - return r.Storage.RemoveAllButCurrent(*repository.Status.Artifact) + if repository.GetArtifact() != nil { + return r.Storage.RemoveAllButCurrent(*repository.GetArtifact()) } return nil } diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 90b20cab..c596ca73 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -140,7 +140,7 @@ var _ = Describe("HelmRepositoryReconciler", func() { Eventually(func() error { r := &sourcev1.HelmRepository{} return k8sClient.Get(context.Background(), key, r) - }).ShouldNot(Succeed()) + }, timeout, interval).ShouldNot(Succeed()) exists := func(path string) bool { // wait for tmp sync on macOS diff --git a/controllers/storage.go b/controllers/storage.go index a137a39e..41883cc9 100644 --- a/controllers/storage.go +++ b/controllers/storage.go @@ -23,8 +23,10 @@ import ( "compress/gzip" "crypto/sha1" "fmt" + "hash" "io" "io/ioutil" + "net/url" "os" "path/filepath" "strings" @@ -36,6 +38,7 @@ import ( "github.com/fluxcd/pkg/lockedfile" sourcev1 "github.com/fluxcd/source-controller/api/v1alpha1" + "github.com/fluxcd/source-controller/internal/fs" ) const ( @@ -61,7 +64,6 @@ func NewStorage(basePath string, hostname string, timeout time.Duration) (*Stora if f, err := os.Stat(basePath); os.IsNotExist(err) || !f.IsDir() { return nil, fmt.Errorf("invalid dir path: %s", basePath) } - return &Storage{ BasePath: basePath, Hostname: hostname, @@ -69,18 +71,34 @@ func NewStorage(basePath string, hostname string, timeout time.Duration) (*Stora }, nil } -// ArtifactFor returns an artifact for the v1alpha1.Source. -func (s *Storage) ArtifactFor(kind string, metadata metav1.Object, fileName, revision, checksum string) sourcev1.Artifact { +// NewArtifactFor returns a new v1alpha1.Artifact. +func (s *Storage) NewArtifactFor(kind string, metadata metav1.Object, revision, fileName string) sourcev1.Artifact { path := sourcev1.ArtifactPath(kind, metadata.GetNamespace(), metadata.GetName(), fileName) - url := fmt.Sprintf("http://%s/%s", s.Hostname, path) - - return sourcev1.Artifact{ - Path: path, - URL: url, - Revision: revision, - Checksum: checksum, - LastUpdateTime: metav1.Now(), + artifact := sourcev1.Artifact{ + Path: path, + Revision: revision, } + s.SetArtifactURL(&artifact) + return artifact +} + +// SetArtifactURL sets the URL on the given v1alpha1.Artifact. +func (s Storage) SetArtifactURL(artifact *sourcev1.Artifact) { + if artifact.Path == "" { + return + } + artifact.URL = fmt.Sprintf("http://%s/%s", s.Hostname, artifact.Path) +} + +// SetHostname sets the hostname of the given URL string to the current Storage.Hostname +// and returns the result. +func (s Storage) SetHostname(URL string) string { + u, err := url.Parse(URL) + if err != nil { + return "" + } + u.Host = s.Hostname + return u.String() } // MkdirAll calls os.MkdirAll for the given v1alpha1.Artifact base dir. @@ -95,7 +113,8 @@ func (s *Storage) RemoveAll(artifact sourcev1.Artifact) error { return os.RemoveAll(dir) } -// RemoveAllButCurrent removes all files for the given artifact base dir excluding the current one +// RemoveAllButCurrent removes all files for the given v1alpha1.Artifact base dir, +// excluding the current one. func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { localPath := s.LocalPath(artifact) dir := filepath.Dir(localPath) @@ -120,8 +139,8 @@ func (s *Storage) RemoveAllButCurrent(artifact sourcev1.Artifact) error { return nil } -// ArtifactExist returns a boolean indicating whether the artifact exists in storage and is a -// regular file. +// ArtifactExist returns a boolean indicating whether the v1alpha1.Artifact exists in storage +// and is a regular file. func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { fi, err := os.Lstat(s.LocalPath(artifact)) if err != nil { @@ -130,33 +149,74 @@ func (s *Storage) ArtifactExist(artifact sourcev1.Artifact) bool { return fi.Mode().IsRegular() } -// Archive creates a tar.gz to the artifact path from the given dir excluding any VCS specific -// files and directories, or any of the excludes defined in the excludeFiles. -func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) error { - if _, err := os.Stat(dir); err != nil { - return err +// Archive atomically archives the given directory as a tarball to the given v1alpha1.Artifact +// path, excluding any VCS specific files and directories, or any of the excludes defined in +// the excludeFiles. If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) Archive(artifact *sourcev1.Artifact, dir string, spec sourcev1.GitRepositorySpec) (err error) { + if f, err := os.Stat(dir); os.IsNotExist(err) || !f.IsDir() { + return fmt.Errorf("invalid dir path: %s", dir) } ps, err := loadExcludePatterns(dir, spec) if err != nil { return err } - matcher := gitignore.NewMatcher(ps) - gzFile, err := os.Create(s.LocalPath(artifact)) + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) if err != nil { return err } - defer gzFile.Close() + tmpName := tf.Name() + defer func() { + if err != nil { + os.Remove(tmpName) + } + }() - gw := gzip.NewWriter(gzFile) - defer gw.Close() + h := newHash() + mw := io.MultiWriter(h, tf) + gw := gzip.NewWriter(mw) tw := tar.NewWriter(gw) - defer tw.Close() + if err := writeToArchiveExcludeMatches(dir, matcher, tw); err != nil { + tw.Close() + gw.Close() + tf.Close() + return err + } - return filepath.Walk(dir, func(p string, fi os.FileInfo, err error) error { + if err := tw.Close(); err != nil { + gw.Close() + tf.Close() + return err + } + if err := gw.Close(); err != nil { + tf.Close() + return err + } + if err := tf.Close(); err != nil { + return err + } + + if err := os.Chmod(tmpName, 0644); err != nil { + return err + } + + if err := fs.RenameWithFallback(tmpName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil +} + +// writeToArchiveExcludeMatches walks over the given dir and writes any regular file that does +// not match the given gitignore.Matcher. +func writeToArchiveExcludeMatches(dir string, matcher gitignore.Matcher, writer *tar.Writer) error { + fn := func(p string, fi os.FileInfo, err error) error { if err != nil { return err } @@ -187,36 +247,99 @@ func (s *Storage) Archive(artifact sourcev1.Artifact, dir string, spec sourcev1. } header.Name = relFilePath - if err := tw.WriteHeader(header); err != nil { + if err := writer.WriteHeader(header); err != nil { return err } f, err := os.Open(p) if err != nil { + f.Close() return err } - if _, err := io.Copy(tw, f); err != nil { + if _, err := io.Copy(writer, f); err != nil { f.Close() return err } return f.Close() - }) + } + return filepath.Walk(dir, fn) } -// WriteFile writes the given bytes to the artifact path if the checksum differs -func (s *Storage) WriteFile(artifact sourcev1.Artifact, data []byte) error { - localPath := s.LocalPath(artifact) - sum := s.Checksum(data) - if file, err := os.Stat(localPath); !os.IsNotExist(err) && !file.IsDir() { - if fb, err := ioutil.ReadFile(localPath); err == nil && sum == s.Checksum(fb) { - return nil +// AtomicWriteFile atomically writes the io.Reader contents to the v1alpha1.Artifact path. +// If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) AtomicWriteFile(artifact *sourcev1.Artifact, reader io.Reader, mode os.FileMode) (err error) { + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) + if err != nil { + return err + } + tfName := tf.Name() + defer func() { + if err != nil { + os.Remove(tfName) } + }() + + h := newHash() + mw := io.MultiWriter(h, tf) + + if _, err := io.Copy(mw, reader); err != nil { + tf.Close() + return err + } + if err := tf.Close(); err != nil { + return err } - return ioutil.WriteFile(localPath, data, 0644) + if err := os.Chmod(tfName, mode); err != nil { + return err + } + + if err := fs.RenameWithFallback(tfName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil } -// Symlink creates or updates a symbolic link for the given artifact +// Copy atomically copies the io.Reader contents to the v1alpha1.Artifact path. +// If successful, it sets the checksum and last update time on the artifact. +func (s *Storage) Copy(artifact *sourcev1.Artifact, reader io.Reader) (err error) { + localPath := s.LocalPath(*artifact) + tf, err := ioutil.TempFile(filepath.Split(localPath)) + if err != nil { + return err + } + tfName := tf.Name() + defer func() { + if err != nil { + os.Remove(tfName) + } + }() + + h := newHash() + mw := io.MultiWriter(h, tf) + + if _, err := io.Copy(mw, reader); err != nil { + tf.Close() + return err + } + if err := tf.Close(); err != nil { + return err + } + + if err := fs.RenameWithFallback(tfName, localPath); err != nil { + return err + } + + artifact.Checksum = fmt.Sprintf("%x", h.Sum(nil)) + artifact.LastUpdateTime = metav1.Now() + return nil +} + +// 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) { localPath := s.LocalPath(artifact) @@ -236,17 +359,18 @@ func (s *Storage) Symlink(artifact sourcev1.Artifact, linkName string) (string, return "", err } - parts := strings.Split(artifact.URL, "/") - url := strings.Replace(artifact.URL, parts[len(parts)-1], linkName, 1) + url := fmt.Sprintf("http://%s/%s", s.Hostname, filepath.Join(filepath.Dir(artifact.Path), linkName)) return url, nil } -// Checksum returns the SHA1 checksum for the given bytes as a string -func (s *Storage) Checksum(b []byte) string { - return fmt.Sprintf("%x", sha1.Sum(b)) +// Checksum returns the SHA1 checksum for the data of the given io.Reader as a string. +func (s *Storage) Checksum(reader io.Reader) string { + h := newHash() + _, _ = io.Copy(h, reader) + return fmt.Sprintf("%x", h.Sum(nil)) } -// Lock creates a file lock for the given artifact +// Lock creates a file lock for the given v1alpha1.Artifact. func (s *Storage) Lock(artifact sourcev1.Artifact) (unlock func(), err error) { lockFile := s.LocalPath(artifact) + ".lock" mutex := lockedfile.MutexAt(lockFile) @@ -262,6 +386,8 @@ func (s *Storage) LocalPath(artifact sourcev1.Artifact) string { return filepath.Join(s.BasePath, artifact.Path) } +// getPatterns collects ignore patterns from the given reader and returns them +// as a gitignore.Pattern slice. func getPatterns(reader io.Reader, path []string) []gitignore.Pattern { var ps []gitignore.Pattern scanner := bufio.NewScanner(reader) @@ -303,3 +429,8 @@ func loadExcludePatterns(dir string, spec sourcev1.GitRepositorySpec) ([]gitigno return ps, nil } + +// newHash returns a new SHA1 hash. +func newHash() hash.Hash { + return sha1.New() +} diff --git a/controllers/storage_test.go b/controllers/storage_test.go index 307f8676..a90a7229 100644 --- a/controllers/storage_test.go +++ b/controllers/storage_test.go @@ -159,7 +159,7 @@ func createArchive(t *testing.T, storage *Storage, filenames []string, sourceIgn t.Fatalf("artifact directory creation failed: %v", err) } - if err := storage.Archive(artifact, gitDir, spec); err != nil { + if err := storage.Archive(&artifact, gitDir, spec); err != nil { t.Fatalf("archiving failed: %v", err) } diff --git a/docs/api/source.md b/docs/api/source.md index b7294757..49c6bfd0 100644 --- a/docs/api/source.md +++ b/docs/api/source.md @@ -744,6 +744,18 @@ are.
observedGeneration
ObservedGeneration is the last observed generation.
+conditions
Conditions holds the conditions for the GitRepository.
observedGeneration
ObservedGeneration is the last observed generation.
+conditions
Conditions holds the conditions for the HelmChart.
observedGeneration
ObservedGeneration is the last observed generation.
+conditions
Conditions holds the conditions for the HelmRepository.