diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 647b8aa7f..c35c2c528 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -19,16 +19,24 @@ package v1beta2 const SourceFinalizer = "finalizers.fluxcd.io" const ( + // ArtifactInStorageCondition indicates the availability of the Artifact in + // the storage. + // If True, the Artifact is stored successfully. + // This Condition is only present on the resource if the Artifact is + // successfully stored. + ArtifactInStorageCondition string = "ArtifactInStorage" + // ArtifactOutdatedCondition indicates the current Artifact of the Source // is outdated. // This is a "negative polarity" or "abnormal-true" type, and is only // present on the resource if it is True. ArtifactOutdatedCondition string = "ArtifactOutdated" - // SourceVerifiedCondition indicates the integrity of the Source has been - // verified. If True, the integrity check succeeded. If False, it failed. - // The Condition is only present on the resource if the integrity has been - // verified. + // SourceVerifiedCondition indicates the integrity verification of the + // Source. + // If True, the integrity check succeeded. If False, it failed. + // This Condition is only present on the resource if the integrity check + // is enabled. SourceVerifiedCondition string = "SourceVerified" // FetchFailedCondition indicates a transient or persistent fetch failure @@ -85,4 +93,8 @@ const ( // SymlinkUpdateFailedReason signals a failure in updating a symlink. SymlinkUpdateFailedReason string = "SymlinkUpdateFailed" + + // ArtifactUpToDateReason signals that an existing Artifact is up-to-date + // with the Source. + ArtifactUpToDateReason string = "ArtifactUpToDate" ) diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index 84a4e38c5..b01236828 100644 --- a/controllers/bucket_controller.go +++ b/controllers/bucket_controller.go @@ -74,23 +74,25 @@ const maxConcurrentBucketFetches = 100 var bucketReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, meta.StalledCondition, meta.ReconcilingCondition, @@ -375,11 +377,14 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.B if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { obj.Status.Artifact = nil obj.Status.URL = "" + // Remove the condition as the artifact doesn't exist. + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) } // Record that we do not have an artifact if obj.GetArtifact() == nil { conditions.MarkReconciling(obj, "NoArtifact", "no artifact for resource in storage") + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) return sreconcile.ResultSuccess, nil } @@ -510,18 +515,18 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1. // Create artifact artifact := r.Storage.NewArtifactFor(obj.Kind, obj, revision, fmt.Sprintf("%s.tar.gz", revision)) - // Always restore the Ready condition in case it got removed due to a transient error + // Set the ArtifactInStorageCondition if there's no drift. defer func() { if obj.GetArtifact().HasRevision(artifact.Revision) { conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) - conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision '%s'", artifact.Revision) } }() // The artifact is up-to-date if obj.GetArtifact().HasRevision(artifact.Revision) { - ctrl.LoggerFrom(ctx).Info("artifact up-to-date", "revision", artifact.Revision) + r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision) return sreconcile.ResultSuccess, nil } diff --git a/controllers/bucket_controller_test.go b/controllers/bucket_controller_test.go index 0732f1f2b..2f432a4bb 100644 --- a/controllers/bucket_controller_test.go +++ b/controllers/bucket_controller_test.go @@ -38,12 +38,14 @@ import ( "k8s.io/client-go/tools/record" kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" gcsmock "github.com/fluxcd/source-controller/internal/mock/gcs" s3mock "github.com/fluxcd/source-controller/internal/mock/s3" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) // Environment variable to set the GCP Storage host for the GCP client. @@ -886,13 +888,13 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) { assertConditions []metav1.Condition }{ { - name: "Archiving artifact to storage makes Ready=True", + name: "Archiving artifact to storage makes ArtifactInStorage=True", beforeFunc: func(t *WithT, obj *sourcev1.Bucket, index *etagIndex, dir string) { obj.Spec.Interval = metav1.Duration{Duration: interval} }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), }, }, { @@ -909,7 +911,7 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), }, }, { @@ -920,7 +922,7 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), }, }, { @@ -937,7 +939,7 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"), }, }, { @@ -1070,3 +1072,94 @@ func Test_etagIndex_Revision(t *testing.T) { }) } } + +func TestBucketReconciler_statusConditions(t *testing.T) { + tests := []struct { + name string + beforeFunc func(obj *sourcev1.Bucket) + assertConditions []metav1.Condition + }{ + { + name: "positive conditions only", + beforeFunc: func(obj *sourcev1.Bucket) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + { + name: "multiple failures", + beforeFunc: func(obj *sourcev1.Bucket) { + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory") + conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"), + }, + }, + { + name: "mixed positive and negative conditions", + beforeFunc: func(obj *sourcev1.Bucket) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &sourcev1.Bucket{ + TypeMeta: metav1.TypeMeta{ + Kind: sourcev1.BucketKind, + APIVersion: "source.toolkit.fluxcd.io/v1beta2", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "bucket", + Namespace: "foo", + }, + } + clientBuilder := fake.NewClientBuilder() + clientBuilder.WithObjects(obj) + c := clientBuilder.Build() + + patchHelper, err := patch.NewHelper(obj, c) + g.Expect(err).ToNot(HaveOccurred()) + + if tt.beforeFunc != nil { + tt.beforeFunc(obj) + } + + ctx := context.TODO() + recResult := sreconcile.ResultSuccess + var retErr error + + summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper) + summarizeOpts := []summarize.Option{ + summarize.WithConditions(bucketReadyCondition), + summarize.WithReconcileResult(recResult), + summarize.WithReconcileError(retErr), + summarize.WithIgnoreNotFound(), + summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}), + summarize.WithPatchFieldOwner("source-controller"), + } + _, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...) + + key := client.ObjectKeyFromObject(obj) + g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred()) + g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 9c9189ca8..2aa0f8589 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -61,28 +61,30 @@ import ( var gitRepositoryReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.SourceVerifiedCondition, - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.IncludeUnavailableCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, + sourcev1.SourceVerifiedCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.IncludeUnavailableCondition, - sourcev1.SourceVerifiedCondition, - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.IncludeUnavailableCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, + sourcev1.SourceVerifiedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ + sourcev1.StorageOperationFailedCondition, sourcev1.FetchFailedCondition, sourcev1.IncludeUnavailableCondition, - sourcev1.StorageOperationFailedCondition, sourcev1.ArtifactOutdatedCondition, meta.StalledCondition, meta.ReconcilingCondition, @@ -279,11 +281,14 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { obj.Status.Artifact = nil obj.Status.URL = "" + // Remove the condition as the artifact doesn't exist. + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) } // Record that we do not have an artifact if obj.GetArtifact() == nil { conditions.MarkReconciling(obj, "NoArtifact", "no artifact for resource in storage") + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) return sreconcile.ResultSuccess, nil } @@ -446,18 +451,18 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, // Create potential new artifact with current available metadata artifact := r.Storage.NewArtifactFor(obj.Kind, obj.GetObjectMeta(), commit.String(), fmt.Sprintf("%s.tar.gz", commit.Hash.String())) - // Always restore the Ready condition in case it got removed due to a transient error + // Set the ArtifactInStorageCondition if there's no drift. defer func() { if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) { conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) - conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision '%s'", artifact.Revision) } }() // The artifact is up-to-date if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) { - ctrl.LoggerFrom(ctx).Info("artifact up-to-date", "revision", artifact.Revision) + r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision) return sreconcile.ResultSuccess, nil } diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 7b6aeba35..88fceb7e7 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -51,11 +51,13 @@ import ( kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/reconcile/summarize" "github.com/fluxcd/source-controller/pkg/git" ) @@ -706,7 +708,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { assertConditions []metav1.Condition }{ { - name: "Archiving artifact to storage makes Ready=True", + name: "Archiving artifact to storage makes ArtifactInStorage=True", dir: "testdata/git/repository", beforeFunc: func(obj *sourcev1.GitRepository) { obj.Spec.Interval = metav1.Duration{Duration: interval} @@ -717,11 +719,11 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { - name: "Archiving artifact to storage with includes makes Ready=True", + name: "Archiving artifact to storage with includes makes ArtifactInStorage=True", dir: "testdata/git/repository", includes: artifactSet{&sourcev1.Artifact{Revision: "main/revision"}}, beforeFunc: func(obj *sourcev1.GitRepository) { @@ -735,7 +737,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -752,7 +754,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -768,7 +770,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -783,7 +785,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -800,7 +802,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -820,7 +822,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'main/revision'"), }, }, { @@ -1171,7 +1173,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { }, }, { - name: "Invalid commit makes SourceVerifiedCondition=False and returns error", + name: "Invalid commit sets no SourceVerifiedCondition and returns error", secret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "existing", @@ -1197,7 +1199,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) { }, }, { - name: "Secret get failure makes SourceVerified=False and returns error", + name: "Secret get failure sets no SourceVerifiedCondition and returns error", beforeFunc: func(obj *sourcev1.GitRepository) { obj.Spec.Interval = metav1.Duration{Duration: interval} obj.Spec.Verification = &sourcev1.GitRepositoryVerification{ @@ -1289,10 +1291,11 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { assertConditions []metav1.Condition }{ { - name: "no condition", + name: "no failure condition", want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, { @@ -1303,6 +1306,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, { @@ -1313,6 +1317,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, { @@ -1326,6 +1331,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, { @@ -1337,6 +1343,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, { @@ -1348,6 +1355,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { want: ctrl.Result{RequeueAfter: interval}, assertConditions: []metav1.Condition{ *conditions.TrueCondition(meta.ReadyCondition, "Succeeded", "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, "Succeeded", "stored artifact for revision"), }, }, } @@ -1531,3 +1539,98 @@ func remoteTagForHead(repo *gogit.Repository, head *plumbing.Reference, tag stri RefSpecs: []config.RefSpec{config.RefSpec(refSpec)}, }) } + +func TestGitRepositoryReconciler_statusConditions(t *testing.T) { + tests := []struct { + name string + beforeFunc func(obj *sourcev1.GitRepository) + assertConditions []metav1.Condition + }{ + { + name: "multiple positive conditions", + beforeFunc: func(obj *sourcev1.GitRepository) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of commit") + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of commit"), + }, + }, + { + name: "multiple failures", + beforeFunc: func(obj *sourcev1.GitRepository) { + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + conditions.MarkTrue(obj, sourcev1.IncludeUnavailableCondition, "IllegalPath", "some error") + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory") + conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.IncludeUnavailableCondition, "IllegalPath", "some error"), + *conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"), + }, + }, + { + name: "mixed positive and negative conditions", + beforeFunc: func(obj *sourcev1.GitRepository) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &sourcev1.GitRepository{ + TypeMeta: metav1.TypeMeta{ + Kind: sourcev1.GitRepositoryKind, + APIVersion: "source.toolkit.fluxcd.io/v1beta2", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "gitrepo", + Namespace: "foo", + }, + } + clientBuilder := fake.NewClientBuilder() + clientBuilder.WithObjects(obj) + c := clientBuilder.Build() + + patchHelper, err := patch.NewHelper(obj, c) + g.Expect(err).ToNot(HaveOccurred()) + + if tt.beforeFunc != nil { + tt.beforeFunc(obj) + } + + ctx := context.TODO() + recResult := sreconcile.ResultSuccess + var retErr error + + summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper) + summarizeOpts := []summarize.Option{ + summarize.WithConditions(gitRepositoryReadyCondition), + summarize.WithReconcileResult(recResult), + summarize.WithReconcileError(retErr), + summarize.WithIgnoreNotFound(), + summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}), + summarize.WithPatchFieldOwner("source-controller"), + } + _, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...) + + key := client.ObjectKeyFromObject(obj) + g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred()) + g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 3fa0c0271..332e0ca4a 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -69,26 +69,28 @@ import ( var helmChartReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.BuildFailedCondition, - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.BuildFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.BuildFailedCondition, - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.BuildFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ - sourcev1.BuildFailedCondition, - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.BuildFailedCondition, sourcev1.ArtifactOutdatedCondition, meta.StalledCondition, meta.ReconcilingCondition, @@ -284,11 +286,14 @@ func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, obj *sourcev if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { obj.Status.Artifact = nil obj.Status.URL = "" + // Remove the condition as the artifact doesn't exist. + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) } // Record that we do not have an artifact if obj.GetArtifact() == nil { conditions.MarkReconciling(obj, "NoArtifact", "no artifact for resource in storage") + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) return sreconcile.ResultSuccess, nil } @@ -620,11 +625,11 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source return sreconcile.ResultRequeue, nil } - // Always restore the conditions in case they got overwritten by transient errors + // Set the ArtifactInStorageCondition if there's no drift. defer func() { if obj.Status.ObservedChartName == b.Name && obj.GetArtifact().HasRevision(b.Version) { conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) - conditions.MarkTrue(obj, meta.ReadyCondition, reasonForBuild(b), b.Summary()) + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, reasonForBuild(b), b.Summary()) } }() @@ -633,7 +638,7 @@ func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *source // Return early if the build path equals the current artifact path if curArtifact := obj.GetArtifact(); curArtifact != nil && r.Storage.LocalPath(*curArtifact) == b.Path { - ctrl.LoggerFrom(ctx).Info("artifact up-to-date", "revision", artifact.Revision) + r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision) return sreconcile.ResultSuccess, nil } diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go index 43d568b85..522908c32 100644 --- a/controllers/helmchart_controller_test.go +++ b/controllers/helmchart_controller_test.go @@ -50,6 +50,7 @@ import ( serror "github.com/fluxcd/source-controller/internal/error" "github.com/fluxcd/source-controller/internal/helm/chart" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) func TestHelmChartReconciler_Reconcile(t *testing.T) { @@ -981,7 +982,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, }, { - name: "Copying artifact to storage from build makes Ready=True", + name: "Copying artifact to storage from build makes ArtifactInStorage=True", build: mockChartBuild("helmchart", "0.1.0", "testdata/charts/helmchart-0.1.0.tgz"), beforeFunc: func(obj *sourcev1.HelmChart) { conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "") @@ -995,7 +996,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), }, }, { @@ -1038,7 +1039,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { t.Expect(obj.Status.URL).To(BeEmpty()) }, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPackageSucceededReason, "packaged 'helmchart' chart with version '0.1.0'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, sourcev1.ChartPackageSucceededReason, "packaged 'helmchart' chart with version '0.1.0'"), }, }, { @@ -1056,7 +1057,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), }, }, { @@ -1073,7 +1074,7 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, sourcev1.ChartPullSucceededReason, "pulled 'helmchart' chart with version '0.1.0'"), }, }, } @@ -1483,3 +1484,96 @@ func mockChartBuild(name, version, path string) *chart.Build { Path: copyP, } } + +func TestHelmChartReconciler_statusConditions(t *testing.T) { + tests := []struct { + name string + beforeFunc func(obj *sourcev1.HelmChart) + assertConditions []metav1.Condition + }{ + { + name: "positive conditions only", + beforeFunc: func(obj *sourcev1.HelmChart) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + { + name: "multiple failures", + beforeFunc: func(obj *sourcev1.HelmChart) { + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory") + conditions.MarkTrue(obj, sourcev1.BuildFailedCondition, "ChartPackageError", "some error") + conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.BuildFailedCondition, "ChartPackageError", "some error"), + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"), + }, + }, + { + name: "mixed positive and negative conditions", + beforeFunc: func(obj *sourcev1.HelmChart) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &sourcev1.HelmChart{ + TypeMeta: metav1.TypeMeta{ + Kind: sourcev1.HelmChartKind, + APIVersion: "source.toolkit.fluxcd.io/v1beta2", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "helmchart", + Namespace: "foo", + }, + } + clientBuilder := fake.NewClientBuilder() + clientBuilder.WithObjects(obj) + c := clientBuilder.Build() + + patchHelper, err := patch.NewHelper(obj, c) + g.Expect(err).ToNot(HaveOccurred()) + + if tt.beforeFunc != nil { + tt.beforeFunc(obj) + } + + ctx := context.TODO() + recResult := sreconcile.ResultSuccess + var retErr error + + summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper) + summarizeOpts := []summarize.Option{ + summarize.WithConditions(helmChartReadyCondition), + summarize.WithReconcileResult(recResult), + summarize.WithReconcileError(retErr), + summarize.WithIgnoreNotFound(), + summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}), + summarize.WithPatchFieldOwner("source-controller"), + } + _, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...) + + key := client.ObjectKeyFromObject(obj) + g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred()) + g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 618cd35a6..cbad94102 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -57,23 +57,25 @@ import ( var helmRepositoryReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ - sourcev1.FetchFailedCondition, sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, sourcev1.ArtifactOutdatedCondition, meta.StalledCondition, meta.ReconcilingCondition, @@ -245,11 +247,14 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *so if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) { obj.Status.Artifact = nil obj.Status.URL = "" + // Remove the condition as the artifact doesn't exist. + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) } // Record that we do not have an artifact if obj.GetArtifact() == nil { conditions.MarkReconciling(obj, "NoArtifact", "no artifact for resource in storage") + conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) return sreconcile.ResultSuccess, nil } @@ -392,11 +397,11 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou // On a successful archive, the Artifact in the Status of the object is set, // and the symlink in the Storage is updated to its path. func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) { - // Always restore the Ready condition in case it got removed due to a transient error. + // Set the ArtifactInStorageCondition if there's no drift. defer func() { if obj.GetArtifact().HasRevision(artifact.Revision) { conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition) - conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision '%s'", artifact.Revision) } @@ -406,7 +411,7 @@ func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *s }() if obj.GetArtifact().HasRevision(artifact.Revision) { - ctrl.LoggerFrom(ctx).Info("artifact up-to-date", "revision", artifact.Revision) + r.eventLogf(ctx, obj, events.EventTypeTrace, sourcev1.ArtifactUpToDateReason, "artifact up-to-date with remote revision: '%s'", artifact.Revision) return sreconcile.ResultSuccess, nil } diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 83cd57bb2..95b770915 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -37,11 +37,13 @@ import ( "k8s.io/client-go/tools/record" kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/helm/repository" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" + "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) func TestHelmRepositoryReconciler_Reconcile(t *testing.T) { @@ -515,13 +517,13 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { assertConditions []metav1.Condition }{ { - name: "Archiving artifact to storage makes Ready=True", + name: "Archiving artifact to storage makes ArtifactInStorage=True", beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, index *repository.ChartRepository) { obj.Spec.Interval = metav1.Duration{Duration: interval} }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), }, }, { @@ -535,7 +537,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), }, }, { @@ -546,7 +548,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), }, }, { @@ -563,7 +565,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) { }, want: sreconcile.ResultSuccess, assertConditions: []metav1.Condition{ - *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"), }, }, } @@ -744,3 +746,94 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) { }) } } + +func TestHelmRepositoryReconciler_statusConditions(t *testing.T) { + tests := []struct { + name string + beforeFunc func(obj *sourcev1.HelmRepository) + assertConditions []metav1.Condition + }{ + { + name: "positive conditions only", + beforeFunc: func(obj *sourcev1.HelmRepository) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + }, + assertConditions: []metav1.Condition{ + *conditions.TrueCondition(meta.ReadyCondition, meta.SucceededReason, "stored artifact for revision"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + { + name: "multiple failures", + beforeFunc: func(obj *sourcev1.HelmRepository) { + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + conditions.MarkTrue(obj, sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory") + conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.DirCreationFailedReason, "failed to create directory"), + *conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "some error"), + }, + }, + { + name: "mixed positive and negative conditions", + beforeFunc: func(obj *sourcev1.HelmRepository) { + conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision") + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret") + }, + assertConditions: []metav1.Condition{ + *conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret"), + *conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision"), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + obj := &sourcev1.HelmRepository{ + TypeMeta: metav1.TypeMeta{ + Kind: sourcev1.HelmRepositoryKind, + APIVersion: "source.toolkit.fluxcd.io/v1beta2", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "helmrepo", + Namespace: "foo", + }, + } + clientBuilder := fake.NewClientBuilder() + clientBuilder.WithObjects(obj) + c := clientBuilder.Build() + + patchHelper, err := patch.NewHelper(obj, c) + g.Expect(err).ToNot(HaveOccurred()) + + if tt.beforeFunc != nil { + tt.beforeFunc(obj) + } + + ctx := context.TODO() + recResult := sreconcile.ResultSuccess + var retErr error + + summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper) + summarizeOpts := []summarize.Option{ + summarize.WithConditions(helmRepositoryReadyCondition), + summarize.WithReconcileResult(recResult), + summarize.WithReconcileError(retErr), + summarize.WithIgnoreNotFound(), + summarize.WithResultBuilder(sreconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.GetRequeueAfter()}), + summarize.WithPatchFieldOwner("source-controller"), + } + _, retErr = summarizeHelper.SummarizeAndPatch(ctx, obj, summarizeOpts...) + + key := client.ObjectKeyFromObject(obj) + g.Expect(c.Get(ctx, key, obj)).ToNot(HaveOccurred()) + g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions)) + }) + } +} diff --git a/docs/spec/v1beta2/buckets.md b/docs/spec/v1beta2/buckets.md index 7fc630989..196c9d617 100644 --- a/docs/spec/v1beta2/buckets.md +++ b/docs/spec/v1beta2/buckets.md @@ -93,6 +93,12 @@ control over. Reason: Succeeded Status: True Type: Ready + Last Transition Time: 2022-02-01T23:43:38Z + Message: stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' + Observed Generation: 1 + Reason: Succeeded + Status: True + Type: ArtifactInStorage Observed Generation: 1 URL: http://source-controller.source-system.svc.cluster.local./bucket/default/minio-bucket/latest.tar.gz Events: @@ -780,6 +786,7 @@ lists ```console LAST SEEN TYPE REASON OBJECT MESSAGE 2m30s Normal NewArtifact bucket/ fetched 16 files with revision from 'my-new-bucket' +36s Normal ArtifactUpToDate bucket/ artifact up-to-date with remote revision: 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' 18s Warning BucketOperationFailed bucket/ bucket 'my-new-bucket' does not exist ``` @@ -892,6 +899,17 @@ This `Ready` Condition will retain a status value of `"True"` until the Bucket is marked as [reconciling](#reconciling-bucket), or e.g. a [transient error](#failed-bucket) occurs due to a temporary network issue. +When the Bucket Artifact is archived in the controller's Artifact +storage, the controller sets a Condition with the following attributes in the +Bucket's `.status.conditions`: + +- `type: ArtifactInStorage` +- `status: "True"` +- `reason: Succeeded` + +This `ArtifactInStorage` Condition will retain a status value of `"True"` until +the Artifact in the storage no longer exists. + #### Failed Bucket The source-controller may get stuck trying to produce an Artifact for a Bucket diff --git a/docs/spec/v1beta2/gitrepositories.md b/docs/spec/v1beta2/gitrepositories.md index 720be7fe0..e922eb131 100644 --- a/docs/spec/v1beta2/gitrepositories.md +++ b/docs/spec/v1beta2/gitrepositories.md @@ -71,6 +71,12 @@ You can run this example by saving the manifest into `gitrepository.yaml`. Reason: Succeeded Status: True Type: Ready + Last Transition Time: 2022-02-14T11:23:36Z + Message: stored artifact for revision 'master/132f4e719209eb10b9485302f8593fc0e680f4fc' + Observed Generation: 1 + Reason: Succeeded + Status: True + Type: ArtifactInStorage Observed Generation: 1 URL: http://source-controller.source-system.svc.cluster.local./gitrepository/default/podinfo/latest.tar.gz Events: @@ -647,6 +653,7 @@ lists ```console LAST SEEN TYPE REASON OBJECT MESSAGE 2m14s Normal NewArtifact gitrepository/ stored artifact for commit 'Merge pull request #160 from stefanprodan/release-6.0.3' +36s Normal ArtifactUpToDate gitrepository/ artifact up-to-date with remote revision: 'master/132f4e719209eb10b9485302f8593fc0e680f4fc' 94s Warning GitOperationFailed gitrepository/ failed to checkout and determine revision: unable to clone 'https://github.com/stefanprodan/podinfo': couldn't find remote ref "refs/heads/invalid" ``` @@ -760,6 +767,17 @@ This `Ready` Condition will retain a status value of `"True"` until the GitRepository is marked as [reconciling](#reconciling-gitrepository), or e.g. a [transient error](#failed-gitrepository) occurs due to a temporary network issue. +When the GitRepository Artifact is archived in the controller's Artifact +storage, the controller sets a Condition with the following attributes in the +GitRepository's `.status.conditions`: + +- `type: ArtifactInStorage` +- `status: "True"` +- `reason: Succeeded` + +This `ArtifactInStorage` Condition will retain a status value of `"True"` until +the Artifact in the storage no longer exists. + #### Failed GitRepository The source-controller may get stuck trying to produce an Artifact for a diff --git a/docs/spec/v1beta2/helmcharts.md b/docs/spec/v1beta2/helmcharts.md index b3f118ab6..8f8f9800a 100644 --- a/docs/spec/v1beta2/helmcharts.md +++ b/docs/spec/v1beta2/helmcharts.md @@ -79,6 +79,12 @@ helm-controller. Reason: ChartPullSucceeded Status: True Type: Ready + Last Transition Time: 2022-02-13T11:24:10Z + Message: pulled 'podinfo' chart with version '5.2.1' + Observed Generation: 1 + Reason: ChartPullSucceeded + Status: True + Type: ArtifactInStorage Observed Chart Name: podinfo Observed Generation: 1 URL: http://source-controller.flux-system.svc.cluster.local./helmchart/default/podinfo/latest.tar.gz @@ -377,6 +383,7 @@ lists LAST SEEN TYPE REASON OBJECT MESSAGE 22s Warning InvalidChartReference helmchart/ invalid chart reference: failed to get chart version for remote reference: no 'podinfo' chart with version matching '9.*' found 2s Normal ChartPullSucceeded helmchart/ pulled 'podinfo' chart with version '6.0.3' +2s Normal ArtifactUpToDate helmchart/ artifact up-to-date with remote revision: '6.0.3' ``` Besides being reported in Events, the reconciliation errors are also logged by @@ -522,6 +529,17 @@ This `Ready` Condition will retain a status value of `"True"` until the HelmChart is marked as [reconciling](#reconciling-helmchart), or e.g. a [transient error](#failed-helmchart) occurs due to a temporary network issue. +When the HelmChart Artifact is archived in the controller's Artifact +storage, the controller sets a Condition with the following attributes in the +HelmChart's `.status.conditions`: + +- `type: ArtifactInStorage` +- `status: "True"` +- `reason: Succeeded` + +This `ArtifactInStorage` Condition will retain a status value of `"True"` until +the Artifact in the storage no longer exists. + #### Failed HelmChart The source-controller may get stuck trying to produce an Artifact for a diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index b3ef08f66..f4dd41dfd 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -69,6 +69,12 @@ You can run this example by saving the manifest into `helmrepository.yaml`. Reason: Succeeded Status: True Type: Ready + Last Transition Time: 2022-02-04T09:55:58Z + Message: stored artifact for revision '83a3c595163a6ff0333e0154c790383b5be441b9db632cb36da11db1c4ece111' + Observed Generation: 1 + Reason: Succeeded + Status: True + Type: ArtifactInStorage Observed Generation: 1 URL: http://source-controller.flux-system.svc.cluster.local./helmrepository/default/podinfo/index.yaml Events: @@ -359,9 +365,10 @@ kubectl get events --field-selector involvedObject.kind=HelmRepository,involvedO lists ```console -LAST SEEN TYPE REASON OBJECT MESSAGE -107s Warning Failed helmrepository/ failed to construct Helm client: scheme "invalid" not supported -7s Normal NewArtifact helmrepository/ fetched index of size 30.88kB from 'https://stefanprodan.github.io/podinfo' +LAST SEEN TYPE REASON OBJECT MESSAGE +107s Warning Failed helmrepository/ failed to construct Helm client: scheme "invalid" not supported +7s Normal NewArtifact helmrepository/ fetched index of size 30.88kB from 'https://stefanprodan.github.io/podinfo' +3s Normal ArtifactUpToDate helmrepository/ artifact up-to-date with remote revision: '83a3c595163a6ff0333e0154c790383b5be441b9db632cb36da11db1c4ece111' ``` Besides being reported in Events, the reconciliation errors are also logged by @@ -464,6 +471,17 @@ HelmRepository is marked as [reconciling](#reconciling-helmrepository), or e.g. a [transient error](#failed-helmrepository) occurs due to a temporary network issue. +When the HelmRepository Artifact is archived in the controller's Artifact +storage, the controller sets a Condition with the following attributes in the +HelmRepository's `.status.conditions`: + +- `type: ArtifactInStorage` +- `status: "True"` +- `reason: Succeeded` + +This `ArtifactInStorage` Condition will retain a status value of `"True"` until +the Artifact in the storage no longer exists. + #### Failed HelmRepository The source-controller may get stuck trying to produce an Artifact for a diff --git a/go.mod b/go.mod index d49ff728b..0ee8fafc4 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/fluxcd/pkg/gitutil v0.1.0 github.com/fluxcd/pkg/helmtestserver v0.7.1 github.com/fluxcd/pkg/lockedfile v0.1.0 - github.com/fluxcd/pkg/runtime v0.13.2 + github.com/fluxcd/pkg/runtime v0.13.4 github.com/fluxcd/pkg/ssh v0.3.2 github.com/fluxcd/pkg/testserver v0.2.0 github.com/fluxcd/pkg/untar v0.1.0 diff --git a/go.sum b/go.sum index b08cc4fd0..4343a6581 100644 --- a/go.sum +++ b/go.sum @@ -365,8 +365,8 @@ github.com/fluxcd/pkg/helmtestserver v0.7.1/go.mod h1:ULIZt2ozO36FLfvjABUwHJn5Ex github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w8RB5eo= github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8= github.com/fluxcd/pkg/runtime v0.13.0-rc.6/go.mod h1:4oKUO19TeudXrnCRnxCfMSS7EQTYpYlgfXwlQuDJ/Eg= -github.com/fluxcd/pkg/runtime v0.13.2 h1:6jkQQUbp17WxHsbozlJFCvHmOS4JIB+yB20CdCd8duE= -github.com/fluxcd/pkg/runtime v0.13.2/go.mod h1:dzWNKqFzFXeittbpFcJzR3cdC9CWlbzw+pNOgaVvF/0= +github.com/fluxcd/pkg/runtime v0.13.4 h1:RJSO+jmAlr6aF5Mia7zZTUrysoRjFSjjuuSTbFURbxg= +github.com/fluxcd/pkg/runtime v0.13.4/go.mod h1:dzWNKqFzFXeittbpFcJzR3cdC9CWlbzw+pNOgaVvF/0= github.com/fluxcd/pkg/ssh v0.3.2 h1:HZlDF6Qu4yplsU4Tisv6hxsRIbIOwwr7rKus8/Q/Dn0= github.com/fluxcd/pkg/ssh v0.3.2/go.mod h1:OVnuv9y2WCx7AoOIid0sxqe9lLKKfDS4PMl+4ta5DIo= github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4= @@ -1151,7 +1151,6 @@ golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= golang.org/x/crypto v0.0.0-20211117183948-ae814b36b871/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220214200702-86341886e292/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= -golang.org/x/crypto v0.0.0-20220315160706-3147a52a75dd h1:XcWmESyNjXJMLahc3mqVQJcgSTDxFxhETVlfk9uGc38= golang.org/x/crypto v0.0.0-20220315160706-3147a52a75dd/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.0.0-20220321153916-2c7772ba3064 h1:S25/rfnfsMVgORT4/J61MJ7rdyseOZOyvLIrZEZ7s6s= golang.org/x/crypto v0.0.0-20220321153916-2c7772ba3064/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=