fix serviceaccount controller remove unnecessary retries (#2188)
Signed-off-by: hongming <coder.scala@gmail.com> Co-authored-by: hongming <coder.scala@gmail.com>
This commit is contained in:
committed by
ks-ci-bot
parent
91c2921733
commit
59d5f0e6d4
@@ -20,6 +20,7 @@ import (
|
||||
"k8s.io/client-go/tools/record"
|
||||
"k8s.io/klog/v2"
|
||||
iamv1beta1 "kubesphere.io/api/iam/v1beta1"
|
||||
tenantv1alpha1 "kubesphere.io/api/tenant/v1beta1"
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller"
|
||||
@@ -200,5 +201,22 @@ func (r *Reconciler) cleanUp(ctx context.Context, namespace *corev1.Namespace) e
|
||||
if err := r.DeleteAllOf(ctx, roleBinding, client.InNamespace(namespace.Name)); err != nil {
|
||||
return errors.Wrapf(err, "failed to delete role bindings")
|
||||
}
|
||||
updated := namespace.DeepCopy()
|
||||
modified := false
|
||||
newOwnerReferences := make([]metav1.OwnerReference, 0, len(updated.OwnerReferences))
|
||||
for _, owner := range updated.OwnerReferences {
|
||||
if owner.Kind != tenantv1alpha1.ResourceKindWorkspace {
|
||||
newOwnerReferences = append(newOwnerReferences, owner)
|
||||
} else {
|
||||
modified = true
|
||||
}
|
||||
}
|
||||
|
||||
if modified {
|
||||
updated.OwnerReferences = newOwnerReferences
|
||||
if err := r.Patch(ctx, updated, client.MergeFrom(namespace)); err != nil {
|
||||
return errors.Wrapf(err, "failed to cleanup ownerReferences for namespace %s", namespace.Name)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -10,18 +10,18 @@ import (
|
||||
"fmt"
|
||||
|
||||
"github.com/go-logr/logr"
|
||||
"github.com/pkg/errors"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
rbacv1 "k8s.io/api/rbac/v1"
|
||||
"k8s.io/apimachinery/pkg/api/errors"
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/client-go/tools/record"
|
||||
iamv1beta1 "kubesphere.io/api/iam/v1beta1"
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
||||
|
||||
iamv1beta1 "kubesphere.io/api/iam/v1beta1"
|
||||
|
||||
kscontroller "kubesphere.io/kubesphere/pkg/controller"
|
||||
rbacutils "kubesphere.io/kubesphere/pkg/utils/rbac"
|
||||
)
|
||||
@@ -58,8 +58,7 @@ func (r *Reconciler) SetupWithManager(mgr *kscontroller.Manager) error {
|
||||
// +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=rolebindings,verbs=get;list;watch;create;update;patch;delete
|
||||
|
||||
func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
|
||||
logger := r.logger.WithValues("serivceaccount", req.NamespacedName)
|
||||
// ctx := context.Background()
|
||||
logger := r.logger.WithValues("serviceaccount", req.NamespacedName)
|
||||
sa := &corev1.ServiceAccount{}
|
||||
if err := r.Get(ctx, req.NamespacedName, sa); err != nil {
|
||||
return ctrl.Result{}, client.IgnoreNotFound(err)
|
||||
@@ -78,12 +77,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
|
||||
func (r *Reconciler) getReferenceRole(ctx context.Context, roleName, namespace string) (*rbacv1.Role, error) {
|
||||
refRole := &rbacv1.Role{}
|
||||
refRoleName := rbacutils.RelatedK8sResourceName(roleName)
|
||||
err := r.Client.Get(ctx, types.NamespacedName{Name: refRoleName, Namespace: namespace}, refRole)
|
||||
if err != nil {
|
||||
if err := r.Get(ctx, types.NamespacedName{Name: refRoleName, Namespace: namespace}, refRole); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if v := refRole.Labels[iamv1beta1.RoleReferenceLabel]; v != roleName {
|
||||
return nil, errors.NewNotFound(rbacv1.Resource("roles"), refRoleName)
|
||||
if refRole.Labels[iamv1beta1.RoleReferenceLabel] != roleName {
|
||||
return nil, apierrors.NewNotFound(rbacv1.Resource("roles"), refRoleName)
|
||||
}
|
||||
return refRole, nil
|
||||
}
|
||||
@@ -93,16 +91,21 @@ func (r *Reconciler) CreateOrUpdateRoleBinding(ctx context.Context, logger logr.
|
||||
if roleName == "" {
|
||||
return nil
|
||||
}
|
||||
var role *rbacv1.Role
|
||||
|
||||
role, err := r.getReferenceRole(ctx, roleName, sa.Namespace)
|
||||
if err != nil {
|
||||
logger.Error(err, "cannot get reference role", "roleName", roleName)
|
||||
return err
|
||||
if apierrors.IsNotFound(err) {
|
||||
logger.V(4).Info("related role not found", "namespace", sa.Namespace, "role", roleName)
|
||||
return nil
|
||||
}
|
||||
return errors.Wrapf(err, "cannot get reference role %s/%s", sa.Namespace, roleName)
|
||||
}
|
||||
|
||||
// Delete existing rolebindings.
|
||||
saRoleBinding := &rbacv1.RoleBinding{}
|
||||
_ = r.Client.DeleteAllOf(ctx, saRoleBinding, client.InNamespace(sa.Namespace), client.MatchingLabels{iamv1beta1.ServiceAccountReferenceLabel: sa.Name})
|
||||
if err = r.DeleteAllOf(ctx, saRoleBinding, client.InNamespace(sa.Namespace), client.MatchingLabels{iamv1beta1.ServiceAccountReferenceLabel: sa.Name}); err != nil {
|
||||
return errors.Wrapf(err, "failed to delete RoleBindings for %s/%s", sa.Namespace, sa.Name)
|
||||
}
|
||||
|
||||
saRoleBinding = &rbacv1.RoleBinding{
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
@@ -125,14 +128,12 @@ func (r *Reconciler) CreateOrUpdateRoleBinding(ctx context.Context, logger logr.
|
||||
}
|
||||
|
||||
if err := controllerutil.SetControllerReference(sa, saRoleBinding, r.Scheme()); err != nil {
|
||||
logger.Error(err, "set controller reference failed")
|
||||
return err
|
||||
return errors.Wrapf(err, "failed to set controller reference for RoleBinding %s/%s", sa.Namespace, saRoleBinding.Name)
|
||||
}
|
||||
|
||||
logger.V(4).Info("create ServiceAccount rolebinding", "ServiceAccount", sa.Name)
|
||||
if err := r.Client.Create(ctx, saRoleBinding); err != nil {
|
||||
logger.Error(err, "create rolebinding failed")
|
||||
return err
|
||||
if err := r.Create(ctx, saRoleBinding); err != nil {
|
||||
return errors.Wrapf(err, "failed to create RoleBinding %s/%s", sa.Namespace, saRoleBinding.Name)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -13,17 +13,15 @@ import (
|
||||
. "github.com/onsi/gomega"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
rbacv1 "k8s.io/api/rbac/v1"
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/client-go/kubernetes/scheme"
|
||||
"k8s.io/client-go/tools/record"
|
||||
iamv1beta1 "kubesphere.io/api/iam/v1beta1"
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/fake"
|
||||
|
||||
iamv1beta1 "kubesphere.io/api/iam/v1beta1"
|
||||
|
||||
"kubesphere.io/kubesphere/pkg/utils/k8sutil"
|
||||
)
|
||||
|
||||
@@ -78,7 +76,6 @@ var _ = Describe("ServiceAccount", func() {
|
||||
ctx := context.Background()
|
||||
|
||||
reconciler := &Reconciler{
|
||||
//nolint:staticcheck
|
||||
Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(),
|
||||
logger: ctrl.Log.WithName("controllers").WithName("serviceaccount"),
|
||||
recorder: record.NewFakeRecorder(5),
|
||||
@@ -93,16 +90,15 @@ var _ = Describe("ServiceAccount", func() {
|
||||
By("Expecting to bind role successfully")
|
||||
rolebindings := &rbacv1.RoleBindingList{}
|
||||
Expect(func() bool {
|
||||
reconciler.List(ctx, rolebindings, client.InNamespace(sa.Namespace), client.MatchingLabels{iamv1beta1.ServiceAccountReferenceLabel: sa.Name})
|
||||
_ = reconciler.List(ctx, rolebindings, client.InNamespace(sa.Namespace), client.MatchingLabels{iamv1beta1.ServiceAccountReferenceLabel: sa.Name})
|
||||
return len(rolebindings.Items) == 1 && k8sutil.IsControlledBy(rolebindings.Items[0].OwnerReferences, "ServiceAccount", saName)
|
||||
}()).Should(BeTrue())
|
||||
})
|
||||
|
||||
It("Should report NotFound error when role doesn't exist", func() {
|
||||
It("Should not report NotFound error when role doesn't exist", func() {
|
||||
ctx := context.Background()
|
||||
|
||||
reconciler := &Reconciler{
|
||||
//nolint:staticcheck
|
||||
Client: fake.NewClientBuilder().WithScheme(scheme.Scheme).Build(),
|
||||
logger: ctrl.Log.WithName("controllers").WithName("serviceaccount"),
|
||||
recorder: record.NewFakeRecorder(5),
|
||||
@@ -110,7 +106,7 @@ var _ = Describe("ServiceAccount", func() {
|
||||
|
||||
Expect(reconciler.Create(ctx, sa)).Should(Succeed())
|
||||
_, err := reconciler.Reconcile(ctx, req)
|
||||
Expect(apierrors.IsNotFound(err)).To(BeTrue())
|
||||
Expect(err).Should(Succeed())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user