Merge branch 'feature-add-interface-fixes' into 'master'

Interface Fixes

circle/dashboard/forms.py

@@ -952,6 +952,25 @@ class VmDownloadDiskForm(forms.Form):
         return helper
 
 
+class VmAddInterfaceForm(forms.Form):
+    def __init__(self, *args, **kwargs):
+        choices = kwargs.pop('choices')
+        super(VmAddInterfaceForm, self).__init__(*args, **kwargs)
+
+        field = forms.ModelChoiceField(
+            queryset=choices, required=True, label=_('Vlan'))
+        if not choices:
+            field.widget.attrs['disabled'] = 'disabled'
+            field.empty_label = _('No more networks.')
+        self.fields['vlan'] = field
+
+    @property
+    def helper(self):
+        helper = FormHelper(self)
+        helper.form_tag = False
+        return helper
+
+
 class CircleAuthenticationForm(AuthenticationForm):
     # fields: username, password
 

circle/dashboard/static/dashboard/vm-common.js

@@ -3,7 +3,7 @@
 $(function() {
 
   /* vm operations */
-  $('#ops, #vm-details-resources-disk, #vm-details-renew-op').on('click', '.operation.btn', function(e) {
+  $('#ops, #vm-details-resources-disk, #vm-details-renew-op, #vm-details-pw-reset, #vm-details-add-interface').on('click', '.operation.btn', function(e) {
     var icon = $(this).children("i").addClass('fa-spinner fa-spin');
 
     $.ajax({
@@ -50,6 +50,9 @@ $(function() {
          */
         if(data.success) {
           $('a[href="#activity"]').trigger("click");
+          if(data.with_reload) {
+            location.reload();
+          }
 
           /* if there are messages display them */
           if(data.messages && data.messages.length > 0) {

circle/dashboard/templates/dashboard/vm-detail/network.html

 {% load i18n %}
 {% load network_tags %}
 <h2>
-  <a href="#" id="vm-details-network-add" class="btn btn-success pull-right no-js-hidden">
-    <i class="fa fa-plus"></i> {% trans "add interface" %}
-  </a>
+    <div id="vm-details-add-interface">
+    {% with op=op.add_interface %}{% if op %}
+    <a href="{{op.get_url}}" class="btn btn-{{op.effect}} operation pull-right"
+        {% if op.disabled %}disabled{% endif %}>
+    <i class="fa fa-{{op.icon}}"></i> {% trans "add interface" %}</a>
+    {% endif %}{% endwith %}
+    </div>
   {% trans "Interfaces" %}
 </h2>
 
-<div class="js-hidden row" id="vm-details-network-add-form">
-  <div class="col-md-12">
-    <div>
-      <hr />
-      <h3>
-        {% trans "Add new network interface!" %}
-      </h3>
-      <form method="POST" action="">
-        {% csrf_token %}
-        <div class="input-group" style="max-width: 330px;">
-          <select name="new_network_vlan" class="form-control font-awesome-font">
-            {% for v in vlans %}
-              <option value="{{ v.pk }}">
-                {% if v.managed %}
-                  &#xf0ac;
-                {% else %}
-                  &#xf0c1;
-                {% endif %}
-                {{ v.name }}
-              </option>
-              {% empty %}
-              <option value="-1">No more networks!</option>
-            {% endfor %}
-          </select>
-          <div class="input-group-btn">
-            <button {% if vlans|length == 0 %}disabled{% endif %}
-              type="submit" class="btn btn-success"><i class="fa fa-plus-circle"></i></button>
-          </div>
-        </div>
-      </form>
-      <hr />
-    </div>
-  </div>
-</div>
 
 {% for i in instance.interface_set.all %}
 <div>

circle/dashboard/tests/test_views.py

@@ -24,8 +24,9 @@ from django.contrib.auth.models import User, Group
 from django.contrib.auth.models import Permission
 from django.contrib.auth import authenticate
 
+from dashboard.views import VmAddInterfaceView
 from vm.models import Instance, InstanceTemplate, Lease, Node, Trait
-from vm.operations import WakeUpOperation
+from vm.operations import WakeUpOperation, AddInterfaceOperation
 from ..models import Profile
 from firewall.models import Vlan, Host, VlanGroup
 from mock import Mock, patch
@@ -168,7 +169,8 @@ class VmDetailTest(LoginMixin, TestCase):
     def test_unpermitted_network_add_wo_perm(self):
         c = Client()
         self.login(c, "user2")
-        response = c.post("/dashboard/vm/1/", {'new_network_vlan': 1})
+        response = c.post("/dashboard/vm/1/op/add_interface/",
+                          {'vlan': 1})
         self.assertEqual(response.status_code, 403)
 
     def test_unpermitted_network_add_wo_vlan_perm(self):
@@ -176,8 +178,18 @@ class VmDetailTest(LoginMixin, TestCase):
         self.login(c, "user2")
         inst = Instance.objects.get(pk=1)
         inst.set_level(self.u2, 'owner')
-        response = c.post("/dashboard/vm/1/", {'new_network_vlan': 1})
-        self.assertEqual(response.status_code, 403)
+        interface_count = inst.interface_set.count()
+
+        with patch.object(AddInterfaceOperation, 'async') as async:
+            async.side_effect = inst.add_interface.call
+            with patch.object(VmAddInterfaceView, 'get_form_kwargs',
+                              autospec=True) as get_form_kwargs:
+                get_form_kwargs.return_value = {'choices': Vlan.objects.all()}
+                response = c.post("/dashboard/vm/1/op/add_interface/",
+                                  {'vlan': 1})
+        self.assertEqual(response.status_code, 302)
+        assert async.called
+        self.assertEqual(inst.interface_set.count(), interface_count)
 
     def test_permitted_network_add(self):
         c = Client()
@@ -187,9 +199,12 @@ class VmDetailTest(LoginMixin, TestCase):
         vlan = Vlan.objects.get(id=1)
         vlan.set_level(self.u1, 'user')
         interface_count = inst.interface_set.count()
-        response = c.post("/dashboard/vm/1/",
-                          {'new_network_vlan': 1})
+        with patch.object(AddInterfaceOperation, 'async') as mock_method:
+            mock_method.side_effect = inst.add_interface
+            response = c.post("/dashboard/vm/1/op/add_interface/",
+                              {'vlan': 1})
         self.assertEqual(response.status_code, 302)
+        assert mock_method.called
         self.assertEqual(inst.interface_set.count(), interface_count + 1)
 
     def test_permitted_network_delete(self):
@@ -401,8 +416,7 @@ class VmDetailTest(LoginMixin, TestCase):
         inst.set_level(self.u2, 'owner')
         vlan = Vlan.objects.get(id=1)
         vlan.set_level(self.u2, 'user')
-        response = c.post("/dashboard/vm/1/",
-                          {'new_network_vlan': 1})
+        inst.add_interface(user=self.u2, vlan=vlan)
         host = Host.objects.get(
             interface__in=inst.interface_set.all())
         self.u2.user_permissions.add(Permission.objects.get(
@@ -421,8 +435,7 @@ class VmDetailTest(LoginMixin, TestCase):
         inst.set_level(self.u2, 'owner')
         vlan = Vlan.objects.get(id=1)
         vlan.set_level(self.u2, 'user')
-        response = c.post("/dashboard/vm/1/",
-                          {'new_network_vlan': 1})
+        inst.add_interface(user=self.u2, vlan=vlan)
         host = Host.objects.get(
             interface__in=inst.interface_set.all())
         self.u2.user_permissions.add(Permission.objects.get(

circle/dashboard/views.py

@@ -61,7 +61,8 @@ from .forms import (
     UserCreationForm, GroupProfileUpdateForm, UnsubscribeForm,
     VmSaveForm, UserKeyForm, VmRenewForm,
     CirclePasswordChangeForm, VmCreateDiskForm, VmDownloadDiskForm,
-    TraitsForm, RawDataForm, GroupPermissionForm, AclUserAddForm
+    TraitsForm, RawDataForm, GroupPermissionForm, AclUserAddForm,
+    VmAddInterfaceForm,
 )
 
 from .tables import (
@@ -309,7 +310,6 @@ class VmDetailView(CheckedDetailView):
             'new_tag': self.__add_tag,
             'to_remove': self.__remove_tag,
             'port': self.__add_port,
-            'new_network_vlan': self.__new_network,
             'abort_operation': self.__abort_operation,
         }
         for k, v in options.iteritems():
@@ -454,24 +454,6 @@ class VmDetailView(CheckedDetailView):
             return redirect(reverse_lazy("dashboard.views.detail",
                                          kwargs={'pk': self.get_object().pk}))
 
-    def __new_network(self, request):
-        self.object = self.get_object()
-        if not self.object.has_level(request.user, 'owner'):
-            raise PermissionDenied()
-
-        vlan = get_object_or_404(Vlan, pk=request.POST.get("new_network_vlan"))
-        if not vlan.has_level(request.user, 'user'):
-            raise PermissionDenied()
-        try:
-            self.object.add_interface(vlan=vlan, user=request.user)
-            messages.success(request, _("Successfully added new interface."))
-        except Exception, e:
-            error = u' '.join(e.messages)
-            messages.error(request, error)
-
-        return redirect("%s#network" % reverse_lazy(
-            "dashboard.views.detail", kwargs={'pk': self.object.pk}))
-
     def __abort_operation(self, request):
         self.object = self.get_object()
 
@@ -643,7 +625,9 @@ class FormOperationMixin(object):
                 request, extra, *args, **kwargs)
             if request.is_ajax():
                 return HttpResponse(
-                    json.dumps({'success': True}),
+                    json.dumps({
+                        'success': True,
+                        'with_reload': getattr(self, 'with_reload', False)}),
                     content_type="application=json"
                 )
             else:
@@ -660,6 +644,25 @@ class RequestFormOperationMixin(FormOperationMixin):
         return val
 
 
+class VmAddInterfaceView(FormOperationMixin, VmOperationView):
+
+    op = 'add_interface'
+    form_class = VmAddInterfaceForm
+    show_in_toolbar = False
+    icon = 'globe'
+    effect = 'success'
+    with_reload = True
+
+    def get_form_kwargs(self):
+        inst = self.get_op().instance
+        choices = Vlan.get_objects_with_level(
+            "user", self.request.user).exclude(
+            vm_interface__instance__in=[inst])
+        val = super(VmAddInterfaceView, self).get_form_kwargs()
+        val.update({'choices': choices})
+        return val
+
+
 class VmCreateDiskView(FormOperationMixin, VmOperationView):
 
     op = 'create_disk'
@@ -854,6 +857,7 @@ vm_ops = OrderedDict([
         op='destroy', icon='times', effect='danger')),
     ('create_disk', VmCreateDiskView),
     ('download_disk', VmDownloadDiskView),
+    ('add_interface', VmAddInterfaceView),
     ('renew', VmRenewView),
     ('resources_change', VmResourcesChangeView),
 ])

circle/vm/operations.py

@@ -94,12 +94,21 @@ class AddInterfaceOperation(InstanceOperation):
                     "the VM.")
     required_perms = ()
 
+    def rollback(self, net, activity):
+        with activity.sub_activity(
+            'destroying_net',
+                readable_name=ugettext_noop("destroy network (rollback)")):
+            net.destroy()
+            net.delete()
+
     def check_precond(self):
         super(AddInterfaceOperation, self).check_precond()
         if self.instance.status not in ['STOPPED', 'PENDING', 'RUNNING']:
             raise self.instance.WrongStateError(self.instance)
 
     def _operation(self, activity, user, system, vlan, managed=None):
+        if not vlan.has_level(user, 'user'):
+            raise PermissionDenied()
         if managed is None:
             managed = vlan.managed
 
@@ -107,8 +116,13 @@ class AddInterfaceOperation(InstanceOperation):
                                managed=managed, owner=user, vlan=vlan)
 
         if self.instance.is_running:
+            try:
                 with activity.sub_activity('attach_network'):
                     self.instance.attach_network(net)
+            except Exception as e:
+                if hasattr(e, 'libvirtError'):
+                    self.rollback(net, activity)
+                raise
             net.deploy()
 
         return net