launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13065
[Merge] lp:~rvb/maas/bug-1062607-url into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/bug-1062607-url into lp:maas.
Commit message:
This branch fixes the urls of the cluster and the cluster interface pages. Instead of using the nodegroup's uuid and the interface's name (interface.interface field), use 'id' everywhere.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1062607 in MAAS: "Broken link in edit cluster controller page"
https://bugs.launchpad.net/maas/+bug/1062607
For more details, see:
https://code.launchpad.net/~rvb/maas/bug-1062607-url/+merge/128353
This branch fixes the urls of the cluster and the cluster interface pages. Instead of using the nodegroup's uuid and the interface's name (interface.interface field), use 'id' everywhere. The reasoning behind that change is that nodegroup.uuid can change (only in one case: the master nodegroup starts with a uuid of 'master' and then that gets changed to a real uuid when the master cluster connects to id) and so does interface.interface. That's why they should not be used in the urls.
Drive-by fixes:
- add a mention on the cluster delete page: "all the nodes in this cluster will be deleted" (!)
- fix the case where a cluster containing nodes is deleted.
- removed an used empty form on nodegroup_edit.html (the result of a copy and paste probably)
--
https://code.launchpad.net/~rvb/maas/bug-1062607-url/+merge/128353
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/bug-1062607-url into lp:maas.
=== modified file 'src/maasserver/dns_connect.py'
--- src/maasserver/dns_connect.py 2012-09-18 16:36:51 +0000
+++ src/maasserver/dns_connect.py 2012-10-06 13:40:27 +0000
@@ -60,8 +60,14 @@
@receiver(post_delete, sender=Node)
def dns_post_delete_Node(sender, instance, **kwargs):
"""When a Node is deleted, update the Node's zone file."""
- from maasserver.dns import change_dns_zones
- change_dns_zones(instance.nodegroup)
+ try:
+ from maasserver.dns import change_dns_zones
+ change_dns_zones(instance.nodegroup)
+ except NodeGroup.DoesNotExist:
+ # If this Node is being deleted because the whole NodeGroup
+ # has been deleted, no need to update the zone file because
+ # this Node got removed.
+ pass
def dns_post_edit_hostname_Node(instance, old_field):
=== modified file 'src/maasserver/templates/maasserver/nodegroup_confirm_delete.html'
--- src/maasserver/templates/maasserver/nodegroup_confirm_delete.html 2012-10-03 07:54:16 +0000
+++ src/maasserver/templates/maasserver/nodegroup_confirm_delete.html 2012-10-06 13:40:27 +0000
@@ -8,7 +8,8 @@
<div class="block auto-width">
<h2>
Are you sure you want to delete the cluster controller
- "{{ cluster_to_delete.cluster_name }}"?
+ "{{ cluster_to_delete.cluster_name }}"? Note that this action
+ will also delete all the nodes inside this cluster controller.
</h2>
<p>This action is permanent and can not be undone.</p>
<p>
=== modified file 'src/maasserver/templates/maasserver/nodegroup_edit.html'
--- src/maasserver/templates/maasserver/nodegroup_edit.html 2012-10-03 07:54:16 +0000
+++ src/maasserver/templates/maasserver/nodegroup_edit.html 2012-10-06 13:40:27 +0000
@@ -38,22 +38,16 @@
<td>{{ interface.network|default_if_none:"Not configured" }}</td>
<td>{{ interface.display_management }}</td>
<td>
- <a href="{% url 'cluster-interface-edit' uuid=cluster.uuid interface=interface.interface %}"
+ <a href="{% url 'cluster-interface-edit' id=cluster.id interface_id=interface.id %}"
title="Edit interface {{ interface.interface }}">
<img src="{{ STATIC_URL }}img/edit.png"
alt="edit"
class="space-right-small" />
</a>
<a title="Delete interface {{ interface.interface }}"
- href="{% url 'cluster-interface-delete' cluster.uuid interface.interface %}">
+ href="{% url 'cluster-interface-delete' cluster.id interface.id %}">
<img src="{{ STATIC_URL }}img/delete.png" alt="delete" />
</a>
- <form method="POST"
- action="{% url 'settings' %}">
- {% csrf_token %}
- <input type="hidden" name="uuid"
- value="{{ interface.interface }}" />
- </form>
</td>
</tr>
{% endfor %}
=== modified file 'src/maasserver/templates/maasserver/nodegroupinterface_confirm_delete.html'
--- src/maasserver/templates/maasserver/nodegroupinterface_confirm_delete.html 2012-10-02 17:53:24 +0000
+++ src/maasserver/templates/maasserver/nodegroupinterface_confirm_delete.html 2012-10-06 13:40:27 +0000
@@ -15,7 +15,7 @@
<form action="." method="post">{% csrf_token %}
<input type="hidden" name="post" value="yes" />
<input type="submit" value="Delete interface" class="right" />
- <a href="{% url 'cluster-edit' interface_to_delete.nodegroup.uuid %}">Cancel</a>
+ <a href="{% url 'cluster-edit' interface_to_delete.nodegroup.id %}">Cancel</a>
</form>
</p>
</div>
=== modified file 'src/maasserver/templates/maasserver/nodegroupinterface_edit.html'
--- src/maasserver/templates/maasserver/nodegroupinterface_edit.html 2012-10-02 17:53:24 +0000
+++ src/maasserver/templates/maasserver/nodegroupinterface_edit.html 2012-10-06 13:40:27 +0000
@@ -15,7 +15,7 @@
</ul>
<input type="submit" value="Save interface" class="button right" />
<a class="link-button"
- href="{% url 'cluster-edit' interface.nodegroup.uuid %}">Cancel</a>
+ href="{% url 'cluster-edit' interface.nodegroup.id %}">Cancel</a>
</form>
</div>
<div class="clear"></div>
=== modified file 'src/maasserver/templates/maasserver/settings_cluster_listing_row.html'
--- src/maasserver/templates/maasserver/settings_cluster_listing_row.html 2012-10-04 07:43:24 +0000
+++ src/maasserver/templates/maasserver/settings_cluster_listing_row.html 2012-10-06 13:40:27 +0000
@@ -2,14 +2,14 @@
id="{{ cluster.cluster_name }}">
<td>{{ cluster.cluster_name }}</td>
<td>
- <a href="{% url 'cluster-edit' cluster.uuid %}"
+ <a href="{% url 'cluster-edit' cluster.id %}"
title="Edit cluster {{ cluster.cluster_name }}">
<img src="{{ STATIC_URL }}img/edit.png"
alt="edit"
class="space-right-small" />
</a>
<a title="Delete cluster {{ cluster.cluster_name }}"
- href="{% url 'cluster-delete' cluster.uuid %}">
+ href="{% url 'cluster-delete' cluster.id %}">
<img src="{{ STATIC_URL }}img/delete.png" alt="delete" />
</a>
</td>
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-10-04 07:55:27 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-10-06 13:40:27 +0000
@@ -258,6 +258,13 @@
def test_work_queue_returns_uuid(self):
nodegroup = factory.make_node_group()
+ factory.make_node(nodegroup=nodegroup)
+ nodegroup.delete()
+ self.assertEqual(nodegroup.uuid, nodegroup.work_queue)
+ self.assertFalse(NodeGroup.objects.filter(id=nodegroup.id).exists())
+
+ def test_delete_cluster(self):
+ nodegroup = factory.make_node_group()
self.assertEqual(nodegroup.uuid, nodegroup.work_queue)
def test_add_dhcp_host_maps_adds_maps_if_managing_dhcp(self):
=== modified file 'src/maasserver/tests/test_views_settings_clusters.py'
--- src/maasserver/tests/test_views_settings_clusters.py 2012-10-03 07:57:55 +0000
+++ src/maasserver/tests/test_views_settings_clusters.py 2012-10-06 13:40:27 +0000
@@ -29,9 +29,7 @@
reload_object,
)
from maasserver.testing.factory import factory
-from maasserver.testing.testcase import (
- AdminLoggedInTestCase,
- )
+from maasserver.testing.testcase import AdminLoggedInTestCase
from maastesting.matchers import ContainsAll
from testtools.matchers import MatchesStructure
@@ -46,10 +44,10 @@
}
links = get_content_links(self.client.get(reverse('settings')))
nodegroup_edit_links = [
- reverse('cluster-edit', args=[nodegroup.uuid])
+ reverse('cluster-edit', args=[nodegroup.id])
for nodegroup in nodegroups]
nodegroup_delete_links = [
- reverse('cluster-delete', args=[nodegroup.uuid])
+ reverse('cluster-delete', args=[nodegroup.id])
for nodegroup in nodegroups]
self.assertThat(
links,
@@ -60,13 +58,13 @@
def test_can_delete_cluster(self):
nodegroup = factory.make_node_group()
- delete_link = reverse('cluster-delete', args=[nodegroup.uuid])
+ delete_link = reverse('cluster-delete', args=[nodegroup.id])
response = self.client.post(delete_link, {'post': 'yes'})
self.assertEqual(
(httplib.FOUND, reverse('settings')),
(response.status_code, extract_redirect(response)))
self.assertFalse(
- NodeGroup.objects.filter(uuid=nodegroup.uuid).exists())
+ NodeGroup.objects.filter(id=nodegroup.id).exists())
class ClusterEditTest(AdminLoggedInTestCase):
@@ -80,14 +78,14 @@
management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
interfaces.add(interface)
links = get_content_links(
- self.client.get(reverse('cluster-edit', args=[nodegroup.uuid])))
+ self.client.get(reverse('cluster-edit', args=[nodegroup.id])))
interface_edit_links = [
reverse('cluster-interface-edit',
- args=[nodegroup.uuid, interface.interface])
+ args=[nodegroup.id, interface.id])
for interface in interfaces]
interface_delete_links = [
reverse('cluster-interface-delete',
- args=[nodegroup.uuid, interface.interface])
+ args=[nodegroup.id, interface.id])
for interface in interfaces]
self.assertThat(
links,
@@ -95,7 +93,7 @@
def test_can_edit_cluster(self):
nodegroup = factory.make_node_group()
- edit_link = reverse('cluster-edit', args=[nodegroup.uuid])
+ edit_link = reverse('cluster-edit', args=[nodegroup.id])
data = {
'cluster_name': factory.make_name('cluster_name'),
'name': factory.make_name('name'),
@@ -117,10 +115,10 @@
management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
delete_link = reverse(
'cluster-interface-delete',
- args=[nodegroup.uuid, interface.interface])
+ args=[nodegroup.id, interface.id])
response = self.client.post(delete_link, {'post': 'yes'})
self.assertEqual(
- (httplib.FOUND, reverse('cluster-edit', args=[nodegroup.uuid])),
+ (httplib.FOUND, reverse('cluster-edit', args=[nodegroup.id])),
(response.status_code, extract_redirect(response)))
self.assertFalse(
NodeGroupInterface.objects.filter(id=interface.id).exists())
@@ -135,7 +133,7 @@
nodegroup=nodegroup)
edit_link = reverse(
'cluster-interface-edit',
- args=[nodegroup.uuid, interface.interface])
+ args=[nodegroup.id, interface.id])
data = factory.get_interface_fields()
response = self.client.post(edit_link, data)
self.assertEqual(httplib.FOUND, response.status_code, response.content)
=== modified file 'src/maasserver/urls.py'
--- src/maasserver/urls.py 2012-10-03 07:47:24 +0000
+++ src/maasserver/urls.py 2012-10-06 13:40:27 +0000
@@ -134,17 +134,17 @@
# Settings views.
urlpatterns += patterns('maasserver.views',
adminurl(
- r'^clusters/(?P<uuid>[\w\-]+)/edit/$', ClusterEdit.as_view(),
+ r'^clusters/(?P<id>\d*)/edit/$', ClusterEdit.as_view(),
name='cluster-edit'),
adminurl(
- r'^clusters/(?P<uuid>[\w\-]+)/delete/$', ClusterDelete.as_view(),
+ r'^clusters/(?P<id>\d*)/delete/$', ClusterDelete.as_view(),
name='cluster-delete'),
adminurl(
- r'^clusters/(?P<uuid>[\w\-]+)/interfaces/(?P<interface>[\w\-]*)/'
+ r'^clusters/(?P<id>\d*)/interfaces/(?P<interface_id>\d*)/'
'edit/$',
ClusterInterfaceEdit.as_view(), name='cluster-interface-edit'),
adminurl(
- r'^clusters/(?P<uuid>[\w\-]+)/interfaces/(?P<interface>[\w\-]*)/'
+ r'^clusters/(?P<id>\d*)/interfaces/(?P<interface_id>\d*)/'
'delete/$',
ClusterInterfaceDelete.as_view(), name='cluster-interface-delete'),
adminurl(r'^settings/$', settings, name='settings'),
=== modified file 'src/maasserver/views/settings_clusters.py'
--- src/maasserver/views/settings_clusters.py 2012-10-02 22:02:11 +0000
+++ src/maasserver/views/settings_clusters.py 2012-10-06 13:40:27 +0000
@@ -51,8 +51,8 @@
return reverse('settings')
def get_object(self):
- uuid = self.kwargs.get('uuid', None)
- return get_object_or_404(NodeGroup, uuid=uuid)
+ id = self.kwargs.get('id', None)
+ return get_object_or_404(NodeGroup, id=id)
def form_valid(self, form):
messages.info(self.request, "Cluster updated.")
@@ -65,8 +65,8 @@
context_object_name = 'cluster_to_delete'
def get_object(self):
- uuid = self.kwargs.get('uuid', None)
- return get_object_or_404(NodeGroup, uuid=uuid)
+ id = self.kwargs.get('id', None)
+ return get_object_or_404(NodeGroup, id=id)
def get_next_url(self):
return reverse('settings')
@@ -83,14 +83,14 @@
context_object_name = 'interface_to_delete'
def get_object(self):
- uuid = self.kwargs.get('uuid', None)
- interface = self.kwargs.get('interface', None)
+ id = self.kwargs.get('id', None)
+ interface_id = self.kwargs.get('interface_id', None)
return get_object_or_404(
- NodeGroupInterface, nodegroup__uuid=uuid, interface=interface)
+ NodeGroupInterface, nodegroup__id=id, id=interface_id)
def get_next_url(self):
- uuid = self.kwargs.get('uuid', None)
- return reverse('cluster-edit', args=[uuid])
+ id = self.kwargs.get('id', None)
+ return reverse('cluster-edit', args=[id])
def delete(self, request, *args, **kwargs):
interface = self.get_object()
@@ -105,15 +105,15 @@
context_object_name = 'interface'
def get_success_url(self):
- uuid = self.kwargs.get('uuid', None)
- return reverse('cluster-edit', args=[uuid])
+ id = self.kwargs.get('id', None)
+ return reverse('cluster-edit', args=[id])
def form_valid(self, form):
messages.info(self.request, "Interface updated.")
return super(ClusterInterfaceEdit, self).form_valid(form)
def get_object(self):
- uuid = self.kwargs.get('uuid', None)
- interface = self.kwargs.get('interface', None)
+ id = self.kwargs.get('id', None)
+ interface_id = self.kwargs.get('interface_id', None)
return get_object_or_404(
- NodeGroupInterface, nodegroup__uuid=uuid, interface=interface)
+ NodeGroupInterface, nodegroup__id=id, id=interface_id)
