diff --git a/compass/db/api/cluster.py b/compass/db/api/cluster.py index 1690788b..000401f9 100644 --- a/compass/db/api/cluster.py +++ b/compass/db/api/cluster.py @@ -275,17 +275,6 @@ def update_cluster(cluster_id, user=None, session=None, **kwargs): 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) @@ -587,17 +576,6 @@ def add_clusterhost_internal( 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( session, host, **host_dict @@ -605,17 +583,6 @@ def add_clusterhost_internal( else: logging.info('host %s is not editable', host.name) 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( session, models.Host, False, machine_id, os=cluster.os, @@ -765,17 +732,6 @@ def _update_clusterhost(session, user, clusterhost, **kwargs): reinstall_os_set=kwargs.get('reinstall_os', 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( session, host, **host_dict @@ -824,7 +780,7 @@ def _update_clusterhost(session, user, clusterhost, **kwargs): is_cluster_editable(session, clusterhost.cluster, user) return update_internal( - clusterhost, **kwargs + clusterhost, **clusterhost_dict ) diff --git a/compass/db/api/database.py b/compass/db/api/database.py index a909e6fc..0e52477e 100644 --- a/compass/db/api/database.py +++ b/compass/db/api/database.py @@ -112,11 +112,15 @@ def session(): logging.error('failed to commit session') logging.exception(error) if isinstance(error, IntegrityError): - raise exception.NotAcceptable( - 'operation error in database' + for item in error.statement.split(): + if item.islower(): + object = item + break + raise exception.DuplicatedRecord( + '%s in %s' % (error.orig, object) ) elif isinstance(error, OperationalError): - raise exception.DatabaseExcedption( + raise exception.DatabaseException( 'operation error in database' ) elif isinstance(error, exception.DatabaseException): diff --git a/compass/db/api/host.py b/compass/db/api/host.py index 0cb93a03..18bb9a1d 100644 --- a/compass/db/api/host.py +++ b/compass/db/api/host.py @@ -285,17 +285,6 @@ def _update_host(session, user, host_id, **kwargs): session, host, user, 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) @@ -597,22 +586,6 @@ def _add_host_network( host = utils.get_db_object( 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) return utils.add_db_object( session, models.HostNetwork, @@ -653,14 +626,26 @@ def add_host_networks( host_networks = [] failed_host_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( session, user, host_id, exception_when_existing, **network )) - except exception.DatabaseException as error: - logging.exception(error) - failed_host_networks.append(network) if host_networks: hosts.append({'host_id': host_id, 'networks': host_networks}) if failed_host_networks: @@ -677,35 +662,6 @@ def add_host_networks( def _update_host_network( 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) return utils.update_db_object(session, host_network, **kwargs) diff --git a/compass/tests/db/api/test_cluster.py b/compass/tests/db/api/test_cluster.py index 90f65437..06fe0bf2 100644 --- a/compass/tests/db/api/test_cluster.py +++ b/compass/tests/db/api/test_cluster.py @@ -379,6 +379,20 @@ class TestUpdateCluster(ClusterTestCase): ) 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): # state is INSTALLING cluster.update_cluster_state( @@ -733,20 +747,30 @@ class TestAddClusterHost(ClusterTestCase): ) add_cluster_hosts = cluster.list_clusterhosts(user=self.user_object) expected = { - 'clusterhost_id': 3, - 'cluster_id': 1, - 'id': 3, - 'switch_ip': '172.29.8.40', 'hostname': 'test_add_cluster_host', 'owner': 'admin@huawei.com', - 'mac': '00:0c:29:5b:ee:eb', - 'host_id': 3, 'name': 'test_add_cluster_host.test_cluster1', } self.assertTrue( all(item in add_cluster_hosts[2].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): # installing cluster.update_cluster_state( @@ -777,7 +801,7 @@ class TestUpdateClusterHost(ClusterTestCase): self.cluster_id, self.host_id[0], user=self.user_object, - roles=['allinone-compute'] + roles=['allinone-compute'], ) update_cluster_hosts = cluster.list_cluster_hosts( self.cluster_id, @@ -789,6 +813,16 @@ class TestUpdateClusterHost(ClusterTestCase): result = item['roles'][0]['display_name'] 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): self.assertRaises( exception.InvalidParameter, @@ -828,6 +862,7 @@ class TestUpdateClusterhost(ClusterTestCase): cluster.update_clusterhost( self.clusterhost_id[0], user=self.user_object, + reinstall_os=False, roles=['allinone-compute'] ) update_clusterhosts = cluster.list_clusterhosts( @@ -839,6 +874,15 @@ class TestUpdateClusterhost(ClusterTestCase): result = item['roles'][0]['display_name'] 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): self.assertRaises( exception.InvalidParameter, diff --git a/compass/tests/db/api/test_host.py b/compass/tests/db/api/test_host.py index 42bd283a..b784aebd 100644 --- a/compass/tests/db/api/test_host.py +++ b/compass/tests/db/api/test_host.py @@ -384,6 +384,20 @@ class TestUpdateHost(HostTestCase): ) 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): host.update_host_state( self.host_ids[0], @@ -398,15 +412,6 @@ class TestUpdateHost(HostTestCase): 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): """Test update hosts.""" @@ -437,6 +442,27 @@ class TestUpdateHosts(HostTestCase): for result in results: 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): """Test delete host.""" @@ -839,7 +865,7 @@ class TestAddHostNetwork(HostTestCase): host.add_host_network( self.host_ids[0], user=self.user_object, - interface='eth1', + interface='eth22', ip='10.145.88.20', subnet_id=self.subnet_ids[0], is_mgmt=True @@ -853,6 +879,25 @@ class TestAddHostNetwork(HostTestCase): result.append(item['ip']) 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): host.add_host_network( self.host_ids[0], @@ -892,17 +937,6 @@ class TestAddHostNetwork(HostTestCase): result.append(item['ip']) 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): """Test add host networks.""" @@ -977,6 +1011,42 @@ class TestUpdateHostNetwork(HostTestCase): self.assertEqual(interface, 'eth10') 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): self.assertRaises( exception.RecordNotExists, @@ -1025,14 +1095,14 @@ class TestUpdateHostnetwork(HostTestCase): is_promiscuous=True ) self.assertRaises( - exception.InvalidParameter, + exception.DuplicatedRecord, host.update_hostnetwork, self.host_ids[0], user=self.user_object, interface='eth11' ) self.assertRaises( - exception.InvalidParameter, + exception.DuplicatedRecord, host.update_hostnetwork, self.host_ids[0], user=self.user_object, diff --git a/compass/tests/db/api/test_machine.py b/compass/tests/db/api/test_machine.py index a94e1bab..2fb271a3 100644 --- a/compass/tests/db/api/test_machine.py +++ b/compass/tests/db/api/test_machine.py @@ -14,6 +14,7 @@ import datetime import logging +import mock import os import unittest2 @@ -165,6 +166,99 @@ class TestDelMachine(BaseTest): 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__': flags.init() logsetting.init()