Merge branch 'issue-248' into 'master'

refactor instance_activity

Closes #248

See merge request !230

circle/dashboard/views/vm.py

@@ -48,7 +48,7 @@ from firewall.models import Vlan, Host, Rule
 from manager.scheduler import SchedulerError
 from storage.models import Disk
 from vm.models import (
-    Instance, instance_activity, InstanceActivity, Node, Lease,
+    Instance, InstanceActivity, Node, Lease,
     InstanceTemplate, InterfaceTemplate, Interface,
 )
 from .util import (
@@ -78,10 +78,10 @@ class VmDetailVncTokenView(CheckedDetailView):
         if not request.user.has_perm('vm.access_console'):
             raise PermissionDenied()
         if self.object.node:
-            with instance_activity(
-                    code_suffix='console-accessed', instance=self.object,
-                    user=request.user, readable_name=ugettext_noop(
-                        "console access"), concurrency_check=False):
+            with self.object.activity(
+                    code_suffix='console-accessed', user=request.user,
+                    readable_name=ugettext_noop("console access"),
+                    concurrency_check=False):
                 port = self.object.vnc_port
                 host = str(self.object.node.host.ipv4)
                 value = signing.dumps({'host': host, 'port': port},
@@ -892,8 +892,7 @@ class VmList(LoginRequiredMixin, FilterMixin, ListView):
                 in [i.name for i in Instance._meta.fields] + ["pk"]):
             queryset = queryset.order_by(sort)
 
-        return queryset.filter(
-            **self.get_queryset_filters()).prefetch_related(
+        return queryset.filter(**self.get_queryset_filters()).prefetch_related(
             "owner", "node", "owner__profile", "interface_set", "lease",
             "interface_set__host").distinct()
 
@@ -1378,9 +1377,8 @@ class TransferOwnershipConfirmView(LoginRequiredMixin, View):
         instance, owner = self.get_instance(key, request.user)
 
         old = instance.owner
-        with instance_activity(code_suffix='ownership-transferred',
-                               concurrency_check=False,
-                               instance=instance, user=request.user):
+        with instance.activity(code_suffix='ownership-transferred',
+                               concurrency_check=False, user=request.user):
             instance.owner = request.user
             instance.clean()
             instance.save()

circle/vm/models/__init__.py

 # flake8: noqa
 from .activity import InstanceActivity
-from .activity import instance_activity
 from .activity import NodeActivity
 from .activity import node_activity
 from .common import BaseResourceConfigModel
@@ -20,7 +19,7 @@ from .node import Node
 __all__ = [
     'InstanceActivity', 'BaseResourceConfigModel',
     'NamedBaseResourceConfig', 'VirtualMachineDescModel', 'InstanceTemplate',
-    'Instance', 'instance_activity', 'post_state_changed', 'pre_state_changed',
-    'InterfaceTemplate', 'Interface', 'Trait', 'Node', 'NodeActivity', 'Lease',
-    'node_activity', 'pwgen'
+    'Instance', 'post_state_changed', 'pre_state_changed', 'InterfaceTemplate',
+    'Interface', 'Trait', 'Node', 'NodeActivity', 'Lease', 'node_activity',
+    'pwgen'
 ]

circle/vm/models/activity.py

@@ -205,21 +205,6 @@ class InstanceActivity(ActivityModel):
             self.activity_code)
 
 
-@contextmanager
-def instance_activity(code_suffix, instance, on_abort=None, on_commit=None,
-                      task_uuid=None, user=None, concurrency_check=True,
-                      readable_name=None, resultant_state=None):
-    """Create a transactional context for an instance activity.
-    """
-    if not readable_name:
-        warn("Set readable_name", stacklevel=3)
-    act = InstanceActivity.create(code_suffix, instance, task_uuid, user,
-                                  concurrency_check,
-                                  readable_name=readable_name,
-                                  resultant_state=resultant_state)
-    return activitycontextimpl(act, on_abort=on_abort, on_commit=on_commit)
-
-
 class NodeActivity(ActivityModel):
     ACTIVITY_CODE_BASE = join_activity_code('vm', 'Node')
     node = ForeignKey('Node', related_name='activity_log',

circle/vm/models/instance.py

@@ -16,6 +16,7 @@
 # with CIRCLE.  If not, see <http://www.gnu.org/licenses/>.
 
 from __future__ import absolute_import, unicode_literals
+from contextlib import contextmanager
 from datetime import timedelta
 from functools import partial
 from importlib import import_module
@@ -41,12 +42,11 @@ from taggit.managers import TaggableManager
 
 from acl.models import AclBase
 from common.models import (
-    create_readable, HumanReadableException,
+    activitycontextimpl, create_readable, HumanReadableException,
 )
 from common.operations import OperatedMixin
 from ..tasks import agent_tasks
-from .activity import (ActivityInProgressError, instance_activity,
-                       InstanceActivity)
+from .activity import (ActivityInProgressError, InstanceActivity)
 from .common import BaseResourceConfigModel, Lease
 from .network import Interface
 from .node import Node, Trait
@@ -365,7 +365,7 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
         def __on_commit(activity):
             activity.resultant_state = 'PENDING'
 
-        with instance_activity(code_suffix='create', instance=inst,
+        with inst.activity(code_suffix='create',
                            readable_name=ugettext_noop("create instance"),
                            on_commit=__on_commit, user=inst.owner) as act:
             # create related entities
@@ -668,7 +668,7 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
                     "%(success)s notifications succeeded."),
                     success=len(success), successes=success)
 
-        with instance_activity('notification_about_expiration', instance=self,
+        with self.activity('notification_about_expiration',
                            readable_name=ugettext_noop(
                                "notify owner about expiration"),
                            on_commit=on_commit):
@@ -852,3 +852,17 @@ class Instance(AclBase, VirtualMachineDescModel, StatusModel, OperatedMixin,
     @property
     def metric_prefix(self):
         return 'vm.%s' % self.vm_name
+
+    @contextmanager
+    def activity(self, code_suffix, readable_name, on_abort=None,
+                 on_commit=None, task_uuid=None, user=None,
+                 concurrency_check=True, resultant_state=None):
+        """Create a transactional context for an instance activity.
+        """
+        if not readable_name:
+            warn("Set readable_name", stacklevel=3)
+        act = InstanceActivity.create(
+            code_suffix=code_suffix, instance=self, task_uuid=task_uuid,
+            user=user, concurrency_check=concurrency_check,
+            readable_name=readable_name, resultant_state=resultant_state)
+        return activitycontextimpl(act, on_abort=on_abort, on_commit=on_commit)

circle/vm/models/network.py

@@ -26,7 +26,6 @@ from django.utils.translation import ugettext_lazy as _, ugettext_noop
 from common.models import create_readable
 from firewall.models import Vlan, Host
 from ..tasks import net_tasks
-from .activity import instance_activity
 
 logger = getLogger(__name__)
 
@@ -120,10 +119,10 @@ class Interface(Model):
             host.hostname = instance.vm_name
             # Get addresses from firewall
             if base_activity is None:
-                act_ctx = instance_activity(
+                act_ctx = instance.activity(
                     code_suffix='allocating_ip',
                     readable_name=ugettext_noop("allocate IP address"),
-                    instance=instance, user=owner)
+                    user=owner)
             else:
                 act_ctx = base_activity.sub_activity(
                     'allocating_ip',

circle/vm/tasks/local_agent_tasks.py

@@ -103,16 +103,15 @@ def create_windows_tar():
 
 @celery.task
 def agent_started(vm, version=None, system=None):
-    from vm.models import Instance, instance_activity, InstanceActivity
+    from vm.models import Instance, InstanceActivity
     instance = Instance.objects.get(id=int(vm.split('-')[-1]))
     queue = instance.get_remote_queue_name("agent")
     initialized = instance.activity_log.filter(
         activity_code='vm.Instance.agent.cleanup').exists()
 
-    with instance_activity(code_suffix='agent',
+    with instance.activity(code_suffix='agent',
                            readable_name=ugettext_noop('agent'),
-                           concurrency_check=False,
-                           instance=instance) as act:
+                           concurrency_check=False) as act:
         with act.sub_activity('starting',
                               readable_name=ugettext_noop('starting')):
             pass
@@ -195,9 +194,8 @@ def update_agent(instance, act=None, system=None, version=None):
                 ugettext_noop('update to %(version)s'),
                 version=settings.AGENT_VERSION))
     else:
-        from vm.models import instance_activity
-        act = instance_activity(
-            code_suffix='agent.update', instance=instance,
+        act = instance.activity(
+            code_suffix='agent.update',
             readable_name=create_readable(
                 ugettext_noop('update agent to %(version)s'),
                 version=settings.AGENT_VERSION))