fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #974 from fluxcd/progressive-status 

Introduce Progressive status
This commit is contained in:
Sunny 2023-01-10 15:13:50 +05:30 committed by GitHub
parent c8feb3ac17 197a03b989
commit 802193c4e6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 1302 additions and 351 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
controllers/bucket_controller.go
View File

@ -47,6 +47,7 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/sourceignore"
@ -119,6 +120,8 @@ type BucketReconciler struct {
Storage *Storage
ControllerName string
patchOptions []patch.Option
}
type BucketReconcilerOptions struct {
@ -151,7 +154,7 @@ type BucketProvider interface {
// bucketReconcileFunc is the function type for all the v1beta2.Bucket
// (sub)reconcile functions. The type implementations are grouped and
// executed serially to perform the complete reconcile of the object.
type bucketReconcileFunc func(ctx context.Context, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error)
type bucketReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error)
// etagIndex is an index of storage object keys and their Etag values.
type etagIndex struct {
@ -234,6 +237,8 @@ func (r *BucketReconciler) SetupWithManager(mgr ctrl.Manager) error {
}
func (r *BucketReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts BucketReconcilerOptions) error {
r.patchOptions = getPatchOptions(bucketReadyCondition.Owned, r.ControllerName)
return ctrl.NewControllerManagedBy(mgr).
For(&sourcev1.Bucket{}).
WithEventFilter(predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{})).
@ -259,10 +264,7 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
@ -270,7 +272,7 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
// Always attempt to patch the object and status after each reconciliation
// NOTE: The final runtime result and error are set in this block.
defer func() {
summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(bucketReadyCondition),
summarize.WithReconcileResult(recResult),
@ -316,19 +318,35 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res
r.reconcileSource,
r.reconcileArtifact,
}
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
return
}
// reconcile iterates through the bucketReconcileFunc tasks for the
// object. It returns early on the first call that returns
// reconcile.ResultRequeue, or produces an error.
func (r *BucketReconciler) reconcile(ctx context.Context, obj *sourcev1.Bucket, reconcilers []bucketReconcileFunc) (sreconcile.Result, error) {
func (r *BucketReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, reconcilers []bucketReconcileFunc) (sreconcile.Result, error) {
oldObj := obj.DeepCopy()
// Mark as reconciling if generation differs.
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var recAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
recAtVal = v
}
// Persist reconciling if generation differs or reconciliation is requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
case recAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
// Create temp working dir
@ -356,7 +374,7 @@ func (r *BucketReconciler) reconcile(ctx context.Context, obj *sourcev1.Bucket,
)
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, index, tmpDir)
recResult, err := rec(ctx, sp, obj, index, tmpDir)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -421,22 +439,31 @@ func (r *BucketReconciler) notify(ctx context.Context, oldObj, newObj *sourcev1.
// condition is added.
// The hostname of any URL in the Status of the object are updated, to ensure
// they match the Storage server hostname of current runtime.
func (r *BucketReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.Bucket, _ *etagIndex, _ string) (sreconcile.Result, error) {
func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, _ *etagIndex, _ string) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
// Determine if the advertised artifact is still in storage
var artifactMissing bool
if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
obj.Status.Artifact = nil
obj.Status.URL = ""
artifactMissing = true
// 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")
msg := "building artifact"
if artifactMissing {
msg += ": disappeared from storage"
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
return sreconcile.ResultSuccess, nil
}
@ -453,7 +480,7 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.B
// When a SecretRef is defined, it attempts to fetch the Secret before calling
// the provider. If this fails, it records v1beta2.FetchFailedCondition=True on
// the object and returns early.
func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error) {
func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error) {
secret, err := r.getBucketSecret(ctx, obj)
if err != nil {
e := &serror.Event{Err: err, Reason: sourcev1.AuthenticationFailedReason}
@ -528,8 +555,14 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
if !obj.GetArtifact().HasRevision(revision) {
message := fmt.Sprintf("new upstream revision '%s'", revision)
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
conditions.MarkReconciling(obj, "NewRevision", message)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
return
}
}
}()
@ -554,7 +587,7 @@ func (r *BucketReconciler) reconcileSource(ctx context.Context, obj *sourcev1.Bu
// early.
// 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 *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error) {
func (r *BucketReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, index *etagIndex, dir string) (sreconcile.Result, error) {
// Calculate revision
revision, err := index.Revision()
if err != nil {
@ -572,7 +605,7 @@ func (r *BucketReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.
if obj.GetArtifact().HasRevision(artifact.Revision) {
conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
"stored artifact for revision '%s'", artifact.Revision)
"stored artifact: revision '%s'", artifact.Revision)
}
}()

257
controllers/bucket_controller_test.go
View File

@ -185,6 +185,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
}
}
testStorage.SetArtifactURL(obj.Status.Artifact)
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
assertArtifact: &sourcev1.Artifact{
@ -201,6 +202,17 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/a.txt",
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "build artifact first time",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
},
},
{
name: "notices missing artifact in storage",
@ -217,7 +229,8 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/invalid.txt",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
},
},
{
@ -235,6 +248,7 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
return err
}
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
want: sreconcile.ResultSuccess,
@ -248,6 +262,9 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
URL: testStorage.Hostname + "/reconcile-storage/hostname.txt",
Size: int64p(int64(len("file"))),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
for _, tt := range tests {
@ -259,22 +276,31 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
}()
r := &BucketReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"),
}
obj := &sourcev1.Bucket{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Generation: 1,
},
}
if tt.beforeFunc != nil {
g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed())
}
index := newEtagIndex()
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
got, err := r.reconcileStorage(context.TODO(), obj, index, "")
index := newEtagIndex()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileStorage(context.TODO(), sp, obj, index, "")
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -292,6 +318,10 @@ func TestBucketReconciler_reconcileStorage(t *testing.T) {
}
g.Expect(absoluteP).NotTo(BeAnExistingFile())
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -327,8 +357,8 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
// TODO(hidde): middleware for mock server
@ -343,11 +373,15 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
obj.Spec.SecretRef = &meta.LocalObjectReference{
Name: "dummy",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/dummy': secrets \"dummy\" not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -362,11 +396,15 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
obj.Spec.SecretRef = &meta.LocalObjectReference{
Name: "dummy",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "invalid 'dummy' secret data: required fields 'accesskey' and 'secretkey'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -374,11 +412,15 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
bucketName: "dummy",
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Spec.BucketName = "invalid"
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "bucket 'invalid' not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -386,11 +428,15 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Spec.Endpoint = "transient.example.com"
obj.Spec.BucketName = "unavailable"
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "failed to confirm existence of 'unavailable' bucket"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -423,8 +469,8 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
},
},
{
@ -462,8 +508,8 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
},
},
{
@ -473,6 +519,8 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: "b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
bucketObjects: []*s3mock.Object{
{
@ -488,7 +536,10 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "Removes FetchFailedCondition after reconciling source",
@ -510,9 +561,38 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
{
name: "Existing artifact makes ArtifactOutdated=True",
bucketName: "dummy",
bucketObjects: []*s3mock.Object{
{
Key: "test.txt",
Content: []byte("test"),
ContentType: "text/plain",
LastModified: time.Now(),
},
},
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Status.Artifact = &sourcev1.Artifact{
Path: "some-path",
Revision: "some-rev",
}
},
want: sreconcile.ResultSuccess,
assertIndex: &etagIndex{
index: map[string]string{
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
}
@ -528,6 +608,7 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Client: builder.Build(),
Storage: testStorage,
patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"),
}
tmpDir := t.TempDir()
@ -536,7 +617,8 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
Kind: sourcev1.BucketKind,
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-bucket",
Name: "test-bucket",
Generation: 1,
},
Spec: sourcev1.BucketSpec{
Timeout: &metav1.Duration{Duration: timeout},
@ -563,14 +645,24 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
tt.beforeFunc(obj)
}
index := newEtagIndex()
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
got, err := r.reconcileSource(context.TODO(), obj, index, tmpDir)
index := newEtagIndex()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(context.TODO(), sp, obj, index, tmpDir)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(index.Index()).To(Equal(tt.assertIndex.Index()))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -620,8 +712,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
{
@ -631,12 +723,16 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
obj.Spec.SecretRef = &meta.LocalObjectReference{
Name: "dummy",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/dummy': secrets \"dummy\" not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -651,12 +747,16 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
obj.Spec.SecretRef = &meta.LocalObjectReference{
Name: "dummy",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "invalid 'dummy' secret data: required fields"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -664,12 +764,16 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
bucketName: "dummy",
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Spec.BucketName = "invalid"
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "bucket 'invalid' not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -677,12 +781,16 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Spec.Endpoint = "transient.example.com"
obj.Spec.BucketName = "unavailable"
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertIndex: newEtagIndex(),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.BucketOperationFailedReason, "failed to confirm existence of 'unavailable' bucket"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -715,8 +823,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision '9fc2ddfc4a6f44e6c3efee40af36578b9e76d4d930eaf384b8435a0aa0bf7a0f'"),
},
},
{
@ -754,8 +862,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision '117f586dc64cfc559329e21d286edcbb94cb6b1581517eaddc0ab5292b470cd5'"),
},
},
{
@ -765,6 +873,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
obj.Status.Artifact = &sourcev1.Artifact{
Revision: "b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
bucketObjects: []*gcsmock.Object{
{
@ -780,7 +890,10 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "Removes FetchFailedCondition after reconciling source",
@ -802,9 +915,38 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
{
name: "Existing artifact makes ArtifactOutdated=True",
bucketName: "dummy",
bucketObjects: []*gcsmock.Object{
{
Key: "test.txt",
ContentType: "text/plain",
Content: []byte("test"),
Generation: 3,
},
},
beforeFunc: func(obj *sourcev1.Bucket) {
obj.Status.Artifact = &sourcev1.Artifact{
Path: "some-path",
Revision: "some-rev",
}
},
want: sreconcile.ResultSuccess,
assertIndex: &etagIndex{
index: map[string]string{
"test.txt": "098f6bcd4621d373cade4e832627b4f6",
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'b4c2a60ce44b67f5b659a95ce4e4cc9e2a86baf13afb72bd397c5384cbc0e479'"),
},
},
// TODO: Middleware for mock server to test authentication using secret.
@ -821,6 +963,7 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Client: builder.Build(),
Storage: testStorage,
patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"),
}
tmpDir := t.TempDir()
@ -830,7 +973,8 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
Kind: sourcev1.BucketKind,
},
ObjectMeta: metav1.ObjectMeta{
Name: "test-bucket",
Name: "test-bucket",
Generation: 1,
},
Spec: sourcev1.BucketSpec{
BucketName: tt.bucketName,
@ -860,15 +1004,25 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
g.Expect(os.Unsetenv(EnvGcpStorageHost)).ToNot(HaveOccurred())
}()
index := newEtagIndex()
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
got, err := r.reconcileSource(context.TODO(), obj, index, tmpDir)
index := newEtagIndex()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(context.TODO(), sp, obj, index, tmpDir)
t.Log(err)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(index.Index()).To(Equal(tt.assertIndex.Index()))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -886,10 +1040,14 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
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}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -899,6 +1057,8 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
// Incomplete artifact
obj.Status.Artifact = &sourcev1.Artifact{Revision: revision}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
afterFunc: func(t *WithT, obj *sourcev1.Bucket, dir string) {
// Still incomplete
@ -906,7 +1066,9 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -914,16 +1076,22 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
beforeFunc: func(t *WithT, obj *sourcev1.Bucket, index *etagIndex, dir string) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "Foo", "")
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "Creates latest symlink to the created artifact",
beforeFunc: func(t *WithT, obj *sourcev1.Bucket, index *etagIndex, dir string) {
obj.Spec.Interval = metav1.Duration{Duration: interval}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
afterFunc: func(t *WithT, obj *sourcev1.Bucket, dir string) {
localPath := testStorage.LocalPath(*obj.GetArtifact())
@ -934,18 +1102,24 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "Dir path deleted",
beforeFunc: func(t *WithT, obj *sourcev1.Bucket, index *etagIndex, dir string) {
t.Expect(os.RemoveAll(dir)).ToNot(HaveOccurred())
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.StatOperationFailedReason, "failed to stat source path"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
@ -957,6 +1131,8 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
f, err := os.Create(dir)
defer f.Close()
t.Expect(err).ToNot(HaveOccurred())
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
afterFunc: func(t *WithT, obj *sourcev1.Bucket, dir string) {
t.Expect(os.RemoveAll(dir)).ToNot(HaveOccurred())
@ -965,6 +1141,8 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.StorageOperationFailedCondition, sourcev1.InvalidPathReason, "is not a directory"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
@ -974,8 +1152,10 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
g := NewWithT(t)
r := &BucketReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"),
}
tmpDir := t.TempDir()
@ -1000,7 +1180,14 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
tt.beforeFunc(g, obj, index, tmpDir)
}
got, err := r.reconcileArtifact(context.TODO(), obj, index, tmpDir)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileArtifact(context.TODO(), sp, obj, index, tmpDir)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -1011,6 +1198,10 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
if tt.afterFunc != nil {
tt.afterFunc(g, obj, tmpDir)
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -1128,8 +1319,7 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
clientBuilder.WithObjects(obj)
c := clientBuilder.Build()
patchHelper, err := patch.NewHelper(obj, c)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, c)
if tt.beforeFunc != nil {
tt.beforeFunc(obj)
@ -1139,7 +1329,7 @@ func TestBucketReconciler_statusConditions(t *testing.T) {
recResult := sreconcile.ResultSuccess
var retErr error
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper)
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(bucketReadyCondition),
summarize.WithReconcileResult(recResult),
@ -1247,6 +1437,7 @@ func TestBucketReconciler_notify(t *testing.T) {
reconciler := &BucketReconciler{
EventRecorder: recorder,
patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"),
}
index := &etagIndex{
index: map[string]string{

90
controllers/gitrepository_controller.go
View File

@ -50,6 +50,7 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/fluxcd/pkg/sourceignore"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
@ -102,6 +103,15 @@ var gitRepositoryFailConditions = []string{
sourcev1.StorageOperationFailedCondition,
}
// getPatchOptions composes patch options based on the given parameters.
// It is used as the options used when patching an object.
func getPatchOptions(ownedConditions []string, controllerName string) []patch.Option {
return []patch.Option{
patch.WithOwnedConditions{Conditions: ownedConditions},
patch.WithFieldOwner(controllerName),
}
}
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=gitrepositories/finalizers,verbs=get;create;update;patch;delete
@ -118,6 +128,8 @@ type GitRepositoryReconciler struct {
requeueDependency time.Duration
features map[string]bool
patchOptions []patch.Option
}
type GitRepositoryReconcilerOptions struct {
@ -128,13 +140,15 @@ type GitRepositoryReconcilerOptions struct {
// gitRepositoryReconcileFunc is the function type for all the
// v1beta2.GitRepository (sub)reconcile functions.
type gitRepositoryReconcileFunc func(ctx context.Context, obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error)
type gitRepositoryReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error)
func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
return r.SetupWithManagerAndOptions(mgr, GitRepositoryReconcilerOptions{})
}
func (r *GitRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts GitRepositoryReconcilerOptions) error {
r.patchOptions = getPatchOptions(gitRepositoryReadyCondition.Owned, r.ControllerName)
r.requeueDependency = opts.DependencyRequeueInterval
if r.features == nil {
@ -167,10 +181,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
@ -178,7 +189,7 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// Always attempt to patch the object and status after each reconciliation
// NOTE: The final runtime result and error are set in this block.
defer func() {
summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(gitRepositoryReadyCondition),
summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
@ -227,19 +238,36 @@ func (r *GitRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.reconcileInclude,
r.reconcileArtifact,
}
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
return
}
// reconcile iterates through the gitRepositoryReconcileFunc tasks for the
// object. It returns early on the first call that returns
// reconcile.ResultRequeue, or produces an error.
func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.GitRepository, reconcilers []gitRepositoryReconcileFunc) (sreconcile.Result, error) {
func (r *GitRepositoryReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.GitRepository, reconcilers []gitRepositoryReconcileFunc) (sreconcile.Result, error) {
oldObj := obj.DeepCopy()
// Mark as reconciling if generation differs
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var recAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
recAtVal = v
}
// Persist reconciling if generation differs or reconciliation is requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
case recAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
// Create temp dir for Git clone
@ -268,7 +296,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G
resErr error
)
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, &commit, &includes, tmpDir)
recResult, err := rec(ctx, sp, obj, &commit, &includes, tmpDir)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -359,23 +387,32 @@ func (r *GitRepositoryReconciler) shouldNotify(oldObj, newObj *sourcev1.GitRepos
// condition is added.
// The hostname of any URL in the Status of the object are updated, to ensure
// they match the Storage server hostname of current runtime.
func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.GitRepository, _ *git.Commit, _ *artifactSet, _ string) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
// Determine if the advertised artifact is still in storage
var artifactMissing bool
if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
obj.Status.Artifact = nil
obj.Status.URL = ""
artifactMissing = true
// 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")
msg := "building artifact"
if artifactMissing {
msg += ": disappeared from storage"
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
return sreconcile.ResultSuccess, nil
}
@ -417,7 +454,7 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context,
// and the local artifact are on the same revision, and no other source content
// related configurations have changed since last reconciliation. If there's a
// change, it short-circuits the whole reconciliation with an early return.
func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
// Remove previously failed source verification status conditions. The
// failing verification should be recalculated. But an existing successful
@ -477,9 +514,15 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// Observe if the artifacts still match the previous included ones
if artifacts.Diff(obj.Status.IncludedArtifacts) {
message := fmt.Sprintf("included artifacts differ from last observed includes")
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
conditions.MarkReconciling(obj, "IncludeChange", message)
if obj.Status.IncludedArtifacts != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
conditions.Delete(obj, sourcev1.ArtifactOutdatedCondition)
// Persist the ArtifactSet.
*includes = *artifacts
@ -540,8 +583,13 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// Mark observations about the revision on the object
if !obj.GetArtifact().HasRevision(commit.String()) {
message := fmt.Sprintf("new upstream revision '%s'", commit.String())
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
conditions.MarkReconciling(obj, "NewRevision", message)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
return sreconcile.ResultSuccess, nil
}
@ -558,7 +606,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context,
// On a successful archive, the Artifact, Includes, observed ignore, recurse
// submodules and observed include in the Status of the object are set, and the
// symlink in the Storage is updated to its path.
func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.GitRepository, commit *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
// Create potential new artifact with current available metadata
@ -672,7 +720,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context,
// v1beta2.IncludeUnavailableCondition from the object.
// When the composed artifactSet differs from the current set in the Status of
// the object, it marks the object with v1beta2.ArtifactOutdatedCondition=True.
func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context,
func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.GitRepository, _ *git.Commit, includes *artifactSet, dir string) (sreconcile.Result, error) {
for i, incl := range obj.Spec.Include {

232
controllers/gitrepository_controller_test.go
View File

@ -38,7 +38,6 @@ import (
sshtestdata "golang.org/x/crypto/ssh/testdata"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"
kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status"
@ -241,16 +240,16 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
assertConditions []metav1.Condition
}{
{
name: "HTTP without secretRef makes ArtifactOutdated=True",
name: "HTTP without secretRef makes Reconciling=True",
protocol: "http",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
},
},
{
name: "HTTP with Basic Auth secret makes ArtifactOutdated=True",
name: "HTTP with Basic Auth secret makes Reconciling=True",
protocol: "http",
server: options{
username: "git",
@ -270,12 +269,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
},
},
{
name: "HTTPS with CAFile secret makes ArtifactOutdated=True",
name: "HTTPS with CAFile secret makes Reconciling=True",
protocol: "https",
server: options{
publicKey: tlsPublicKey,
@ -295,8 +294,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
},
},
{
@ -317,6 +316,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "invalid-ca"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
},
wantErr: true,
assertConditions: []metav1.Condition{
@ -326,10 +327,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
//
// Trimming the expected error message for consistent results.
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitOperationFailedReason, "x509: "),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
},
},
{
name: "SSH with private key secret makes ArtifactOutdated=True",
name: "SSH with private key secret makes Reconciling=True",
protocol: "ssh",
server: options{
username: "git",
@ -348,12 +351,12 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>"),
},
},
{
name: "SSH with password protected private key secret makes ArtifactOutdated=True",
name: "SSH with password protected private key secret makes Reconciling=True",
protocol: "ssh",
server: options{
username: "git",
@ -373,8 +376,8 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
},
},
{
@ -385,10 +388,46 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
},
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "non-existing"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to get secret '/non-existing': secrets \"non-existing\" not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
},
},
{
name: "Existing artifact makes ArtifactOutdated=True",
protocol: "http",
server: options{
username: "git",
password: "1234",
},
secret: &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-auth",
},
Data: map[string][]byte{
"username": []byte("git"),
"password": []byte("1234"),
},
},
beforeFunc: func(obj *sourcev1.GitRepository) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "basic-auth"}
obj.Status = sourcev1.GitRepositoryStatus{
Artifact: &sourcev1.Artifact{
Revision: "staging/some-revision",
Path: randStringRunes(10),
},
}
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new upstream revision 'master/<commit>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new upstream revision 'master/<commit>'"),
},
},
}
@ -400,6 +439,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
obj := &sourcev1.GitRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
Generation: 1,
},
Spec: sourcev1.GitRepositorySpec{
Interval: metav1.Duration{Duration: interval},
@ -468,6 +508,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
features: map[string]bool{
features.OptimizedGitClones: true,
},
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
tmpDir := t.TempDir()
@ -479,14 +520,24 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", obj.Spec.URL)
}
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
var commit git.Commit
var includes artifactSet
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(context.TODO(), obj, &commit, &includes, tmpDir)
got, err := r.reconcileSource(context.TODO(), sp, obj, &commit, &includes, tmpDir)
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(commit).ToNot(BeNil())
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -505,30 +556,31 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
wantErr bool
wantRevision string
wantArtifactOutdated bool
wantReconciling bool
}{
{
name: "Nil reference (default branch)",
want: sreconcile.ResultSuccess,
wantRevision: "master/<commit>",
wantArtifactOutdated: true,
name: "Nil reference (default branch)",
want: sreconcile.ResultSuccess,
wantRevision: "master/<commit>",
wantReconciling: true,
},
{
name: "Branch",
reference: &sourcev1.GitRepositoryRef{
Branch: "staging",
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: true,
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantReconciling: true,
},
{
name: "Tag",
reference: &sourcev1.GitRepositoryRef{
Tag: "v0.1.0",
},
want: sreconcile.ResultSuccess,
wantRevision: "v0.1.0/<commit>",
wantArtifactOutdated: true,
want: sreconcile.ResultSuccess,
wantRevision: "v0.1.0/<commit>",
wantReconciling: true,
},
{
name: "Branch commit",
@ -536,36 +588,56 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
Branch: "staging",
Commit: "<commit>",
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: true,
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantReconciling: true,
},
{
name: "SemVer",
reference: &sourcev1.GitRepositoryRef{
SemVer: "*",
},
want: sreconcile.ResultSuccess,
wantRevision: "v2.0.0/<commit>",
wantArtifactOutdated: true,
want: sreconcile.ResultSuccess,
wantRevision: "v2.0.0/<commit>",
wantReconciling: true,
},
{
name: "SemVer range",
reference: &sourcev1.GitRepositoryRef{
SemVer: "<v0.2.1",
},
want: sreconcile.ResultSuccess,
wantRevision: "0.2.0/<commit>",
wantArtifactOutdated: true,
want: sreconcile.ResultSuccess,
wantRevision: "0.2.0/<commit>",
wantReconciling: true,
},
{
name: "SemVer prerelease",
reference: &sourcev1.GitRepositoryRef{
SemVer: ">=1.0.0-0 <1.1.0-0",
},
wantRevision: "v1.0.0-alpha/<commit>",
wantRevision: "v1.0.0-alpha/<commit>",
want: sreconcile.ResultSuccess,
wantReconciling: true,
},
{
name: "Existing artifact makes ArtifactOutdated=True",
reference: &sourcev1.GitRepositoryRef{
Branch: "staging",
},
beforeFunc: func(obj *sourcev1.GitRepository, latestRev string) {
obj.Status = sourcev1.GitRepositoryStatus{
Artifact: &sourcev1.Artifact{
Revision: "staging/some-revision",
Path: randStringRunes(10),
},
}
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: true,
wantReconciling: true,
},
{
name: "Optimized clone",
@ -581,11 +653,12 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
},
}
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
},
want: sreconcile.ResultEmpty,
wantErr: true,
wantRevision: "staging/<commit>",
wantArtifactOutdated: false,
want: sreconcile.ResultEmpty,
wantErr: true,
wantRevision: "staging/<commit>",
wantReconciling: false,
},
{
name: "Optimized clone different ignore",
@ -603,10 +676,11 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
},
}
conditions.MarkTrue(obj, sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "foo")
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "foo")
},
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantArtifactOutdated: false,
want: sreconcile.ResultSuccess,
wantRevision: "staging/<commit>",
wantReconciling: false,
},
}
@ -632,12 +706,13 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
}
r := &GitRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(runtime.NewScheme()).Build(),
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: map[string]bool{
features.OptimizedGitClones: true,
},
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -645,6 +720,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
obj := &sourcev1.GitRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "checkout-strategy-",
Generation: 1,
},
Spec: sourcev1.GitRepositorySpec{
Interval: metav1.Duration{Duration: interval},
@ -664,9 +740,15 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
tt.beforeFunc(obj, headRef.Hash().String())
}
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
var commit git.Commit
var includes artifactSet
got, err := r.reconcileSource(ctx, obj, &commit, &includes, tmpDir)
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(ctx, sp, obj, &commit, &includes, tmpDir)
if err != nil {
println(err.Error())
}
@ -676,7 +758,11 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
revision := strings.ReplaceAll(tt.wantRevision, "<commit>", headRef.Hash().String())
g.Expect(commit.String()).To(Equal(revision))
g.Expect(conditions.IsTrue(obj, sourcev1.ArtifactOutdatedCondition)).To(Equal(tt.wantArtifactOutdated))
g.Expect(conditions.IsTrue(obj, meta.ReconcilingCondition)).To(Equal(tt.wantReconciling))
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -855,6 +941,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{
@ -873,8 +960,9 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
Hash: []byte("revision"),
Reference: "refs/heads/main",
}
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileArtifact(ctx, obj, &commit, &tt.includes, tt.dir)
got, err := r.reconcileArtifact(ctx, sp, obj, &commit, &tt.includes, tt.dir)
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -1000,6 +1088,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
Storage: storage,
requeueDependency: dependencyInterval,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{
@ -1036,7 +1125,9 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
g.Expect(err).ToNot(HaveOccurred())
includes = *artifactSet
got, err := r.reconcileInclude(ctx, obj, &commit, &includes, tmpDir)
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileInclude(ctx, sp, obj, &commit, &includes, tmpDir)
g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
g.Expect(err != nil).To(Equal(tt.wantErr))
if err == nil {
@ -1093,6 +1184,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
}
}
testStorage.SetArtifactURL(obj.Status.Artifact)
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
assertArtifact: &sourcev1.Artifact{
@ -1109,6 +1201,17 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/a.txt",
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "build artifact first time",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
},
},
{
name: "notices missing artifact in storage",
@ -1125,7 +1228,8 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/invalid.txt",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
},
},
{
@ -1143,6 +1247,7 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
return err
}
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
want: sreconcile.ResultSuccess,
@ -1156,6 +1261,9 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
URL: testStorage.Hostname + "/reconcile-storage/hostname.txt",
Size: int64p(int64(len("file"))),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
for _, tt := range tests {
@ -1167,23 +1275,32 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
}()
r := &GitRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Generation: 1,
},
}
if tt.beforeFunc != nil {
g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed())
}
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
var c *git.Commit
var as artifactSet
got, err := r.reconcileStorage(context.TODO(), obj, c, &as, "")
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileStorage(context.TODO(), sp, obj, c, &as, "")
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -1201,6 +1318,10 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) {
}
g.Expect(absoluteP).NotTo(BeAnExistingFile())
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -1212,6 +1333,7 @@ func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{
@ -1350,6 +1472,7 @@ func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Client: builder.Build(),
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{
@ -1492,6 +1615,7 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
key := client.ObjectKeyFromObject(obj)
@ -1710,8 +1834,7 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
clientBuilder.WithObjects(obj)
c := clientBuilder.Build()
patchHelper, err := patch.NewHelper(obj, c)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, c)
if tt.beforeFunc != nil {
tt.beforeFunc(obj)
@ -1721,9 +1844,10 @@ func TestGitRepositoryReconciler_statusConditions(t *testing.T) {
recResult := sreconcile.ResultSuccess
var retErr error
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper)
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(gitRepositoryReadyCondition),
summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
summarize.WithReconcileResult(recResult),
summarize.WithReconcileError(retErr),
summarize.WithIgnoreNotFound(),
@ -1857,6 +1981,7 @@ func TestGitRepositoryReconciler_notify(t *testing.T) {
reconciler := &GitRepositoryReconciler{
EventRecorder: recorder,
features: features.FeatureGates(),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
reconciler.notify(ctx, oldObj, newObj, tt.commit, tt.res, tt.resErr)
@ -1996,6 +2121,7 @@ func TestGitRepositoryReconciler_fetchIncludes(t *testing.T) {
r := &GitRepositoryReconciler{
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.GitRepository{

75
controllers/helmchart_controller.go
View File

@ -30,6 +30,8 @@ import (
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
soci "github.com/fluxcd/source-controller/internal/oci"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/v1/remote"
helmgetter "helm.sh/helm/v3/pkg/getter"
helmreg "helm.sh/helm/v3/pkg/registry"
corev1 "k8s.io/api/core/v1"
@ -56,9 +58,8 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/fluxcd/pkg/untar"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/v1/remote"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
"github.com/fluxcd/source-controller/internal/cache"
@ -133,6 +134,8 @@ type HelmChartReconciler struct {
Cache *cache.Cache
TTL time.Duration
*cache.CacheRecorder
patchOptions []patch.Option
}
func (r *HelmChartReconciler) SetupWithManager(mgr ctrl.Manager) error {
@ -147,9 +150,11 @@ type HelmChartReconcilerOptions struct {
// helmChartReconcileFunc is the function type for all the v1beta2.HelmChart
// (sub)reconcile functions. The type implementations are grouped and
// executed serially to perform the complete reconcile of the object.
type helmChartReconcileFunc func(ctx context.Context, obj *sourcev1.HelmChart, build *chart.Build) (sreconcile.Result, error)
type helmChartReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, build *chart.Build) (sreconcile.Result, error)
func (r *HelmChartReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmChartReconcilerOptions) error {
r.patchOptions = getPatchOptions(helmChartReadyCondition.Owned, r.ControllerName)
if err := mgr.GetCache().IndexField(context.TODO(), &sourcev1.HelmRepository{}, sourcev1.HelmRepositoryURLIndexKey,
r.indexHelmRepositoryByURL); err != nil {
return fmt.Errorf("failed setting index fields: %w", err)
@ -200,10 +205,7 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
@ -211,7 +213,7 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
// Always attempt to patch the object after each reconciliation.
// NOTE: The final runtime result and error are set in this block.
defer func() {
summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(helmChartReadyCondition),
summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
@ -259,19 +261,35 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
r.reconcileSource,
r.reconcileArtifact,
}
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
return
}
// reconcile iterates through the helmChartReconcileFunc tasks for the
// object. It returns early on the first call that returns
// reconcile.ResultRequeue, or produces an error.
func (r *HelmChartReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmChart, reconcilers []helmChartReconcileFunc) (sreconcile.Result, error) {
func (r *HelmChartReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, reconcilers []helmChartReconcileFunc) (sreconcile.Result, error) {
oldObj := obj.DeepCopy()
// Mark as reconciling if generation differs.
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var reconcileAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
reconcileAtVal = v
}
// Persist reconciling if generation differs or reconciliation is requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
// Run the sub-reconcilers and build the result of reconciliation.
@ -281,7 +299,7 @@ func (r *HelmChartReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmC
resErr error
)
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, &build)
recResult, err := rec(ctx, sp, obj, &build)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -344,22 +362,31 @@ func (r *HelmChartReconciler) notify(ctx context.Context, oldObj, newObj *source
// condition is added.
// The hostname of any URL in the Status of the object are updated, to ensure
// they match the Storage server hostname of current runtime.
func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.HelmChart, build *chart.Build) (sreconcile.Result, error) {
func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, build *chart.Build) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
// Determine if the advertised artifact is still in storage
var artifactMissing bool
if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
obj.Status.Artifact = nil
obj.Status.URL = ""
artifactMissing = true
// 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")
msg := "building artifact"
if artifactMissing {
msg += ": disappeared from storage"
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
return sreconcile.ResultSuccess, nil
}
@ -371,7 +398,7 @@ func (r *HelmChartReconciler) reconcileStorage(ctx context.Context, obj *sourcev
return sreconcile.ResultSuccess, nil
}
func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmChart, build *chart.Build) (_ sreconcile.Result, retErr error) {
func (r *HelmChartReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, build *chart.Build) (_ sreconcile.Result, retErr error) {
// Remove any failed verification condition.
// The reason is that a failing verification should be recalculated.
if conditions.IsFalse(obj, sourcev1.SourceVerifiedCondition) {
@ -418,7 +445,7 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1
// Defer observation of build result
defer func() {
// Record both success and error observations on the object
observeChartBuild(obj, build, retErr)
observeChartBuild(ctx, sp, r.patchOptions, obj, build, retErr)
// If we actually build a chart, take a historical note of any dependencies we resolved.
// The reason this is a done conditionally, is because if we have a cached one in storage,
@ -810,7 +837,7 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
// early.
// 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 *HelmChartReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.HelmChart, b *chart.Build) (sreconcile.Result, error) {
func (r *HelmChartReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmChart, b *chart.Build) (sreconcile.Result, error) {
// Without a complete chart build, there is little to reconcile
if !b.Complete() {
return sreconcile.ResultRequeue, nil
@ -1265,10 +1292,16 @@ func (r *HelmChartReconciler) eventLogf(ctx context.Context, obj runtime.Object,
}
// observeChartBuild records the observation on the given given build and error on the object.
func observeChartBuild(obj *sourcev1.HelmChart, build *chart.Build, err error) {
func observeChartBuild(ctx context.Context, sp *patch.SerialPatcher, pOpts []patch.Option, obj *sourcev1.HelmChart, build *chart.Build, err error) {
if build.HasMetadata() {
if build.Name != obj.Status.ObservedChartName || !obj.GetArtifact().HasRevision(build.Version) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewChart", build.Summary())
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewChart", build.Summary())
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", build.Summary())
if err := sp.Patch(ctx, obj, pOpts...); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
}
}
}

218
controllers/helmchart_controller_test.go
View File

@ -309,6 +309,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
}
}
testStorage.SetArtifactURL(obj.Status.Artifact)
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
assertArtifact: &sourcev1.Artifact{
@ -325,6 +326,17 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/a.txt",
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "build artifact first time",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
},
},
{
name: "notices missing artifact in storage",
@ -341,7 +353,8 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/invalid.txt",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
},
},
{
@ -359,6 +372,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
return err
}
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
want: sreconcile.ResultSuccess,
@ -372,6 +386,9 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
URL: testStorage.Hostname + "/reconcile-storage/hostname.txt",
Size: int64p(int64(len("file"))),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
for _, tt := range tests {
@ -383,20 +400,30 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
}()
r := &HelmChartReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Generation: 1,
},
}
if tt.beforeFunc != nil {
g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed())
}
got, err := r.reconcileStorage(context.TODO(), obj, nil)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileStorage(context.TODO(), sp, obj, nil)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -414,6 +441,10 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
}
g.Expect(absoluteP).NotTo(BeAnExistingFile())
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -468,13 +499,51 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(obj.Status.ObservedSourceArtifactRevision).To(Equal(gitArtifact.Revision))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled 'helmchart' chart with version '0.1.0'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
}))
},
cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed())
},
},
{
name: "Existing artifact makes AritfactOutdated=True",
source: &sourcev1.GitRepository{
ObjectMeta: metav1.ObjectMeta{
Name: "gitrepository",
Namespace: "default",
},
Status: sourcev1.GitRepositoryStatus{
Artifact: gitArtifact,
},
},
beforeFunc: func(obj *sourcev1.HelmChart) {
obj.Spec.Chart = "testdata/charts/helmchart-0.1.0.tgz"
obj.Spec.SourceRef = sourcev1.LocalHelmChartSourceReference{
Name: "gitrepository",
Kind: sourcev1.GitRepositoryKind,
}
obj.Status.Artifact = &sourcev1.Artifact{
Path: "some-path",
Revision: "some-rev",
}
},
want: sreconcile.ResultSuccess,
assertFunc: func(g *WithT, build chart.Build, obj sourcev1.HelmChart) {
g.Expect(build.Complete()).To(BeTrue())
g.Expect(build.Name).To(Equal("helmchart"))
g.Expect(build.Version).To(Equal("0.1.0"))
g.Expect(build.Path).To(BeARegularFile())
g.Expect(obj.Status.ObservedSourceArtifactRevision).To(Equal(gitArtifact.Revision))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled 'helmchart' chart with version '0.1.0'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
}))
},
},
{
name: "Error on unavailable source",
beforeFunc: func(obj *sourcev1.HelmChart) {
@ -482,6 +551,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
Name: "unavailable",
Kind: sourcev1.GitRepositoryKind,
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: &serror.Event{Err: errors.New("gitrepositories.source.toolkit.fluxcd.io \"unavailable\" not found")},
@ -490,6 +561,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "SourceUnavailable", "failed to get source: gitrepositories.source.toolkit.fluxcd.io \"unavailable\" not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
}))
},
},
@ -500,6 +573,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
Name: "unavailable",
Kind: "Unsupported",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
},
want: sreconcile.ResultEmpty,
wantErr: &serror.Stalling{Err: errors.New("unsupported source kind 'Unsupported'")},
@ -508,6 +583,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "SourceUnavailable", "failed to get source: unsupported source kind"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
}))
},
},
@ -529,6 +606,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
Kind: sourcev1.GitRepositoryKind,
}
obj.Spec.ValuesFiles = []string{"invalid.yaml"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
},
want: sreconcile.ResultEmpty,
wantErr: &serror.Stalling{Err: errors.New("values files merge error: no values file found at path")},
@ -537,6 +616,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.BuildFailedCondition, "ValuesFilesError", "values files merge error: no values file found at path"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
}))
},
},
@ -556,6 +637,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
Kind: sourcev1.GitRepositoryKind,
}
obj.Status.ObservedSourceArtifactRevision = "foo"
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, meta.ProgressingReason, "foo")
},
want: sreconcile.ResultRequeue,
assertFunc: func(g *WithT, build chart.Build, obj sourcev1.HelmChart) {
@ -564,6 +647,8 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
g.Expect(obj.Status.ObservedSourceArtifactRevision).To(Equal("foo"))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions([]metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, "NoSourceArtifact", "no artifact available"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "foo"),
}))
},
},
@ -572,7 +657,7 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
clientBuilder := fake.NewClientBuilder()
clientBuilder := fake.NewClientBuilder().WithScheme(testEnv.GetScheme())
if tt.source != nil {
clientBuilder.WithRuntimeObjects(tt.source)
}
@ -581,12 +666,14 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
Client: clientBuilder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: storage,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := sourcev1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
Name: "chart",
Namespace: "default",
Name: "chart",
Namespace: "default",
Generation: 1,
},
Spec: sourcev1.HelmChartSpec{},
}
@ -599,7 +686,14 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
defer tt.cleanFunc(g, &b)
}
got, err := r.reconcileSource(context.TODO(), &obj, &b)
g.Expect(r.Client.Create(context.TODO(), &obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), &obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(&obj, r.Client)
got, err := r.reconcileSource(context.TODO(), sp, &obj, &b)
g.Expect(err != nil).To(Equal(tt.wantErr != nil))
if tt.wantErr != nil {
@ -611,6 +705,10 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
if tt.assertFunc != nil {
tt.assertFunc(g, b, obj)
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, &obj)
})
}
}
@ -840,6 +938,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Getters: testGetters,
Storage: storage,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
repository := &sourcev1.HelmRepository{
@ -1006,8 +1105,6 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
assertFunc: func(g *WithT, obj *sourcev1.HelmChart, build chart.Build) {
g.Expect(build.Name).To(Equal(metadata.Name))
g.Expect(build.Version).To(Equal(metadata.Version))
fmt.Println("buildpath", build.Path)
fmt.Println("storage Path", storage.LocalPath(*cachedArtifact.DeepCopy()))
g.Expect(build.Path).ToNot(Equal(storage.LocalPath(*cachedArtifact.DeepCopy())))
g.Expect(build.Path).To(BeARegularFile())
},
@ -1071,6 +1168,7 @@ func TestHelmChartReconciler_buildFromOCIHelmRepository(t *testing.T) {
Getters: testGetters,
Storage: storage,
RegistryClientGenerator: registry.ClientGenerator,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
repository := &sourcev1.HelmRepository{
@ -1276,6 +1374,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
Storage: storage,
Getters: testGetters,
RegistryClientGenerator: registry.ClientGenerator,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
@ -1433,8 +1532,10 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
g := NewWithT(t)
r := &HelmChartReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
@ -1448,7 +1549,14 @@ func TestHelmChartReconciler_reconcileArtifact(t *testing.T) {
tt.beforeFunc(obj)
}
got, err := r.reconcileArtifact(ctx, obj, tt.build)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileArtifact(ctx, sp, obj, tt.build)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
@ -1477,7 +1585,8 @@ func TestHelmChartReconciler_getHelmRepositorySecret(t *testing.T) {
clientBuilder.WithObjects(mock)
r := &HelmChartReconciler{
Client: clientBuilder.Build(),
Client: clientBuilder.Build(),
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
tests := []struct {
@ -1572,7 +1681,8 @@ func TestHelmChartReconciler_getSource(t *testing.T) {
clientBuilder.WithObjects(mocks...)
r := &HelmChartReconciler{
Client: clientBuilder.Build(),
Client: clientBuilder.Build(),
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
tests := []struct {
@ -1678,6 +1788,7 @@ func TestHelmChartReconciler_reconcileDelete(t *testing.T) {
r := &HelmChartReconciler{
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
@ -1704,7 +1815,7 @@ func TestHelmChartReconciler_reconcileDelete(t *testing.T) {
func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
// Helper to build simple helmChartReconcileFunc with result and error.
buildReconcileFuncs := func(r sreconcile.Result, e error) helmChartReconcileFunc {
return func(_ context.Context, _ *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
return func(_ context.Context, _ *patch.SerialPatcher, _ *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
return r, e
}
}
@ -1725,6 +1836,10 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultSuccess,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "successful reconciliation with generation difference",
@ -1736,7 +1851,8 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
wantResult: sreconcile.ResultSuccess,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "reconciling new object generation (3)"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
},
},
{
@ -1746,15 +1862,19 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "multiple object status conditions mutations",
reconcileFuncs: []helmChartReconcileFunc{
func(_ context.Context, obj *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
func(_ context.Context, _ *patch.SerialPatcher, obj *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision")
return sreconcile.ResultSuccess, nil
},
func(_ context.Context, obj *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
func(_ context.Context, _ *patch.SerialPatcher, obj *sourcev1.HelmChart, _ *chart.Build) (sreconcile.Result, error) {
conditions.MarkTrue(obj, meta.ReconcilingCondition, "Progressing", "creating artifact")
return sreconcile.ResultSuccess, nil
},
@ -1763,7 +1883,8 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "Progressing", "creating artifact"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "creating artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
@ -1775,6 +1896,10 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultRequeue,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "subrecs with error before result=Requeue",
@ -1785,6 +1910,10 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
}
@ -1792,7 +1921,10 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
r := &HelmChartReconciler{}
r := &HelmChartReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
@ -1803,7 +1935,14 @@ func TestHelmChartReconciler_reconcileSubRecs(t *testing.T) {
},
}
got, err := r.reconcile(context.TODO(), obj, tt.reconcileFuncs)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcile(context.TODO(), sp, obj, tt.reconcileFuncs)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.wantResult))
@ -1898,8 +2037,7 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
clientBuilder.WithObjects(obj)
c := clientBuilder.Build()
patchHelper, err := patch.NewHelper(obj, c)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, c)
if tt.beforeFunc != nil {
tt.beforeFunc(obj)
@ -1909,9 +2047,10 @@ func TestHelmChartReconciler_statusConditions(t *testing.T) {
recResult := sreconcile.ResultSuccess
var retErr error
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper)
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(helmChartReadyCondition),
summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
summarize.WithReconcileResult(recResult),
summarize.WithReconcileError(retErr),
summarize.WithIgnoreNotFound(),
@ -2012,6 +2151,7 @@ func TestHelmChartReconciler_notify(t *testing.T) {
reconciler := &HelmChartReconciler{
EventRecorder: recorder,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
build := &chart.Build{
Name: "foo",
@ -2061,7 +2201,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
name: "HTTP without basic auth",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
@ -2075,7 +2216,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
password: testRegistryPassword,
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
{
@ -2114,7 +2256,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
},
provider: "azure",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<helmchart>' chart with version '<version>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled 'helmchart' chart with version '0.1.0'"),
},
},
}
@ -2200,6 +2343,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Getters: testGetters,
RegistryClientGenerator: registry.ClientGenerator,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
var b chart.Build
@ -2216,7 +2360,14 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_authStrategy(t *testing.T) {
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<version>", metadata.Version)
}
got, err := r.reconcileSource(ctx, obj, &b)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(ctx, sp, obj, &b)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
@ -2323,8 +2474,9 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<name>' chart with version '<version>'"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of version <version>"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled '<name>' chart with version '<version>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled '<name>' chart with version '<version>'"),
},
cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed())
@ -2342,6 +2494,8 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewChart", "pulled '<name>' chart with version '<version>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: pulled '<name>' chart with version '<version>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: pulled '<name>' chart with version '<version>'"),
},
cleanFunc: func(g *WithT, build *chart.Build) {
g.Expect(os.Remove(build.Path)).To(Succeed())
@ -2383,6 +2537,7 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
Getters: testGetters,
Storage: storage,
RegistryClientGenerator: registry.ClientGenerator,
patchOptions: getPatchOptions(helmChartReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmChart{
@ -2434,7 +2589,14 @@ func TestHelmChartReconciler_reconcileSourceFromOCI_verifySignature(t *testing.T
defer tt.cleanFunc(g, &b)
}
got, err := r.reconcileSource(ctx, obj, &b)
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(ctx, sp, obj, &b)
if tt.wantErr {
tt.wantErrMsg = strings.ReplaceAll(tt.wantErrMsg, "<url>", chartUrl)
g.Expect(err).ToNot(BeNil())

74
controllers/helmrepository_controller.go
View File

@ -43,6 +43,7 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
"github.com/fluxcd/source-controller/internal/cache"
@ -109,6 +110,8 @@ type HelmRepositoryReconciler struct {
Cache *cache.Cache
TTL time.Duration
*cache.CacheRecorder
patchOptions []patch.Option
}
type HelmRepositoryReconcilerOptions struct {
@ -120,13 +123,15 @@ type HelmRepositoryReconcilerOptions struct {
// v1beta2.HelmRepository (sub)reconcile functions. The type implementations
// are grouped and executed serially to perform the complete reconcile of the
// object.
type helmRepositoryReconcileFunc func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error)
type helmRepositoryReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error)
func (r *HelmRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
return r.SetupWithManagerAndOptions(mgr, HelmRepositoryReconcilerOptions{})
}
func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error {
r.patchOptions = getPatchOptions(helmRepositoryReadyCondition.Owned, r.ControllerName)
return ctrl.NewControllerManagedBy(mgr).
For(&sourcev1.HelmRepository{}).
WithEventFilter(
@ -160,10 +165,7 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
@ -171,7 +173,7 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
// Always attempt to patch the object after each reconciliation.
// NOTE: The final runtime result and error are set in this block.
defer func() {
summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(helmRepositoryReadyCondition),
summarize.WithReconcileResult(recResult),
@ -219,19 +221,36 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque
r.reconcileSource,
r.reconcileArtifact,
}
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
return
}
// reconcile iterates through the helmRepositoryReconcileFunc tasks for the
// object. It returns early on the first call that returns
// reconcile.ResultRequeue, or produces an error.
func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) {
func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.HelmRepository, reconcilers []helmRepositoryReconcileFunc) (sreconcile.Result, error) {
oldObj := obj.DeepCopy()
// Mark as reconciling if generation differs.
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var reconcileAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
reconcileAtVal = v
}
// Persist reconciling if generation differs or reconciliation is requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
var chartRepo repository.ChartRepository
@ -241,7 +260,7 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.
var res sreconcile.Result
var resErr error
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, &artifact, &chartRepo)
recResult, err := rec(ctx, sp, obj, &artifact, &chartRepo)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -311,22 +330,32 @@ func (r *HelmRepositoryReconciler) notify(ctx context.Context, oldObj, newObj *s
// condition is added.
// The hostname of any URL in the Status of the object are updated, to ensure
// they match the Storage server hostname of current runtime.
func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.HelmRepository, _ *sourcev1.Artifact, _ *repository.ChartRepository) (sreconcile.Result, error) {
func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.HelmRepository, _ *sourcev1.Artifact, _ *repository.ChartRepository) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
// Determine if the advertised artifact is still in storage
var artifactMissing bool
if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
obj.Status.Artifact = nil
obj.Status.URL = ""
artifactMissing = true
// 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")
msg := "building artifact"
if artifactMissing {
msg += ": disappeared from storage"
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
return sreconcile.ResultSuccess, nil
}
@ -346,7 +375,8 @@ func (r *HelmRepositoryReconciler) reconcileStorage(ctx context.Context, obj *so
// If successful and the index is valid, any previous
// v1beta2.FetchFailedCondition is removed, and the repository.ChartRepository
// pointer is set to the newly fetched index.
func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, error) {
var tlsConfig *tls.Config
// Configure Helm client to access repository
@ -455,8 +485,14 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
// Mark observations about the revision on the object.
if !obj.GetArtifact().HasRevision(chartRepo.Checksum) {
message := fmt.Sprintf("new index revision '%s'", checksum)
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
conditions.MarkReconciling(obj, "NewRevision", message)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
return sreconcile.ResultEmpty, err
}
}
// Create potential new artifact.
@ -482,13 +518,13 @@ func (r *HelmRepositoryReconciler) reconcileSource(ctx context.Context, obj *sou
// early.
// 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) {
func (r *HelmRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, chartRepo *repository.ChartRepository) (sreconcile.Result, 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, sourcev1.ArtifactInStorageCondition, meta.SucceededReason,
"stored artifact for revision '%s'", artifact.Revision)
"stored artifact: revision '%s'", artifact.Revision)
}
chartRepo.Unload()

61
controllers/helmrepository_controller_oci.go
View File

@ -24,6 +24,7 @@ import (
"os"
"time"
"github.com/google/go-containerregistry/pkg/authn"
helmgetter "helm.sh/helm/v3/pkg/getter"
helmreg "helm.sh/helm/v3/pkg/registry"
corev1 "k8s.io/api/core/v1"
@ -45,7 +46,7 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
"github.com/google/go-containerregistry/pkg/authn"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/fluxcd/source-controller/api/v1beta2"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
@ -79,6 +80,8 @@ type HelmRepositoryOCIReconciler struct {
Getters helmgetter.Providers
ControllerName string
RegistryClientGenerator RegistryClientGeneratorFunc
patchOptions []patch.Option
}
// RegistryClientGeneratorFunc is a function that returns a registry client
@ -92,6 +95,8 @@ func (r *HelmRepositoryOCIReconciler) SetupWithManager(mgr ctrl.Manager) error {
}
func (r *HelmRepositoryOCIReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts HelmRepositoryReconcilerOptions) error {
r.patchOptions = getPatchOptions(helmRepositoryOCIOwnedConditions, r.ControllerName)
return ctrl.NewControllerManagedBy(mgr).
For(&sourcev1.HelmRepository{}).
WithEventFilter(
@ -122,34 +127,26 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// Always attempt to patch the object after each reconciliation.
defer func() {
// Patch the object, prioritizing the conditions owned by the controller in
// case of any conflicts.
patchOpts := []patch.Option{
patch.WithOwnedConditions{
Conditions: helmRepositoryOCIOwnedConditions,
},
}
patchOpts = append(patchOpts, patch.WithFieldOwner(r.ControllerName))
// If a reconcile annotation value is found, set it in the object status
// as status.lastHandledReconcileAt.
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
object.SetStatusLastHandledReconcileAt(obj, v)
}
patchOpts := []patch.Option{}
patchOpts = append(patchOpts, r.patchOptions...)
// Set status observed generation option if the object is stalled, or
// if the object is ready.
if conditions.IsStalled(obj) || conditions.IsReady(obj) {
patchOpts = append(patchOpts, patch.WithStatusObservedGeneration{})
}
if err = patchHelper.Patch(ctx, obj, patchOpts...); err != nil {
if err := serialPatcher.Patch(ctx, obj, patchOpts...); err != nil {
// Ignore patch error "not found" when the object is being deleted.
if !obj.GetDeletionTimestamp().IsZero() {
err = kerrors.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) })
@ -188,7 +185,7 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
return ctrl.Result{}, nil
}
result, retErr = r.reconcile(ctx, obj)
result, retErr = r.reconcile(ctx, serialPatcher, obj)
return
}
@ -198,7 +195,7 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re
// status conditions and the returned results are evaluated in the deferred
// block at the very end to summarize the conditions to be in a consistent
// state.
func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta2.HelmRepository) (result ctrl.Result, retErr error) {
func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *v1beta2.HelmRepository) (result ctrl.Result, retErr error) {
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
defer cancel()
@ -224,6 +221,15 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
}
}
// Presence of reconciling means that the reconciliation didn't succeed.
// Set the Reconciling reason to ProgressingWithRetry to indicate a
// failure retry.
if conditions.IsReconciling(obj) {
reconciling := conditions.Get(obj, meta.ReconcilingCondition)
reconciling.Reason = meta.ProgressingWithRetryReason
conditions.Set(obj, reconciling)
}
// If it's still a successful reconciliation and it's not reconciling or
// stalled, mark Ready=True.
if !conditions.IsReconciling(obj) && !conditions.IsStalled(obj) &&
@ -244,8 +250,27 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
}()
// Set reconciling condition.
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var reconcileAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
reconcileAtVal = v
}
// Persist reconciling if generation differs or reconciliation is requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
result, retErr = ctrl.Result{}, err
return
}
case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
result, retErr = ctrl.Result{}, err
return
}
}
// Ensure that it's an OCI URL before continuing.

20
controllers/helmrepository_controller_oci_test.go
View File

@ -208,6 +208,7 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
password: "wrong-pass",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"),
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to login to registry"),
},
},
@ -217,6 +218,7 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
provider: "aws",
providerImg: "oci://123456789000.dkr.ecr.us-east-2.amazonaws.com/test",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "processing object: new generation"),
*conditions.FalseCondition(meta.ReadyCondition, sourcev1.AuthenticationFailedReason, "failed to get credential from"),
},
},
@ -249,6 +251,7 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
obj := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
Generation: 1,
},
Spec: sourcev1.HelmRepositorySpec{
Interval: metav1.Duration{Duration: interval},
@ -293,12 +296,27 @@ func TestHelmRepositoryOCIReconciler_authStrategy(t *testing.T) {
EventRecorder: record.NewFakeRecorder(32),
Getters: testGetters,
RegistryClientGenerator: registry.ClientGenerator,
patchOptions: getPatchOptions(helmRepositoryOCIOwnedConditions, "sc"),
}
got, err := r.reconcile(ctx, obj)
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcile(ctx, sp, obj)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
// NOTE: Check the object directly as reconcile() doesn't apply the
// final patch, the object has unapplied changes.
checker.DisableFetch = true
checker.CheckErr(ctx, obj)
})
}
}

177
controllers/helmrepository_controller_test.go
View File

@ -159,6 +159,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
}
}
testStorage.SetArtifactURL(obj.Status.Artifact)
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
assertArtifact: &sourcev1.Artifact{
@ -175,6 +176,17 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/a.txt",
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "build artifact first time",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
},
},
{
name: "notices missing artifact in storage",
@ -191,7 +203,8 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
"!/reconcile-storage/invalid.txt",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
},
},
{
@ -209,6 +222,7 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
return err
}
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
want: sreconcile.ResultSuccess,
@ -222,6 +236,9 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
URL: testStorage.Hostname + "/reconcile-storage/hostname.txt",
Size: int64p(int64(len("file"))),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
for _, tt := range tests {
@ -229,23 +246,32 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
g := NewWithT(t)
r := &HelmRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Generation: 1,
},
}
if tt.beforeFunc != nil {
g.Expect(tt.beforeFunc(obj, testStorage)).To(Succeed())
}
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
var chartRepo repository.ChartRepository
var artifact sourcev1.Artifact
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileStorage(context.TODO(), obj, &artifact, &chartRepo)
got, err := r.reconcileStorage(context.TODO(), sp, obj, &artifact, &chartRepo)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -263,6 +289,10 @@ func TestHelmRepositoryReconciler_reconcileStorage(t *testing.T) {
}
g.Expect(absoluteP).NotTo(BeAnExistingFile())
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -302,8 +332,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
},
{
@ -311,8 +341,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
protocol: "http",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@ -342,8 +372,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@ -373,8 +403,8 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@ -401,10 +431,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "invalid-ca"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "failed to create TLS client config with secret data: cannot append certificate into certificate pool: invalid caFile"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// No repo index due to fetch fail.
@ -419,11 +453,15 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
protocol: "http",
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "")
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.URLInvalidReason, "first path segment in URL cannot contain colon"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// No repo index due to fetch fail.
@ -438,11 +476,15 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
protocol: "http",
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Spec.URL = strings.ReplaceAll(obj.Spec.URL, "http://", "ftp://")
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
want: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, meta.FailedReason, "scheme \"ftp\" not supported"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// No repo index due to fetch fail.
@ -457,10 +499,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
protocol: "http",
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "non-existing"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "secrets \"non-existing\" not found"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// No repo index due to fetch fail.
@ -483,10 +529,14 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Spec.SecretRef = &meta.LocalObjectReference{Name: "malformed-basic-auth"}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.AuthenticationFailedReason, "required fields 'username' and 'password"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// No repo index due to fetch fail.
@ -504,6 +554,12 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
Revision: checksum,
Checksum: checksum,
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
// chartRepo.Checksum isn't populated, artifact.Checksum is
@ -523,6 +579,12 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
Revision: checksum,
Checksum: "foo",
}
conditions.MarkReconciling(obj, meta.ProgressingReason, "foo")
conditions.MarkUnknown(obj, meta.ReadyCondition, "foo", "bar")
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "foo"),
*conditions.UnknownCondition(meta.ReadyCondition, "foo", "bar"),
},
afterFunc: func(t *WithT, obj *sourcev1.HelmRepository, artifact sourcev1.Artifact, chartRepo repository.ChartRepository) {
t.Expect(chartRepo.Checksum).ToNot(BeEmpty())
@ -532,12 +594,29 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
},
want: sreconcile.ResultSuccess,
},
{
name: "Existing artifact makes ArtifactOutdated=True",
protocol: "http",
beforeFunc: func(t *WithT, obj *sourcev1.HelmRepository, checksum string) {
obj.Status.Artifact = &sourcev1.Artifact{
Path: "some-path",
Revision: "some-rev",
}
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new index revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new index revision"),
},
},
}
for _, tt := range tests {
obj := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
Generation: 1,
},
Spec: sourcev1.HelmRepositorySpec{
Interval: metav1.Duration{Duration: interval},
@ -642,11 +721,19 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
Client: builder.Build(),
Storage: testStorage,
Getters: testGetters,
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
var chartRepo repository.ChartRepository
var artifact sourcev1.Artifact
got, err := r.reconcileSource(context.TODO(), obj, &artifact, &chartRepo)
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileSource(context.TODO(), sp, obj, &artifact, &chartRepo)
defer os.Remove(chartRepo.CachePath)
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))
@ -656,6 +743,10 @@ func TestHelmRepositoryReconciler_reconcileSource(t *testing.T) {
if tt.afterFunc != nil {
tt.afterFunc(g, obj, artifact, chartRepo)
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -676,7 +767,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
},
},
{
@ -690,7 +781,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
},
},
{
@ -701,7 +792,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
},
},
{
@ -718,7 +809,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact for revision 'existing'"),
*conditions.TrueCondition(sourcev1.ArtifactInStorageCondition, meta.SucceededReason, "stored artifact: revision 'existing'"),
},
},
}
@ -728,8 +819,10 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
g := NewWithT(t)
r := &HelmRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmRepository{
@ -766,8 +859,9 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
if tt.beforeFunc != nil {
tt.beforeFunc(g, obj, artifact, chartRepo)
}
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileArtifact(context.TODO(), obj, &artifact, chartRepo)
got, err := r.reconcileArtifact(context.TODO(), sp, obj, &artifact, chartRepo)
g.Expect(err != nil).To(Equal(tt.wantErr))
g.Expect(got).To(Equal(tt.want))
@ -788,7 +882,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
// Helper to build simple helmRepositoryReconcileFunc with result and error.
buildReconcileFuncs := func(r sreconcile.Result, e error) helmRepositoryReconcileFunc {
return func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
return func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
return r, e
}
}
@ -809,6 +903,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultSuccess,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "successful reconciliation with generation difference",
@ -820,7 +918,8 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
wantResult: sreconcile.ResultSuccess,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "reconciling new object generation (3)"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "processing object: new generation 2 -> 3"),
},
},
{
@ -830,16 +929,20 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "multiple object status conditions mutations",
reconcileFuncs: []helmRepositoryReconcileFunc{
func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision")
return sreconcile.ResultSuccess, nil
},
func(ctx context.Context, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
conditions.MarkTrue(obj, meta.ReconcilingCondition, "Progressing", "creating artifact")
func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.HelmRepository, artifact *sourcev1.Artifact, repo *repository.ChartRepository) (sreconcile.Result, error) {
conditions.MarkTrue(obj, meta.ReconcilingCondition, meta.ProgressingReason, "creating artifact")
return sreconcile.ResultSuccess, nil
},
},
@ -847,7 +950,8 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "Progressing", "creating artifact"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "creating artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
@ -859,6 +963,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultRequeue,
wantErr: false,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
{
name: "subrecs with error before result=Requeue",
@ -869,6 +977,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
},
wantResult: sreconcile.ResultEmpty,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "reconciliation in progress"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "reconciliation in progress"),
},
},
}
@ -876,7 +988,10 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
g := NewWithT(t)
r := &HelmRepositoryReconciler{}
r := &HelmRepositoryReconciler{
Client: fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()).Build(),
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.HelmRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
@ -887,9 +1002,15 @@ func TestHelmRepositoryReconciler_reconcileSubRecs(t *testing.T) {
},
}
ctx := context.TODO()
g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred())
}()
gotRes, gotErr := r.reconcile(ctx, obj, tt.reconcileFuncs)
ctx := context.TODO()
sp := patch.NewSerialPatcher(obj, r.Client)
gotRes, gotErr := r.reconcile(ctx, sp, obj, tt.reconcileFuncs)
g.Expect(gotErr != nil).To(Equal(tt.wantErr))
g.Expect(gotRes).To(Equal(tt.wantResult))
@ -960,8 +1081,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
clientBuilder.WithObjects(obj)
c := clientBuilder.Build()
patchHelper, err := patch.NewHelper(obj, c)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, c)
if tt.beforeFunc != nil {
tt.beforeFunc(obj)
@ -971,7 +1091,7 @@ func TestHelmRepositoryReconciler_statusConditions(t *testing.T) {
recResult := sreconcile.ResultSuccess
var retErr error
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), patchHelper)
summarizeHelper := summarize.NewHelper(record.NewFakeRecorder(32), serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(helmRepositoryReadyCondition),
summarize.WithReconcileResult(recResult),
@ -1084,6 +1204,7 @@ func TestHelmRepositoryReconciler_notify(t *testing.T) {
reconciler := &HelmRepositoryReconciler{
EventRecorder: recorder,
patchOptions: getPatchOptions(helmRepositoryReadyCondition.Owned, "sc"),
}
chartRepo := repository.ChartRepository{
URL: "some-address",

76
controllers/ocirepository_controller.go
View File

@ -31,7 +31,6 @@ import (
"time"
"github.com/Masterminds/semver/v3"
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
soci "github.com/fluxcd/source-controller/internal/oci"
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/authn/k8schain"
@ -54,6 +53,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/predicate"
"sigs.k8s.io/controller-runtime/pkg/ratelimiter"
eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
"github.com/fluxcd/pkg/apis/meta"
"github.com/fluxcd/pkg/oci"
"github.com/fluxcd/pkg/oci/auth/login"
@ -61,9 +61,11 @@ import (
helper "github.com/fluxcd/pkg/runtime/controller"
"github.com/fluxcd/pkg/runtime/patch"
"github.com/fluxcd/pkg/runtime/predicates"
rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
"github.com/fluxcd/pkg/sourceignore"
"github.com/fluxcd/pkg/untar"
"github.com/fluxcd/pkg/version"
sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
serror "github.com/fluxcd/source-controller/internal/error"
sreconcile "github.com/fluxcd/source-controller/internal/reconcile"
@ -120,7 +122,7 @@ func (e invalidOCIURLError) Error() string {
// ociRepositoryReconcileFunc is the function type for all the v1beta2.OCIRepository
// (sub)reconcile functions. The type implementations are grouped and
// executed serially to perform the complete reconcile of the object.
type ociRepositoryReconcileFunc func(ctx context.Context, obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error)
type ociRepositoryReconcileFunc func(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error)
// OCIRepositoryReconciler reconciles a v1beta2.OCIRepository object
type OCIRepositoryReconciler struct {
@ -131,6 +133,8 @@ type OCIRepositoryReconciler struct {
Storage *Storage
ControllerName string
requeueDependency time.Duration
patchOptions []patch.Option
}
type OCIRepositoryReconcilerOptions struct {
@ -145,6 +149,8 @@ func (r *OCIRepositoryReconciler) SetupWithManager(mgr ctrl.Manager) error {
}
func (r *OCIRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager, opts OCIRepositoryReconcilerOptions) error {
r.patchOptions = getPatchOptions(ociRepositoryReadyCondition.Owned, r.ControllerName)
r.requeueDependency = opts.DependencyRequeueInterval
return ctrl.NewControllerManagedBy(mgr).
@ -178,10 +184,7 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.RecordSuspend(ctx, obj, obj.Spec.Suspend)
// Initialize the patch helper with the current version of the object.
patchHelper, err := patch.NewHelper(obj, r.Client)
if err != nil {
return ctrl.Result{}, err
}
serialPatcher := patch.NewSerialPatcher(obj, r.Client)
// recResult stores the abstracted reconcile result.
var recResult sreconcile.Result
@ -189,7 +192,7 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
// Always attempt to patch the object and status after each reconciliation
// NOTE: The final runtime result and error are set in this block.
defer func() {
summarizeHelper := summarize.NewHelper(r.EventRecorder, patchHelper)
summarizeHelper := summarize.NewHelper(r.EventRecorder, serialPatcher)
summarizeOpts := []summarize.Option{
summarize.WithConditions(ociRepositoryReadyCondition),
summarize.WithBiPolarityConditionTypes(sourcev1.SourceVerifiedCondition),
@ -236,19 +239,36 @@ func (r *OCIRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reques
r.reconcileSource,
r.reconcileArtifact,
}
recResult, retErr = r.reconcile(ctx, obj, reconcilers)
recResult, retErr = r.reconcile(ctx, serialPatcher, obj, reconcilers)
return
}
// reconcile iterates through the ociRepositoryReconcileFunc tasks for the
// object. It returns early on the first call that returns
// reconcile.ResultRequeue, or produces an error.
func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.OCIRepository, reconcilers []ociRepositoryReconcileFunc) (sreconcile.Result, error) {
func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.OCIRepository, reconcilers []ociRepositoryReconcileFunc) (sreconcile.Result, error) {
oldObj := obj.DeepCopy()
// Mark as reconciling if generation differs.
if obj.Generation != obj.Status.ObservedGeneration {
conditions.MarkReconciling(obj, "NewGeneration", "reconciling new object generation (%d)", obj.Generation)
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
var reconcileAtVal string
if v, ok := meta.ReconcileAnnotationValue(obj.GetAnnotations()); ok {
reconcileAtVal = v
}
// Persist reconciling status if generation differs or reconciliation is
// requested.
switch {
case obj.Generation != obj.Status.ObservedGeneration:
rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
case reconcileAtVal != obj.Status.GetLastHandledReconcileRequest():
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
}
// Create temp working dir
@ -276,7 +296,7 @@ func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.O
// Run the sub-reconcilers and build the result of reconciliation.
for _, rec := range reconcilers {
recResult, err := rec(ctx, obj, &metadata, tmpDir)
recResult, err := rec(ctx, sp, obj, &metadata, tmpDir)
// Exit immediately on ResultRequeue.
if recResult == sreconcile.ResultRequeue {
return sreconcile.ResultRequeue, nil
@ -299,7 +319,8 @@ func (r *OCIRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.O
// reconcileSource fetches the upstream OCI artifact metadata and content.
// If this fails, it records v1beta2.FetchFailedCondition=True on the object and returns early.
func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
var auth authn.Authenticator
ctxTimeout, cancel := context.WithTimeout(ctx, obj.Spec.Timeout.Duration)
@ -385,8 +406,14 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, obj *sour
defer func() {
if !obj.GetArtifact().HasRevision(revision) {
message := fmt.Sprintf("new revision '%s' for '%s'", revision, url)
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
conditions.MarkReconciling(obj, "NewRevision", message)
if obj.GetArtifact() != nil {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", message)
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, "building artifact: %s", message)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
ctrl.LoggerFrom(ctx).Error(err, "failed to patch")
return
}
}
}()
@ -876,22 +903,32 @@ func oidcAuth(ctx context.Context, url, provider string) (authn.Authenticator, e
// condition is added.
// The hostname of any URL in the Status of the object are updated, to ensure
// they match the Storage server hostname of current runtime.
func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sourcev1.OCIRepository, _ *sourcev1.Artifact, _ string) (sreconcile.Result, error) {
func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.OCIRepository, _ *sourcev1.Artifact, _ string) (sreconcile.Result, error) {
// Garbage collect previous advertised artifact(s) from storage
_ = r.garbageCollect(ctx, obj)
// Determine if the advertised artifact is still in storage
var artifactMissing bool
if artifact := obj.GetArtifact(); artifact != nil && !r.Storage.ArtifactExist(*artifact) {
obj.Status.Artifact = nil
obj.Status.URL = ""
artifactMissing = true
// 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")
msg := "building artifact"
if artifactMissing {
msg += ": disappeared from storage"
}
rreconcile.ProgressiveStatus(true, obj, meta.ProgressingReason, msg)
conditions.Delete(obj, sourcev1.ArtifactInStorageCondition)
if err := sp.Patch(ctx, obj, r.patchOptions...); err != nil {
return sreconcile.ResultEmpty, err
}
return sreconcile.ResultSuccess, nil
}
@ -911,7 +948,8 @@ func (r *OCIRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sou
// early.
// 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 *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
func (r *OCIRepositoryReconciler) reconcileArtifact(ctx context.Context, sp *patch.SerialPatcher,
obj *sourcev1.OCIRepository, metadata *sourcev1.Artifact, dir string) (sreconcile.Result, error) {
revision := metadata.Revision
// Create artifact

165
controllers/ocirepository_controller_test.go
View File

@ -335,7 +335,7 @@ func TestOCIRepository_Reconcile_MediaType(t *testing.T) {
return false
}
readyCondition := conditions.Get(obj, meta.ReadyCondition)
return readyCondition != nil
return readyCondition != nil && !conditions.IsUnknown(obj, meta.ReadyCondition)
}, timeout).Should(BeTrue())
g.Expect(conditions.IsReady(obj)).To(BeIdenticalTo(!tt.wantErr))
@ -383,8 +383,8 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
name: "HTTP without basic auth",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
},
},
{
@ -404,8 +404,8 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
includeSecret: true,
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
},
},
{
@ -425,8 +425,8 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
includeSA: true,
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
},
},
{
@ -508,8 +508,8 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
},
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
},
},
{
@ -580,8 +580,8 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
},
provider: "azure",
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
},
},
}
@ -595,6 +595,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "auth-strategy-",
Generation: 1,
},
Spec: sourcev1.OCIRepositorySpec{
Interval: metav1.Duration{Duration: interval},
@ -667,6 +668,7 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
opts := craneOptions(ctx, true)
@ -680,8 +682,15 @@ func TestOCIRepository_reconcileSource_authStrategy(t *testing.T) {
assertConditions[k].Message = strings.ReplaceAll(assertConditions[k].Message, "<url>", repoURL)
}
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
tmpDir := t.TempDir()
got, err := r.reconcileSource(ctx, obj, &sourcev1.Artifact{}, tmpDir)
got, err := r.reconcileSource(ctx, sp, obj, &sourcev1.Artifact{}, tmpDir)
if tt.wantErr {
g.Expect(err).ToNot(BeNil())
} else {
@ -778,6 +787,7 @@ func TestOCIRepository_CertSecret(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
GenerateName: "ocirepository-test-resource",
Namespace: ns.Name,
Generation: 1,
},
Spec: sourcev1.OCIRepositorySpec{
URL: tt.url,
@ -818,7 +828,7 @@ func TestOCIRepository_CertSecret(t *testing.T) {
return false
}
readyCondition := conditions.Get(&resultobj, meta.ReadyCondition)
if readyCondition == nil {
if readyCondition == nil || conditions.IsUnknown(&resultobj, meta.ReadyCondition) {
return false
}
return obj.Generation == readyCondition.ObservedGeneration &&
@ -866,8 +876,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
want: sreconcile.ResultSuccess,
wantRevision: fmt.Sprintf("latest/%s", img6.digest.Hex),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
{
@ -878,8 +888,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
want: sreconcile.ResultSuccess,
wantRevision: fmt.Sprintf("%s/%s", img6.tag, img6.digest.Hex),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
{
@ -890,8 +900,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
want: sreconcile.ResultSuccess,
wantRevision: fmt.Sprintf("%s/%s", img6.tag, img6.digest.Hex),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
{
@ -902,8 +912,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
wantRevision: img6.digest.Hex,
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
{
@ -948,8 +958,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
want: sreconcile.ResultSuccess,
wantRevision: fmt.Sprintf("%s/%s", img6.tag, img6.digest.Hex),
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
{
@ -962,8 +972,8 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
want: sreconcile.ResultSuccess,
wantRevision: img5.digest.Hex,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision"),
},
},
}
@ -974,6 +984,7 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -981,6 +992,7 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "checkout-strategy-",
Generation: 1,
},
Spec: sourcev1.OCIRepositorySpec{
URL: fmt.Sprintf("oci://%s/podinfo", server.registryHost),
@ -993,9 +1005,16 @@ func TestOCIRepository_reconcileSource_remoteReference(t *testing.T) {
obj.Spec.Reference = tt.reference
}
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
artifact := &sourcev1.Artifact{}
tmpDir := t.TempDir()
got, err := r.reconcileSource(ctx, obj, artifact, tmpDir)
got, err := r.reconcileSource(ctx, sp, obj, artifact, tmpDir)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
@ -1043,8 +1062,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
shouldSign: true,
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, meta.SucceededReason, "verified signature of revision <digest>"),
},
},
@ -1058,8 +1077,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
wantErrMsg: "failed to verify the signature using provider 'cosign': no matching signatures were found for '<url>'",
want: sreconcile.ResultEmpty,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider '<provider>': no matching signatures were found for '<url>'"),
},
},
@ -1073,8 +1092,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
want: sreconcile.ResultEmpty,
keyless: true,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "failed to verify the signature using provider '<provider> keyless': no matching signatures"),
},
},
@ -1132,8 +1151,8 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
wantErr: true,
want: sreconcile.ResultEmpty,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new revision '<digest>' for '<url>'"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: new revision '<digest>' for '<url>'"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, sourcev1.VerificationError, "cosign does not support insecure registries"),
},
},
@ -1145,6 +1164,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
pf := func(b bool) ([]byte, error) {
@ -1175,6 +1195,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "verify-oci-source-signature-",
Generation: 1,
},
Spec: sourcev1.OCIRepositorySpec{
URL: fmt.Sprintf("oci://%s/podinfo", server.registryHost),
@ -1236,8 +1257,15 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignature(t *testing.T) {
tt.beforeFunc(obj)
}
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
artifact := &sourcev1.Artifact{}
got, err := r.reconcileSource(ctx, obj, artifact, tmpDir)
got, err := r.reconcileSource(ctx, sp, obj, artifact, tmpDir)
if tt.wantErr {
tt.wantErrMsg = strings.ReplaceAll(tt.wantErrMsg, "<url>", artifactURL)
g.Expect(err).ToNot(BeNil())
@ -1373,6 +1401,7 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -1382,6 +1411,7 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "noop-",
Generation: 1,
},
Spec: sourcev1.OCIRepositorySpec{
URL: fmt.Sprintf("oci://%s/podinfo", server.registryHost),
@ -1395,9 +1425,16 @@ func TestOCIRepository_reconcileSource_noop(t *testing.T) {
tt.beforeFunc(obj)
}
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
artifact := &sourcev1.Artifact{}
tmpDir := t.TempDir()
got, err := r.reconcileSource(ctx, obj, artifact, tmpDir)
got, err := r.reconcileSource(ctx, sp, obj, artifact, tmpDir)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(got).To(Equal(sreconcile.ResultSuccess))
@ -1593,6 +1630,7 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -1602,6 +1640,7 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "reconcile-artifact-",
Generation: 1,
},
}
if tt.beforeFunc != nil {
@ -1612,7 +1651,15 @@ func TestOCIRepository_reconcileArtifact(t *testing.T) {
if tt.artifact != nil {
artifact = tt.artifact
}
got, err := r.reconcileArtifact(ctx, obj, artifact, tt.targetPath)
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileArtifact(ctx, sp, obj, artifact, tt.targetPath)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
@ -1698,6 +1745,7 @@ func TestOCIRepository_getArtifactURL(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -1763,7 +1811,7 @@ func TestOCIRepository_stalled(t *testing.T) {
return false
}
return obj.Generation == readyCondition.ObservedGeneration &&
!conditions.IsReady(&resultobj)
!conditions.IsUnknown(&resultobj, meta.ReadyCondition)
}, timeout).Should(BeTrue())
// Verify that stalled condition is present in status
@ -1809,6 +1857,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
}
testStorage.SetArtifactURL(obj.Status.Artifact)
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
assertArtifact: &sourcev1.Artifact{
@ -1825,6 +1874,17 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
"!/oci-reconcile-storage/a.txt",
},
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
{
name: "build artifact first time",
want: sreconcile.ResultSuccess,
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact"),
},
},
{
name: "notices missing artifact in storage",
@ -1841,7 +1901,8 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
"!/oci-reconcile-storage/invalid.txt",
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, "NoArtifact", "no artifact for resource in storage"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
*conditions.UnknownCondition(meta.ReadyCondition, meta.ProgressingReason, "building artifact: disappeared from storage"),
},
},
{
@ -1859,6 +1920,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
if err := testStorage.AtomicWriteFile(obj.Status.Artifact, strings.NewReader("file"), 0o640); err != nil {
return err
}
conditions.MarkTrue(obj, meta.ReadyCondition, "foo", "bar")
return nil
},
want: sreconcile.ResultSuccess,
@ -1872,6 +1934,9 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
URL: testStorage.Hostname + "/oci-reconcile-storage/hostname.txt",
Size: int64p(int64(len("file"))),
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReadyCondition, "foo", "bar"),
},
},
}
@ -1880,6 +1945,7 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
Client: builder.Build(),
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
for _, tt := range tests {
@ -1888,11 +1954,22 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
obj := &sourcev1.OCIRepository{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-",
Generation: 1,
},
}
g.Expect(tt.beforeFunc(obj)).To(Succeed())
got, err := r.reconcileStorage(ctx, obj, &sourcev1.Artifact{}, "")
if tt.beforeFunc != nil {
g.Expect(tt.beforeFunc(obj)).To(Succeed())
}
g.Expect(r.Client.Create(ctx, obj)).ToNot(HaveOccurred())
defer func() {
g.Expect(r.Client.Delete(ctx, obj)).ToNot(HaveOccurred())
}()
sp := patch.NewSerialPatcher(obj, r.Client)
got, err := r.reconcileStorage(ctx, sp, obj, &sourcev1.Artifact{}, "")
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
} else {
@ -1916,6 +1993,10 @@ func TestOCIRepository_reconcileStorage(t *testing.T) {
g.Expect(absoluteP).ToNot(BeAnExistingFile())
}
// In-progress status condition validity.
checker := conditionscheck.NewInProgressChecker(r.Client)
checker.CheckErr(ctx, obj)
})
}
}
@ -1926,6 +2007,7 @@ func TestOCIRepository_ReconcileDelete(t *testing.T) {
r := &OCIRepositoryReconciler{
EventRecorder: record.NewFakeRecorder(32),
Storage: testStorage,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
obj := &sourcev1.OCIRepository{
@ -2058,6 +2140,7 @@ func TestOCIRepositoryReconciler_notify(t *testing.T) {
reconciler := &OCIRepositoryReconciler{
EventRecorder: recorder,
patchOptions: getPatchOptions(ociRepositoryReadyCondition.Owned, "sc"),
}
reconciler.notify(ctx, oldObj, newObj, tt.res, tt.resErr)

16
docs/spec/v1beta2/buckets.md
View File

@ -866,9 +866,9 @@ Status:
...
Conditions:
Last Transition Time: 2022-02-02T13:26:55Z
Message: reconciling new object generation (2)
Message: processing object: new generation 1 -> 2
Observed Generation: 2
Reason: NewGeneration
Reason: ProgressingWithRetry
Status: True
Type: Reconciling
Last Transition Time: 2022-02-02T13:26:55Z
@ -978,13 +978,13 @@ is true:
- The generation of the Bucket is newer than the [Observed Generation](#observed-generation).
- The newly calculated Artifact revision differs from the current Artifact.
When the Bucket is "reconciling", the `Ready` Condition status becomes `False`,
and the controller adds a Condition with the following attributes to the
Bucket's `.status.conditions`:
When the Bucket is "reconciling", the `Ready` Condition status becomes
`Unknown` when the controller detects drift, and the controller adds a Condition
with the following attributes to the Bucket's `.status.conditions`:
- `type: Reconciling`
- `status: "True"`
- `reason: NewGeneration` | `reason: NoArtifact` | `reason: NewRevision`
- `reason: Progressing` | `reason: ProgressingWithRetry`
If the reconciling state is due to a new revision, an additional Condition is
added with the following attributes:
@ -1062,7 +1062,9 @@ it succeeds and the Bucket is marked as [ready](#ready-bucket).
Note that a Bucket can be [reconciling](#reconciling-bucket) while failing at
the same time, for example due to a newly introduced configuration issue in the
Bucket spec.
Bucket spec. When a reconciliation fails, the `Reconciling` Condition reason
would be `ProgressingWithRetry`. When the reconciliation is performed again
after the failure, the reason is updated to `Progressing`.
### Observed Ignore

16
docs/spec/v1beta2/gitrepositories.md
View File

@ -610,9 +610,9 @@ Status:
...
Conditions:
Last Transition Time: 2022-02-14T09:40:27Z
Message: reconciling new object generation (2)
Message: processing object: new generation 1 -> 2
Observed Generation: 2
Reason: NewGeneration
Reason: ProgressingWithRetry
Status: True
Type: Reconciling
Last Transition Time: 2022-02-14T09:40:27Z
@ -724,12 +724,13 @@ following is true:
- The newly resolved Artifact revision differs from the current Artifact.
When the GitRepository is "reconciling", the `Ready` Condition status becomes
`False`, and the controller adds a Condition with the following attributes to
the GitRepository's `.status.conditions`:
`Unknown` when the controller detects drift, and the controller adds a Condition
with the following attributes to the GitRepository's
`.status.conditions`:
- `type: Reconciling`
- `status: "True"`
- `reason: NewGeneration` | `reason: NoArtifact` | `reason: NewRevision`
- `reason: Progressing` | `reason: ProgressingWithRetry`
If the reconciling state is due to a new revision, an additional Condition is
added with the following attributes:
@ -819,7 +820,10 @@ exponential backoff, until it succeeds and the GitRepository is marked as
Note that a GitRepository can be [reconciling](#reconciling-gitrepository)
while failing at the same time, for example due to a newly introduced
configuration issue in the GitRepository spec.
configuration issue in the GitRepository spec. When a reconciliation fails, the
`Reconciling` Condition reason would be `ProgressingWithRetry`. When the
reconciliation is performed again after the failure, the reason is updated to
`Progressing`.
### Content Configuration Checksum

11
docs/spec/v1beta2/helmcharts.md
View File

@ -633,12 +633,12 @@ following is true:
- The newly fetched Artifact revision differs from the current Artifact.
When the HelmChart is "reconciling", the `Ready` Condition status becomes
`False`, and the controller adds a Condition with the following attributes to
the HelmChart's `.status.conditions`:
`Unknown` when the controller detects drift, and the controller adds a Condition
with the following attributes to the HelmChart's `.status.conditions`:
- `type: Reconciling`
- `status: "True"`
- `reason: NewGeneration` | `reason: NoArtifact`
- `reason: Progressing` | `reason: ProgressingWithRetry`
If the reconciling state is due to a new version, it adds an additional
Condition with the following attributes:
@ -715,7 +715,10 @@ until it succeeds and the HelmChart is marked as [ready](#ready-helmchart).
Note that a HelmChart can be [reconciling](#reconciling-helmchart)
while failing at the same time, for example due to a newly introduced
configuration issue in the HelmChart spec.
configuration issue in the HelmChart spec. When a reconciliation fails, the
`Reconciling` Condition reason would be `ProgressingWithRetry`. When the
reconciliation is performed again after the failure, the reason is updated to
`Progressing`.
#### Stalled HelmChart

11
docs/spec/v1beta2/helmrepositories.md
View File

@ -676,12 +676,12 @@ is true:
- The newly fetched Artifact revision differs from the current Artifact.
When the HelmRepository is "reconciling", the `Ready` Condition status becomes
`False`, and the controller adds a Condition with the following attributes to
the HelmRepository's `.status.conditions`:
`Unknown` when the controller detects drift, and the controller adds a Condition
with the following attributes to the HelmRepository's `.status.conditions`:
- `type: Reconciling`
- `status: "True"`
- `reason: NewGeneration` | `reason: NoArtifact` | `reason: NewRevision`
- `reason: Progressing` | `reason: ProgressingWithRetry`
If the reconciling state is due to a new revision, it adds an additional
Condition with the following attributes:
@ -760,7 +760,10 @@ until it succeeds and the HelmRepository is marked as [ready](#ready-helmreposit
Note that a HelmRepository can be [reconciling](#reconciling-helmrepository)
while failing at the same time, for example due to a newly introduced
configuration issue in the HelmRepository spec.
configuration issue in the HelmRepository spec. When a reconciliation fails, the
`Reconciling` Condition reason would be `ProgressingWithRetry`. When the
reconciliation is performed again after the failure, the reason is updated to
`Progressing`.
#### Stalled HelmRepository

15
docs/spec/v1beta2/ocirepositories.md
View File

@ -642,9 +642,9 @@ Status:
...
Conditions:
Last Transition Time: 2022-02-14T09:40:27Z
Message: reconciling new object generation (2)
Message: processing object: new generation 1 -> 2
Observed Generation: 2
Reason: NewGeneration
Reason: ProgressingWithRetry
Status: True
Type: Reconciling
Last Transition Time: 2022-02-14T09:40:27Z
@ -769,12 +769,12 @@ following is true:
- The newly resolved Artifact digest differs from the current Artifact.
When the OCIRepository is "reconciling", the `Ready` Condition status becomes
`False`, and the controller adds a Condition with the following attributes to
the OCIRepository's `.status.conditions`:
`Unknown` when the controller detects drift, and the controller adds a Condition
with the following attributes to the OCIRepository's `.status.conditions`:
- `type: Reconciling`
- `status: "True"`
- `reason: NewGeneration` | `reason: NoArtifact` | `reason: NewRevision`
- `reason: Progressing` | `reason: ProgressingWithRetry`
If the reconciling state is due to a new revision, an additional Condition is
added with the following attributes:
@ -862,7 +862,10 @@ exponential backoff, until it succeeds and the OCIRepository is marked as
Note that a OCIRepository can be [reconciling](#reconciling-ocirepository)
while failing at the same time, for example due to a newly introduced
configuration issue in the OCIRepository spec.
configuration issue in the OCIRepository spec. When a reconciliation fails, the
`Reconciling` Condition reason would be `ProgressingWithRetry`. When the
reconciliation is performed again after the failure, the reason is updated to
`Progressing`.
### Content Configuration Checksum

2
go.mod
View File

@ -31,7 +31,7 @@ require (
github.com/fluxcd/pkg/lockedfile v0.1.0
github.com/fluxcd/pkg/masktoken v0.2.0
github.com/fluxcd/pkg/oci v0.17.0
github.com/fluxcd/pkg/runtime v0.24.0
github.com/fluxcd/pkg/runtime v0.25.0
github.com/fluxcd/pkg/sourceignore v0.3.0
github.com/fluxcd/pkg/ssh v0.7.0
github.com/fluxcd/pkg/testserver v0.4.0

4
go.sum
View File

@ -549,8 +549,8 @@ github.com/fluxcd/pkg/masktoken v0.2.0 h1:HoSPTk4l1fz5Fevs2vVRvZGru33blfMwWSZKsH
github.com/fluxcd/pkg/masktoken v0.2.0/go.mod h1:EA7GleAHL33kN6kTW06m5R3/Q26IyuGO7Ef/0CtpDI0=
github.com/fluxcd/pkg/oci v0.17.0 h1:DYoT0HG3DogEmeXRif6ZzTYwAZe+iqYWP4QpsP37ZBE=
github.com/fluxcd/pkg/oci v0.17.0/go.mod h1:UjxCQcdcKtog/ad9Vr2yPYjz9keNSoLdTOOiUNqCRiY=
github.com/fluxcd/pkg/runtime v0.24.0 h1:rQmm5Xq8K7f8xcPj1oNOInM1x4YwmgTucZJOP51Xmr4=
github.com/fluxcd/pkg/runtime v0.24.0/go.mod h1:I2T+HWVNzX0cxm9TgH+SVNHTwqlmEDiSke43JXsq9iY=
github.com/fluxcd/pkg/runtime v0.25.0 h1:Lk5WrKDJKsayymLnnSCY/Zn77/mrlIf+skYz64suoww=
github.com/fluxcd/pkg/runtime v0.25.0/go.mod h1:I2T+HWVNzX0cxm9TgH+SVNHTwqlmEDiSke43JXsq9iY=
github.com/fluxcd/pkg/sourceignore v0.3.0 h1:pFO3hKV9ub+2SrNZPZE7xfiRhxsycRrd7JK7qB26nVw=
github.com/fluxcd/pkg/sourceignore v0.3.0/go.mod h1:ak3Tve/KwVzytZ5V2yBlGGpTJ/2oQ9kcP3iuwBOAHGo=
github.com/fluxcd/pkg/ssh v0.7.0 h1:FX5ky8SU9dYwbM6zEIDR3TSveLF01iyS95CtB5Ykpno=

9
internal/reconcile/reconcile.go
View File

@ -124,6 +124,15 @@ func ComputeReconcileResult(obj conditions.Setter, res Result, recErr error, rb
conditions.Delete(obj, meta.ReconcilingCondition)
}
// Presence of reconciling means that the reconciliation didn't succeed.
// Set the Reconciling reason to ProgressingWithRetry to indicate a failure
// retry.
if conditions.IsReconciling(obj) {
reconciling := conditions.Get(obj, meta.ReconcilingCondition)
reconciling.Reason = meta.ProgressingWithRetryReason
conditions.Set(obj, reconciling)
}
// Analyze the reconcile error.
switch t := recErr.(type) {
case *serror.Stalling:

15
internal/reconcile/reconcile_test.go
View File

@ -206,6 +206,21 @@ func TestComputeReconcileResult(t *testing.T) {
t.Expect(conditions.IsUnknown(obj, meta.StalledCondition)).To(BeTrue())
},
},
{
name: "failed with Reconciling=True adds ProgressingWithRetry reason",
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, meta.ProgressingReason, "some msg")
},
result: ResultEmpty,
recErr: fmt.Errorf("some error"),
wantResult: ctrl.Result{},
wantErr: true,
afterFunc: func(t *WithT, obj conditions.Setter, patchOpts *patch.HelperOptions) {
},
assertConditions: []metav1.Condition{
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "some msg"),
},
},
}
for _, tt := range tests {

12
internal/reconcile/summarize/summary.go
View File

@ -50,15 +50,15 @@ type Conditions struct {
// Helper is SummarizeAndPatch helper.
type Helper struct {
recorder kuberecorder.EventRecorder
patchHelper *patch.Helper
recorder kuberecorder.EventRecorder
serialPatcher *patch.SerialPatcher
}
// NewHelper returns an initialized Helper.
func NewHelper(recorder kuberecorder.EventRecorder, patchHelper *patch.Helper) *Helper {
func NewHelper(recorder kuberecorder.EventRecorder, serialPatcher *patch.SerialPatcher) *Helper {
return &Helper{
recorder: recorder,
patchHelper: patchHelper,
recorder: recorder,
serialPatcher: serialPatcher,
}
}
@ -250,7 +250,7 @@ func (h *Helper) SummarizeAndPatch(ctx context.Context, obj conditions.Setter, o
}
// Finally, patch the resource.
if err := h.patchHelper.Patch(ctx, obj, patchOpts...); err != nil {
if err := h.serialPatcher.Patch(ctx, obj, patchOpts...); err != nil {
// Ignore patch error "not found" when the object is being deleted.
if opts.IgnoreNotFound && !obj.GetDeletionTimestamp().IsZero() {
err = kerrors.FilterOut(err, func(e error) bool { return apierrors.IsNotFound(e) })

26
internal/reconcile/summarize/summary_test.go
View File

@ -128,7 +128,7 @@ func TestSummarizeAndPatch(t *testing.T) {
name: "Success, removes reconciling for successful result",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, "NewRevision", "new index version")
conditions.MarkReconciling(obj, meta.ProgressingReason, "new index version")
conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "stored artifact")
},
conditions: []Conditions{testReadyConditions},
@ -167,7 +167,7 @@ func TestSummarizeAndPatch(t *testing.T) {
generation: 7,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision")
conditions.MarkReconciling(obj, "NewRevision", "new index revision")
conditions.MarkReconciling(obj, meta.ProgressingReason, "new index revision")
},
conditions: []Conditions{testReadyConditions},
reconcileErr: fmt.Errorf("failed to create dir"),
@ -175,7 +175,7 @@ func TestSummarizeAndPatch(t *testing.T) {
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new index revision"),
},
afterFunc: func(t *WithT, obj client.Object) {
t.Expect(obj).ToNot(HaveStatusObservedGeneration(7))
@ -264,7 +264,7 @@ func TestSummarizeAndPatch(t *testing.T) {
name: "Fail, reconciling with bipolar condition False, Ready gets bipolar failure value",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, "NewRevision", "new index revision")
conditions.MarkReconciling(obj, meta.ProgressingReason, "new index revision")
conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed")
},
result: reconcile.ResultEmpty,
@ -275,14 +275,14 @@ func TestSummarizeAndPatch(t *testing.T) {
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "VerifyFailed", "verify failed"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new index revision"),
},
},
{
name: "Fail, bipolar condition True, negative polarity True, Ready gets negative polarity value",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, "NewGeneration", "new obj gen")
conditions.MarkReconciling(obj, meta.ProgressingReason, "new obj gen")
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest")
conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Success", "verified")
},
@ -294,7 +294,7 @@ func TestSummarizeAndPatch(t *testing.T) {
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "NewRevision", "new digest"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "new obj gen"),
*conditions.TrueCondition(meta.ReconcilingCondition, meta.ProgressingWithRetryReason, "new obj gen"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Success", "verified"),
},
},
@ -345,10 +345,9 @@ func TestSummarizeAndPatch(t *testing.T) {
ctx := context.TODO()
g.Expect(client.Create(ctx, obj)).To(Succeed())
patchHelper, err := patch.NewHelper(obj, client)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, client)
summaryHelper := NewHelper(record.NewFakeRecorder(32), patchHelper)
summaryHelper := NewHelper(record.NewFakeRecorder(32), serialPatcher)
summaryOpts := []Option{
WithReconcileResult(tt.result),
WithReconcileError(tt.reconcileErr),
@ -471,15 +470,14 @@ func TestSummarizeAndPatch_Intermediate(t *testing.T) {
ctx := context.TODO()
g.Expect(kclient.Create(ctx, obj)).To(Succeed())
patchHelper, err := patch.NewHelper(obj, kclient)
g.Expect(err).ToNot(HaveOccurred())
serialPatcher := patch.NewSerialPatcher(obj, kclient)
summaryHelper := NewHelper(record.NewFakeRecorder(32), patchHelper)
summaryHelper := NewHelper(record.NewFakeRecorder(32), serialPatcher)
summaryOpts := []Option{
WithConditions(tt.conditions...),
WithResultBuilder(reconcile.AlwaysRequeueResultBuilder{RequeueAfter: interval}),
}
_, err = summaryHelper.SummarizeAndPatch(ctx, obj, summaryOpts...)
_, err := summaryHelper.SummarizeAndPatch(ctx, obj, summaryOpts...)
g.Expect(err).ToNot(HaveOccurred())
g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(tt.assertConditions))