refactor rbacfinalizercontroller to fix cluster ns is terminating after delete clustermanager (#211)

Signed-off-by: Zhiwei Yin <zyin@redhat.com>
This commit is contained in:
Zhiwei Yin
2023-07-05 09:33:41 +08:00
committed by GitHub
parent eb9b7fac80
commit 0adc7664ae
5 changed files with 127 additions and 191 deletions

1
.gitignore vendored
View File

@@ -24,6 +24,7 @@ munge-csv
# Output of the go coverage tool, specifically when used with LiteIDE
*.out
_output/
.idea/
.kubeconfig
.hub-kubeconfig

View File

@@ -204,11 +204,12 @@ func NewManifestWork(namespace, name string, finalizers []string, deletionTimest
return work
}
func NewRole(namespace, name string, finalizers []string, terminated bool) *rbacv1.Role {
func NewRole(namespace, name string, finalizers []string, labels map[string]string, terminated bool) *rbacv1.Role {
role := &rbacv1.Role{}
role.Namespace = namespace
role.Name = name
role.Finalizers = finalizers
role.Labels = labels
if terminated {
now := metav1.Now()
role.DeletionTimestamp = &now
@@ -216,11 +217,12 @@ func NewRole(namespace, name string, finalizers []string, terminated bool) *rbac
return role
}
func NewRoleBinding(namespace, name string, finalizers []string, terminated bool) *rbacv1.RoleBinding {
func NewRoleBinding(namespace, name string, finalizers []string, labels map[string]string, terminated bool) *rbacv1.RoleBinding {
rolebinding := &rbacv1.RoleBinding{}
rolebinding.Namespace = namespace
rolebinding.Name = name
rolebinding.Finalizers = finalizers
rolebinding.Labels = labels
if terminated {
now := metav1.Now()
rolebinding.DeletionTimestamp = &now

View File

@@ -177,10 +177,9 @@ func (m *HubManagerOptions) RunControllerManager(ctx context.Context, controller
)
rbacFinalizerController := rbacfinalizerdeletion.NewFinalizeController(
kubeInfomers.Rbac().V1().Roles(),
kubeInfomers.Rbac().V1().RoleBindings(),
kubeInfomers.Core().V1().Namespaces().Lister(),
clusterInformers.Cluster().V1().ManagedClusters().Lister(),
kubeInfomers.Rbac().V1().RoleBindings().Lister(),
kubeInfomers.Core().V1().Namespaces(),
clusterInformers.Cluster().V1().ManagedClusters(),
workInformers.Work().V1().ManifestWorks().Lister(),
kubeClient.RbacV1(),
controllerContext.EventRecorder,

View File

@@ -2,25 +2,26 @@ package rbacfinalizerdeletion
import (
"context"
"fmt"
"reflect"
"time"
"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/operator/events"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
rbacv1informers "k8s.io/client-go/informers/rbac/v1"
"k8s.io/apimachinery/pkg/selection"
corev1informers "k8s.io/client-go/informers/core/v1"
rbacv1client "k8s.io/client-go/kubernetes/typed/rbac/v1"
corelisters "k8s.io/client-go/listers/core/v1"
rbacv1listers "k8s.io/client-go/listers/rbac/v1"
"k8s.io/client-go/tools/cache"
"k8s.io/klog/v2"
informerv1 "open-cluster-management.io/api/client/cluster/informers/externalversions/cluster/v1"
clusterv1listers "open-cluster-management.io/api/client/cluster/listers/cluster/v1"
worklister "open-cluster-management.io/api/client/work/listers/work/v1"
clusterv1 "open-cluster-management.io/api/cluster/v1"
@@ -33,7 +34,6 @@ const (
)
type finalizeController struct {
roleLister rbacv1listers.RoleLister
roleBindingLister rbacv1listers.RoleBindingLister
rbacClient rbacv1client.RbacV1Interface
clusterLister clusterv1listers.ManagedClusterLister
@@ -42,128 +42,98 @@ type finalizeController struct {
eventRecorder events.Recorder
}
// NewFinalizeController ensures all manifestworks are deleted before role/rolebinding for work
// NewFinalizeController ensures all manifestworks are deleted before rolebinding for work
// agent are deleted in a terminating cluster namespace.
func NewFinalizeController(
roleInformer rbacv1informers.RoleInformer,
roleBindingInformer rbacv1informers.RoleBindingInformer,
namespaceLister corelisters.NamespaceLister,
clusterLister clusterv1listers.ManagedClusterLister,
roleBindingLister rbacv1listers.RoleBindingLister,
namespaceInformer corev1informers.NamespaceInformer,
clusterInformer informerv1.ManagedClusterInformer,
manifestWorkLister worklister.ManifestWorkLister,
rbacClient rbacv1client.RbacV1Interface,
eventRecorder events.Recorder,
) factory.Controller {
controller := &finalizeController{
roleLister: roleInformer.Lister(),
roleBindingLister: roleBindingInformer.Lister(),
namespaceLister: namespaceLister,
clusterLister: clusterLister,
roleBindingLister: roleBindingLister,
namespaceLister: namespaceInformer.Lister(),
clusterLister: clusterInformer.Lister(),
manifestWorkLister: manifestWorkLister,
rbacClient: rbacClient,
eventRecorder: eventRecorder,
}
return factory.New().
WithInformersQueueKeysFunc(queue.QueueKeyByMetaNamespaceName, roleInformer.Informer(), roleBindingInformer.Informer()).
WithInformersQueueKeysFunc(queue.QueueKeyByMetaNamespaceName, clusterInformer.Informer(), namespaceInformer.Informer()).
WithSync(controller.sync).ToController("FinalizeController", eventRecorder)
}
func (m *finalizeController) sync(ctx context.Context, controllerContext factory.SyncContext) error {
key := controllerContext.QueueKey()
namespace, name, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
// ignore role/rolebinding whose key is not in format: namespace/name
if key == "" {
return nil
}
cluster, err := m.clusterLister.Get(namespace)
_, clusterName, err := cache.SplitMetaNamespaceKey(key)
if err != nil {
return nil
}
cluster, err := m.clusterLister.Get(clusterName)
if err != nil && !errors.IsNotFound(err) {
return err
}
ns, err := m.namespaceLister.Get(namespace)
if err != nil {
return err
}
// cluster is deleted or being deleted
role, rolebinding, err := m.getRoleAndRoleBinding(namespace, name)
if err != nil {
return err
}
err = m.syncRoleAndRoleBinding(ctx, controllerContext, role, rolebinding, ns, cluster)
if err != nil {
klog.Errorf("Reconcile role/rolebinding %s fails with err: %v", key, err)
}
return err
}
func (m *finalizeController) syncRoleAndRoleBinding(ctx context.Context, controllerContext factory.SyncContext,
role *rbacv1.Role, rolebinding *rbacv1.RoleBinding, ns *corev1.Namespace, cluster *clusterv1.ManagedCluster) error {
// Skip if neither role nor rolebinding has the finalizer
if !hasFinalizer(role, manifestWorkFinalizer) && !hasFinalizer(rolebinding, manifestWorkFinalizer) {
ns, err := m.namespaceLister.Get(clusterName)
if errors.IsNotFound(err) {
return nil
}
if err != nil {
return err
}
// There are two possible cases that we need to remove finalizers on role/rolebindings based on
// There are two possible cases that we need to remove finalizers on rolebindings based on
// clean of manifestworks.
// 1. The namespace is finalizing.
// 2. The cluster is finalizing but namespace fails to be deleted.
if !ns.DeletionTimestamp.IsZero() || (cluster != nil && !cluster.DeletionTimestamp.IsZero()) {
// 2. The cluster is finalizing or not found.
if !ns.DeletionTimestamp.IsZero() || cluster == nil ||
(cluster != nil && !cluster.DeletionTimestamp.IsZero()) {
works, err := m.manifestWorkLister.ManifestWorks(ns.Name).List(labels.Everything())
if err != nil {
return err
}
if len(works) != 0 {
return fmt.Errorf("still having %d works in the cluster namespace %s", len(works), ns.Name)
}
}
// remove finalizer from role/rolebinding
if pendingFinalization(role) {
if err := m.removeFinalizerFromRole(ctx, role, manifestWorkFinalizer); err != nil {
return err
}
}
if pendingFinalization(rolebinding) {
if err := m.removeFinalizerFromRoleBinding(ctx, rolebinding, manifestWorkFinalizer); err != nil {
return err
controllerContext.Queue().AddAfter(clusterName, 10*time.Second)
klog.Warningf("still having %d works in the cluster namespace %s", len(works), ns.Name)
return nil
}
return m.syncRoleBindings(ctx, controllerContext, clusterName)
}
return nil
}
func (m *finalizeController) getRoleAndRoleBinding(namespace, name string) (*rbacv1.Role, *rbacv1.RoleBinding, error) {
role, err := m.roleLister.Roles(namespace).Get(name)
func (m *finalizeController) syncRoleBindings(ctx context.Context, controllerContext factory.SyncContext,
namespace string) error {
requirement, _ := labels.NewRequirement(clusterv1.ClusterNameLabelKey, selection.Exists, []string{})
selector := labels.NewSelector().Add(*requirement)
roleBindings, err := m.roleBindingLister.RoleBindings(namespace).List(selector)
if err != nil && !errors.IsNotFound(err) {
return nil, nil, err
return err
}
rolebinding, err := m.roleBindingLister.RoleBindings(namespace).Get(name)
if err != nil && !errors.IsNotFound(err) {
return nil, nil, err
for _, roleBinding := range roleBindings {
// Skip if roleBinding has no the finalizer
if !hasFinalizer(roleBinding, manifestWorkFinalizer) {
continue
}
// remove finalizer from roleBinding
if pendingFinalization(roleBinding) {
if err := m.removeFinalizerFromRoleBinding(ctx, roleBinding, manifestWorkFinalizer); err != nil {
return err
}
}
}
return role, rolebinding, nil
}
// removeFinalizerFromRole removes the particular finalizer from role
func (m *finalizeController) removeFinalizerFromRole(ctx context.Context, role *rbacv1.Role, finalizer string) error {
if role == nil {
return nil
}
role = role.DeepCopy()
if changed := removeFinalizer(role, finalizer); !changed {
return nil
}
_, err := m.rbacClient.Roles(role.Namespace).Update(ctx, role, metav1.UpdateOptions{})
return err
return nil
}
// removeFinalizerFromRoleBinding removes the particular finalizer from rolebinding

View File

@@ -7,7 +7,6 @@ import (
"time"
"github.com/openshift/library-go/pkg/operator/events"
corev1 "k8s.io/api/core/v1"
rbacv1 "k8s.io/api/rbac/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
@@ -20,7 +19,6 @@ import (
fakeworkclient "open-cluster-management.io/api/client/work/clientset/versioned/fake"
workinformers "open-cluster-management.io/api/client/work/informers/externalversions"
clusterv1 "open-cluster-management.io/api/cluster/v1"
workapiv1 "open-cluster-management.io/api/work/v1"
testingcommon "open-cluster-management.io/ocm/pkg/common/testing"
testinghelpers "open-cluster-management.io/ocm/pkg/registration/helpers/testing"
@@ -34,29 +32,43 @@ func TestSync(t *testing.T) {
key string
clusters []runtime.Object
namespaces []runtime.Object
roles []runtime.Object
roleBindings []runtime.Object
works []runtime.Object
expectedErr string
}{
{
name: "key is empty",
key: "",
expectedErr: "",
},
{
name: "managed cluster namespace is not found",
key: fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
expectedErr: "namespace \"testmanagedcluster\" not found",
key: testinghelpers.TestManagedClusterName,
expectedErr: "",
},
{
name: "there are no resources in managed cluster namespace",
key: testinghelpers.TestManagedClusterName,
namespaces: []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true)},
expectedErr: "",
},
{
name: "cluster and ns are not deleting",
key: testinghelpers.TestManagedClusterName,
namespaces: []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false)},
clusters: []runtime.Object{testinghelpers.NewManagedCluster()},
expectedErr: "",
},
{
name: "there are no resources in managed cluster namespace",
key: fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
name: "still have works in deleting managed cluster namespace",
key: testinghelpers.TestManagedClusterName,
namespaces: []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true)},
},
{
name: "still have works in deleting managed cluster namespace",
key: fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
namespaces: []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true)},
roles: []runtime.Object{testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true)},
roleBindings: []runtime.Object{testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true)},
works: []runtime.Object{testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil)},
expectedErr: "still having 1 works in the cluster namespace testmanagedcluster",
roleBindings: []runtime.Object{testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName,
[]string{manifestWorkFinalizer}, map[string]string{clusterv1.ClusterNameLabelKey: testinghelpers.TestManagedClusterName}, true)},
works: []runtime.Object{testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil)},
expectedErr: "",
},
}
for _, c := range cases {
@@ -69,12 +81,7 @@ func TestSync(t *testing.T) {
t.Fatal(err)
}
}
roleStore := kubeInformerFactory.Rbac().V1().Roles().Informer().GetStore()
for _, role := range c.roles {
if err := roleStore.Add(role); err != nil {
t.Fatal(err)
}
}
roleBindingStore := kubeInformerFactory.Rbac().V1().RoleBindings().Informer().GetStore()
for _, roleBinding := range c.roleBindings {
if err := roleBindingStore.Add(roleBinding); err != nil {
@@ -84,7 +91,12 @@ func TestSync(t *testing.T) {
clusterClient := fakeclusterclient.NewSimpleClientset()
clusterInformerFactory := clusterinformers.NewSharedInformerFactory(clusterClient, time.Minute*10)
clusterStore := clusterInformerFactory.Cluster().V1().ManagedClusters().Informer().GetStore()
for _, cluster := range c.clusters {
if err := clusterStore.Add(cluster); err != nil {
t.Fatal(err)
}
}
workClient := fakeworkclient.NewSimpleClientset()
workInformerFactory := workinformers.NewSharedInformerFactory(workClient, 5*time.Minute)
workStore := workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore()
@@ -94,8 +106,16 @@ func TestSync(t *testing.T) {
}
}
_ = NewFinalizeController(
kubeInformerFactory.Rbac().V1().RoleBindings().Lister(),
kubeInformerFactory.Core().V1().Namespaces(),
clusterInformerFactory.Cluster().V1().ManagedClusters(),
workInformerFactory.Work().V1().ManifestWorks().Lister(),
kubeClient.RbacV1(),
events.NewInMemoryRecorder(""),
)
ctrl := &finalizeController{
roleLister: kubeInformerFactory.Rbac().V1().Roles().Lister(),
roleBindingLister: kubeInformerFactory.Rbac().V1().RoleBindings().Lister(),
namespaceLister: kubeInformerFactory.Core().V1().Namespaces().Lister(),
clusterLister: clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(),
@@ -112,93 +132,57 @@ func TestSync(t *testing.T) {
func TestSyncRoleAndRoleBinding(t *testing.T) {
cases := []struct {
name string
role *rbacv1.Role
roleBinding *rbacv1.RoleBinding
cluster *clusterv1.ManagedCluster
namespace *corev1.Namespace
work *workapiv1.ManifestWork
namespace string
expectedRoleFinalizers []string
expectedRoleBindingFinalizers []string
expectedWorkFinalizers []string
expectedQueueLen int
validateRbacActions func(t *testing.T, actions []clienttesting.Action)
}{
{
name: "skip if neither role nor rolebinding exists",
cluster: testinghelpers.NewManagedCluster(),
namespace: testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
work: testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", nil, nil),
name: "skip if rolebinding does not exists",
namespace: testinghelpers.TestManagedClusterName,
validateRbacActions: testingcommon.AssertNoActions,
},
{
name: "skip if neither role nor rolebinding has finalizer",
role: testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, nil, false),
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, nil, false),
cluster: testinghelpers.NewManagedCluster(),
namespace: testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
work: testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil),
expectedWorkFinalizers: []string{manifestWorkFinalizer},
validateRbacActions: testingcommon.AssertNoActions,
name: "skip if rolebinding has no finalizer", roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, nil, nil, false),
namespace: testinghelpers.TestManagedClusterName,
validateRbacActions: testingcommon.AssertNoActions,
},
{
name: "remove finalizer from deleting role within non-terminating namespace",
role: testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, false),
cluster: testinghelpers.NewManagedCluster(),
namespace: testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
work: testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil),
name: "skip if rolebinding has no labels",
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, nil, false),
namespace: testinghelpers.TestManagedClusterName,
expectedRoleBindingFinalizers: []string{manifestWorkFinalizer},
expectedWorkFinalizers: []string{manifestWorkFinalizer},
validateRbacActions: testingcommon.AssertNoActions,
},
{
name: "remove finalizer from deleting roleBinding",
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer},
map[string]string{clusterv1.ClusterNameLabelKey: testinghelpers.TestManagedClusterName}, true),
namespace: testinghelpers.TestManagedClusterName,
expectedRoleFinalizers: []string{manifestWorkFinalizer},
validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "update")
},
},
{
name: "remove finalizer from role/rolebinding within terminating cluster",
role: testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
cluster: testinghelpers.NewDeletingManagedCluster(),
namespace: testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "update", "update")
},
},
{
name: "remove finalizer from role/rolebinding within terminating ns",
role: testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
namespace: testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true),
validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
testingcommon.AssertActions(t, actions, "update", "update")
},
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
objects := []runtime.Object{}
if c.role != nil {
objects = append(objects, c.role)
}
if c.roleBinding != nil {
objects = append(objects, c.roleBinding)
}
fakeClient := fakeclient.NewSimpleClientset(objects...)
var fakeManifestWorkClient *fakeworkclient.Clientset
if c.work == nil {
fakeManifestWorkClient = fakeworkclient.NewSimpleClientset()
} else {
fakeManifestWorkClient = fakeworkclient.NewSimpleClientset(c.work)
}
workInformerFactory := workinformers.NewSharedInformerFactory(fakeManifestWorkClient, 5*time.Minute)
kubeInformerFactory := kubeinformers.NewSharedInformerFactory(fakeClient, time.Minute*10)
recorder := events.NewInMemoryRecorder("")
controller := finalizeController{
manifestWorkLister: workInformerFactory.Work().V1().ManifestWorks().Lister(),
eventRecorder: recorder,
rbacClient: fakeClient.RbacV1(),
roleBindingLister: kubeInformerFactory.Rbac().V1().RoleBindings().Lister(),
eventRecorder: recorder,
rbacClient: fakeClient.RbacV1(),
}
controllerContext := testingcommon.NewFakeSyncContext(t, "")
@@ -207,23 +191,15 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
ctx, cancel := context.WithTimeout(context.TODO(), 15*time.Second)
defer cancel()
workInformerFactory.Start(ctx.Done())
workInformerFactory.WaitForCacheSync(ctx.Done())
if err := controller.syncRoleAndRoleBinding(context.TODO(), controllerContext, c.role, c.roleBinding, c.namespace, c.cluster); err != nil {
kubeInformerFactory.Start(ctx.Done())
kubeInformerFactory.WaitForCacheSync(ctx.Done())
fakeClient.ClearActions()
if err := controller.syncRoleBindings(context.TODO(), controllerContext, c.namespace); err != nil {
t.Fatal(err)
}
c.validateRbacActions(t, fakeClient.Actions())
if c.role != nil {
role, err := fakeClient.RbacV1().Roles(c.role.Namespace).Get(context.TODO(), c.role.Name, metav1.GetOptions{})
if err != nil {
t.Fatal(err)
}
testinghelpers.AssertFinalizers(t, role, c.expectedRoleFinalizers)
}
if c.roleBinding != nil {
rolebinding, err := fakeClient.RbacV1().RoleBindings(c.roleBinding.Namespace).Get(context.TODO(), c.roleBinding.Name, metav1.GetOptions{})
if err != nil {
@@ -232,18 +208,6 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
testinghelpers.AssertFinalizers(t, rolebinding, c.expectedRoleBindingFinalizers)
}
if c.work != nil {
work, err := fakeManifestWorkClient.WorkV1().ManifestWorks(c.work.Namespace).Get(context.TODO(), c.work.Name, metav1.GetOptions{})
if err != nil {
t.Fatal(err)
}
testinghelpers.AssertFinalizers(t, work, c.expectedWorkFinalizers)
}
actual := controllerContext.Queue().Len()
if actual != c.expectedQueueLen {
t.Errorf("Expect queue with length: %d, but got %d", c.expectedQueueLen, actual)
}
}()
})
}