From 63073104665ee4597cf3b7aa8dc2295a8a7db794 Mon Sep 17 00:00:00 2001
From: Alessio Ababilov <aababilov@griddynamics.com>
Date: Sat, 22 Dec 2012 20:12:20 +0400
Subject: [PATCH] Unify Manager._update behaviour
Now _update call usually returns an instance of self.resource_class.
This simplifies the code and makes novaclient closer to keystoneclient.
Also, update hosts and services API according to changes on nova.
(If50a6b6e20f9b3fe66d486bb9b15d3eb4b62daf9).
Change-Id: I447e49e5fce0afba8a9c1a5df6dfa7200cc93e18
---
novaclient/base.py | 14 +++----
novaclient/v1_1/agents.py | 2 +-
novaclient/v1_1/aggregates.py | 6 +--
novaclient/v1_1/floating_ip_dns.py | 13 ++++---
novaclient/v1_1/hosts.py | 5 +--
novaclient/v1_1/quota_classes.py | 6 ++-
novaclient/v1_1/quotas.py | 4 +-
novaclient/v1_1/servers.py | 2 +-
novaclient/v1_1/services.py | 31 ++++++++-------
novaclient/v1_1/shell.py | 2 +-
tests/v1_1/fakes.py | 60 +++++++++++++++---------------
tests/v1_1/test_agents.py | 2 +-
tests/v1_1/test_hosts.py | 12 +++---
tests/v1_1/test_services.py | 14 +++----
tests/v1_1/test_shell.py | 20 +++++-----
15 files changed, 99 insertions(+), 94 deletions(-)
diff --git a/novaclient/base.py b/novaclient/base.py
index c5aa4a769..351399a78 100644
--- a/novaclient/base.py
+++ b/novaclient/base.py
@@ -136,12 +136,9 @@ class Manager(utils.HookableMixin):
if cache:
cache.write("%s\n" % val)
- def _get(self, url, response_key=None):
+ def _get(self, url, response_key):
_resp, body = self.api.client.get(url)
- if response_key:
- return self.resource_class(self, body[response_key], loaded=True)
- else:
- return self.resource_class(self, body, loaded=True)
+ return self.resource_class(self, body[response_key], loaded=True)
def _create(self, url, body, response_key, return_raw=False, **kwargs):
self.run_hooks('modify_body_for_create', body, **kwargs)
@@ -156,10 +153,13 @@ class Manager(utils.HookableMixin):
def _delete(self, url):
_resp, _body = self.api.client.delete(url)
- def _update(self, url, body, **kwargs):
+ def _update(self, url, body, response_key=None, **kwargs):
self.run_hooks('modify_body_for_update', body, **kwargs)
_resp, body = self.api.client.put(url, body=body)
- return body
+ if response_key:
+ return self.resource_class(self, body[response_key])
+ else:
+ return body
class ManagerWithFind(Manager):
diff --git a/novaclient/v1_1/agents.py b/novaclient/v1_1/agents.py
index 8552119d4..17e3cad31 100644
--- a/novaclient/v1_1/agents.py
+++ b/novaclient/v1_1/agents.py
@@ -49,7 +49,7 @@ class AgentsManager(base.ManagerWithFind):
'version': version,
'url': url,
'md5hash': md5hash}}
- return self._update('/os-agents/%s' % id, body)
+ return self._update('/os-agents/%s' % id, body, 'agent')
def create(self, os, architecture, version,
url, md5hash, hypervisor):
diff --git a/novaclient/v1_1/aggregates.py b/novaclient/v1_1/aggregates.py
index f55f486bc..2a57703f8 100644
--- a/novaclient/v1_1/aggregates.py
+++ b/novaclient/v1_1/aggregates.py
@@ -62,9 +62,9 @@ class AggregateManager(base.ManagerWithFind):
def update(self, aggregate, values):
"""Update the name and/or availability zone."""
body = {'aggregate': values}
- result = self._update("/os-aggregates/%s" % base.getid(aggregate),
- body)
- return self.resource_class(self, result["aggregate"])
+ return self._update("/os-aggregates/%s" % base.getid(aggregate),
+ body,
+ "aggregate")
def add_host(self, aggregate, host):
"""Add a host into the Host Aggregate."""
diff --git a/novaclient/v1_1/floating_ip_dns.py b/novaclient/v1_1/floating_ip_dns.py
index c8bd563fd..1c00bdeb1 100644
--- a/novaclient/v1_1/floating_ip_dns.py
+++ b/novaclient/v1_1/floating_ip_dns.py
@@ -59,9 +59,9 @@ class FloatingIPDNSDomainManager(base.ManagerWithFind):
body = {'domain_entry':
{'scope': 'private',
'availability_zone': availability_zone}}
-
return self._update('/os-floating-ip-dns/%s' % _quote_domain(fqdomain),
- body)
+ body,
+ 'domain_entry')
def create_public(self, fqdomain, project):
"""Add or modify a public DNS domain."""
@@ -70,7 +70,8 @@ class FloatingIPDNSDomainManager(base.ManagerWithFind):
'project': project}}
return self._update('/os-floating-ip-dns/%s' % _quote_domain(fqdomain),
- body)
+ body,
+ 'domain_entry')
def delete(self, fqdomain):
"""Delete the specified domain"""
@@ -115,7 +116,8 @@ class FloatingIPDNSEntryManager(base.ManagerWithFind):
return self._update("/os-floating-ip-dns/%s/entries/%s" %
(_quote_domain(domain), name),
- body)
+ body,
+ "dns_entry")
def modify_ip(self, domain, name, ip):
"""Add a new DNS entry."""
@@ -125,7 +127,8 @@ class FloatingIPDNSEntryManager(base.ManagerWithFind):
return self._update("/os-floating-ip-dns/%s/entries/%s" %
(_quote_domain(domain), name),
- body)
+ body,
+ "dns_entry")
def delete(self, domain, name):
"""Delete entry specified by name and domain."""
diff --git a/novaclient/v1_1/hosts.py b/novaclient/v1_1/hosts.py
index b2377800d..a6f5a04a0 100644
--- a/novaclient/v1_1/hosts.py
+++ b/novaclient/v1_1/hosts.py
@@ -54,13 +54,12 @@ class HostManager(base.ManagerWithFind):
def update(self, host, values):
"""Update status or maintenance mode for the host."""
- result = self._update("/os-hosts/%s" % host, values)
- return self.resource_class(self, result)
+ return self._update("/os-hosts/%s" % host, {"host": values}, "host")
def host_action(self, host, action):
"""Performs an action on a host."""
url = "/os-hosts/%s/%s" % (host, action)
- return self._get(url)
+ return self._create(url, None, "host")
def list_all(self, zone=None):
url = '/os-hosts'
diff --git a/novaclient/v1_1/quota_classes.py b/novaclient/v1_1/quota_classes.py
index 3b52d64f4..066428461 100644
--- a/novaclient/v1_1/quota_classes.py
+++ b/novaclient/v1_1/quota_classes.py
@@ -25,7 +25,7 @@ class QuotaClassSet(base.Resource):
return self.class_name
def update(self, *args, **kwargs):
- self.manager.update(self.class_name, *args, **kwargs)
+ return self.manager.update(self.class_name, *args, **kwargs)
class QuotaClassSetManager(base.ManagerWithFind):
@@ -62,4 +62,6 @@ class QuotaClassSetManager(base.ManagerWithFind):
if body['quota_class_set'][key] is None:
body['quota_class_set'].pop(key)
- self._update('/os-quota-class-sets/%s' % (class_name), body)
+ return self._update('/os-quota-class-sets/%s' % (class_name),
+ body,
+ 'quota_class_set')
diff --git a/novaclient/v1_1/quotas.py b/novaclient/v1_1/quotas.py
index cc2556384..c63cb4694 100644
--- a/novaclient/v1_1/quotas.py
+++ b/novaclient/v1_1/quotas.py
@@ -25,7 +25,7 @@ class QuotaSet(base.Resource):
return self.tenant_id
def update(self, *args, **kwargs):
- self.manager.update(self.tenant_id, *args, **kwargs)
+ return self.manager.update(self.tenant_id, *args, **kwargs)
class QuotaSetManager(base.ManagerWithFind):
@@ -63,7 +63,7 @@ class QuotaSetManager(base.ManagerWithFind):
if body['quota_set'][key] is None:
body['quota_set'].pop(key)
- self._update('/os-quota-sets/%s' % (tenant_id), body)
+ return self._update('/os-quota-sets/%s' % tenant_id, body, 'quota_set')
def defaults(self, tenant_id):
return self._get('/os-quota-sets/%s/defaults' % tenant_id,
diff --git a/novaclient/v1_1/servers.py b/novaclient/v1_1/servers.py
index 9ea2cb008..3aa4ec967 100644
--- a/novaclient/v1_1/servers.py
+++ b/novaclient/v1_1/servers.py
@@ -525,7 +525,7 @@ class ServerManager(local_base.BootingManagerWithFind):
},
}
- self._update("/servers/%s" % base.getid(server), body)
+ return self._update("/servers/%s" % base.getid(server), body, "server")
def change_password(self, server, password):
"""
diff --git a/novaclient/v1_1/services.py b/novaclient/v1_1/services.py
index 8f54a6727..7aa26f7bf 100644
--- a/novaclient/v1_1/services.py
+++ b/novaclient/v1_1/services.py
@@ -34,29 +34,28 @@ class Service(base.Resource):
class ServiceManager(base.ManagerWithFind):
resource_class = Service
- def list(self, host=None, service=None):
+ def list(self, host=None, binary=None):
"""
Describes cpu/memory/hdd info for host.
:param host: destination host name.
"""
url = "/os-services"
+ filters = []
if host:
- url = "/os-services?host=%s" % host
- if service:
- url = "/os-services?service=%s" % service
- if host and service:
- url = "/os-services?host=%s&service=%s" % (host, service)
+ filters.append("host=%s" % host)
+ if binary:
+ filters.append("binary=%s" % binary)
+ if filters:
+ url = "%s?%s" % (url, "&".join(filters))
return self._list(url, "services")
- def enable(self, host, service):
- """Enable the service specified by hostname and servicename"""
- body = {"host": host, "service": service}
- result = self._update("/os-services/enable", body)
- return self.resource_class(self, result)
+ def enable(self, host, binary):
+ """Enable the service specified by hostname and binary"""
+ body = {"host": host, "binary": binary}
+ return self._update("/os-services/enable", body, "service")
- def disable(self, host, service):
- """Enable the service specified by hostname and servicename"""
- body = {"host": host, "service": service}
- result = self._update("/os-services/disable", body)
- return self.resource_class(self, result)
+ def disable(self, host, binary):
+ """Enable the service specified by hostname and binary"""
+ body = {"host": host, "binary": binary}
+ return self._update("/os-services/disable", body, "service")
diff --git a/novaclient/v1_1/shell.py b/novaclient/v1_1/shell.py
index b3fe46b1a..133785918 100644
--- a/novaclient/v1_1/shell.py
+++ b/novaclient/v1_1/shell.py
@@ -2077,7 +2077,7 @@ def do_agent_modify(cs, args):
"""Modify an existing agent build."""
result = cs.agents.update(args.id, args.version,
args.url, args.md5hash)
- utils.print_dict(result['agent'])
+ utils.print_dict(result._info)
def do_aggregate_list(cs, args):
diff --git a/tests/v1_1/fakes.py b/tests/v1_1/fakes.py
index 545701ba9..4dd17f6e7 100644
--- a/tests/v1_1/fakes.py
+++ b/tests/v1_1/fakes.py
@@ -363,7 +363,7 @@ class FakeHTTPClient(base_client.HTTPClient):
def put_servers_1234(self, body, **kw):
assert body.keys() == ['server']
fakes.assert_has_keys(body['server'], optional=['name', 'adminPass'])
- return (204, {}, None)
+ return (204, {}, body)
def delete_servers_1234(self, **kw):
return (202, {}, None)
@@ -711,12 +711,12 @@ class FakeHTTPClient(base_client.HTTPClient):
else:
fakes.assert_has_keys(body['domain_entry'],
required=['project', 'scope'])
- return (205, {}, None)
+ return (205, {}, body)
def put_os_floating_ip_dns_testdomain_entries_testname(self, body, **kw):
fakes.assert_has_keys(body['dns_entry'],
required=['ip', 'dns_type'])
- return (205, {}, None)
+ return (205, {}, body)
def delete_os_floating_ip_dns_testdomain(self, **kw):
return (200, {}, None)
@@ -1080,7 +1080,7 @@ class FakeHTTPClient(base_client.HTTPClient):
#
def get_os_services(self, **kw):
host = kw.get('host', 'host1')
- service = kw.get('service', 'nova-compute')
+ service = kw.get('binary', 'nova-compute')
return (200, {}, {'services':
[{'binary': service,
'host': host,
@@ -1097,12 +1097,14 @@ class FakeHTTPClient(base_client.HTTPClient):
]})
def put_os_services_enable(self, body, **kw):
- return (200, {}, {'host': body['host'], 'service': body['service'],
- 'disabled': False})
+ return (200, {}, {'service': {'host': body['host'],
+ 'binary': body['binary'],
+ 'disabled': False}})
def put_os_services_disable(self, body, **kw):
- return (200, {}, {'host': body['host'], 'service': body['service'],
- 'disabled': True})
+ return (200, {}, {'service': {'host': body['host'],
+ 'binary': body['binary'],
+ 'disabled': True}})
#
# Fixed IPs
@@ -1134,10 +1136,10 @@ class FakeHTTPClient(base_client.HTTPClient):
def get_os_hosts(self, **kw):
zone = kw.get('zone', 'nova1')
return (200, {}, {'hosts':
- [{'host': 'host1',
+ [{'host_name': 'host1',
'service': 'nova-compute',
'zone': zone},
- {'host': 'host1',
+ {'host_name': 'host1',
'service': 'nova-cert',
'zone': zone}]})
@@ -1145,34 +1147,34 @@ class FakeHTTPClient(base_client.HTTPClient):
return (200, {}, {'host': [{'resource': {'host': 'sample_host'}}], })
def put_os_hosts_sample_host_1(self, body, **kw):
- return (200, {}, {'host': 'sample-host_1',
- 'status': 'enabled'})
+ return (200, {}, {'host': {'host_name': 'sample-host_1',
+ 'status': 'enabled'}})
def put_os_hosts_sample_host_2(self, body, **kw):
- return (200, {}, {'host': 'sample-host_2',
- 'maintenance_mode': 'on_maintenance'})
+ return (200, {}, {'host': {'host_name': 'sample-host_2',
+ 'maintenance_mode': 'on_maintenance'}})
def put_os_hosts_sample_host_3(self, body, **kw):
- return (200, {}, {'host': 'sample-host_3',
- 'status': 'enabled',
- 'maintenance_mode': 'on_maintenance'})
+ return (200, {}, {'host': {'host_name': 'sample-host_3',
+ 'status': 'enabled',
+ 'maintenance_mode': 'on_maintenance'}})
- def get_os_hosts_sample_host_startup(self, **kw):
- return (200, {}, {'host': 'sample_host',
- 'power_action': 'startup'})
+ def post_os_hosts_sample_host_startup(self, **kw):
+ return (200, {}, {'host': {'host_name': 'sample_host',
+ 'power_action': 'startup'}})
- def get_os_hosts_sample_host_reboot(self, **kw):
- return (200, {}, {'host': 'sample_host',
- 'power_action': 'reboot'})
+ def post_os_hosts_sample_host_reboot(self, **kw):
+ return (200, {}, {'host': {'host_name': 'sample_host',
+ 'power_action': 'reboot'}})
- def get_os_hosts_sample_host_shutdown(self, **kw):
- return (200, {}, {'host': 'sample_host',
- 'power_action': 'shutdown'})
+ def post_os_hosts_sample_host_shutdown(self, **kw):
+ return (200, {}, {'host': {'host_name': 'sample_host',
+ 'power_action': 'shutdown'}})
def put_os_hosts_sample_host(self, body, **kw):
- result = {'host': 'dummy'}
- result.update(body)
- return (200, {}, result)
+ result = {'host_name': 'dummy'}
+ result.update(body['host'])
+ return (200, {}, {'host': result})
def get_os_hypervisors(self, **kw):
return (200, {}, {"hypervisors": [
diff --git a/tests/v1_1/test_agents.py b/tests/v1_1/test_agents.py
index 97d537994..14ddcf7fc 100644
--- a/tests/v1_1/test_agents.py
+++ b/tests/v1_1/test_agents.py
@@ -65,4 +65,4 @@ class AgentsTest(utils.TestCase):
"version": "8.0",
"md5hash": "add6bb58e139be103324d04d82d8f546"}}
cs.assert_called('PUT', '/os-agents/1', body)
- self.assertEqual(1, ag['agent']['id'])
+ self.assertEqual(1, ag.id)
diff --git a/tests/v1_1/test_hosts.py b/tests/v1_1/test_hosts.py
index 7daa135cb..57c2891a8 100644
--- a/tests/v1_1/test_hosts.py
+++ b/tests/v1_1/test_hosts.py
@@ -29,14 +29,14 @@ class HostsTest(utils.TestCase):
host = cs.hosts.get('sample_host')[0]
values = {"status": "enabled"}
result = host.update(values)
- cs.assert_called('PUT', '/os-hosts/sample_host', values)
+ cs.assert_called('PUT', '/os-hosts/sample_host', {'host': values})
self.assertTrue(isinstance(result, hosts.Host))
def test_update_maintenance(self):
host = cs.hosts.get('sample_host')[0]
values = {"maintenance_mode": "enable"}
result = host.update(values)
- cs.assert_called('PUT', '/os-hosts/sample_host', values)
+ cs.assert_called('PUT', '/os-hosts/sample_host', {'host': values})
self.assertTrue(isinstance(result, hosts.Host))
def test_update_both(self):
@@ -44,23 +44,23 @@ class HostsTest(utils.TestCase):
values = {"status": "enabled",
"maintenance_mode": "enable"}
result = host.update(values)
- cs.assert_called('PUT', '/os-hosts/sample_host', values)
+ cs.assert_called('PUT', '/os-hosts/sample_host', {'host': values})
self.assertTrue(isinstance(result, hosts.Host))
def test_host_startup(self):
host = cs.hosts.get('sample_host')[0]
result = host.startup()
- cs.assert_called('GET', '/os-hosts/sample_host/startup')
+ cs.assert_called('POST', '/os-hosts/sample_host/startup')
self.assertTrue(isinstance(result, hosts.Host))
def test_host_reboot(self):
host = cs.hosts.get('sample_host')[0]
result = host.reboot()
- cs.assert_called('GET', '/os-hosts/sample_host/reboot')
+ cs.assert_called('POST', '/os-hosts/sample_host/reboot')
self.assertTrue(isinstance(result, hosts.Host))
def test_host_shutdown(self):
host = cs.hosts.get('sample_host')[0]
result = host.shutdown()
- cs.assert_called('GET', '/os-hosts/sample_host/shutdown')
+ cs.assert_called('POST', '/os-hosts/sample_host/shutdown')
self.assertTrue(isinstance(result, hosts.Host))
diff --git a/tests/v1_1/test_services.py b/tests/v1_1/test_services.py
index 4f78c7048..d6d58837e 100644
--- a/tests/v1_1/test_services.py
+++ b/tests/v1_1/test_services.py
@@ -39,30 +39,30 @@ class ServicesTest(utils.TestCase):
[self.assertEqual(s.binary, 'nova-compute') for s in svs]
[self.assertEqual(s.host, 'host2') for s in svs]
- def test_list_services_with_service(self):
- svs = cs.services.list(service='nova-cert')
- cs.assert_called('GET', '/os-services?service=nova-cert')
+ def test_list_services_with_binary(self):
+ svs = cs.services.list(binary='nova-cert')
+ cs.assert_called('GET', '/os-services?binary=nova-cert')
[self.assertTrue(isinstance(s, services.Service)) for s in svs]
[self.assertEqual(s.binary, 'nova-cert') for s in svs]
[self.assertEqual(s.host, 'host1') for s in svs]
- def test_list_services_with_host_service(self):
+ def test_list_services_with_host_binary(self):
svs = cs.services.list('host2', 'nova-cert')
- cs.assert_called('GET', '/os-services?host=host2&service=nova-cert')
+ cs.assert_called('GET', '/os-services?host=host2&binary=nova-cert')
[self.assertTrue(isinstance(s, services.Service)) for s in svs]
[self.assertEqual(s.binary, 'nova-cert') for s in svs]
[self.assertEqual(s.host, 'host2') for s in svs]
def test_services_enable(self):
service = cs.services.enable('host1', 'nova-cert')
- values = {"host": "host1", 'service': 'nova-cert'}
+ values = {"host": "host1", 'binary': 'nova-cert'}
cs.assert_called('PUT', '/os-services/enable', values)
self.assertTrue(isinstance(service, services.Service))
self.assertFalse(service.disabled)
def test_services_disable(self):
service = cs.services.disable('host1', 'nova-cert')
- values = {"host": "host1", 'service': 'nova-cert'}
+ values = {"host": "host1", 'binary': 'nova-cert'}
cs.assert_called('PUT', '/os-services/disable', values)
self.assertTrue(isinstance(service, services.Service))
self.assertTrue(service.disabled)
diff --git a/tests/v1_1/test_shell.py b/tests/v1_1/test_shell.py
index de85a3e57..6565f1317 100644
--- a/tests/v1_1/test_shell.py
+++ b/tests/v1_1/test_shell.py
@@ -578,20 +578,20 @@ class ShellTest(utils.TestCase):
def test_services_list_with_servicename(self):
self.run_command('service-list --servicename nova-cert')
- self.assert_called('GET', '/os-services?service=nova-cert')
+ self.assert_called('GET', '/os-services?binary=nova-cert')
def test_services_list_with_host_servicename(self):
self.run_command('service-list --host host1 --servicename nova-cert')
- self.assert_called('GET', '/os-services?host=host1&service=nova-cert')
+ self.assert_called('GET', '/os-services?host=host1&binary=nova-cert')
def test_services_enable(self):
self.run_command('service-enable host1 nova-cert')
- body = {'host': 'host1', 'service': 'nova-cert'}
+ body = {'host': 'host1', 'binary': 'nova-cert'}
self.assert_called('PUT', '/os-services/enable', body)
def test_services_disable(self):
self.run_command('service-disable host1 nova-cert')
- body = {'host': 'host1', 'service': 'nova-cert'}
+ body = {'host': 'host1', 'binary': 'nova-cert'}
self.assert_called('PUT', '/os-services/disable', body)
def test_fixed_ips_get(self):
@@ -618,31 +618,31 @@ class ShellTest(utils.TestCase):
def test_host_update_status(self):
self.run_command('host-update sample-host_1 --status enabled')
- body = {'status': 'enabled'}
+ body = {'host': {'status': 'enabled'}}
self.assert_called('PUT', '/os-hosts/sample-host_1', body)
def test_host_update_maintenance(self):
self.run_command('host-update sample-host_2 --maintenance enable')
- body = {'maintenance_mode': 'enable'}
+ body = {'host': {'maintenance_mode': 'enable'}}
self.assert_called('PUT', '/os-hosts/sample-host_2', body)
def test_host_update_multiple_settings(self):
self.run_command('host-update sample-host_3 '
'--status disabled --maintenance enable')
- body = {'status': 'disabled', 'maintenance_mode': 'enable'}
+ body = {'host': {'status': 'disabled', 'maintenance_mode': 'enable'}}
self.assert_called('PUT', '/os-hosts/sample-host_3', body)
def test_host_startup(self):
self.run_command('host-action sample-host --action startup')
- self.assert_called('GET', '/os-hosts/sample-host/startup')
+ self.assert_called('POST', '/os-hosts/sample-host/startup')
def test_host_shutdown(self):
self.run_command('host-action sample-host --action shutdown')
- self.assert_called('GET', '/os-hosts/sample-host/shutdown')
+ self.assert_called('POST', '/os-hosts/sample-host/shutdown')
def test_host_reboot(self):
self.run_command('host-action sample-host --action reboot')
- self.assert_called('GET', '/os-hosts/sample-host/reboot')
+ self.assert_called('POST', '/os-hosts/sample-host/reboot')
def test_coverage_start(self):
self.run_command('coverage-start')