From 9e9410bf0c416b1c03e9b688f3e3ff5972e5850b Mon Sep 17 00:00:00 2001 From: Michelle Dhanani Date: Tue, 5 Mar 2024 23:08:23 -0500 Subject: [PATCH] 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 --- go.mod | 1 + .../controller/spinappexecutor_controller.go | 20 +- .../spinappexecutor_controller_test.go | 194 +++++++++++------- 3 files changed, 140 insertions(+), 75 deletions(-) diff --git a/go.mod b/go.mod index a82bc5ec..83689618 100644 --- a/go.mod +++ b/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 diff --git a/internal/controller/spinappexecutor_controller.go b/internal/controller/spinappexecutor_controller.go index f5501e61..b7a67f27 100644 --- a/internal/controller/spinappexecutor_controller.go +++ b/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 } diff --git a/internal/controller/spinappexecutor_controller_test.go b/internal/controller/spinappexecutor_controller_test.go index c4e3aea3..423bc282 100644 --- a/internal/controller/spinappexecutor_controller_test.go +++ b/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", + }, + }, + } +}