reana-admin: fix setting storage quota for users after global default…

AUTHORS.rst

@@ -7,6 +7,7 @@ The list of contributors in alphabetical order:
 - `Audrius Mecionis <https://orcid.org/0000-0002-3759-1663>`_
 - `Anton Khodak <https://orcid.org/0000-0003-3263-4553>`_
 - `Camila Diaz <https://orcid.org/0000-0001-5543-797X>`_
+- `Daan Rosendal <https://github.com/DaanRosendal>`_
 - `Diego Rodriguez <https://orcid.org/0000-0003-0649-2002>`_
 - `Dinos Kousidis <https://orcid.org/0000-0002-4914-4289>`_
 - `Domenic Gosein <https://orcid.org/0000-0002-1546-0435>`_

CHANGES.rst

@@ -4,6 +4,7 @@ Changes
 Version 0.9.1 (UNRELEASED)
 --------------------------
 
+- Fixes ``quota-set-default-limits`` command to propagate default quota limits to all users without custom quota limit values..
 - Changes ``launch`` endpoint also include the warnings of the validation of the workflow specification.
 - Changes OpenAPI specification with respect to return the maximum inactivity time before automatic closure of interactive sessions in ``info`` endpoint.
 - Adds the timestamp of when the workflow was stopped (``run_stopped_at``) to the workflow list and the workflow status endpoints.

reana_server/reana_admin/cli.py

@@ -16,20 +16,20 @@
 from typing import List, Optional
 
 import click
-import tablib
 import requests
+import tablib
 from flask.cli import with_appcontext
 from invenio_accounts.utils import register_user
 from kubernetes.client.rest import ApiException
 from reana_commons.config import (
-    REANAConfig,
     REANA_RESOURCE_HEALTH_COLORS,
     REANA_RUNTIME_KUBERNETES_NAMESPACE,
+    REANAConfig,
 )
-from reana_commons.email import send_email, REANA_EMAIL_SENDER
+from reana_commons.email import REANA_EMAIL_SENDER, send_email
 from reana_commons.errors import REANAEmailNotificationError
-from reana_commons.utils import click_table_printer
 from reana_commons.k8s.api_client import current_k8s_corev1_api_client
+from reana_commons.utils import click_table_printer
 from reana_db.config import DEFAULT_QUOTA_LIMITS
 from reana_db.database import Session
 from reana_db.models import (
@@ -46,6 +46,7 @@
 )
 from reana_db.utils import update_workspace_retention_rules
 
+from reana_server.api_client import current_rwc_api_client
 from reana_server.config import ADMIN_USER_ID, REANA_HOSTNAME
 from reana_server.reana_admin.check_workflows import check_workspaces
 from reana_server.reana_admin.options import (
@@ -55,8 +56,8 @@
 )
 from reana_server.reana_admin.retention_rule_deleter import RetentionRuleDeleter
 from reana_server.status import STATUS_OBJECT_TYPES
-from reana_server.api_client import current_rwc_api_client
 from reana_server.utils import (
+    JinjaEnv,
     _create_user,
     _export_users,
     _get_user_by_criteria,
@@ -65,7 +66,6 @@
     _validate_email,
     _validate_password,
     create_user_workspace,
-    JinjaEnv,
 )
 
 
@@ -638,24 +638,53 @@
 
 @reana_admin.command(
     "quota-set-default-limits",
-    help="Set default quota limits to users that do not have any.",
+    help="""Set default quota limits for users who do not have any custom limits
+         defined.
+
+    Note that any previously set user limits, either via old defaults or via
+    custom settings, will be kept during the upgrade, and won't be automatically
+    updated to match the new default limit value.""",
 )
 @admin_access_token_option
 @click.pass_context
 def set_default_quota_limit(ctx, admin_access_token: str):
-    """Set default quota limits to users that do not have any."""
-    users_without_quota_limits = User.query.filter(~User.resources.any()).all()
+    """Set default quota limits for users who do not have any custom limits defined."""
+    users_without_quota_limits = (
+        Session.query(User)
+        .filter(
+            User.id_.in_(
+                Session.query(UserResource.user_id).filter(
+                    UserResource.quota_limit == 0
+                )
+            )
+        )
+        .all()
+    )
+
     if not users_without_quota_limits:
         click.secho("There are no users without quota limits.", fg="green")
         sys.exit(0)
-    for resource in Resource.query:
-        ctx.invoke(
-            set_quota_limit,
-            emails=[user.email for user in users_without_quota_limits],
-            resource_name=resource.name,
-            resource_type=resource.type_.name,
-            limit=DEFAULT_QUOTA_LIMITS.get(resource.type_.name),
-        )
+
+    resources = Resource.query.all()
+
+    for user in users_without_quota_limits:
+        for resource in resources:
+            user_resource = UserResource.query.filter_by(
+                user_id=user.id_, resource_id=resource.id_
+            ).first()
+
+            if user_resource and user_resource.quota_limit == 0:
+                # If no limit exists, set the default limit
+                default_limit = DEFAULT_QUOTA_LIMITS.get(resource.type_.name)
+                if default_limit is not None and default_limit != 0:
+                    ctx.invoke(
+                        set_quota_limit,
+                        emails=[user.email],
+                        resource_name=resource.name,
+                        resource_type=resource.type_.name,
+                        limit=default_limit,
+                        admin_access_token=admin_access_token,
+                    )
 
 
 @reana_admin.command("queue-consume")
@@ -901,10 +930,10 @@
 ) -> None:
     """Check consistency of selected workflow run statuses between database, message queue and Kubernetes."""
     from .check_workflows import (
-        check_workflows,
+        InfoCollectionError,
         check_interactive_sessions,
+        check_workflows,
         display_results,
-        InfoCollectionError,
     )
 
     click.secho("Checking if workflows are in-sync...", fg="yellow")

tests/test_cli.py

@@ -8,26 +8,28 @@
 
 """Test command line application."""
 
-import datetime
 import csv
+import datetime
 import io
 import pathlib
 import secrets
-from unittest.mock import Mock, MagicMock, patch
 import uuid
+from unittest.mock import MagicMock, Mock, patch
 
-from click.testing import CliRunner
 import pytest
+from click.testing import CliRunner
 from pytest_reana.test_utils import make_mock_api_client
 from reana_db.models import (
-    generate_uuid,
     AuditLogAction,
     InteractiveSession,
+    Resource,
+    RunStatus,
     User,
+    UserResource,
     UserTokenStatus,
-    RunStatus,
     Workflow,
     WorkspaceRetentionRuleStatus,
+    generate_uuid,
 )
 
 from reana_server.api_client import WorkflowSubmissionPublisher
@@ -968,3 +970,33 @@ def test_check_workspaces(
         if workflow:
             assert len(result.errors) == 2
             assert any(workflow.workspace_path in str(error) for error in result.errors)
+
+
+def test_quota_set_default_limits_for_user_with_custom_limits(default_user, session):
+    """Test setting default quota when there are is one user with custom quota limits."""
+    runner = CliRunner()
+
+    resources = session.query(Resource).all()
+
+    for resource in resources:
+        user_resource = (
+            session.query(UserResource)
+            .filter_by(user_id=default_user.id_, resource_id=resource.id_)
+            .first()
+        )
+
+        if user_resource:
+            user_resource.quota_limit = 12345
+
+    session.commit()
+
+    result = runner.invoke(
+        reana_admin,
+        [
+            "quota-set-default-limits",
+            "--admin-access-token",
+            default_user.access_token,
+        ],
+    )
+
+    assert "There are no users without quota limits." in result.output