Wait for Artifact before generation.

The artifact may or may not be available, if we've already generated
resources, generating no resources means that the existing resources
are removed.

This changes it to return a marker error, which we can record and do
nothing about.

We can do nothing, because when the Artifact is applied, this will
trigger a watch notification, and we can generate.

Author: bigkevmcd

controllers/gitopsset_controller.go

@@ -2,6 +2,7 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"sort"
 	"time"
@@ -164,6 +165,16 @@ func (r *GitOpsSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	inventory, requeue, err := r.reconcileResources(ctx, k8sClient, &gitOpsSet)
 
 	if err != nil {
+		// We can return here because when the resource artifact is updated, this
+		// will trigger a reconciliation.
+		if errors.As(err, &generators.NoArtifactError{}) {
+			templatesv1.SetGitOpsSetReadiness(&gitOpsSet, metav1.ConditionFalse, templatesv1.ReconciliationFailedReason, "waiting for artifact")
+			if err := r.patchStatus(ctx, req, gitOpsSet.Status); err != nil {
+				logger.Error(err, "failed to reconcile")
+			}
+			return ctrl.Result{}, nil
+		}
+
 		templatesv1.SetGitOpsSetReadiness(&gitOpsSet, metav1.ConditionFalse, templatesv1.ReconciliationFailedReason, err.Error())
 		if err := r.patchStatus(ctx, req, gitOpsSet.Status); err != nil {
 			logger.Error(err, "failed to reconcile")

controllers/gitopsset_controller_test.go

@@ -11,6 +11,7 @@ import (
 	kustomizev1 "github.com/fluxcd/kustomize-controller/api/v1beta2"
 	"github.com/fluxcd/pkg/apis/meta"
 	fluxMeta "github.com/fluxcd/pkg/apis/meta"
+	sourcev1beta2 "github.com/fluxcd/source-controller/api/v1beta2"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
@@ -32,6 +33,7 @@ import (
 	clustersv1 "github.com/weaveworks/cluster-controller/api/v1alpha1"
 	templatesv1 "github.com/weaveworks/gitopssets-controller/api/v1alpha1"
 	"github.com/weaveworks/gitopssets-controller/controllers/templates/generators"
+	"github.com/weaveworks/gitopssets-controller/controllers/templates/generators/gitrepository"
 	"github.com/weaveworks/gitopssets-controller/controllers/templates/generators/list"
 	"github.com/weaveworks/gitopssets-controller/test"
 )
@@ -54,6 +56,7 @@ func TestReconciliation(t *testing.T) {
 		CRDDirectoryPaths: []string{
 			filepath.Join("..", "config", "crd", "bases"),
 			"testdata/crds",
+			"../tests/e2e/testdata/crds",
 		},
 	}
 	testEnv.ControlPlane.GetAPIServer().Configure().Append("--authorization-mode=RBAC")
@@ -73,6 +76,7 @@ func TestReconciliation(t *testing.T) {
 	// Kustomizations.
 	test.AssertNoError(t, clientgoscheme.AddToScheme(scheme))
 	test.AssertNoError(t, templatesv1.AddToScheme(scheme))
+	test.AssertNoError(t, sourcev1beta2.AddToScheme(scheme))
 
 	k8sClient, err := client.New(cfg, client.Options{Scheme: scheme})
 	test.AssertNoError(t, err)
@@ -86,7 +90,8 @@ func TestReconciliation(t *testing.T) {
 		Scheme:                scheme,
 		DefaultServiceAccount: "",
 		Generators: map[string]generators.GeneratorFactory{
-			"List": list.GeneratorFactory,
+			"List":          list.GeneratorFactory,
+			"GitRepository": gitrepository.GeneratorFactory(nil),
 		},
 		Config:        cfg,
 		EventRecorder: eventRecorder,
@@ -200,7 +205,7 @@ func TestReconciliation(t *testing.T) {
 			}
 		})
 		test.AssertNoError(t, k8sClient.Create(ctx, test.ToUnstructured(t, devKS)))
-		defer cleanupResource(t, k8sClient, test.ToUnstructured(t, devKS))
+		defer deleteObject(t, k8sClient, test.ToUnstructured(t, devKS))
 
 		_, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)})
 		test.AssertErrorMatch(t, "failed to create Resource.*already exists", err)
@@ -731,6 +736,37 @@ func TestReconciliation(t *testing.T) {
 		assertNoKustomizationsExistInNamespace(t, k8sClient, "default")
 	})
 
+	t.Run("reconciling when gitrepository has no artifact", func(t *testing.T) {
+		ctx := context.TODO()
+		emptyGR := test.NewGitRepository()
+		test.AssertNoError(t, k8sClient.Create(ctx, test.ToUnstructured(t, emptyGR)))
+		defer deleteObject(t, k8sClient, emptyGR)
+
+		gs := createAndReconcileToFinalizedState(t, k8sClient, reconciler, makeTestGitOpsSet(t, func(gs *templatesv1.GitOpsSet) {
+			gs.Spec.Generators = []templatesv1.GitOpsSetGenerator{
+				{
+					GitRepository: &templatesv1.GitRepositoryGenerator{
+						RepositoryRef: "test-repository",
+						Files: []templatesv1.RepositoryGeneratorFileItem{
+							{Path: "files/dev.yaml"},
+							{Path: "files/production.yaml"},
+							{Path: "files/staging.yaml"},
+						},
+					},
+				},
+			}
+		}))
+		defer deleteGitOpsSetAndFinalize(t, k8sClient, reconciler, gs)
+
+		_, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)})
+		test.AssertNoError(t, err)
+
+		test.AssertNoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(gs), gs))
+		assertInventoryHasNoItems(t, gs)
+		assertNoKustomizationsExistInNamespace(t, k8sClient, "default")
+		assertGitOpsSetCondition(t, gs, meta.ReadyCondition, "waiting for artifact")
+	})
+
 	t.Run("error conditions - existing resource", func(t *testing.T) {
 		ctx := context.TODO()
 		gs := makeTestGitOpsSet(t, func(gs *templatesv1.GitOpsSet) {
@@ -760,7 +796,7 @@ func TestReconciliation(t *testing.T) {
 			}
 		})
 		test.AssertNoError(t, k8sClient.Create(ctx, test.ToUnstructured(t, devKS)))
-		defer cleanupResource(t, k8sClient, gs)
+		defer deleteObject(t, k8sClient, gs)
 
 		_, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(gs)})
 		test.AssertErrorMatch(t, "failed to create Resource.*already exists", err)
@@ -1098,7 +1134,7 @@ func assertInventoryHasNoItems(t *testing.T, gs *templatesv1.GitOpsSet) {
 	}
 }
 
-func cleanupResource(t *testing.T, cl client.Client, obj client.Object) {
+func deleteObject(t *testing.T, cl client.Client, obj client.Object) {
 	t.Helper()
 	if err := cl.Delete(context.TODO(), obj); err != nil {
 		t.Fatal(err)
@@ -1226,7 +1262,7 @@ func createRBACForServiceAccount(t *testing.T, cl client.Client, serviceAccountN
 		t.Fatalf("failed to write role: %s", err)
 	}
 	t.Cleanup(func() {
-		cleanupResource(t, cl, role)
+		deleteObject(t, cl, role)
 	})
 	binding := &rbacv1.RoleBinding{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1248,7 +1284,7 @@ func createRBACForServiceAccount(t *testing.T, cl client.Client, serviceAccountN
 		t.Fatalf("failed to write role-binding: %s", err)
 	}
 	t.Cleanup(func() {
-		cleanupResource(t, cl, binding)
+		deleteObject(t, cl, binding)
 	})
 }
 func deleteGitOpsSetAndFinalize(t *testing.T, cl client.Client, reconciler *GitOpsSetReconciler, gs *templatesv1.GitOpsSet) {

controllers/templates/generators/errors.go

@@ -0,0 +1,25 @@
+package generators
+
+import (
+	"fmt"
+
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+// NoArtifactError indicates that a Repository's artifact is not available.
+type NoArtifactError struct {
+	Kind string
+	Name client.ObjectKey
+}
+
+func (e NoArtifactError) Error() string {
+	return fmt.Sprintf("no artifact for %s %s", e.Kind, e.Name)
+}
+
+// ArtifactError creates and returns a new Artifact error.
+func ArtifactError(kind string, name client.ObjectKey) error {
+	return NoArtifactError{
+		Kind: kind,
+		Name: name,
+	}
+}

controllers/templates/generators/gitrepository/git_repository.go

@@ -73,7 +73,7 @@ func (g *GitRepositoryGenerator) generateParamsFromGitFiles(ctx context.Context,
 	// No artifact? nothing to generate...
 	if gr.Status.Artifact == nil {
 		g.Logger.Info("GitRepository does not have an artifact", "repository", repoName)
-		return []map[string]any{}, nil
+		return nil, generators.ArtifactError("GitRepository", repoName)
 	}
 
 	g.Logger.Info("fetching archive URL", "repoURL", gr.Spec.URL, "artifactURL", gr.Status.Artifact.URL,

controllers/templates/generators/gitrepository/git_repository_test.go

@@ -24,8 +24,6 @@ const testRetries int = 3
 
 var _ generators.Generator = (*GitRepositoryGenerator)(nil)
 
-const testNamespace = "generation"
-
 var testFetcher = fetch.NewArchiveFetcher(testRetries, tar.UnlimitedUntarSize, tar.UnlimitedUntarSize, "")
 
 func TestGenerate_with_no_GitRepository(t *testing.T) {
@@ -48,19 +46,6 @@ func TestGenerate(t *testing.T) {
 		objects   []runtime.Object
 		want      []map[string]any
 	}{
-		{
-			"no artifact in GitRepository",
-			&templatesv1.GitRepositoryGenerator{
-				RepositoryRef: "test-repository",
-				Files: []templatesv1.RepositoryGeneratorFileItem{
-					{Path: "files/dev.yaml"},
-					{Path: "files/production.yaml"},
-					{Path: "files/staging.yaml"},
-				},
-			},
-			[]runtime.Object{newGitRepository()},
-			[]map[string]any{},
-		},
 		{
 			"file list case",
 			&templatesv1.GitRepositoryGenerator{
@@ -71,7 +56,7 @@ func TestGenerate(t *testing.T) {
 					{Path: "files/staging.yaml"},
 				},
 			},
-			[]runtime.Object{newGitRepository(
+			[]runtime.Object{test.NewGitRepository(
 				withArchiveURLAndChecksum(srv.URL+"/files.tar.gz",
 					"sha256:f0a57ec1cdebda91cf00d89dfa298c6ac27791e7fdb0329990478061755eaca8"))},
 			[]map[string]any{
@@ -88,7 +73,7 @@ func TestGenerate(t *testing.T) {
 					{Path: "applications/*"},
 				},
 			},
-			[]runtime.Object{newGitRepository(
+			[]runtime.Object{test.NewGitRepository(
 				withArchiveURLAndChecksum(srv.URL+"/directories.tar.gz",
 					"sha256:a8bb41d733c5cc9bdd13d926a2edbe4c85d493c6c90271da1e1b991880935dc1"))},
 			[]map[string]any{
@@ -107,7 +92,7 @@ func TestGenerate(t *testing.T) {
 				&templatesv1.GitOpsSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-generator",
-						Namespace: testNamespace,
+						Namespace: "default",
 					},
 					Spec: templatesv1.GitOpsSetSpec{
 						Generators: []templatesv1.GitOpsSetGenerator{
@@ -163,6 +148,19 @@ func TestGenerate_errors(t *testing.T) {
 			},
 			wantErr: "GitOpsSet is empty",
 		},
+		{
+			name: "no artifact in GitRepository",
+			generator: &templatesv1.GitRepositoryGenerator{
+				RepositoryRef: "test-repository",
+				Files: []templatesv1.RepositoryGeneratorFileItem{
+					{Path: "files/dev.yaml"},
+					{Path: "files/production.yaml"},
+					{Path: "files/staging.yaml"},
+				},
+			},
+			objects: []runtime.Object{test.NewGitRepository()},
+			wantErr: "no artifact for GitRepository default/test-repository",
+		},
 	}
 
 	for _, tt := range testCases {
@@ -174,7 +172,7 @@ func TestGenerate_errors(t *testing.T) {
 				&templatesv1.GitOpsSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-generator",
-						Namespace: testNamespace,
+						Namespace: "default",
 					},
 					Spec: templatesv1.GitOpsSetSpec{
 						Generators: []templatesv1.GitOpsSetGenerator{
@@ -199,21 +197,6 @@ func withArchiveURLAndChecksum(archiveURL, xsum string) func(*sourcev1beta2.GitR
 	}
 }
 
-func newGitRepository(opts ...func(*sourcev1beta2.GitRepository)) *sourcev1beta2.GitRepository {
-	gr := &sourcev1beta2.GitRepository{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-repository",
-			Namespace: testNamespace,
-		},
-	}
-
-	for _, opt := range opts {
-		opt(gr)
-	}
-
-	return gr
-}
-
 func newFakeClient(t *testing.T, objs ...runtime.Object) client.WithWatch {
 	t.Helper()
 	scheme := runtime.NewScheme()

controllers/templates/generators/interface.go

@@ -31,9 +31,8 @@ type Generator interface {
 // ErrEmptyGitOpsSetGenerator is returned when GitOpsSet is
 // empty.
 var ErrEmptyGitOpsSet = errors.New("GitOpsSet is empty")
-var NoRequeueInterval time.Duration
 
-var ErrIncorrectNumberOfGenerators = errors.New("matrix generator needs two generators")
+var NoRequeueInterval time.Duration
 
 // DefaultInterval is used when Interval is not specified, it
 // is the default time to wait before the next reconcile loop.

controllers/templates/generators/ocirepository/oci_repository.go

@@ -73,7 +73,7 @@ func (g *OCIRepositoryGenerator) generateParamsFromOCIFiles(ctx context.Context,
 	// No artifact? nothing to generate...
 	if gr.Status.Artifact == nil {
 		g.Logger.Info("OCIRepository does not have an artifact", "repository", repoName)
-		return []map[string]any{}, nil
+		return nil, generators.ArtifactError("OCIRepository", repoName)
 	}
 
 	g.Logger.Info("fetching archive URL", "repoURL", gr.Spec.URL, "artifactURL", gr.Status.Artifact.URL,

controllers/templates/generators/ocirepository/oci_repository_test.go

@@ -24,8 +24,6 @@ const testRetries int = 3
 
 var _ generators.Generator = (*OCIRepositoryGenerator)(nil)
 
-const testNamespace = "generation"
-
 var testFetcher = fetch.NewArchiveFetcher(testRetries, tar.UnlimitedUntarSize, tar.UnlimitedUntarSize, "")
 
 func TestGenerate_with_no_OCIRepository(t *testing.T) {
@@ -48,19 +46,6 @@ func TestGenerate(t *testing.T) {
 		objects   []runtime.Object
 		want      []map[string]any
 	}{
-		{
-			"no artifact in OCIRepository",
-			&templatesv1.OCIRepositoryGenerator{
-				RepositoryRef: "test-repository",
-				Files: []templatesv1.RepositoryGeneratorFileItem{
-					{Path: "files/dev.yaml"},
-					{Path: "files/production.yaml"},
-					{Path: "files/staging.yaml"},
-				},
-			},
-			[]runtime.Object{newOCIRepository()},
-			[]map[string]any{},
-		},
 		{
 			"file list case",
 			&templatesv1.OCIRepositoryGenerator{
@@ -107,7 +92,7 @@ func TestGenerate(t *testing.T) {
 				&templatesv1.GitOpsSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-generator",
-						Namespace: testNamespace,
+						Namespace: "default",
 					},
 					Spec: templatesv1.GitOpsSetSpec{
 						Generators: []templatesv1.GitOpsSetGenerator{
@@ -163,6 +148,19 @@ func TestGenerate_errors(t *testing.T) {
 			},
 			wantErr: "GitOpsSet is empty",
 		},
+		{
+			name: "no artifact in OCIRepository",
+			generator: &templatesv1.OCIRepositoryGenerator{
+				RepositoryRef: "test-repository",
+				Files: []templatesv1.RepositoryGeneratorFileItem{
+					{Path: "files/dev.yaml"},
+					{Path: "files/production.yaml"},
+					{Path: "files/staging.yaml"},
+				},
+			},
+			objects: []runtime.Object{newOCIRepository()},
+			wantErr: "no artifact for OCIRepository default/test-repository",
+		},
 	}
 
 	for _, tt := range testCases {
@@ -174,7 +172,7 @@ func TestGenerate_errors(t *testing.T) {
 				&templatesv1.GitOpsSet{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      "test-generator",
-						Namespace: testNamespace,
+						Namespace: "default",
 					},
 					Spec: templatesv1.GitOpsSetSpec{
 						Generators: []templatesv1.GitOpsSetGenerator{
@@ -203,7 +201,7 @@ func newOCIRepository(opts ...func(*sourcev1beta2.OCIRepository)) *sourcev1beta2
 	gr := &sourcev1beta2.OCIRepository{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "test-repository",
-			Namespace: testNamespace,
+			Namespace: "default",
 		},
 	}