diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 647b8aa7..4425cddf 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 diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 9c9189ca..2460df32 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,11 +451,11 @@ 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) } }() diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 7b6aeba3..88fceb7e 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/go.mod b/go.mod index d49ff728..0ee8fafc 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 b08cc4fd..4343a658 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=