Additional Git tests

Signed-off-by: Hidde Beydals <hello@hidde.co>

Author: hiddeco

controllers/gitrepository_controller.go

@@ -30,6 +30,7 @@ import (
 	"github.com/fluxcd/source-controller/pkg/sourceignore"
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -338,6 +339,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
 	// The artifact is up-to-date
 	if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) {
 		logr.FromContext(ctx).Info("Artifact is up-to-date")
+		conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
 		return ctrl.Result{RequeueAfter: obj.GetInterval().Duration}, nil
 	}
 
@@ -383,7 +385,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
 	// Record it on the object
 	obj.Status.Artifact = artifact.DeepCopy()
 	obj.Status.IncludedArtifacts = includes
-	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision %s", artifact.Revision)
+	conditions.MarkTrue(obj, sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision %s", artifact.Revision)
 	r.Events.Eventf(ctx, obj, map[string]string{
 		"revision": obj.GetArtifact().Revision,
 	}, events.EventSeverityInfo, sourcev1.GitOperationSucceedReason, conditions.Get(obj, sourcev1.ArtifactAvailableCondition).Message)
@@ -392,7 +394,6 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
 	url, err := r.Storage.Symlink(artifact, "latest.tar.gz")
 	if err != nil {
 		r.Events.Eventf(ctx, obj, nil, events.EventSeverityError, sourcev1.StorageOperationFailedReason, "Failed to update status URL symlink: %s", err)
-		return ctrl.Result{}, err
 	}
 	if url != "" {
 		obj.Status.URL = url
@@ -410,16 +411,20 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so
 // statuses is recorded in a condition on the given object.
 func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sourcev1.GitRepository, artifacts artifactSet, dir string) (ctrl.Result, error) {
 	includes := make([]conditions.Getter, len(obj.Spec.Include))
+	artifacts = make(artifactSet, len(obj.Spec.Include))
 
 	for i, incl := range obj.Spec.Include {
 		dep := &sourcev1.GitRepository{}
 		if err := r.Get(ctx, types.NamespacedName{Namespace: obj.Namespace, Name: incl.GitRepositoryRef.Name}, dep); err != nil {
-			return ctrl.Result{RequeueAfter: r.requeueDependency}, client.IgnoreNotFound(err)
+			if apierrors.IsNotFound(err){
+				conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include %q: %s", incl.GitRepositoryRef.Name, err.Error())
+			}
+			return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, client.IgnoreNotFound(err)
 		}
 
 		// Confirm include has an artifact
 		if dep.GetArtifact() == nil {
-			conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "No artifact available for include %s", incl.GitRepositoryRef)
+			conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include %q", incl.GitRepositoryRef.Name)
 			return ctrl.Result{RequeueAfter: r.requeueDependency}, nil
 		}
 
@@ -428,20 +433,25 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sou
 		// Copy artifact (sub)contents to configured directory
 		toPath, err := securejoin.SecureJoin(dir, incl.GetToPath())
 		if err != nil {
-			conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %s: %s", incl.GitRepositoryRef, err.Error())
+			conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeFailure", "Failed to calculate path for include %q: %s", incl.GitRepositoryRef.Name, err.Error())
 			return ctrl.Result{}, err
 		}
 		if err = r.Storage.CopyToPath(dep.GetArtifact(), incl.GetFromPath(), toPath); err != nil {
-			conditions.MarkFalse(obj, sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy %s include from %s to %s: %s", incl.GitRepositoryRef, incl.GetFromPath(), toPath, err.Error())
+			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())
 			return ctrl.Result{}, err
 		}
 
 		artifacts[i] = dep.GetArtifact().DeepCopy()
 	}
 
-	// Record an aggregation of all includes' Ready state to the object
-	// condition
-	conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes)
+	// Record an aggregation of all includes Stalled or Ready state to
+	// the object condition
+	conditions.SetAggregate(obj, sourcev1.SourceAvailableCondition, includes,
+		conditions.WithConditions(meta.StalledCondition, meta.ReadyCondition),
+		conditions.WithNegativePolarityConditions(meta.StalledCondition),
+		conditions.WithSourceRefIf(meta.StalledCondition),
+		conditions.WithCounter(),
+		conditions.WithCounterIfOnly(meta.ReadyCondition))
 
 	return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil
 }

controllers/gitrepository_controller_test.go

@@ -48,6 +48,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -591,7 +592,7 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 			},
 			want: ctrl.Result{RequeueAfter: mockInterval},
 			assertConditions: []metav1.Condition{
-				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Archived artifact revision main/revision"),
+				*conditions.TrueCondition(sourcev1.ArtifactAvailableCondition, "ArchivedArtifact", "Artifact revision main/revision"),
 			},
 		},
 		{
@@ -662,6 +663,251 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) {
 	}
 }
 
+func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
+	g := NewWithT(t)
+
+	storage, err := newTestStorage()
+	g.Expect(err).NotTo(HaveOccurred())
+	defer os.RemoveAll(storage.BasePath)
+
+	dependencyInterval := 5 * time.Second
+
+	type dependency struct {
+		name         string
+		withArtifact bool
+		conditions   []metav1.Condition
+	}
+
+	type include struct {
+		name     string
+		fromPath string
+		toPath   string
+		shouldExist bool
+	}
+
+	tests := []struct {
+		name             string
+		dependencies     []dependency
+		includes         []include
+		want             ctrl.Result
+		wantErr          bool
+		assertConditions []metav1.Condition
+	}{
+		{
+			name: "Includes artifacts",
+			dependencies: []dependency{
+				{
+					name:         "a",
+					withArtifact: true,
+					conditions: []metav1.Condition{
+						*conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"),
+					},
+				},
+				{
+					name:         "b",
+					withArtifact: true,
+					conditions: []metav1.Condition{
+						*conditions.TrueCondition(meta.ReadyCondition, "Bar", "bar ready"),
+					},
+				},
+			},
+			includes: []include{
+				{name: "a", toPath: "a/"},
+				{name: "b", toPath: "b/"},
+			},
+			want: ctrl.Result{RequeueAfter: mockInterval},
+			assertConditions: []metav1.Condition{
+				*conditions.TrueCondition(sourcev1.SourceAvailableCondition, "Foo", "2 of 2 Ready"),
+			},
+		},
+		{
+			name: "Non existing artifact",
+			includes: []include{
+				{name: "a", toPath: "a/"},
+			},
+			want: ctrl.Result{RequeueAfter: mockInterval},
+			wantErr: false,
+			assertConditions: []metav1.Condition{
+				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeNotFound", "Could not find resource for include \"a\": gitrepositories.source.toolkit.fluxcd.io \"a\" not found"),
+			},
+		},
+		{
+			name: "Missing artifact",
+			dependencies: []dependency{
+				{
+					name:         "a",
+					withArtifact: false,
+					conditions: []metav1.Condition{
+						*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Foo", "foo unavailable"),
+					},
+				},
+			},
+			includes: []include{
+				{name: "a", toPath: "a/"},
+			},
+			want: ctrl.Result{RequeueAfter: dependencyInterval},
+			assertConditions: []metav1.Condition{
+				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeUnavailable", "No artifact available for include \"a\""),
+			},
+		},
+		{
+			name: "Invalid FromPath",
+			dependencies: []dependency{
+				{
+					name:         "a",
+					withArtifact: true,
+				},
+			},
+			includes: []include{
+				{name: "a", fromPath: "../../../path", shouldExist: false},
+			},
+			wantErr: true,
+			assertConditions: []metav1.Condition{
+				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "IncludeCopyFailure", "Failed to copy \"a\" include from ../../../path to a"),
+			},
+		},
+		{
+			name: "Stalled include",
+			dependencies: []dependency{
+				{
+					name:         "a",
+					withArtifact: true,
+					conditions: []metav1.Condition{
+						*conditions.TrueCondition(meta.ReadyCondition, "Foo", "foo ready"),
+					},
+				},
+				{
+					name:         "b",
+					withArtifact: true,
+					conditions: []metav1.Condition{
+						*conditions.TrueCondition(meta.StalledCondition, "Bar", "bar stalled"),
+					},
+				},
+			},
+			includes: []include{
+				{name: "a", toPath: "a/"},
+				{name: "b", toPath: "b/"},
+			},
+			want: ctrl.Result{RequeueAfter: mockInterval},
+			assertConditions: []metav1.Condition{
+				*conditions.FalseCondition(sourcev1.SourceAvailableCondition, "Bar @ GitRepository/a", "bar stalled"),
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			var depObjs []client.Object
+			for _, d := range tt.dependencies {
+				obj := &sourcev1.GitRepository{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: d.name,
+					},
+					Status: sourcev1.GitRepositoryStatus{
+						Conditions: d.conditions,
+					},
+				}
+				if d.withArtifact {
+					obj.Status.Artifact = &sourcev1.Artifact{
+						Path:           d.name + ".tar.gz",
+						Revision:       d.name,
+						LastUpdateTime: metav1.Now(),
+					}
+					g.Expect(storage.Archive(obj.GetArtifact(), "testdata/git/repository", nil)).To(Succeed())
+				}
+				depObjs = append(depObjs, obj)
+			}
+
+			s := runtime.NewScheme()
+			utilruntime.Must(sourcev1.AddToScheme(s))
+			builder := fakeclient.NewClientBuilder().WithScheme(s)
+			if len(tt.dependencies) > 0 {
+				builder.WithObjects(depObjs...)
+			}
+
+			r := &GitRepositoryReconciler{
+				Client:  builder.Build(),
+				Storage: storage,
+				requeueDependency: dependencyInterval,
+			}
+
+			obj := &sourcev1.GitRepository{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "reconcile-include",
+				},
+				Spec: sourcev1.GitRepositorySpec{
+					Interval: metav1.Duration{Duration: mockInterval},
+				},
+			}
+
+			for i, incl := range tt.includes {
+				incl := sourcev1.GitRepositoryInclude{
+					GitRepositoryRef: meta.LocalObjectReference{Name: incl.name},
+					FromPath:         incl.fromPath,
+					ToPath:           incl.toPath,
+				}
+				tt.includes[i].fromPath = incl.GetFromPath()
+				tt.includes[i].toPath = incl.GetToPath()
+				obj.Spec.Include = append(obj.Spec.Include, incl)
+			}
+
+			tmpDir, err := ioutil.TempDir("", "include-")
+			g.Expect(err).NotTo(HaveOccurred())
+
+			var artifacts artifactSet
+			got, err := r.reconcileInclude(ctx, obj, artifacts, tmpDir)
+			g.Expect(obj.GetConditions()).To(conditions.MatchConditions(tt.assertConditions))
+			g.Expect(err != nil).To(Equal(tt.wantErr))
+			g.Expect(got).To(Equal(tt.want))
+			for _, i := range tt.includes {
+				if i.toPath != "" {
+					expect := g.Expect(filepath.Join(storage.BasePath, i.toPath))
+					if i.shouldExist {
+						expect.To(BeADirectory())
+					} else {
+						expect.NotTo(BeADirectory())
+					}
+				}
+				if i.shouldExist {
+					g.Expect(filepath.Join(storage.BasePath, i.toPath)).Should(BeADirectory())
+				} else {
+					g.Expect(filepath.Join(storage.BasePath, i.toPath)).ShouldNot(BeADirectory())
+				}
+			}
+		})
+	}
+}
+
+func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) {
+	g := NewWithT(t)
+
+	r := &GitRepositoryReconciler{
+		Storage: storage,
+	}
+
+	obj := &sourcev1.GitRepository{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "reconcile-delete-",
+			DeletionTimestamp: &metav1.Time{Time: time.Now()},
+			Finalizers: []string{
+				sourcev1.SourceFinalizer,
+			},
+		},
+		Status: sourcev1.GitRepositoryStatus{},
+	}
+
+	artifact := storage.NewArtifactFor(sourcev1.GitRepositoryKind, obj.GetObjectMeta(), "revision", "foo.txt")
+	obj.Status.Artifact = &artifact
+
+	got, err := r.reconcileDelete(ctx, obj)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(got).To(Equal(ctrl.Result{}))
+	g.Expect(controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer)).To(BeFalse())
+	g.Expect(obj.Status.Artifact).To(BeNil())
+
+}
+
 func TestGitRepositoryReconciler_verifyCommitSignature(t *testing.T) {
 	tests := []struct {
 		name             string