fix(TidbMonitor): do not set OwnerReferences for ClusterRole and Clus…

…terRoleBinding (#5956)

pkg/apis/label/label.go

@@ -192,6 +192,9 @@ const (
 	// TiDBMonitorVal is Monitor label value
 	TiDBMonitorVal string = "monitor"
 
+	// TiDBMonitorProtectionFinalizer is the name of finalizer on TidbMonitors
+	TiDBMonitorProtectionFinalizer string = "tidb.pingcap.com/monitor-protection"
+
 	// CleanJobLabelVal is clean job label value
 	CleanJobLabelVal string = "clean"
 	// RestoreJobLabelVal is restore job label value

pkg/controller/generic_control.go

@@ -103,7 +103,7 @@ func (w *typedWrapper) CreateOrUpdateClusterRoleBinding(controller client.Object
 		existingCRB.RoleRef = desiredCRB.RoleRef
 		existingCRB.Subjects = desiredCRB.Subjects
 		return nil
-	}, true)
+	}, false)
 	if err != nil {
 		return nil, err
 	}
@@ -118,7 +118,7 @@ func (w *typedWrapper) CreateOrUpdateClusterRole(controller client.Object, clust
 		existingCRole.Labels = desiredCRole.Labels
 		existingCRole.Rules = desiredCRole.Rules
 		return nil
-	}, true)
+	}, false)
 	if err != nil {
 		return nil, err
 	}

pkg/monitor/monitor/monitor_manager.go

@@ -14,11 +14,25 @@
 package monitor
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"sort"
 	"strings"
 
+	appsv1 "k8s.io/api/apps/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbac "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
+	"k8s.io/client-go/discovery"
+	discoverycachedmemory "k8s.io/client-go/discovery/cached/memory"
+	"k8s.io/klog/v2"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+
 	"github.com/pingcap/tidb-operator/pkg/apis/label"
 	"github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1"
 	v1alpha1validation "github.com/pingcap/tidb-operator/pkg/apis/pingcap/v1alpha1/validation"
@@ -31,17 +45,6 @@ import (
 	"github.com/pingcap/tidb-operator/pkg/pdapi"
 	"github.com/pingcap/tidb-operator/pkg/util"
 	utildiscovery "github.com/pingcap/tidb-operator/pkg/util/discovery"
-	appsv1 "k8s.io/api/apps/v1"
-	corev1 "k8s.io/api/core/v1"
-	rbac "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/selection"
-	"k8s.io/client-go/discovery"
-	discoverycachedmemory "k8s.io/client-go/discovery/cached/memory"
-	"k8s.io/klog/v2"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type MonitorManager struct {
@@ -67,9 +70,13 @@ func NewMonitorManager(deps *controller.Dependencies) *MonitorManager {
 }
 
 func (m *MonitorManager) SyncMonitor(monitor *v1alpha1.TidbMonitor) error {
+	if err := m.addProtectionFinalizerIfNeed(monitor); err != nil {
+		return err
+	}
 	if monitor.DeletionTimestamp != nil {
-		return nil
+		return m.cleanAndRemoveProtectionFinalizerIfNeed(monitor)
 	}
+
 	if len(monitor.Spec.Clusters) < 1 && (monitor.Spec.DM == nil || len(monitor.Spec.DM.Clusters) < 1) {
 		klog.Errorf("tm[%s/%s] does not configure the target tidbcluster", monitor.Namespace, monitor.Name)
 		return nil
@@ -187,6 +194,63 @@ func (m *MonitorManager) SyncMonitor(monitor *v1alpha1.TidbMonitor) error {
 	return nil
 }
 
+func (m *MonitorManager) addProtectionFinalizerIfNeed(tm *v1alpha1.TidbMonitor) error {
+	if tm.DeletionTimestamp != nil {
+		return nil
+	}
+	if !controllerutil.ContainsFinalizer(tm, label.TiDBMonitorProtectionFinalizer) {
+		controllerutil.AddFinalizer(tm, label.TiDBMonitorProtectionFinalizer)
+		_, err := m.deps.Clientset.PingcapV1alpha1().TidbMonitors(tm.Namespace).Update(context.TODO(), tm, metav1.UpdateOptions{})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (m *MonitorManager) cleanAndRemoveProtectionFinalizerIfNeed(tm *v1alpha1.TidbMonitor) error {
+	if tm.DeletionTimestamp.IsZero() {
+		return nil
+	}
+
+	if tm.Spec.ClusterScoped {
+		// we need to clean ClusterRole and ClusterRoleBinding as we can't set ownerReference for them
+		// so we call DELETE directly for them (but without waiting for the deletion to complete)
+		clusterRole, err := m.deps.KubeClientset.RbacV1().ClusterRoles().Get(context.TODO(), GetMonitorObjectNameCrossNamespace(tm), metav1.GetOptions{})
+		if err != nil && !errors.IsNotFound(err) {
+			return err
+		} else if err == nil {
+			if clusterRole.Labels[label.ManagedByLabelKey] == label.TiDBOperator {
+				err = m.deps.KubeClientset.RbacV1().ClusterRoles().Delete(context.TODO(), clusterRole.Name, metav1.DeleteOptions{})
+				if err != nil {
+					return err
+				}
+			}
+		}
+
+		clusterRoleBinding, err := m.deps.KubeClientset.RbacV1().ClusterRoleBindings().Get(context.TODO(), GetMonitorObjectNameCrossNamespace(tm), metav1.GetOptions{})
+		if err != nil && !errors.IsNotFound(err) {
+			return err
+		} else if err == nil {
+			if clusterRoleBinding.Labels[label.ManagedByLabelKey] == label.TiDBOperator {
+				err = m.deps.KubeClientset.RbacV1().ClusterRoleBindings().Delete(context.TODO(), clusterRoleBinding.Name, metav1.DeleteOptions{})
+				if err != nil {
+					return err
+				}
+			}
+		}
+	}
+
+	if controllerutil.ContainsFinalizer(tm, label.TiDBMonitorProtectionFinalizer) {
+		controllerutil.RemoveFinalizer(tm, label.TiDBMonitorProtectionFinalizer)
+		_, err := m.deps.Clientset.PingcapV1alpha1().TidbMonitors(tm.Namespace).Update(context.TODO(), tm, metav1.UpdateOptions{})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (m *MonitorManager) syncTidbMonitorStatus(monitor *v1alpha1.TidbMonitor) error {
 	sts, err := m.deps.StatefulSetLister.StatefulSets(monitor.Namespace).Get(GetMonitorObjectName(monitor))
 	if err != nil {

pkg/monitor/monitor/monitor_manager_test.go

@@ -14,6 +14,7 @@
 package monitor
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -72,6 +73,8 @@ func TestTidbMonitorSyncCreate(t *testing.T) {
 		if tm.Spec.Shards == nil {
 			tm.Spec.Shards = pointer.Int32Ptr(0)
 		}
+		_, err = tmm.deps.Clientset.PingcapV1alpha1().TidbMonitors(tm.Namespace).Create(context.Background(), tm, metav1.CreateOptions{})
+		g.Expect(err).NotTo(HaveOccurred())
 
 		err = tmm.SyncMonitor(tm)
 		if test.errExpectFn != nil {
@@ -548,6 +551,8 @@ func TestTidbMonitorSyncUpdate(t *testing.T) {
 		if test.prepare != nil {
 			test.prepare(tmm, tm)
 		}
+		_, err = tmm.deps.Clientset.PingcapV1alpha1().TidbMonitors(tm.Namespace).Create(context.Background(), tm, metav1.CreateOptions{})
+		g.Expect(err).NotTo(HaveOccurred())
 
 		err = tmm.SyncMonitor(tm)
 

pkg/monitor/monitor/util.go

@@ -253,10 +253,9 @@ func getMonitorRole(monitor *v1alpha1.TidbMonitor, policyRules []rbac.PolicyRule
 func getMonitorClusterRole(monitor *v1alpha1.TidbMonitor, policyRules []rbac.PolicyRule) *rbac.ClusterRole {
 	return &rbac.ClusterRole{
 		ObjectMeta: meta.ObjectMeta{
-			Name:            GetMonitorObjectNameCrossNamespace(monitor),
-			Namespace:       monitor.Namespace,
-			Labels:          buildTidbMonitorLabel(monitor.Name),
-			OwnerReferences: []meta.OwnerReference{controller.GetTiDBMonitorOwnerRef(monitor)},
+			Name:      GetMonitorObjectNameCrossNamespace(monitor),
+			Namespace: monitor.Namespace,
+			Labels:    buildTidbMonitorLabel(monitor.Name),
 		},
 		Rules: policyRules,
 	}
@@ -265,10 +264,9 @@ func getMonitorClusterRole(monitor *v1alpha1.TidbMonitor, policyRules []rbac.Pol
 func getMonitorClusterRoleBinding(sa *core.ServiceAccount, role *rbac.ClusterRole, monitor *v1alpha1.TidbMonitor) *rbac.ClusterRoleBinding {
 	return &rbac.ClusterRoleBinding{
 		ObjectMeta: meta.ObjectMeta{
-			Name:            GetMonitorObjectNameCrossNamespace(monitor),
-			Namespace:       monitor.Namespace,
-			Labels:          buildTidbMonitorLabel(monitor.Name),
-			OwnerReferences: []meta.OwnerReference{controller.GetTiDBMonitorOwnerRef(monitor)},
+			Name:      GetMonitorObjectNameCrossNamespace(monitor),
+			Namespace: monitor.Namespace,
+			Labels:    buildTidbMonitorLabel(monitor.Name),
 		},
 		Subjects: []rbac.Subject{
 			{