Skip to content

Commit 63d65f9

Browse files
hiddecohiddeco
committed
Additional Git tests
Signed-off-by: Hidde Beydals <hello@hidde.co>
1 parent b61c6c3 commit 63d65f9

File tree

5 files changed

+286
-24
lines changed

5 files changed

+286
-24
lines changed

‎controllers/gitrepository_controller.go

+20-9
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/fluxcd/source-controller/pkg/sourceignore"
3131
"github.com/go-logr/logr"
3232
corev1 "k8s.io/api/core/v1"
33+
apierrors "k8s.io/apimachinery/pkg/api/errors"
3334
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3435
"k8s.io/apimachinery/pkg/types"
3536
kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -338,6 +339,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
338339
// The artifact is up-to-date
339340
if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) {
340341
logr.FromContext(ctx).Info("Artifact is up-to-date")
342+
conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
341343
return ctrl.Result{RequeueAfter: obj.GetInterval().Duration}, nil
342344
}
343345

@@ -383,7 +385,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
383385
// Record it on the object
384386
obj.Status.Artifact = artifact.DeepCopy()
385387
obj.Status.IncludedArtifacts = includes
386-
conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision %s", artifact.Revision)
388+
conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
387389
r.Events.Eventf(ctx, obj, map[string]string{
388390
"revision": obj.GetArtifact().Revision,
389391
}, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason, conditions.Get(obj, sourcev1.ArtifactAvailableCondition).Message)
@@ -392,7 +394,6 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
392394
url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
393395
if err != nil {
394396
r.Events.Eventf(ctx, obj, nil, events.EventSeverityError, sourcev1.StorageOperationFailedReason, "Failed to update status URL symlink: %s", err)
395-
return ctrl.Result{}, err
396397
}
397398
if url != "" {
398399
obj.Status.URL = url
@@ -410,16 +411,21 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
410411
// statuses is recorded in a condition on the given object.
411412
func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sourcev1.GitRepository, artifacts artifactSet, dir string) (ctrl.Result, error) {
412413
includes := make([]conditions.Getter, len(obj.Spec.Include))
414+
artifacts = make(artifactSet, len(obj.Spec.Include))
413415

414416
for i, incl := range obj.Spec.Include {
415417
dep := &sourcev1.GitRepository{}
416418
if err := r.Get(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: incl.GitRepositoryRef.Name}, dep); err != nil {
417-
return ctrl.Result{RequeueAfter: r.requeueDependency}, client.IgnoreNotFound(err)
419+
if apierrors.IsNotFound(err){
420+
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include %q: %s", incl.GitRepositoryRef.Name, err.Error())
421+
}
422+
//r.Eventf()
423+
return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, client.IgnoreNotFound(err)
418424
}
419425

420426
// Confirm include has an artifact
421427
if dep.GetArtifact() == nil {
422-
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "No artifact available for include %s", incl.GitRepositoryRef)
428+
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include %q", incl.GitRepositoryRef.Name)
423429
return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
424430
}
425431

@@ -428,20 +434,25 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sou
428434
// Copy artifact (sub)contents to configured directory
429435
toPath, err := securejoin.SecureJoin(dir, incl.GetToPath())
430436
if err != nil {
431-
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %s: %s", incl.GitRepositoryRef, err.Error())
437+
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %q: %s", incl.GitRepositoryRef.Name, err.Error())
432438
return ctrl.Result{}, err
433439
}
434440
if err = r.Storage.CopyToPath(dep.GetArtifact(), incl.GetFromPath(), toPath); err != nil {
435-
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy %s include from %s to %s: %s", incl.GitRepositoryRef, incl.GetFromPath(), toPath, err.Error())
441+
conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy %q include from %s to %s: %s", incl.GitRepositoryRef.Name, incl.GetFromPath(), incl.GetToPath(), err.Error())
436442
return ctrl.Result{}, err
437443
}
438444

439445
artifacts[i] = dep.GetArtifact().DeepCopy()
440446
}
441447

442-
// Record an aggregation of all includes' Ready state to the object
443-
// condition
444-
conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes)
448+
// Record an aggregation of all includes Stalled or Ready state to
449+
// the object condition
450+
conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes,
451+
conditions.WithConditions(meta.StalledCondition, meta.ReadyCondition),
452+
conditions.WithNegativePolarityConditions(meta.StalledCondition),
453+
conditions.WithSourceRefIf(meta.StalledCondition),
454+
conditions.WithCounter(),
455+
conditions.WithCounterIfOnly(meta.ReadyCondition))
445456

446457
return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil
447458
}

‎controllers/gitrepository_controller_test.go

+247-1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
ctrl "sigs.k8s.io/controller-runtime"
4949
"sigs.k8s.io/controller-runtime/pkg/client"
5050
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
51+
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
5152
"sigs.k8s.io/controller-runtime/pkg/log"
5253

5354
"github.com/fluxcd/pkg/apis/meta"
@@ -591,7 +592,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
591592
},
592593
want: ctrl.Result{RequeueAfter: mockInterval},
593594
assertConditions: []metav1.Condition{
594-
*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision main/revision"),
595+
*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision main/revision"),
595596
},
596597
},
597598
{
@@ -662,6 +663,251 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
662663
}
663664
}
664665

666+
func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
667+
g := NewWithT(t)
668+
669+
storage, err := newTestStorage()
670+
g.Expect(err).NotTo(HaveOccurred())
671+
defer os.RemoveAll(storage.BasePath)
672+
673+
dependencyInterval := 5 * time.Second
674+
675+
type dependency struct {
676+
name string
677+
withArtifact bool
678+
conditions []metav1.Condition
679+
}
680+
681+
type include struct {
682+
name string
683+
fromPath string
684+
toPath string
685+
shouldExist bool
686+
}
687+
688+
tests := []struct {
689+
name string
690+
dependencies []dependency
691+
includes []include
692+
want ctrl.Result
693+
wantErr bool
694+
assertConditions []metav1.Condition
695+
}{
696+
{
697+
name: "Includes artifacts",
698+
dependencies: []dependency{
699+
{
700+
name: "a",
701+
withArtifact: true,
702+
conditions: []metav1.Condition{
703+
*conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"),
704+
},
705+
},
706+
{
707+
name: "b",
708+
withArtifact: true,
709+
conditions: []metav1.Condition{
710+
*conditions.TrueCondition(meta.ReadyCondition, "Bar", "bar ready"),
711+
},
712+
},
713+
},
714+
includes: []include{
715+
{name: "a", toPath: "a/"},
716+
{name: "b", toPath: "b/"},
717+
},
718+
want: ctrl.Result{RequeueAfter: mockInterval},
719+
assertConditions: []metav1.Condition{
720+
*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "Foo", "2 of 2 Ready"),
721+
},
722+
},
723+
{
724+
name: "Non existing artifact",
725+
includes: []include{
726+
{name: "a", toPath: "a/"},
727+
},
728+
want: ctrl.Result{RequeueAfter: mockInterval},
729+
wantErr: false,
730+
assertConditions: []metav1.Condition{
731+
*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include \"a\": gitrepositories.source.toolkit.fluxcd.io \"a\" not found"),
732+
},
733+
},
734+
{
735+
name: "Missing artifact",
736+
dependencies: []dependency{
737+
{
738+
name: "a",
739+
withArtifact: false,
740+
conditions: []metav1.Condition{
741+
*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Foo", "foo unavailable"),
742+
},
743+
},
744+
},
745+
includes: []include{
746+
{name: "a", toPath: "a/"},
747+
},
748+
want: ctrl.Result{RequeueAfter: dependencyInterval},
749+
assertConditions: []metav1.Condition{
750+
*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include \"a\""),
751+
},
752+
},
753+
{
754+
name: "Invalid FromPath",
755+
dependencies: []dependency{
756+
{
757+
name: "a",
758+
withArtifact: true,
759+
},
760+
},
761+
includes: []include{
762+
{name: "a", fromPath: "../../../path", shouldExist: false},
763+
},
764+
wantErr: true,
765+
assertConditions: []metav1.Condition{
766+
*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy \"a\" include from ../../../path to a"),
767+
},
768+
},
769+
{
770+
name: "Stalled include",
771+
dependencies: []dependency{
772+
{
773+
name: "a",
774+
withArtifact: true,
775+
conditions: []metav1.Condition{
776+
*conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"),
777+
},
778+
},
779+
{
780+
name: "b",
781+
withArtifact: true,
782+
conditions: []metav1.Condition{
783+
*conditions.TrueCondition(meta.StalledCondition, "Bar", "bar stalled"),
784+
},
785+
},
786+
},
787+
includes: []include{
788+
{name: "a", toPath: "a/"},
789+
{name: "b", toPath: "b/"},
790+
},
791+
want: ctrl.Result{RequeueAfter: mockInterval},
792+
assertConditions: []metav1.Condition{
793+
*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Bar @ GitRepository/a", "bar stalled"),
794+
},
795+
},
796+
}
797+
for _, tt := range tests {
798+
t.Run(tt.name, func(t *testing.T) {
799+
g := NewWithT(t)
800+
801+
var depObjs []client.Object
802+
for _, d := range tt.dependencies {
803+
obj := &sourcev1.GitRepository{
804+
ObjectMeta: metav1.ObjectMeta{
805+
Name: d.name,
806+
},
807+
Status: sourcev1.GitRepositoryStatus{
808+
Conditions: d.conditions,
809+
},
810+
}
811+
if d.withArtifact {
812+
obj.Status.Artifact = &sourcev1.Artifact{
813+
Path: d.name + ".tar.gz",
814+
Revision: d.name,
815+
LastUpdateTime: metav1.Now(),
816+
}
817+
g.Expect(storage.Archive(obj.GetArtifact(), "testdata/git/repository", nil)).To(Succeed())
818+
}
819+
depObjs = append(depObjs, obj)
820+
}
821+
822+
s := runtime.NewScheme()
823+
utilruntime.Must(sourcev1.AddToScheme(s))
824+
builder := fakeclient.NewClientBuilder().WithScheme(s)
825+
if len(tt.dependencies) > 0 {
826+
builder.WithObjects(depObjs...)
827+
}
828+
829+
r := &GitRepositoryReconciler{
830+
Client: builder.Build(),
831+
Storage: storage,
832+
requeueDependency: dependencyInterval,
833+
}
834+
835+
obj := &sourcev1.GitRepository{
836+
ObjectMeta: metav1.ObjectMeta{
837+
Name: "reconcile-include",
838+
},
839+
Spec: sourcev1.GitRepositorySpec{
840+
Interval: metav1.Duration{Duration: mockInterval},
841+
},
842+
}
843+
844+
for i, incl := range tt.includes {
845+
incl := sourcev1.GitRepositoryInclude{
846+
GitRepositoryRef: meta.LocalObjectReference{Name: incl.name},
847+
FromPath: incl.fromPath,
848+
ToPath: incl.toPath,
849+
}
850+
tt.includes[i].fromPath = incl.GetFromPath()
851+
tt.includes[i].toPath = incl.GetToPath()
852+
obj.Spec.Include = append(obj.Spec.Include, incl)
853+
}
854+
855+
tmpDir, err := ioutil.TempDir("", "include-")
856+
g.Expect(err).NotTo(HaveOccurred())
857+
858+
var artifacts artifactSet
859+
got, err := r.reconcileInclude(ctx, obj, artifacts, tmpDir)
860+
g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
861+
g.Expect(err != nil).To(Equal(tt.wantErr))
862+
g.Expect(got).To(Equal(tt.want))
863+
for _, i := range tt.includes {
864+
if i.toPath != "" {
865+
expect := g.Expect(filepath.Join(storage.BasePath, i.toPath))
866+
if i.shouldExist {
867+
expect.To(BeADirectory())
868+
} else {
869+
expect.NotTo(BeADirectory())
870+
}
871+
}
872+
if i.shouldExist {
873+
g.Expect(filepath.Join(storage.BasePath, i.toPath)).Should(BeADirectory())
874+
} else {
875+
g.Expect(filepath.Join(storage.BasePath, i.toPath)).ShouldNot(BeADirectory())
876+
}
877+
}
878+
})
879+
}
880+
}
881+
882+
func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) {
883+
g := NewWithT(t)
884+
885+
r := &GitRepositoryReconciler{
886+
Storage: storage,
887+
}
888+
889+
obj := &sourcev1.GitRepository{
890+
ObjectMeta: metav1.ObjectMeta{
891+
Name: "reconcile-delete-",
892+
DeletionTimestamp: &metav1.Time{Time: time.Now()},
893+
Finalizers: []string{
894+
sourcev1.SourceFinalizer,
895+
},
896+
},
897+
Status: sourcev1.GitRepositoryStatus{},
898+
}
899+
900+
artifact := storage.NewArtifactFor(sourcev1.GitRepositoryKind, obj.GetObjectMeta(), "revision", "foo.txt")
901+
obj.Status.Artifact = &artifact
902+
903+
got, err := r.reconcileDelete(ctx, obj)
904+
g.Expect(err).NotTo(HaveOccurred())
905+
g.Expect(got).To(Equal(ctrl.Result{}))
906+
g.Expect(controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer)).To(BeFalse())
907+
g.Expect(obj.Status.Artifact).To(BeNil())
908+
909+
}
910+
665911
func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
666912
tests := []struct {
667913
name string

0 commit comments

Comments
 (0)