CLOUDP-80607: Implement project delete logic (#88)

Author: vasilevp

pkg/controller/atlascluster/atlascluster_controller.go

@@ -24,8 +24,9 @@ import (
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/watch"
@@ -104,22 +105,25 @@ func (r *AtlasClusterReconciler) readProjectResource(cluster *mdbv1.AtlasCluster
 }
 
 func (r *AtlasClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewControllerManagedBy(mgr).
-		For(&mdbv1.AtlasCluster{}).
-		WithEventFilter(watch.CommonPredicates()).
-		Watches(
-			&source.Kind{Type: &mdbv1.AtlasCluster{}},
-			&watch.AtlasResourceEventHandler{Controller: r},
-			builder.WithPredicates(watch.DeleteOnly()),
-		).
-		Complete(r)
+	c, err := controller.New("AtlasCluster", mgr, controller.Options{Reconciler: r})
+	if err != nil {
+		return err
+	}
+
+	// Watch for changes to primary resource AtlasCluster & handle delete separately
+	err = c.Watch(&source.Kind{Type: &mdbv1.AtlasCluster{}}, &watch.EventHandlerWithDelete{Controller: r}, watch.CommonPredicates())
+	if err != nil {
+		return err
+	}
+
+	return nil
 }
 
 // Delete implements a handler for the Delete event.
-func (r *AtlasClusterReconciler) Delete(obj runtime.Object) error {
-	cluster, ok := obj.(*mdbv1.AtlasCluster)
+func (r *AtlasClusterReconciler) Delete(e event.DeleteEvent) error {
+	cluster, ok := e.Object.(*mdbv1.AtlasCluster)
 	if !ok {
-		r.Log.Errorf("Ignoring malformed Delete() call (expected type %T, got %T)", &mdbv1.AtlasCluster{}, obj)
+		r.Log.Errorf("Ignoring malformed Delete() call (expected type %T, got %T)", &mdbv1.AtlasCluster{}, e.Object)
 		return nil
 	}
 

pkg/controller/atlasproject/atlasproject_controller.go

@@ -17,12 +17,17 @@ limitations under the License.
 package atlasproject
 
 import (
+	"context"
+	"errors"
+	"fmt"
+
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
@@ -32,6 +37,7 @@ import (
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/customresource"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/statushandler"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/watch"
+	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/kube"
 )
 
 // AtlasProjectReconciler reconciles a AtlasProject object
@@ -96,8 +102,34 @@ func (r *AtlasProjectReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error
 	return ctrl.Result{}, nil
 }
 
-func (r *AtlasProjectReconciler) Delete(obj runtime.Object) error {
-	// TODO CLOUDP-80607
+func (r *AtlasProjectReconciler) Delete(e event.DeleteEvent) error {
+	project, ok := e.Object.(*mdbv1.AtlasProject)
+	if !ok {
+		r.Log.Errorf("Ignoring malformed Delete() call (expected type %T, got %T)", &mdbv1.AtlasProject{}, e.Object)
+		return nil
+	}
+
+	log := r.Log.With("atlasproject", kube.ObjectKeyFromObject(project))
+
+	log.Infow("-> Starting AtlasProject deletion", "spec", project.Spec)
+
+	connection, result := atlas.ReadConnection(log, r.Client, "TODO!", project.ConnectionSecretObjectKey())
+	if !result.IsOk() {
+		return errors.New("cannot read Atlas connection")
+	}
+
+	atlasClient, err := atlas.Client(r.AtlasDomain, connection, log)
+	if err != nil {
+		return fmt.Errorf("cannot build Atlas client: %w", err)
+	}
+
+	_, err = atlasClient.Projects.Delete(context.Background(), project.Status.ID)
+	if err != nil {
+		return fmt.Errorf("cannot delete Atlas project: %w", err)
+	}
+
+	log.Infow("Successfully deleted Atlas project", "projectID", project.Status.ID)
+
 	return nil
 }
 
@@ -107,8 +139,8 @@ func (r *AtlasProjectReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		return err
 	}
 
-	// Watch for changes to primary resource MongoDbReplicaSet
-	err = c.Watch(&source.Kind{Type: &mdbv1.AtlasProject{}}, &watch.AtlasResourceEventHandler{Controller: r}, watch.CommonPredicates())
+	// Watch for changes to primary resource AtlasProject & handle delete separately
+	err = c.Watch(&source.Kind{Type: &mdbv1.AtlasProject{}}, &watch.EventHandlerWithDelete{Controller: r}, watch.CommonPredicates())
 	if err != nil {
 		return err
 	}

pkg/controller/watch/delete_handler.go

@@ -0,0 +1,29 @@
+package watch
+
+import (
+	"go.uber.org/zap"
+	"k8s.io/client-go/util/workqueue"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+
+	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/kube"
+)
+
+// EventHandlerWithDelete is an extension of EnqueueRequestForObject that will _not_ trigger a reconciliation for a Delete event.
+// Instead, it will call an external controller's Delete() method and pass the event argument unchanged.
+type EventHandlerWithDelete struct {
+	handler.EnqueueRequestForObject
+	Controller interface {
+		Delete(e event.DeleteEvent) error
+	}
+}
+
+func (d *EventHandlerWithDelete) Delete(e event.DeleteEvent, _ workqueue.RateLimitingInterface) {
+	objectKey := kube.ObjectKeyFromObject(e.Meta)
+	log := zap.S().With("resource", objectKey)
+
+	if err := d.Controller.Delete(e); err != nil {
+		log.Errorf("Object (%s) removed from Kubernetes, but controller could not delete it: %s", e.Object.GetObjectKind(), err)
+		return
+	}
+}

pkg/controller/watch/event_handler.go

@@ -2,7 +2,6 @@ package int
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"net/http"
 	"time"
@@ -15,7 +14,6 @@ import (
 
 	mdbv1 "github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/api/v1/status"
-	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/atlas"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/controller/workflow"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/kube"
 	"github.com/mongodb/mongodb-atlas-kubernetes/pkg/util/testutil"
@@ -52,17 +50,22 @@ var _ = Describe("AtlasCluster", func() {
 		By("Creating the project " + createdProject.Name)
 		Expect(k8sClient.Create(context.Background(), createdProject)).To(Succeed())
 		Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)),
-			10, interval).Should(BeTrue())
+			20, interval).Should(BeTrue())
 	})
 
 	AfterEach(func() {
 		if createdProject != nil && createdProject.Status.ID != "" {
 			if createdCluster != nil {
 				By("Removing Atlas Cluster " + createdCluster.Name)
 				Expect(k8sClient.Delete(context.Background(), createdCluster)).To(Succeed())
-
 				Eventually(checkAtlasClusterRemoved(createdProject.Status.ID, createdCluster.Name), 600, interval).Should(BeTrue())
 			}
+
+			// TODO: CLOUDP-82115
+			// By("Removing Atlas Project " + createdProject.Status.ID)
+			// Expect(k8sClient.Delete(context.Background(), createdProject)).To(Succeed())
+			// Eventually(checkAtlasProjectRemoved(createdProject.Status.ID), 20, interval).Should(BeTrue())
+
 			By("Removing Atlas Project " + createdProject.Status.ID)
 			// This is a bit strange but the delete request right after the cluster is removed may fail with "Still active cluster" error
 			// UI shows the cluster being deleted though. Seems to be the issue only if removal is done using API,
@@ -237,20 +240,8 @@ func checkAtlasClusterRemoved(projectID string, clusterName string) func() bool
 				return true
 			}
 		}
-		return false
-	}
-}
 
-func removeAtlasProject(projectID string) func() bool {
-	return func() bool {
-		_, err := atlasClient.Projects.Delete(context.Background(), projectID)
-		if err != nil {
-			var apiError *mongodbatlas.ErrorResponse
-			Expect(errors.As(err, &apiError)).To(BeTrue())
-			Expect(apiError.ErrorCode).To(Equal(atlas.CannotCloseGroupActiveAtlasCluster))
-			return false
-		}
-		return true
+		return false
 	}
 }
 

test/int/cluster_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/http"
 	"time"
 
 	. "github.com/onsi/ginkgo"
@@ -47,8 +48,8 @@ var _ = Describe("AtlasProject", func() {
 	AfterEach(func() {
 		if createdProject != nil && createdProject.Status.ID != "" {
 			By("Removing Atlas Project " + createdProject.Status.ID)
-			_, err := atlasClient.Projects.Delete(context.Background(), createdProject.Status.ID)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(k8sClient.Delete(context.Background(), createdProject)).To(Succeed())
+			Eventually(checkAtlasProjectRemoved(createdProject.Status.ID), 20, interval).Should(BeTrue())
 		}
 		removeControllersAndNamespace()
 	})
@@ -108,7 +109,7 @@ var _ = Describe("AtlasProject", func() {
 
 			expectedCondition := status.FalseCondition(status.ProjectReadyType).WithReason(string(workflow.AtlasCredentialsNotProvided))
 			Eventually(testutil.WaitFor(k8sClient, createdProject, expectedCondition),
-				10, interval).Should(BeTrue())
+				20, interval).Should(BeTrue())
 
 			Expect(createdProject.Status.ObservedGeneration).To(Equal(createdProject.Generation))
 			expectedConditionsMatchers := testutil.MatchConditions(
@@ -147,7 +148,7 @@ var _ = Describe("AtlasProject", func() {
 			Expect(k8sClient.Update(context.Background(), createdProject)).To(Succeed())
 
 			Eventually(testutil.WaitFor(k8sClient, createdProject, status.TrueCondition(status.ReadyType)),
-				10, interval).Should(BeTrue())
+				20, interval).Should(BeTrue())
 
 			Expect(testutil.ReadAtlasResource(k8sClient, createdProject)).To(BeTrue())
 			Expect(createdProject.Status.Conditions).To(ContainElement(testutil.MatchCondition(status.TrueCondition(status.ProjectReadyType))))
@@ -163,9 +164,9 @@ var _ = Describe("AtlasProject", func() {
 		var secondProject *mdbv1.AtlasProject
 		AfterEach(func() {
 			if secondProject != nil && secondProject.Status.ID != "" {
-				By("Removing (second) Atlas Project " + secondProject.ID())
-				_, err := atlasClient.Projects.Delete(context.Background(), secondProject.ID())
-				Expect(err).ToNot(HaveOccurred())
+				By("Removing (second) Atlas Project " + secondProject.Status.ID)
+				Expect(k8sClient.Delete(context.Background(), secondProject)).To(Succeed())
+				Eventually(checkAtlasProjectRemoved(secondProject.Status.ID), 20, interval).Should(BeTrue())
 			}
 		})
 		It("Should Succeed", func() {
@@ -280,6 +281,7 @@ var _ = Describe("AtlasProject", func() {
 			checkExpiredAccessLists([]mdbv1.ProjectIPAccessList{})
 		})
 	})
+
 	Describe("Updating the project IP access list", func() {
 		It("Should Succeed (single)", func() {
 			By("Creating the project first", func() {
@@ -305,6 +307,7 @@ var _ = Describe("AtlasProject", func() {
 				checkIPAccessListInAtlas()
 			})
 		})
+
 		It("Should Succeed (multiple)", func() {
 			By("Creating the project first", func() {
 				createdProject = testAtlasProject(namespace.Name, "test-project", namespace.Name, connectionSecret.Name)
@@ -336,7 +339,6 @@ var _ = Describe("AtlasProject", func() {
 			})
 		})
 	})
-
 })
 
 // TODO builders
@@ -353,6 +355,32 @@ func testAtlasProject(namespace, name, atlasName, connectionSecretName string) *
 	}
 }
 
+func removeAtlasProject(projectID string) func() bool {
+	return func() bool {
+		_, err := atlasClient.Projects.Delete(context.Background(), projectID)
+		if err != nil {
+			var apiError *mongodbatlas.ErrorResponse
+			Expect(errors.As(err, &apiError)).To(BeTrue())
+			Expect(apiError.ErrorCode).To(Equal(atlas.CannotCloseGroupActiveAtlasCluster))
+			return false
+		}
+		return true
+	}
+}
+
+// checkAtlasProjectRemoved returns true if the Atlas Project is removed from Atlas.
+func checkAtlasProjectRemoved(projectID string) func() bool {
+	return func() bool {
+		_, r, err := atlasClient.Projects.GetOneProject(context.Background(), projectID)
+		if err != nil {
+			if r != nil && r.StatusCode == http.StatusNotFound {
+				return true
+			}
+		}
+		return false
+	}
+}
+
 // validateNoErrorsIPAccessListDuringCreate performs check that no problems happen to IP Access list during the create.
 // This allows the test to fail fast instead by timeout if there are any troubles.
 func validateNoErrorsIPAccessListDuringCreate(a mdbv1.AtlasCustomResource) {