Remove extra duplicate checking code

Change-Id: Idec921b67d87b822949b940d50c7efef139a9271
This commit is contained in:
Lei Lei 2015-02-11 16:35:59 -08:00
parent 1eed8ce893
commit 89201e8f12
6 changed files with 262 additions and 138 deletions

View File

@ -275,17 +275,6 @@ def update_cluster(cluster_id, user=None, session=None, **kwargs):
kwargs.get('reinstall_distributed_system', False) kwargs.get('reinstall_distributed_system', False)
) )
) )
if 'name' in kwargs:
clustername = kwargs['name']
cluster_by_name = utils.get_db_object(
session, models.Cluster, False, name=clustername
)
if cluster_by_name and cluster_by_name.id != cluster.id:
raise exception.InvalidParameter(
'cluster name %s is already exists in cluster %s' % (
clustername, cluster_by_name.id
)
)
return utils.update_db_object(session, cluster, **kwargs) return utils.update_db_object(session, cluster, **kwargs)
@ -587,17 +576,6 @@ def add_clusterhost_internal(
exception_when_not_editable=False exception_when_not_editable=False
) )
): ):
if 'name' in host_dict:
hostname = host_dict['name']
host_by_name = utils.get_db_object(
session, models.Host, False, name=hostname
)
if host_by_name and host_by_name.id != host.id:
raise exception.InvalidParameter(
'host name %s exists in host %s' % (
hostname, host_by_name.id
)
)
utils.update_db_object( utils.update_db_object(
session, host, session, host,
**host_dict **host_dict
@ -605,17 +583,6 @@ def add_clusterhost_internal(
else: else:
logging.info('host %s is not editable', host.name) logging.info('host %s is not editable', host.name)
else: else:
if 'name' in host_dict:
hostname = host_dict['name']
host = utils.get_db_object(
session, models.Host, False, name=hostname
)
if host and host.machine_id != machine_id:
raise exception.InvalidParameter(
'host name %s exists in host %s' % (
hostname, host.id
)
)
host = utils.add_db_object( host = utils.add_db_object(
session, models.Host, False, machine_id, session, models.Host, False, machine_id,
os=cluster.os, os=cluster.os,
@ -765,17 +732,6 @@ def _update_clusterhost(session, user, clusterhost, **kwargs):
reinstall_os_set=kwargs.get('reinstall_os', False), reinstall_os_set=kwargs.get('reinstall_os', False),
exception_when_not_editable=False exception_when_not_editable=False
): ):
if 'name' in host_dict:
hostname = host_dict['name']
host_by_name = utils.get_db_object(
session, models.Host, False, name=hostname
)
if host_by_name and host_by_name.id != host.id:
raise exception.InvalidParameter(
'host name %s exists in host %s' % (
hostname, host_by_name.id
)
)
utils.update_db_object( utils.update_db_object(
session, host, session, host,
**host_dict **host_dict
@ -824,7 +780,7 @@ def _update_clusterhost(session, user, clusterhost, **kwargs):
is_cluster_editable(session, clusterhost.cluster, user) is_cluster_editable(session, clusterhost.cluster, user)
return update_internal( return update_internal(
clusterhost, **kwargs clusterhost, **clusterhost_dict
) )

View File

@ -112,11 +112,15 @@ def session():
logging.error('failed to commit session') logging.error('failed to commit session')
logging.exception(error) logging.exception(error)
if isinstance(error, IntegrityError): if isinstance(error, IntegrityError):
raise exception.NotAcceptable( for item in error.statement.split():
'operation error in database' if item.islower():
object = item
break
raise exception.DuplicatedRecord(
'%s in %s' % (error.orig, object)
) )
elif isinstance(error, OperationalError): elif isinstance(error, OperationalError):
raise exception.DatabaseExcedption( raise exception.DatabaseException(
'operation error in database' 'operation error in database'
) )
elif isinstance(error, exception.DatabaseException): elif isinstance(error, exception.DatabaseException):

View File

@ -285,17 +285,6 @@ def _update_host(session, user, host_id, **kwargs):
session, host, user, session, host, user,
reinstall_os_set=kwargs.get('reinstall_os', False) reinstall_os_set=kwargs.get('reinstall_os', False)
) )
if 'name' in kwargs:
hostname = kwargs['name']
host_by_name = utils.get_db_object(
session, models.Host, False, name=hostname
)
if host_by_name and host_by_name.id != host.id:
raise exception.InvalidParameter(
'hostname %s is already exists in host %s' % (
hostname, host_by_name.id
)
)
return utils.update_db_object(session, host, **kwargs) return utils.update_db_object(session, host, **kwargs)
@ -597,22 +586,6 @@ def _add_host_network(
host = utils.get_db_object( host = utils.get_db_object(
session, models.Host, id=host_id session, models.Host, id=host_id
) )
ip_int = long(netaddr.IPAddress(ip))
host_network = utils.get_db_object(
session, models.HostNetwork, False,
ip_int=ip_int
)
if (
host_network and not (
host_network.host_id == host_id and
host_network.interface == interface
)
):
raise exception.InvalidParameter(
'ip %s exists in host network %s' % (
ip, host_network.id
)
)
is_host_editable(session, host, user) is_host_editable(session, host, user)
return utils.add_db_object( return utils.add_db_object(
session, models.HostNetwork, session, models.HostNetwork,
@ -653,14 +626,26 @@ def add_host_networks(
host_networks = [] host_networks = []
failed_host_networks = [] failed_host_networks = []
for network in networks: for network in networks:
try: ip_int = long(netaddr.IPAddress(network['ip']))
host_network = utils.get_db_object(
session, models.HostNetwork, False,
ip_int=ip_int
)
if (
host_network and not (
host_network.host_id == host_id and
host_network.interface == network['interface']
)
):
logging.error('ip %s exists in host network %s' % (
network['ip'], host_network.id
))
failed_host_networks.append(network)
else:
host_networks.append(_add_host_network( host_networks.append(_add_host_network(
session, user, host_id, exception_when_existing, session, user, host_id, exception_when_existing,
**network **network
)) ))
except exception.DatabaseException as error:
logging.exception(error)
failed_host_networks.append(network)
if host_networks: if host_networks:
hosts.append({'host_id': host_id, 'networks': host_networks}) hosts.append({'host_id': host_id, 'networks': host_networks})
if failed_host_networks: if failed_host_networks:
@ -677,35 +662,6 @@ def add_host_networks(
def _update_host_network( def _update_host_network(
session, user, host_network, **kwargs session, user, host_network, **kwargs
): ):
if 'interface' in kwargs:
interface = kwargs['interface']
host_network_by_interface = utils.get_db_object(
session, models.HostNetwork, False,
host_id=host_network.host_id,
interface=interface
)
if (
host_network_by_interface and
host_network_by_interface.id != host_network.id
):
raise exception.InvalidParameter(
'interface %s exists in host network %s' % (
interface, host_network_by_interface.id
)
)
if 'ip' in kwargs:
ip = kwargs['ip']
ip_int = long(netaddr.IPAddress(ip))
host_network_by_ip = utils.get_db_object(
session, models.HostNetwork, False,
ip_int=ip_int
)
if host_network_by_ip and host_network_by_ip.id != host_network.id:
raise exception.InvalidParameter(
'ip %s exist in host network %s' % (
ip, host_network_by_ip.id
)
)
is_host_editable(session, host_network.host, user) is_host_editable(session, host_network.host, user)
return utils.update_db_object(session, host_network, **kwargs) return utils.update_db_object(session, host_network, **kwargs)

View File

@ -379,6 +379,20 @@ class TestUpdateCluster(ClusterTestCase):
) )
self.assertEqual(update_cluster['name'], 'test_update_cluster') self.assertEqual(update_cluster['name'], 'test_update_cluster')
def test_duplicate_name(self):
cluster.update_cluster(
self.cluster_id,
user=self.user_object,
name='test_update_cluster'
)
self.assertRaises(
exception.DuplicatedRecord,
cluster.update_cluster,
2,
user=self.user_object,
name='test_update_cluster'
)
def test_is_cluster_editable(self): def test_is_cluster_editable(self):
# state is INSTALLING # state is INSTALLING
cluster.update_cluster_state( cluster.update_cluster_state(
@ -733,20 +747,30 @@ class TestAddClusterHost(ClusterTestCase):
) )
add_cluster_hosts = cluster.list_clusterhosts(user=self.user_object) add_cluster_hosts = cluster.list_clusterhosts(user=self.user_object)
expected = { expected = {
'clusterhost_id': 3,
'cluster_id': 1,
'id': 3,
'switch_ip': '172.29.8.40',
'hostname': 'test_add_cluster_host', 'hostname': 'test_add_cluster_host',
'owner': 'admin@huawei.com', 'owner': 'admin@huawei.com',
'mac': '00:0c:29:5b:ee:eb',
'host_id': 3,
'name': 'test_add_cluster_host.test_cluster1', 'name': 'test_add_cluster_host.test_cluster1',
} }
self.assertTrue( self.assertTrue(
all(item in add_cluster_hosts[2].items() all(item in add_cluster_hosts[2].items()
for item in expected.items())) for item in expected.items()))
def test_duplicate_name(self):
cluster.add_cluster_host(
self.cluster_id,
user=self.user_object,
machine_id=self.add_machine_id,
name='test_add_cluster_host'
)
self.assertRaises(
exception.DuplicatedRecord,
cluster.add_cluster_host,
2,
user=self.user_object,
machine_id=2,
name='test_add_cluster_host'
)
def test_is_cluster_editable(self): def test_is_cluster_editable(self):
# installing # installing
cluster.update_cluster_state( cluster.update_cluster_state(
@ -777,7 +801,7 @@ class TestUpdateClusterHost(ClusterTestCase):
self.cluster_id, self.cluster_id,
self.host_id[0], self.host_id[0],
user=self.user_object, user=self.user_object,
roles=['allinone-compute'] roles=['allinone-compute'],
) )
update_cluster_hosts = cluster.list_cluster_hosts( update_cluster_hosts = cluster.list_cluster_hosts(
self.cluster_id, self.cluster_id,
@ -789,6 +813,16 @@ class TestUpdateClusterHost(ClusterTestCase):
result = item['roles'][0]['display_name'] result = item['roles'][0]['display_name']
self.assertEqual(result, 'all in one compute') self.assertEqual(result, 'all in one compute')
def test_duplicate_name(self):
self.assertRaises(
exception.DuplicatedRecord,
cluster.update_cluster_host,
self.cluster_id,
self.host_id[0],
user=self.user_object,
name='newname2'
)
def test_invalid_role(self): def test_invalid_role(self):
self.assertRaises( self.assertRaises(
exception.InvalidParameter, exception.InvalidParameter,
@ -828,6 +862,7 @@ class TestUpdateClusterhost(ClusterTestCase):
cluster.update_clusterhost( cluster.update_clusterhost(
self.clusterhost_id[0], self.clusterhost_id[0],
user=self.user_object, user=self.user_object,
reinstall_os=False,
roles=['allinone-compute'] roles=['allinone-compute']
) )
update_clusterhosts = cluster.list_clusterhosts( update_clusterhosts = cluster.list_clusterhosts(
@ -839,6 +874,15 @@ class TestUpdateClusterhost(ClusterTestCase):
result = item['roles'][0]['display_name'] result = item['roles'][0]['display_name']
self.assertEqual(result, 'all in one compute') self.assertEqual(result, 'all in one compute')
def test_duplicate_name(self):
self.assertRaises(
exception.DuplicatedRecord,
cluster.update_clusterhost,
self.clusterhost_id[0],
user=self.user_object,
name='newname2'
)
def test_invalid_role(self): def test_invalid_role(self):
self.assertRaises( self.assertRaises(
exception.InvalidParameter, exception.InvalidParameter,

View File

@ -384,6 +384,20 @@ class TestUpdateHost(HostTestCase):
) )
self.assertEqual(update_host['name'], 'update_test_name') self.assertEqual(update_host['name'], 'update_test_name')
def test_duplicate_name(self):
host.update_host(
self.host_ids[0],
user=self.user_object,
name='update_test_name'
)
self.assertRaises(
exception.DuplicatedRecord,
host.update_host,
self.host_ids[1],
user=self.user_object,
name='update_test_name'
)
def test_is_host_etitable(self): def test_is_host_etitable(self):
host.update_host_state( host.update_host_state(
self.host_ids[0], self.host_ids[0],
@ -398,15 +412,6 @@ class TestUpdateHost(HostTestCase):
name='invalid' name='invalid'
) )
def test_invalid_parameter(self):
self.assertRaises(
exception.InvalidParameter,
host.update_host,
self.host_ids[1],
user=self.user_object,
name='newname1'
)
class TestUpdateHosts(HostTestCase): class TestUpdateHosts(HostTestCase):
"""Test update hosts.""" """Test update hosts."""
@ -437,6 +442,27 @@ class TestUpdateHosts(HostTestCase):
for result in results: for result in results:
self.assertIn(result, ['test_update1', 'test_update2']) self.assertIn(result, ['test_update1', 'test_update2'])
def test_duplicate_name(self):
host.update_hosts(
user=self.user_object,
data=[
{
'host_id': self.host_ids[0],
'name': 'test_update1'
}
])
self.assertRaises(
exception.DuplicatedRecord,
host.update_hosts,
user=self.user_object,
data=[
{
'host_id': self.host_ids[1],
'name': 'test_update1'
}
]
)
class TestDelHost(HostTestCase): class TestDelHost(HostTestCase):
"""Test delete host.""" """Test delete host."""
@ -839,7 +865,7 @@ class TestAddHostNetwork(HostTestCase):
host.add_host_network( host.add_host_network(
self.host_ids[0], self.host_ids[0],
user=self.user_object, user=self.user_object,
interface='eth1', interface='eth22',
ip='10.145.88.20', ip='10.145.88.20',
subnet_id=self.subnet_ids[0], subnet_id=self.subnet_ids[0],
is_mgmt=True is_mgmt=True
@ -853,6 +879,25 @@ class TestAddHostNetwork(HostTestCase):
result.append(item['ip']) result.append(item['ip'])
self.assertIn('10.145.88.20', result) self.assertIn('10.145.88.20', result)
def test_duplicate(self):
host.add_host_network(
self.host_ids[0],
user=self.user_object,
interface='eth1',
ip='10.145.88.20',
subnet_id=self.subnet_ids[0],
is_mgmt=True
)
self.assertRaises(
exception.DuplicatedRecord,
host.add_host_network,
self.host_ids[0],
user=self.user_object,
interface='eth3',
subnet_id=self.subnet_ids[0],
ip='10.145.88.20'
)
def test_add_host_network_position(self): def test_add_host_network_position(self):
host.add_host_network( host.add_host_network(
self.host_ids[0], self.host_ids[0],
@ -892,17 +937,6 @@ class TestAddHostNetwork(HostTestCase):
result.append(item['ip']) result.append(item['ip'])
self.assertIn('10.145.88.40', result) self.assertIn('10.145.88.40', result)
def test_invalid_parameter(self):
self.assertRaises(
exception.InvalidParameter,
host.add_host_network,
self.host_ids[0],
user=self.user_object,
interface='eth3',
ip='10.145.88.0',
subnet_id=self.subnet_ids[0]
)
class TestAddHostNetworks(HostTestCase): class TestAddHostNetworks(HostTestCase):
"""Test add host networks.""" """Test add host networks."""
@ -977,6 +1011,42 @@ class TestUpdateHostNetwork(HostTestCase):
self.assertEqual(interface, 'eth10') self.assertEqual(interface, 'eth10')
self.assertEqual(ip, '10.145.88.100') self.assertEqual(ip, '10.145.88.100')
def test_duplicate_ip(self):
host.add_host_network(
self.host_ids[0],
user=self.user_object,
interface='eth2',
ip='10.145.88.10',
subnet_id=self.subnet_ids[0],
is_mgmt=True
)
self.assertRaises(
exception.DuplicatedRecord,
host.update_host_network,
self.host_ids[0],
3,
user=self.user_object,
ip='10.145.88.0'
)
def test_duplicate_interface(self):
host.add_host_network(
self.host_ids[0],
interface='eth1',
user=self.user_object,
ip='10.145.88.10',
subnet_id=self.subnet_ids[0],
is_mgmt=True
)
self.assertRaises(
exception.DuplicatedRecord,
host.update_host_network,
self.host_ids[0],
3,
user=self.user_object,
interface='eth0'
)
def test_record_not_exists(self): def test_record_not_exists(self):
self.assertRaises( self.assertRaises(
exception.RecordNotExists, exception.RecordNotExists,
@ -1025,14 +1095,14 @@ class TestUpdateHostnetwork(HostTestCase):
is_promiscuous=True is_promiscuous=True
) )
self.assertRaises( self.assertRaises(
exception.InvalidParameter, exception.DuplicatedRecord,
host.update_hostnetwork, host.update_hostnetwork,
self.host_ids[0], self.host_ids[0],
user=self.user_object, user=self.user_object,
interface='eth11' interface='eth11'
) )
self.assertRaises( self.assertRaises(
exception.InvalidParameter, exception.DuplicatedRecord,
host.update_hostnetwork, host.update_hostnetwork,
self.host_ids[0], self.host_ids[0],
user=self.user_object, user=self.user_object,

View File

@ -14,6 +14,7 @@
import datetime import datetime
import logging import logging
import mock
import os import os
import unittest2 import unittest2
@ -165,6 +166,99 @@ class TestDelMachine(BaseTest):
self.assertEqual([], del_machine) self.assertEqual([], del_machine)
class TestPoweronMachine(BaseTest):
"""Test poweron machine."""
def setUp(self):
super(TestPoweronMachine, self).setUp()
def tearDown(self):
super(TestPoweronMachine, self).tearDown()
def test_poweron_machine(self):
switch.add_switch_machine(
1,
mac='28:6e:d4:46:c4:25',
port='1',
user=self.user_object,
)
from compass.tasks import client as celery_client
celery_client.celery.send_task = mock.Mock()
poweron_machine = machine.poweron_machine(
1,
poweron={'poweron': True},
user=self.user_object
)
expected = {
'status': 'poweron 28:6e:d4:46:c4:25 action sent',
}
self.assertTrue(all(
item in poweron_machine.items() for item in expected.items())
)
class TestPoweroffMachine(BaseTest):
"""Test poweroff machine."""
def setUp(self):
super(TestPoweroffMachine, self).setUp()
def tearDown(self):
super(TestPoweroffMachine, self).tearDown()
def test_poweroff_machine(self):
switch.add_switch_machine(
1,
mac='28:6e:d4:46:c4:25',
port='1',
user=self.user_object,
)
from compass.tasks import client as celery_client
celery_client.celery.send_task = mock.Mock()
poweroff_machine = machine.poweroff_machine(
1,
{'poweroff': True},
user=self.user_object
)
expected = {
'status': 'poweroff 28:6e:d4:46:c4:25 action sent'
}
self.assertTrue(all(
item in poweroff_machine.items() for item in expected.items())
)
class TestResetMachine(BaseTest):
"""Test reset machine."""
def setUp(self):
super(TestResetMachine, self).setUp()
def tearDown(self):
super(TestResetMachine, self).tearDown()
def test_reset_machine(self):
switch.add_switch_machine(
1,
mac='28:6e:d4:46:c4:25',
port='1',
user=self.user_object,
)
from compass.tasks import client as celery_client
celery_client.celery.send_task = mock.Mock()
reset_machine = machine.reset_machine(
1,
{'reset_machine': True},
user=self.user_object
)
expected = {
'status': 'reset 28:6e:d4:46:c4:25 action sent'
}
self.assertTrue(all(
item in reset_machine.items() for item in expected.items())
)
if __name__ == '__main__': if __name__ == '__main__':
flags.init() flags.init()
logsetting.init() logsetting.init()