adds indexing for spec.executor field

To query/list a custom resource by a field in spec,
it must be indexed by the controller executing the query.
This adds indexing for spec.executor so that upon delete,
we can filter and list spinapps by spec.executor.
This resolves the behavior we were seeing around not being
able to delete a spinappexecutor even after spinapp workloads
were deleted.

resolves #102

Signed-off-by: Michelle Dhanani <michelle@fermyon.com>

go.mod

@@ -19,6 +19,7 @@ require (
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/emicklei/go-restful/v3 v3.11.0 // indirect
+	github.com/evanphx/json-patch v4.12.0+incompatible // indirect
 	github.com/evanphx/json-patch/v5 v5.8.0 // indirect
 	github.com/fsnotify/fsnotify v1.7.0 // indirect
 	github.com/go-logr/zapr v1.3.0 // indirect

internal/controller/spinappexecutor_controller.go

@@ -29,6 +29,10 @@ import (
 	"github.com/spinkube/spin-operator/internal/logging"
 )
 
+var (
+	spinAppExecutorKey = "spec.executor"
+)
+
 // SpinAppExecutorReconciler reconciles a SpinAppExecutor object
 type SpinAppExecutorReconciler struct {
 	client.Client
@@ -41,6 +45,15 @@ type SpinAppExecutorReconciler struct {
 
 // SetupWithManager sets up the controller with the Manager.
 func (r *SpinAppExecutorReconciler) SetupWithManager(mgr ctrl.Manager) error {
+
+	if err := mgr.GetFieldIndexer().IndexField(context.Background(), &spinv1.SpinApp{}, spinAppExecutorKey, func(rawObj client.Object) []string {
+		// grab the spinapp object, extract the executor...
+		spinapp := rawObj.(*spinv1.SpinApp)
+		return []string{spinapp.Spec.Executor}
+	}); err != nil {
+		return err
+	}
+
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&spinv1.SpinAppExecutor{}).
 		Complete(r)
@@ -84,10 +97,11 @@ func (r *SpinAppExecutorReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 // handleDeletion makes sure no SpinApps are dependent on the SpinAppExecutor
 // before allowing it to be deleted.
 func (r *SpinAppExecutorReconciler) handleDeletion(ctx context.Context, executor *spinv1.SpinAppExecutor) error {
+	log := logging.FromContext(ctx)
+
 	var spinApps spinv1.SpinAppList
-	if err := r.Client.List(ctx, &spinApps, client.MatchingFields{"spec.runtime": executor.Name}); err != nil {
-		// TODO: Log this
-		// TODO: Emit k8s event
+	if err := r.Client.List(ctx, &spinApps, client.MatchingFields{spinAppExecutorKey: executor.Name}); err != nil {
+		log.Error(err, "Unable to list SpinApps")
 		return err
 	}
 

internal/controller/spinappexecutor_controller_test.go

@@ -16,75 +16,125 @@ limitations under the License.
 
 package controller
 
-// import (
-// 	"fmt"
-// 	"path/filepath"
-// 	"runtime"
-// 	"testing"
-
-// 	. "github.com/onsi/ginkgo/v2"
-// 	. "github.com/onsi/gomega"
-
-// 	"k8s.io/client-go/kubernetes/scheme"
-// 	"k8s.io/client-go/rest"
-// 	"sigs.k8s.io/controller-runtime/pkg/client"
-// 	"sigs.k8s.io/controller-runtime/pkg/envtest"
-// 	logf "sigs.k8s.io/controller-runtime/pkg/log"
-// 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
-
-// 	spinv1 "github.com/spinkube/spin-operator/api/v1"
-// 	//+kubebuilder:scaffold:imports
-// )
-
-// // These tests use Ginkgo (BDD-style Go testing framework). Refer to
-// // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
-
-// var cfg *rest.Config
-// var k8sClient client.Client
-// var testEnv *envtest.Environment
-
-// func TestControllers(t *testing.T) {
-// 	RegisterFailHandler(Fail)
-
-// 	RunSpecs(t, "Controller Suite")
-// }
-
-// var _ = BeforeSuite(func() {
-// 	logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true)))
-
-// 	By("bootstrapping test environment")
-// 	testEnv = &envtest.Environment{
-// 		CRDDirectoryPaths:     []string{filepath.Join("..", "..", "config", "crd", "bases")},
-// 		ErrorIfCRDPathMissing: true,
-
-// 		// The BinaryAssetsDirectory is only required if you want to run the tests directly
-// 		// without call the makefile target test. If not informed it will look for the
-// 		// default path defined in controller-runtime which is /usr/local/kubebuilder/.
-// 		// Note that you must have the required binaries setup under the bin directory to perform
-// 		// the tests directly. When we run make test it will be setup and used automatically.
-// 		BinaryAssetsDirectory: filepath.Join("..", "..", "bin", "k8s",
-// 			fmt.Sprintf("1.28.3-%s-%s", runtime.GOOS, runtime.GOARCH)),
-// 	}
-
-// 	var err error
-// 	// cfg is defined in this file globally.
-// 	cfg, err = testEnv.Start()
-// 	Expect(err).NotTo(HaveOccurred())
-// 	Expect(cfg).NotTo(BeNil())
-
-// 	err = spinv1.AddToScheme(scheme.Scheme)
-// 	Expect(err).NotTo(HaveOccurred())
-
-// 	//+kubebuilder:scaffold:scheme
-
-// 	k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})
-// 	Expect(err).NotTo(HaveOccurred())
-// 	Expect(k8sClient).NotTo(BeNil())
-
-// })
-
-// var _ = AfterSuite(func() {
-// 	By("tearing down the test environment")
-// 	err := testEnv.Stop()
-// 	Expect(err).NotTo(HaveOccurred())
-// })
+import (
+	"context"
+	"sync"
+	"testing"
+	"time"
+
+	spinv1 "github.com/spinkube/spin-operator/api/v1"
+	"github.com/stretchr/testify/require"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
+)
+
+func setupExecutorController(t *testing.T) (*envTestState, ctrl.Manager, *SpinAppExecutorReconciler) {
+	t.Helper()
+
+	envTest := SetupEnvTest(t)
+
+	opts := zap.Options{
+		Development: true,
+	}
+	logger := zap.New(zap.UseFlagOptions(&opts))
+
+	mgr, err := ctrl.NewManager(envTest.cfg, manager.Options{
+		Metrics: metricsserver.Options{BindAddress: "0"},
+		Scheme:  envTest.scheme,
+		// Provide a real logger to controllers - this means that when tests fail we
+		// get to see the controller logs that lead to the failure - if we decide this
+		// is too noisy then we can gate this behind an env var like SPINKUBE_TEST_LOGS.
+		Logger: logger,
+	})
+
+	require.NoError(t, err)
+
+	ctrlr := &SpinAppExecutorReconciler{
+		Client: envTest.k8sClient,
+		Scheme: envTest.scheme,
+	}
+
+	require.NoError(t, ctrlr.SetupWithManager(mgr))
+
+	return envTest, mgr, ctrlr
+}
+func TestSpinAppExecutorReconcile_StartupShutdown(t *testing.T) {
+	t.Parallel()
+
+	_, mgr, _ := setupExecutorController(t)
+
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancelFunc()
+	require.NoError(t, mgr.Start(ctx))
+}
+
+func TestSpinAppExecutorReconcile_ContainerDShimSpinExecutorCreate(t *testing.T) {
+	t.Parallel()
+
+	envTest, mgr, _ := setupExecutorController(t)
+
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancelFunc()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		require.NoError(t, mgr.Start(ctx))
+		wg.Done()
+	}()
+
+	executor := testContainerdShimSpinExecutor()
+	list := &spinv1.SpinAppExecutorList{}
+	require.NoError(t, envTest.k8sClient.List(ctx, list))
+	require.True(t, len(list.Items) == 0)
+	require.NoError(t, envTest.k8sClient.Create(ctx, executor))
+	require.NoError(t, envTest.k8sClient.List(ctx, list))
+	require.True(t, len(list.Items) == 1)
+}
+
+func TestSpinAppExecutorReconcile_ContainerDShimSpinExecutorDelete(t *testing.T) {
+	t.Parallel()
+
+	envTest, mgr, _ := setupExecutorController(t)
+	executor := testContainerdShimSpinExecutor()
+
+	envTest.k8sClient = fake.NewClientBuilder().WithScheme(envTest.scheme).WithObjects(
+		executor,
+	).Build()
+
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancelFunc()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		require.NoError(t, mgr.Start(ctx))
+		wg.Done()
+	}()
+
+	list := &spinv1.SpinAppExecutorList{}
+	require.NoError(t, envTest.k8sClient.List(ctx, list))
+	require.True(t, len(list.Items) == 1)
+	require.NoError(t, envTest.k8sClient.Delete(ctx, executor))
+	require.NoError(t, envTest.k8sClient.List(ctx, list))
+	require.True(t, len(list.Items) == 0)
+}
+
+func testContainerdShimSpinExecutor() *spinv1.SpinAppExecutor {
+	return &spinv1.SpinAppExecutor{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-executor",
+			Namespace: "default",
+		},
+		Spec: spinv1.SpinAppExecutorSpec{
+			CreateDeployment: true,
+			DeploymentConfig: &spinv1.ExecutorDeploymentConfig{
+				RuntimeClassName: "test-runtime",
+			},
+		},
+	}
+}