fix: Handle MySQL deletion before finalizer addition

internal/controller/mysql_controller.go

@@ -78,13 +78,33 @@ func (r *MySQLReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		return ctrl.Result{}, err
 	}
 
-	// Add a finalizer if not exists
-	if controllerutil.AddFinalizer(mysql, mysqlFinalizer) {
-		if err := r.Update(ctx, mysql); err != nil {
+	// Handle deletion first
+	if !mysql.GetDeletionTimestamp().IsZero() {
+		if !controllerutil.ContainsFinalizer(mysql, mysqlFinalizer) {
+			// If being deleted and no finalizer, nothing to do
+			return ctrl.Result{}, nil
+		}
+		if r.finalizeMySQL(ctx, mysql) {
+			if controllerutil.RemoveFinalizer(mysql, mysqlFinalizer) {
+				if err := r.Update(ctx, mysql); err != nil {
+					return ctrl.Result{}, err
+				}
+			}
+			return ctrl.Result{}, nil
+		}
+		log.Info("Could not complete finalizer. waiting another second")
+		return ctrl.Result{RequeueAfter: time.Second}, nil
+	}
 
+	// Add finalizer first if it doesn't exist
+	if !controllerutil.ContainsFinalizer(mysql, mysqlFinalizer) {
+		controllerutil.AddFinalizer(mysql, mysqlFinalizer)
+		if err := r.Update(ctx, mysql); err != nil {
 			log.Error(err, "Failed to update MySQL after adding finalizer")
 			return ctrl.Result{}, err
 		}
+		// Requeue to continue with other operations after finalizer is added
+		return ctrl.Result{Requeue: true}, nil
 	}
 
 	// Get referenced number
@@ -137,18 +157,6 @@ func (r *MySQLReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
 		}
 	}
 
-	if !mysql.GetDeletionTimestamp().IsZero() && controllerutil.ContainsFinalizer(mysql, mysqlFinalizer) {
-		if r.finalizeMySQL(ctx, mysql) {
-			if controllerutil.RemoveFinalizer(mysql, mysqlFinalizer) {
-				if err := r.Update(ctx, mysql); err != nil {
-					return ctrl.Result{}, err
-				}
-			}
-		} else {
-			log.Info("Could not complete finalizer. waiting another second")
-			return ctrl.Result{RequeueAfter: time.Second}, nil
-		}
-	}
 	return ctrl.Result{}, nil
 }
 
@@ -271,10 +279,24 @@ func (r *MySQLReconciler) countReferencesByMySQLDB(ctx context.Context, mysql *m
 // finalizeMySQL return true if no user and no db is referencing the given MySQL
 func (r *MySQLReconciler) finalizeMySQL(ctx context.Context, mysql *mysqlv1alpha1.MySQL) bool {
 	log := log.FromContext(ctx).WithName("MySQLReconciler")
-	if mysql.Status.UserCount > 0 || mysql.Status.DBCount > 0 {
-		log.Info("there's referencing user or database", "UserCount", mysql.Status.UserCount, "DBCount", mysql.Status.DBCount)
+
+	// Check actual references instead of status
+	userCount, err := r.countReferencesByMySQLUser(ctx, mysql)
+	if err != nil {
+		log.Error(err, "Failed to count user references")
 		return false
 	}
+	dbCount, err := r.countReferencesByMySQLDB(ctx, mysql)
+	if err != nil {
+		log.Error(err, "Failed to count db references")
+		return false
+	}
+
+	if userCount > 0 || dbCount > 0 {
+		log.Info("there's referencing user or database", "UserCount", userCount, "DBCount", dbCount)
+		return false
+	}
+
 	if db, ok := r.MySQLClients[mysql.GetKey()]; ok {
 		if err := db.Close(); err != nil {
 			return false
@@ -285,4 +307,4 @@ func (r *MySQLReconciler) finalizeMySQL(ctx context.Context, mysql *mysqlv1alpha
 		log.Info("MySQL client doesn't exist", "mysql.Key", mysql.GetKey())
 	}
 	return true
-}
+}

internal/controller/mysql_finalizer_test.go

@@ -0,0 +1,118 @@
+package controllers
+
+import (
+	"context"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
+	mysqlv1alpha1 "github.com/nakamasato/mysql-operator/api/v1alpha1"
+)
+
+var _ = Describe("MySQL Finalizer", func() {
+	const testNamespace = "default"
+	ctx := context.Background()
+
+	BeforeEach(func() {
+		// Clean up any existing resources
+		cleanUpMySQLUser(ctx, k8sClient, testNamespace)
+		cleanUpMySQLDB(ctx, k8sClient, testNamespace)
+		cleanUpMySQL(ctx, k8sClient, testNamespace)
+	})
+
+	AfterEach(func() {
+		// Clean up test resources
+		cleanUpMySQLUser(ctx, k8sClient, testNamespace)
+		cleanUpMySQLDB(ctx, k8sClient, testNamespace)
+		cleanUpMySQL(ctx, k8sClient, testNamespace)
+	})
+
+	Context("Finalizer Behavior", func() {
+		It("Should add finalizer before creating dependent resources", func() {
+			// Create MySQL without finalizer
+			mysql := &mysqlv1alpha1.MySQL{
+				TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: "MySQL"},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-mysql-finalizer",
+					Namespace: testNamespace,
+				},
+				Spec: mysqlv1alpha1.MySQLSpec{
+					Host:          "localhost",
+					AdminUser:     mysqlv1alpha1.Secret{Name: "root", Type: "raw"},
+					AdminPassword: mysqlv1alpha1.Secret{Name: "password", Type: "raw"},
+				},
+			}
+			Expect(k8sClient.Create(ctx, mysql)).Should(Succeed())
+
+			// Verify finalizer is added
+			Eventually(func() bool {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "test-mysql-finalizer", Namespace: testNamespace}, mysql)
+				if err != nil {
+					return false
+				}
+				return controllerutil.ContainsFinalizer(mysql, mysqlFinalizer)
+			}, time.Second*10).Should(BeTrue())
+
+			// Create dependent MySQLUser
+			mysqlUser := &mysqlv1alpha1.MySQLUser{
+				TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: "MySQLUser"},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-user",
+					Namespace: testNamespace,
+				},
+				Spec: mysqlv1alpha1.MySQLUserSpec{
+					MysqlName: "test-mysql-finalizer",
+					Host:      "%",
+				},
+			}
+			Expect(k8sClient.Create(ctx, mysqlUser)).Should(Succeed())
+
+			// Delete MySQL
+			Expect(k8sClient.Delete(ctx, mysql)).Should(Succeed())
+
+			// Verify MySQL is not immediately deleted due to finalizer
+			Consistently(func() error {
+				return k8sClient.Get(ctx, types.NamespacedName{Name: "test-mysql-finalizer", Namespace: testNamespace}, mysql)
+			}, time.Second*2).Should(Succeed())
+
+			// Verify dependent resources are cleaned up
+			Eventually(func() error {
+				return k8sClient.Get(ctx, types.NamespacedName{Name: "test-user", Namespace: testNamespace}, mysqlUser)
+			}, time.Second*10).ShouldNot(Succeed())
+
+			// Verify MySQL is eventually deleted
+			Eventually(func() error {
+				return k8sClient.Get(ctx, types.NamespacedName{Name: "test-mysql-finalizer", Namespace: testNamespace}, mysql)
+			}, time.Second*5).ShouldNot(Succeed())
+		})
+
+		It("Should handle deletion before finalizer addition", func() {
+			// Create MySQL without finalizer
+			mysql := &mysqlv1alpha1.MySQL{
+				TypeMeta: metav1.TypeMeta{APIVersion: APIVersion, Kind: "MySQL"},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-mysql-no-finalizer",
+					Namespace: testNamespace,
+				},
+				Spec: mysqlv1alpha1.MySQLSpec{
+					Host:          "localhost",
+					AdminUser:     mysqlv1alpha1.Secret{Name: "root", Type: "raw"},
+					AdminPassword: mysqlv1alpha1.Secret{Name: "password", Type: "raw"},
+				},
+			}
+			Expect(k8sClient.Create(ctx, mysql)).Should(Succeed())
+
+			// Delete immediately
+			Expect(k8sClient.Delete(ctx, mysql)).Should(Succeed())
+
+			// Verify it's deleted without issues
+			Eventually(func() error {
+				return k8sClient.Get(ctx, types.NamespacedName{Name: "test-mysql-no-finalizer", Namespace: testNamespace}, mysql)
+			}, time.Second*5).ShouldNot(Succeed())
+		})
+	})
+})

internal/controller/suite_test.go

@@ -23,13 +23,16 @@ import (
 	"time"
 
 	mysqlv1alpha1 "github.com/nakamasato/mysql-operator/api/v1alpha1"
+	mysqlinternal "github.com/nakamasato/mysql-operator/internal/mysql"
+	"github.com/nakamasato/mysql-operator/internal/secret"
 	testdbdriver "github.com/nakamasato/test-db-driver"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
+	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -96,11 +99,33 @@ var _ = BeforeSuite(func() {
 	Expect(k8sClient).NotTo(BeNil())
 
 	StartDebugTool(ctx, cfg, scheme)
+
+	// Start controller manager
+	k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
+		Scheme: scheme,
+	})
+	Expect(err).ToNot(HaveOccurred())
+
+	// Set up MySQL controller
+	err = (&MySQLReconciler{
+		Client:          k8sManager.GetClient(),
+		Scheme:          k8sManager.GetScheme(),
+		MySQLClients:    mysqlinternal.MySQLClients{},
+		MySQLDriverName: "testdbdriver",
+		SecretManagers:  map[string]secret.SecretManager{"raw": secret.RawSecretManager{}},
+	}).SetupWithManager(k8sManager)
+	Expect(err).ToNot(HaveOccurred())
+
+	go func() {
+		err = k8sManager.Start(ctx)
+		Expect(err).ToNot(HaveOccurred())
+	}()
+	time.Sleep(100 * time.Millisecond)
 })
 
 var _ = AfterSuite(func() {
 	By("tearing down the test environment")
 	cancel()
 	err := testEnv.Stop()
 	Expect(err).NotTo(HaveOccurred())
-})
+})