Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tenant cascade deletion #747

Merged
merged 3 commits into from
May 6, 2022
Merged

Fix tenant cascade deletion #747

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b8ec6fd
fix tenant cascade deletion of the product
eguzki May 5, 2022
5e9ef38
fix tenant cascade deletion of the backend
eguzki May 5, 2022
e7f80b6
fix early return when backend status ID is nil
eguzki May 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 73 additions & 45 deletions controllers/capabilities/backend_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ import (
threescaleapi "github.com/3scale/3scale-porta-go-client/client"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
Expand Down Expand Up @@ -85,17 +87,19 @@ func (r *BackendReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
// Ignore deleted Backends, this can happen when foregroundDeletion is enabled
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
if backend.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(backend, backendFinalizer) {
// Attempt to remove backend only if backend.Status.ID is present
if backend.Status.ID != nil {
requeue, err := r.removeBackend(backend.Spec.ProviderAccountRef, *backend.Status.ID, backend.Namespace, backend.Spec.SystemName)
if err != nil {
return ctrl.Result{}, err
}
if requeue {
return ctrl.Result{RequeueAfter: requeueTime}, nil
}
} else {
reqLogger.Info("ERROR", "could not remove backend because backend ID is missing for backend name", backend.Name)
res, err := r.removeBackendReferencesFromProducts(backend)
if err != nil {
return ctrl.Result{}, err
}

if res.Requeue {
reqLogger.Info("Removed backend references from product CRs. Requeueing.")
return res, nil
}

err = r.removeBackendFrom3scale(backend)
if err != nil {
return ctrl.Result{}, err
}

controllerutil.RemoveFinalizer(backend, backendFinalizer)
Expand Down Expand Up @@ -186,12 +190,6 @@ func (r *BackendReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
return ctrl.Result{}, nil
}

func (r *BackendReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&capabilitiesv1beta1.Backend{}).
Complete(r)
}

func (r *BackendReconciler) reconcile(backendResource *capabilitiesv1beta1.Backend) (*BackendStatusReconciler, error) {
logger := r.Logger().WithValues("backend", backendResource.Name)

Expand Down Expand Up @@ -240,71 +238,95 @@ func (r *BackendReconciler) validateSpec(backendResource *capabilitiesv1beta1.Ba
}
}

func (r *BackendReconciler) removeBackend(providerAccountRef *corev1.LocalObjectReference, backendID int64, backendNamespace string, systemName string) (requeue bool, err error) {
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), backendNamespace, providerAccountRef, r.Logger())
if err != nil {
return false, err
}

threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount)
if err != nil {
return false, err
}

func (r *BackendReconciler) removeBackendReferencesFromProducts(backend *capabilitiesv1beta1.Backend) (ctrl.Result, error) {
// Retrieve all product CRs that are under the same ns as the backend CR
opts := k8sclient.ListOptions{
Namespace: backendNamespace,
Namespace: backend.Namespace,
}
productCRsList := &capabilitiesv1beta1.ProductList{}
err = r.Client().List(context.TODO(), productCRsList, &opts)
err := r.Client().List(context.TODO(), productCRsList, &opts)
if err != nil {
return false, err
return ctrl.Result{}, err
}

// fetch CRs that belong to a tenant and require removal of the backend mentions in
// Application Plan pricing rules
// Application Plan limits
// Backend usages
tenantProductCRs, err := r.fetchTenantProductCRs(productCRsList, providerAccountRef, backendNamespace, systemName)
tenantProductCRs, err := r.fetchTenantProductCRs(productCRsList, backend)
if err != nil {
return false, err
return ctrl.Result{}, err
}

var productCrUpdated = false
productCRUpdated := false

// update backendUsages for each product retrieved
for _, productCR := range tenantProductCRs {
if productCR.RemoveBackendReferences(systemName) {
if productCR.RemoveBackendReferences(backend.Spec.SystemName) {
err = r.UpdateResource(&productCR)
if err != nil {
return false, err
return ctrl.Result{}, err
}
productCrUpdated = true
productCRUpdated = true
}
}

if productCrUpdated {
return true, nil
if productCRUpdated {
return ctrl.Result{Requeue: productCRUpdated, RequeueAfter: requeueTime}, nil
}

return ctrl.Result{}, nil
}

func (r *BackendReconciler) removeBackendFrom3scale(backend *capabilitiesv1beta1.Backend) error {
logger := r.Logger().WithValues("backend", client.ObjectKey{Name: backend.Name, Namespace: backend.Namespace})

// Attempt to remove backend only if backend.Status.ID is present
if backend.Status.ID == nil {
logger.Info("could not remove backend because backend ID is missing for backend name")
Copy link
Contributor

@MStokluska MStokluska May 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the ID is nil I think line 305 will be throwing nil pointer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! I forgot the early return nil

}

providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), backend.Namespace, backend.Spec.ProviderAccountRef, logger)
if apierrors.IsNotFound(err) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be cleaner that way:

if err != nil {
   if apierrors.IsNotFound(err) {
       		logger.Info("backend not deleted from 3scale, provider account not found")
		return nil
   }
   return err
 }

logger.Info("backend not deleted from 3scale, provider account not found")
return nil
}

if err != nil {
return err
}

threescaleAPIClient, err := controllerhelper.PortaClient(providerAccount)
if err != nil {
return err
}

// Attempt to remove backendAPI - expect error on first attempt as the backendUsage has not been removed yet from 3scale
err = threescaleAPIClient.DeleteBackendApi(backendID)
err = threescaleAPIClient.DeleteBackendApi(*backend.Status.ID)
if err != nil && !threescaleapi.IsNotFound(err) {
return false, err
return err
}

return false, nil
return nil
}

func (r *BackendReconciler) fetchTenantProductCRs(productsCRsList *capabilitiesv1beta1.ProductList, providerAccountRef *corev1.LocalObjectReference, backendNs string, systemName string) ([]capabilitiesv1beta1.Product, error) {
func (r *BackendReconciler) fetchTenantProductCRs(productsCRsList *capabilitiesv1beta1.ProductList, backendResource *capabilitiesv1beta1.Backend) ([]capabilitiesv1beta1.Product, error) {
logger := r.Logger().WithValues("backend", client.ObjectKey{Name: backendResource.Name, Namespace: backendResource.Namespace})

var productsList []capabilitiesv1beta1.Product
backendProviderAccount, err := controllerhelper.LookupProviderAccount(r.Client(), backendNs, providerAccountRef, r.Logger())
backendProviderAccount, err := controllerhelper.LookupProviderAccount(r.Client(), backendResource.Namespace, backendResource.Spec.ProviderAccountRef, logger)

if apierrors.IsNotFound(err) {
logger.Info("could not look up for products of the same tenant. Tenant not found")
return nil, nil
}

if err != nil {
return nil, err
}

for _, productCR := range productsCRsList.Items {
productProviderAccount, err := controllerhelper.LookupProviderAccount(r.Client(), productCR.Namespace, productCR.Spec.ProviderAccountRef, r.Logger())
productProviderAccount, err := controllerhelper.LookupProviderAccount(r.Client(), productCR.Namespace, productCR.Spec.ProviderAccountRef, logger)
if err != nil {
// skip product CR if productProviderAccount is not found
continue
Expand All @@ -317,3 +339,9 @@ func (r *BackendReconciler) fetchTenantProductCRs(productsCRsList *capabilitiesv

return productsList, nil
}

func (r *BackendReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&capabilitiesv1beta1.Backend{}).
Complete(r)
}
27 changes: 18 additions & 9 deletions controllers/capabilities/product_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@ import (
threescaleapi "github.com/3scale/3scale-porta-go-client/client"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

Expand Down Expand Up @@ -82,7 +84,7 @@ func (r *ProductReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
// https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion
if product.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(product, productFinalizer) {
if product.Status.ID != nil {
err = r.removeProduct(product)
err = r.removeProductFrom3scale(product)
if err != nil {
r.EventRecorder().Eventf(product, corev1.EventTypeWarning, "Failed to delete product", "%v", err)
return ctrl.Result{}, err
Expand Down Expand Up @@ -183,12 +185,6 @@ func (r *ProductReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
return ctrl.Result{}, reconcileErr
}

func (r *ProductReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&capabilitiesv1beta1.Product{}).
Complete(r)
}

func (r *ProductReconciler) reconcile(productResource *capabilitiesv1beta1.Product) (*ProductStatusReconciler, error) {
logger := r.Logger().WithValues("product", productResource.Name)

Expand Down Expand Up @@ -386,8 +382,15 @@ func computeBackendUsageList(list []capabilitiesv1beta1.Backend, backendUsageMap
return result
}

func (r *ProductReconciler) removeProduct(productResource *capabilitiesv1beta1.Product) error {
providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), productResource.Namespace, productResource.Spec.ProviderAccountRef, r.Logger())
func (r *ProductReconciler) removeProductFrom3scale(productResource *capabilitiesv1beta1.Product) error {
logger := r.Logger().WithValues("product", client.ObjectKey{Name: productResource.Name, Namespace: productResource.Namespace})

providerAccount, err := controllerhelper.LookupProviderAccount(r.Client(), productResource.Namespace, productResource.Spec.ProviderAccountRef, logger)
if apierrors.IsNotFound(err) {
logger.Info("product not deleted from 3scale, provider account not found")
return nil
}

if err != nil {
return err
}
Expand All @@ -404,3 +407,9 @@ func (r *ProductReconciler) removeProduct(productResource *capabilitiesv1beta1.P

return nil
}

func (r *ProductReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&capabilitiesv1beta1.Product{}).
Complete(r)
}
6 changes: 3 additions & 3 deletions pkg/controller/helper/lookup_provider_account.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func LookupProviderAccount(cl client.Client, ns string, providerAccountRef *core
for _, source := range orderedSources {
providerAccount, err := source(cl, ns, providerAccountRef, logger)
if err != nil {
return nil, fmt.Errorf("LookupProviderAccount: %w", err)
return nil, err
}

if providerAccount != nil {
Expand All @@ -63,11 +63,11 @@ func providerAccountFromSecretReferenceSource(cl client.Client, ns string, provi
secretSource := helper.NewSecretSource(cl, ns)
adminURLStr, err := secretSource.RequiredFieldValueFromRequiredSecret(providerAccountRef.Name, providerAccountSecretURLFieldName)
if err != nil {
return nil, fmt.Errorf("providerAccountFromSecretReferenceSource: %w", err)
return nil, err
}
token, err := secretSource.RequiredFieldValueFromRequiredSecret(providerAccountRef.Name, providerAccountSecretTokenFieldName)
if err != nil {
return nil, fmt.Errorf("providerAccountFromSecretReferenceSource: %w", err)
return nil, err
}

return &ProviderAccount{AdminURLStr: adminURLStr, Token: token}, nil
Expand Down