From cb91077c589656190ab576fac1dfbed6f3dc9d40 Mon Sep 17 00:00:00 2001 From: Jiahui <4543bxy@gmail.com> Date: Wed, 27 Sep 2023 18:18:17 +0800 Subject: [PATCH] Fix: billing error (#4006) * fix billing nil point error * fix non-user delete ns role * add team resource quota limit * fix init balance * price query optimize --- controllers/account/api/v1/debt_webhook.go | 3 -- .../account/controllers/account_controller.go | 48 ++++++++----------- .../account/controllers/billing_controller.go | 15 ++++-- .../billingrecordquery_controller.go | 31 ++++-------- 4 files changed, 41 insertions(+), 56 deletions(-) diff --git a/controllers/account/api/v1/debt_webhook.go b/controllers/account/api/v1/debt_webhook.go index d4ee33b94..7c023290a 100644 --- a/controllers/account/api/v1/debt_webhook.go +++ b/controllers/account/api/v1/debt_webhook.go @@ -69,9 +69,6 @@ func (d DebtValidate) Handle(ctx context.Context, req admission.Request) admissi logger.V(1).Info("checking user", "userInfo", req.UserInfo, "req.Namespace", req.Namespace, "req.Name", req.Name, "req.gvrk", getGVRK(req), "req.Operation", req.Operation) // skip delete request (删除quota资源除外) if req.Operation == admissionV1.Delete && !strings.Contains(getGVRK(req), "quotas") { - if req.Kind.Kind == "Namespace" { - return admission.Denied(fmt.Sprintf("ns %s request %s %s permission denied", req.Namespace, req.Kind.Kind, req.Operation)) - } return admission.Allowed("") } diff --git a/controllers/account/controllers/account_controller.go b/controllers/account/controllers/account_controller.go index ced464182..88c549595 100644 --- a/controllers/account/controllers/account_controller.go +++ b/controllers/account/controllers/account_controller.go @@ -38,7 +38,6 @@ import ( corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -97,6 +96,9 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct if owner = user.Annotations[userv1.UserAnnotationOwnerKey]; owner == "" { return ctrl.Result{}, fmt.Errorf("user owner is empty") } + // This is only used to monitor and initialize user resource creation data, + // determine the resource quota created by the owner user and the resource quota initialized by the account user, + // and only the resource quota created by the team user _, err = r.syncAccount(ctx, owner, r.AccountSystemNamespace, "ns-"+user.Name) return ctrl.Result{}, err } else if client.IgnoreNotFound(err) != nil { @@ -117,7 +119,7 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct return ctrl.Result{}, nil } - account, err := r.syncAccount(ctx, payment.Spec.UserID, r.AccountSystemNamespace, payment.Namespace) + account, err := r.syncAccount(ctx, getUsername(payment.Spec.UserID), r.AccountSystemNamespace, payment.Namespace) if err != nil { return ctrl.Result{}, fmt.Errorf("get account failed: %v", err) } @@ -200,13 +202,8 @@ func (r *AccountReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct func (r *AccountReconciler) syncAccount(ctx context.Context, owner, accountNamespace string, userNamespace string) (*accountv1.Account, error) { if err := r.syncResourceQuotaAndLimitRange(ctx, userNamespace); err != nil { - //return nil, fmt.Errorf("sync resource resourceQuota and limitRange failed: %v", err) r.Logger.Error(err, "sync resource resourceQuota and limitRange failed") } - //TODO delete after nodeport count quota already in resource-quota - if err := r.adaptNodePortCountQuota(ctx, userNamespace); err != nil { - r.Logger.Error(err, "adapt nodeport count quota failed") - } account := accountv1.Account{ ObjectMeta: metav1.ObjectMeta{ Name: owner, @@ -221,6 +218,7 @@ func (r *AccountReconciler) syncAccount(ctx context.Context, owner, accountNames }); err != nil { return nil, fmt.Errorf("failed to create account %v, err: %v", account, err) } + // If the user is not the owner, the user represents the team and does not perform subsequent account initialization operations if owner != getUsername(userNamespace) { return &account, nil } @@ -228,7 +226,7 @@ func (r *AccountReconciler) syncAccount(ctx context.Context, owner, accountNames if err := r.syncRoleAndRoleBinding(ctx, owner, userNamespace); err != nil { return nil, fmt.Errorf("sync role and rolebinding failed: %v", err) } - err := r.initBalance(&account) + err := initBalance(&account) if err != nil { return nil, fmt.Errorf("sync init balance failed: %v", err) } @@ -258,7 +256,7 @@ func (r *AccountReconciler) syncAccount(ctx context.Context, owner, accountNames }); err != nil { return nil, err } - err = r.initBalance(&account) + err = initBalance(&account) if err != nil { return nil, fmt.Errorf("sync init balance failed: %v", err) } @@ -290,18 +288,18 @@ func (r *AccountReconciler) syncResourceQuotaAndLimitRange(ctx context.Context, return nil } -func (r *AccountReconciler) adaptNodePortCountQuota(ctx context.Context, nsName string) error { - quota := resources.GetDefaultResourceQuota(nsName, ResourceQuotaPrefix+nsName) - return retry.Retry(10, 1*time.Second, func() error { - _, err := controllerutil.CreateOrUpdate(ctx, r.Client, quota, func() error { - if _, ok := quota.Spec.Hard[corev1.ResourceServicesNodePorts]; !ok { - quota.Spec.Hard[corev1.ResourceServicesNodePorts] = resource.MustParse(env.GetEnvWithDefault(resources.QuotaLimitsNodePorts, resources.DefaultQuotaLimitsNodePorts)) - } - return nil - }) - return err - }) -} +//func (r *AccountReconciler) adaptNodePortCountQuota(ctx context.Context, nsName string) error { +// quota := resources.GetDefaultResourceQuota(nsName, ResourceQuotaPrefix+nsName) +// return retry.Retry(10, 1*time.Second, func() error { +// _, err := controllerutil.CreateOrUpdate(ctx, r.Client, quota, func() error { +// if _, ok := quota.Spec.Hard[corev1.ResourceServicesNodePorts]; !ok { +// quota.Spec.Hard[corev1.ResourceServicesNodePorts] = resource.MustParse(env.GetEnvWithDefault(resources.QuotaLimitsNodePorts, resources.DefaultQuotaLimitsNodePorts)) +// } +// return nil +// }) +// return err +// }) +//} func (r *AccountReconciler) syncRoleAndRoleBinding(ctx context.Context, name, namespace string) error { role := rbacv1.Role{ @@ -407,7 +405,7 @@ func SyncAccountStatus(ctx context.Context, client client.Client, account *accou return client.Status().Update(ctx, account) } -func (r *AccountReconciler) initBalance(account *accountv1.Account) (err error) { +func initBalance(account *accountv1.Account) (err error) { if account.Status.EncryptBalance == nil { encryptBalance, err := crypto.EncryptInt64(account.Status.Balance) if err != nil { @@ -430,11 +428,7 @@ func (r *AccountReconciler) SetupWithManager(mgr ctrl.Manager, rateOpts controll r.Logger = ctrl.Log.WithName("account_controller") r.AccountSystemNamespace = env.GetEnvWithDefault(ACCOUNTNAMESPACEENV, DEFAULTACCOUNTNAMESPACE) return ctrl.NewControllerManagedBy(mgr). - For(&userv1.User{}, builder.WithPredicates(predicate.And(OnlyCreatePredicate{}, predicate.Funcs{ - CreateFunc: func(createEvent event.CreateEvent) bool { - return createEvent.Object.GetAnnotations()[userv1.UserAnnotationOwnerKey] == createEvent.Object.GetName() - }, - }))). + For(&userv1.User{}, builder.WithPredicates(predicate.And(OnlyCreatePredicate{}))). Watches(&source.Kind{Type: &accountv1.Payment{}}, &handler.EnqueueRequestForObject{}). WithOptions(rateOpts). Complete(r) diff --git a/controllers/account/controllers/billing_controller.go b/controllers/account/controllers/billing_controller.go index 02b442c60..e80497138 100644 --- a/controllers/account/controllers/billing_controller.go +++ b/controllers/account/controllers/billing_controller.go @@ -122,13 +122,15 @@ func (r *BillingReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct orderList = append(orderList, ids...) consumAmount += amount } - if err := r.rechargeBalance(owner, consumAmount); err != nil { - for i := range orderList { - if err := r.DBClient.UpdateBillingStatus(orderList[i], resources.Unsettled); err != nil { - r.Logger.Error(err, "update billing status failed", "id", orderList[i]) + if consumAmount > 0 { + if err := r.rechargeBalance(owner, consumAmount); err != nil { + for i := range orderList { + if err := r.DBClient.UpdateBillingStatus(orderList[i], resources.Unsettled); err != nil { + r.Logger.Error(err, "update billing status failed", "id", orderList[i]) + } } + return ctrl.Result{}, fmt.Errorf("recharge balance failed: %w", err) } - return ctrl.Result{}, fmt.Errorf("recharge balance failed: %w", err) } return ctrl.Result{Requeue: true, RequeueAfter: time.Until(currentHourTime.Add(1*time.Hour + 10*time.Minute))}, nil } @@ -141,6 +143,9 @@ func (r *BillingReconciler) rechargeBalance(owner string, amount int64) (err err if err = r.Get(context.Background(), types.NamespacedName{Name: owner, Namespace: r.AccountSystemNamespace}, account); err != nil { return fmt.Errorf("get account cr failed: %w", err) } + if err = initBalance(account); err != nil { + return fmt.Errorf("failed to init balance: %v", err) + } if err = crypto.RechargeBalance(account.Status.EncryptDeductionBalance, amount); err != nil { return fmt.Errorf("recharge balance failed: %w", err) } diff --git a/controllers/account/controllers/billingrecordquery_controller.go b/controllers/account/controllers/billingrecordquery_controller.go index e0a75d083..0133bc08a 100644 --- a/controllers/account/controllers/billingrecordquery_controller.go +++ b/controllers/account/controllers/billingrecordquery_controller.go @@ -26,7 +26,6 @@ import ( accountv1 "github.com/labring/sealos/controllers/account/api/v1" "github.com/labring/sealos/controllers/pkg/database" - "github.com/labring/sealos/controllers/pkg/gpu" "github.com/labring/sealos/controllers/pkg/resources" "github.com/labring/sealos/controllers/pkg/utils/env" @@ -86,7 +85,7 @@ func (r *BillingRecordQueryReconciler) Reconcile(ctx context.Context, req ctrl.R priceQuery := &accountv1.PriceQuery{} err = r.Get(ctx, req.NamespacedName, priceQuery) if err == nil { - return r.ReconcilePriceQuery(ctx, priceQuery, dbClient) + return r.ReconcilePriceQuery(ctx, priceQuery) } else if client.IgnoreNotFound(err) != nil { return ctrl.Result{}, err } @@ -148,36 +147,26 @@ func (r *BillingRecordQueryReconciler) SetupWithManager(mgr ctrl.Manager, rateOp Complete(r) } -func (r *BillingRecordQueryReconciler) ReconcilePriceQuery(ctx context.Context, priceQuery *accountv1.PriceQuery, dbClient database.Interface) (ctrl.Result, error) { +func (r *BillingRecordQueryReconciler) ReconcilePriceQuery(ctx context.Context, priceQuery *accountv1.PriceQuery) (ctrl.Result, error) { // TODO query price if time.Since(priceQuery.CreationTimestamp.Time) > (3 * time.Minute) { err := r.Delete(ctx, priceQuery) return ctrl.Result{}, err } - pricesMap, err := dbClient.GetAllPricesMap() - if err != nil { - r.Logger.Error(err, "get all prices failed") - pricesMap = resources.DefaultPrices - } priceQuery.Status.BillingRecords = make([]accountv1.BillingRecord, 0) - alias, err := gpu.GetGPUAlias(r.Client) - if errors.IsNotFound(err) { - r.Logger.Error(err, "get gpu alias failed") - } - for property, v := range pricesMap { - if resources.IsGpuResource(property) && alias != nil { - if propertyAlias := alias[resources.GetGpuResourceProduct(property)]; propertyAlias != "" { - property = string(resources.NewGpuResource(propertyAlias)) - } + for _, property := range resources.DefaultPropertyTypeLS.Types { + displayName, displayPrice := property.Name, property.UnitPrice + if resources.IsGpuResource(property.Name) && property.Alias != "" { + displayName = string(resources.NewGpuResource(property.Alias)) } priceQuery.Status.BillingRecords = append(priceQuery.Status.BillingRecords, accountv1.BillingRecord{ - ResourceType: property, - Price: v.Price, + ResourceType: displayName, + Price: displayPrice, }) } - if err = r.Status().Update(ctx, priceQuery); err != nil { + if err := r.Status().Update(ctx, priceQuery); err != nil { r.Logger.Error(err, "update price query status failed") return ctrl.Result{Requeue: true}, err } - return ctrl.Result{Requeue: true, RequeueAfter: time.Minute * 4}, err + return ctrl.Result{Requeue: true, RequeueAfter: time.Minute * 4}, nil }