openstack
/
fuel-web
Code Issues Proposed changes

Add cleanup on delete action for NetworkGroupHandler 

Part of the functionality from NetworkManager.update_networks was moved
to NetworkGroup object. Tests updated accordingly.

Change-Id: Id0b1aebb329da1fd895c7edd9d7a88f137fa36e6
Closes-Bug: #1484476
This commit is contained in:
Artem Roma 2015-08-13 19:08:31 +03:00 committed by Ivan Kliuk
parent 2d04787440
commit e1d31f6ae1
9 changed files with 374 additions and 130 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
nailgun/nailgun/db/migration/alembic_migrations/versions/fuel_7_0.py
View File

@ -101,9 +101,11 @@ def upgrade():
upgrade_node_labels()
extend_segmentation_type()
network_groups_name_upgrade()
upgrade_ip_addr_ranges_constraint()
def downgrade():
downgrade_ip_addr_ranges_constraint()
network_groups_name_downgrade()
downgrade_node_labels()
extensions_field_downgrade()
@ -134,6 +136,24 @@ def downgrade():
type_='foreignkey')
def upgrade_ip_addr_ranges_constraint():
op.drop_constraint(
'ip_addr_ranges_network_group_id_fkey', 'ip_addr_ranges')
op.create_foreign_key(
'ip_addr_ranges_network_group_id_fkey', 'ip_addr_ranges',
'network_groups', ['network_group_id'], ['id'], ondelete="CASCADE"
)
def downgrade_ip_addr_ranges_constraint():
op.drop_constraint(
'ip_addr_ranges_network_group_id_fkey', 'ip_addr_ranges')
op.create_foreign_key(
'ip_addr_ranges_network_group_id_fkey', 'ip_addr_ranges',
'network_groups', ['network_group_id'], ['id']
)
def network_groups_name_upgrade():
op.alter_column('network_groups',
'name',

7
nailgun/nailgun/db/sqlalchemy/models/network.py
View File

@ -41,7 +41,8 @@ class IPAddr(Base):
class IPAddrRange(Base):
__tablename__ = 'ip_addr_ranges'
id = Column(Integer, primary_key=True)
network_group_id = Column(Integer, ForeignKey('network_groups.id'))
network_group_id = Column(Integer, ForeignKey('network_groups.id',
ondelete="CASCADE"))
first = Column(String(25), nullable=False)
last = Column(String(25), nullable=False)
@ -66,7 +67,9 @@ class NetworkGroup(Base):
nodes = relationship(
"Node",
secondary=IPAddr.__table__,
backref="networks")
backref="networks",
passive_deletes=True
)
meta = Column(MutableDict.as_mutable(JSON), default={})

86
nailgun/nailgun/network/manager.py
View File

@ -51,24 +51,6 @@ from nailgun.settings import settings
class NetworkManager(object):
@classmethod
def update_range_mask_from_cidr(cls, network_group, cidr,
use_gateway=False):
"""Update network ranges for cidr
"""
db().query(IPAddrRange).filter_by(
network_group_id=network_group.id).delete()
first_idx = 2 if use_gateway else 1
new_cidr = IPNetwork(cidr)
ip_range = IPAddrRange(
network_group_id=network_group.id,
first=str(new_cidr[first_idx]),
last=str(new_cidr[-2]))
db().add(ip_range)
db().flush()
@classmethod
def get_admin_network_group_id(cls, node_id=None):
"""Method for receiving Admin NetworkGroup ID.
@ -1215,19 +1197,6 @@ class NetworkManager(object):
NodeBondInterface.node_id == node_id
).first()
@classmethod
def _set_ip_ranges(cls, network_group_id, ip_ranges):
# deleting old ip ranges
db().query(IPAddrRange).filter_by(
network_group_id=network_group_id).delete()
for r in ip_ranges:
new_ip_range = IPAddrRange(
first=r[0],
last=r[1],
network_group_id=network_group_id)
db().add(new_ip_range)
@classmethod
def create_admin_network_group(cls, cluster_id, group_id):
cluster_db = objects.Cluster.get_by_uid(cluster_id)
@ -1312,7 +1281,7 @@ class NetworkManager(object):
cls.cleanup_network_group(nw_group)
@classmethod
def update_networks(cls, cluster, network_configuration):
def update_networks(cls, network_configuration):
if 'networks' in network_configuration:
for ng in network_configuration['networks']:
if ng['id'] == cls.get_admin_network_group_id():
@ -1320,43 +1289,36 @@ class NetworkManager(object):
ng_db = db().query(NetworkGroup).get(ng['id'])
# NOTE: Skip the ip_ranges key because it intersects
# with the NetworkGroup.ip_ranges attribute that
# is a relation. Skip meta as only certain fields
# can be changed there.
for key, value in six.iteritems(ng):
if key not in ("ip_ranges", "meta"):
setattr(ng_db, key, value)
if 'meta' not in ng:
# there are no restrictions on update process if
# meta is not supplied
objects.NetworkGroup.update(ng_db, ng)
continue
# If 'notation' is present in 'network_group.meta'
# save it in the model.
meta = ng.get('meta', {})
# only 'notation' and 'use_gateway' attributes is
# allowed to be updated in network group metadata for
# old clusters so here we updated it manually with data
# which doesn't contain 'meta' key
meta_to_update = {}
notation = meta.get('notation', ng_db.meta['notation'])
if notation != ng_db.meta['notation']:
ng_db.meta['notation'] = notation
for param in ('notation', 'use_gateway'):
if param in ng.get('meta', {}):
meta_to_update[param] = ng['meta'][param]
use_gateway = meta.get('use_gateway',
ng_db.get('use_gateway', False))
if use_gateway != ng_db.meta.get('use_gateway'):
ng_db.meta['use_gateway'] = use_gateway
# update particular keys in data
objects.NetworkGroup.update_meta(
ng_db, meta_to_update
)
if notation == consts.NETWORK_NOTATION.ip_ranges:
ip_ranges = ng.get("ip_ranges") or \
[(r.first, r.last) for r in ng_db.ip_ranges]
cls._set_ip_ranges(ng['id'], ip_ranges)
elif notation == consts.NETWORK_NOTATION.cidr:
cidr = ng["cidr"] or ng_db.cidr
cls.update_range_mask_from_cidr(
ng_db, cidr, use_gateway=use_gateway)
if notation and not cluster.is_locked:
cls.cleanup_network_group(ng_db)
objects.Cluster.add_pending_changes(cluster, 'networks')
# preserve original input dict but remove 'meta' key
# for proper update of the network group instance
data_to_update = dict(ng)
del data_to_update['meta']
objects.NetworkGroup.update(ng_db, data_to_update)
@classmethod
def update(cls, cluster, network_configuration):
cls.update_networks(cluster, network_configuration)
cls.update_networks(network_configuration)
if 'networking_parameters' in network_configuration:
if cluster.is_locked:

120
nailgun/nailgun/objects/network_group.py
View File

@ -16,15 +16,14 @@
from netaddr import IPNetwork
from nailgun.objects.serializers.network_group import NetworkGroupSerializer
from nailgun import consts
from nailgun.db import db
from nailgun.db.sqlalchemy import models
from nailgun.errors import errors
from nailgun.logger import logger
from nailgun.objects import NailgunCollection
from nailgun.objects import NailgunObject
from nailgun.objects.serializers.network_group import NetworkGroupSerializer
class NetworkGroup(NailgunObject):
@ -77,6 +76,119 @@ class NetworkGroup(NailgunObject):
db().flush()
return instance
@classmethod
def update_meta(cls, instance, data):
"""Updates particular keys in object's meta.
Is used by NetworkManager.update_networks as
for old clusters only those data in meta is
allowed for updating
"""
meta_copy = dict(instance.meta)
meta_copy.update(data)
instance.meta = meta_copy
@classmethod
def update(cls, instance, data):
# cleanup stalled data and generate new for the group
cls._regenerate_ip_ranges_on_notation(instance, data)
# as ip ranges were regenerated we must update instance object
# in order to prevent possible SQAlchemy errors with operating
# on stale data
db().refresh(instance)
# remove 'ip_ranges' (if) any from data as this is relation
# attribute for the orm model object
data.pop('ip_ranges', None)
return super(NetworkGroup, cls).update(instance, data)
@classmethod
def _regenerate_ip_ranges_on_notation(cls, instance, data):
"""Regenerate IP-address ranges basing on 'notation' field of
Network group 'meta' content.
:param instance: NetworkGroup instance
:type instance: models.NetworkGroup
:param data: network data
:type data: dict
:return: None
"""
notation = instance.meta['notation']
data_meta = data.get('meta', {})
# if notation data is present change ip ranges and remove
# stalled ip addresses for the network group
if notation and not instance.nodegroup.cluster.is_locked:
cls._delete_ips(instance)
notation = data_meta.get('notation', notation)
if notation == consts.NETWORK_NOTATION.ip_ranges:
ip_ranges = data.get("ip_ranges") or \
[(r.first, r.last) for r in instance.ip_ranges]
cls._set_ip_ranges(instance, ip_ranges)
elif notation == consts.NETWORK_NOTATION.cidr:
use_gateway = data_meta.get(
'use_gateway', instance.meta.get('use_gateway'))
cidr = data.get('cidr', instance.cidr)
cls._update_range_from_cidr(
instance, cidr, use_gateway=use_gateway)
@classmethod
def _set_ip_ranges(cls, instance, ip_ranges):
"""Set IP-address ranges.
:param instance: NetworkGroup instance being updated
:type instance: models.NetworkGroup
:param ip_ranges: IP-address ranges sequence
:type ip_ranges: iterable of pairs
:return: None
"""
# deleting old ip ranges
db().query(models.IPAddrRange).filter_by(
network_group_id=instance.id).delete()
for r in ip_ranges:
new_ip_range = models.IPAddrRange(
first=r[0],
last=r[1],
network_group_id=instance.id)
db().add(new_ip_range)
db().flush()
@classmethod
def _update_range_from_cidr(
cls, instance, cidr, use_gateway=False):
"""Update network ranges for CIDR.
:param instance: NetworkGroup instance being updated
:type instance: models.NetworkGroup
:param cidr: CIDR network representation
:type cidr: basestring
:param use_gateway: whether gateway is taken into account
:type use_gateway: bool
:return: None
"""
first_idx = 2 if use_gateway else 1
new_cidr = IPNetwork(cidr)
ip_range = (str(new_cidr[first_idx]), str(new_cidr[-2]))
cls._set_ip_ranges(instance, [ip_range])
@classmethod
def _delete_ips(cls, instance):
"""Network group cleanup - deletes all IPs were assigned within
the network group.
:param instance: NetworkGroup instance
:type instance: models.NetworkGroup
:returns: None
"""
logger.debug("Deleting old IPs for network with id=%s, cidr=%s",
instance.id, instance.cidr)
db().query(models.IPAddr).filter(
models.IPAddr.network == instance.id
).delete()
db().flush()
class NetworkGroupCollection(NailgunCollection):

4
nailgun/nailgun/test/base.py
View File

@ -1054,7 +1054,9 @@ class EnvironmentManager(object):
"cidr": "10.3.0.0/24",
"gateway": "10.3.0.1",
"group_id": Cluster.get_default_group(cluster).id,
"meta": {"notation": "cidr", 'map_priority': 2}
"meta": {
"notation": consts.NETWORK_NOTATION.cidr,
"map_priority": 2}
}
ng.update(kwargs)
resp = self.app.post(

27
nailgun/nailgun/test/integration/test_changes_model.py
View File

@ -133,33 +133,6 @@ class TestClusterChanges(BaseIntegrationTest):
).all()
self.assertEqual(len(pending_changes), 1)
def test_network_changing_adds_pending_changes(self):
cluster = self.env.create_cluster(api=True)
cluster_db = self.env.clusters[0]
objects.Cluster.clear_pending_changes(cluster_db)
all_changes = self.db.query(ClusterChanges).all()
self.assertEqual(len(all_changes), 0)
resp = self.app.get(
reverse(
'NovaNetworkConfigurationHandler',
kwargs={'cluster_id': cluster['id']}),
headers=self.default_headers
)
net_id = resp.json_body['networks'][0]["id"]
resp = self.app.put(
reverse(
'NovaNetworkConfigurationHandler',
kwargs={'cluster_id': cluster['id']}),
jsonutils.dumps({'networks': [{
"id": net_id, "gateway": "10.0.0.1"}
]}),
headers=self.default_headers
)
pending_changes = self.db.query(ClusterChanges).filter_by(
name="networks"
).all()
self.assertEqual(len(pending_changes), 1)
@fake_tasks(override_state={"progress": 100, "status": "ready"})
def test_successful_deployment_drops_all_changes(self):
self.env.create(

96
nailgun/nailgun/test/integration/test_network_manager.py
View File

@ -22,6 +22,7 @@ from mock import patch
from netaddr import IPAddress
from netaddr import IPNetwork
from netaddr import IPRange
import six
from sqlalchemy import not_
import nailgun
@ -541,25 +542,6 @@ class TestNetworkManager(BaseNetworkManagerTest):
self.env.network_manager.assign_given_vips_for_net_groups(
cluster, vips_to_assign)
def test_upgrade_range_mask_from_cidr(self):
cluster = self.env.create_cluster(api=False)
ng = cluster.network_groups[0]
self.env.network_manager.update_range_mask_from_cidr(
ng, "192.168.10.0/24")
ip_range = ng.ip_ranges[0]
self.assertEqual("192.168.10.1", ip_range.first)
self.assertEqual("192.168.10.254", ip_range.last)
def test_upgrade_range_mask_from_cidr_use_gateway(self):
cluster = self.env.create_cluster(api=False)
ng = cluster.network_groups[0]
self.env.network_manager.update_range_mask_from_cidr(
ng, "192.168.10.0/24",
use_gateway=True)
ip_range = ng.ip_ranges[0]
self.assertEqual("192.168.10.2", ip_range.first)
self.assertEqual("192.168.10.254", ip_range.last)
def test_update_networks_idempotent(self):
cluster = self.env.create_cluster(
api=False,
@ -569,17 +551,17 @@ class TestNetworkManager(BaseNetworkManagerTest):
get_network_config = network_configuration.\
NeutronNetworkConfigurationSerializer.serialize_for_cluster
nets = get_network_config(cluster)
self.env.network_manager.update_networks(cluster, nets)
self.env.network_manager.update_networks(nets)
updated_nets = get_network_config(cluster)
self.assertEqual(nets, updated_nets)
def test_update_networks(self):
def get_net_by_name(networks, name):
for net in networks:
if net["meta"]["name"] == name:
return net
raise Exception("Network with name {0} not found".format(name))
def get_net_by_name(self, networks, name):
for net in networks:
if net["meta"]["name"] == name:
return net
raise Exception("Network with name {0} not found".format(name))
def test_update_networks(self):
updates = {
consts.NETWORKS.public: {
"cidr": "172.16.42.0/24",
@ -587,8 +569,8 @@ class TestNetworkManager(BaseNetworkManagerTest):
"ip_ranges": [["172.16.42.2", "172.16.42.126"]],
},
consts.NETWORKS.management: {
'cidr': u'192.10.2.0/24',
'ip_ranges': [[u'192.10.2.1', u'192.10.2.254']],
'cidr': "192.10.2.0/24",
'ip_ranges': [["192.10.2.1", "192.10.2.254"]],
},
}
cluster = self.env.create_cluster(
@ -599,16 +581,64 @@ class TestNetworkManager(BaseNetworkManagerTest):
get_network_config = network_configuration.\
NeutronNetworkConfigurationSerializer.serialize_for_cluster
nets = get_network_config(cluster)
for net_name, net_changes in updates.items():
ng = get_net_by_name(nets["networks"], net_name)
for net_name, net_changes in six.iteritems(updates):
ng = self.get_net_by_name(nets["networks"], net_name)
ng.update(net_changes)
self.env.network_manager.update_networks(cluster, nets)
self.env.network_manager.update_networks(nets)
updated_nets = get_network_config(cluster)
for net_name in updates.keys():
expected_ng = get_net_by_name(nets["networks"], net_name)
updated_ng = get_net_by_name(updated_nets["networks"], net_name)
expected_ng = self.get_net_by_name(nets["networks"], net_name)
updated_ng = self.get_net_by_name(updated_nets["networks"],
net_name)
self.assertEqual(expected_ng, updated_ng)
def test_update_networks_meta(self):
updates = {
consts.NETWORKS.public: {
"meta": {"notation": consts.NETWORK_NOTATION.cidr},
},
consts.NETWORKS.management: {
"meta": {"use_gateway": True}
}
}
cluster = self.env.create_cluster(
api=False,
net_provider=consts.CLUSTER_NET_PROVIDERS.neutron,
net_l23_provider=consts.NEUTRON_L23_PROVIDERS.ovs,
)
get_network_config = network_configuration.\
NeutronNetworkConfigurationSerializer.serialize_for_cluster
nets = get_network_config(cluster)
# Lower value of range is "192.168.0.1".
mgmt_ng = self.get_net_by_name(
nets["networks"],
consts.NETWORKS.management)
self.assertEqual(mgmt_ng['ip_ranges'][0][0], "192.168.0.1")
for net_name, net_changes in six.iteritems(updates):
ng = self.get_net_by_name(nets["networks"], net_name)
ng.update(net_changes)
self.env.network_manager.update_networks(nets)
nets_updated = get_network_config(cluster)
public_ng = self.get_net_by_name(
nets_updated["networks"],
consts.NETWORKS.public)
self.assertEqual(public_ng["meta"]["notation"],
consts.NETWORK_NOTATION.cidr)
mgmt_ng_updated = self.get_net_by_name(
nets_updated["networks"],
consts.NETWORKS.management)
self.assertTrue(mgmt_ng_updated["meta"]["use_gateway"])
# Check whether ranges are changed after 'use_gateway=True' was set.
# Lower value of range is changed from "192.168.0.1" to "192.168.0.2".
self.assertEqual(mgmt_ng_updated['ip_ranges'][0][0], "192.168.0.2")
@fake_tasks(fake_rpc=False, mock_rpc=False)
@patch('nailgun.rpc.cast')
def test_admin_ip_cobbler(self, mocked_rpc):

122
nailgun/nailgun/test/unit/test_network_group_handler.py
View File

@ -15,8 +15,11 @@
# under the License.
import mock
from netaddr import IPNetwork
from oslo_serialization import jsonutils
from nailgun import consts
from nailgun.db.sqlalchemy import models
from nailgun import objects
from nailgun.test.base import BaseIntegrationTest
from nailgun.utils import reverse
@ -28,6 +31,20 @@ class TestHandlers(BaseIntegrationTest):
super(TestHandlers, self).setUp()
self.cluster = self.env.create_cluster(api=False)
def _create_network_group(self, expect_errors=False, **kwargs):
meta = {
"meta": {
"notation": consts.NETWORK_NOTATION.cidr,
"use_gateway": False
}
}
meta.update(kwargs)
return self.env._create_network_group(
expect_errors=expect_errors,
cluster=None,
**meta)
def test_create_network_group_w_cidr(self):
resp = self.env._create_network_group()
self.assertEqual(201, resp.status_code)
@ -122,6 +139,51 @@ class TestHandlers(BaseIntegrationTest):
)
self.assertEqual(204, resp.status_code)
def test_delete_network_group_cleanup_ip_range(self):
ng_id = self._create_network_group(
meta={
"notation": "ip_ranges",
"ip_range": ["10.3.0.33", "10.3.0.158"]
}
).json["id"]
self.app.delete(
reverse(
'NetworkGroupHandler',
kwargs={'obj_id': ng_id}
),
headers=self.default_headers,
)
ip_range = self.db.query(models.IPAddrRange)\
.filter_by(network_group_id=ng_id)\
.first()
self.assertIsNone(ip_range)
def test_delete_network_group_cleanup_ip_addrs(self):
ng_id = self._create_network_group().json["id"]
node = self.env.create_node(api=False)
ip_address = []
for ip_addr in ('10.3.0.2', '10.3.0.3'):
ip_addr_data = {'network': ng_id,
'node': node.id,
'ip_addr': ip_addr}
ip_address.append(ip_addr_data)
self.db.add_all([models.IPAddr(**ips) for ips in ip_address])
self.db.flush()
self.app.delete(
reverse(
'NetworkGroupHandler',
kwargs={'obj_id': ng_id}
),
headers=self.default_headers,
)
ips_db = self.db.query(models.IPAddr)\
.filter_by(network=ng_id)\
.all()
self.assertFalse(ips_db)
def test_cannot_delete_admin_network_group(self):
admin = objects.Cluster.get_network_manager().get_admin_network_group()
resp = self.app.delete(
@ -164,8 +226,8 @@ class TestHandlers(BaseIntegrationTest):
def test_modify_network_group(self):
resp = self.env._create_network_group(name='test')
new_ng = resp.json_body
new_ng = resp.json_body
new_ng['name'] = 'test2'
resp = self.env._update_network_group(new_ng)
@ -173,6 +235,64 @@ class TestHandlers(BaseIntegrationTest):
self.assertEquals('test2', updated_ng['name'])
def test_update_network_group_ipranges_regenerated(self):
resp = self._create_network_group(
meta={
"notation": "ip_ranges",
"ip_range": ["10.3.0.33", "10.3.0.158"],
}
)
new_ng = resp.json_body
new_ng["name"] = "test"
new_ip_ranges = [["172.16.0.1", "172.16.0.10"],
["10.20.0.2", "10.20.0.20"]]
new_ng["ip_ranges"] = new_ip_ranges
self.app.put(
reverse(
'NetworkGroupHandler',
kwargs={'obj_id': new_ng['id']}
),
jsonutils.dumps(new_ng),
headers=self.default_headers
)
ip_ranges = self.db.query(models.IPAddrRange)\
.filter_by(network_group_id=new_ng["id"])\
.all()
self.assertItemsEqual(
new_ip_ranges,
[[ipr.first, ipr.last] for ipr in ip_ranges]
)
def test_update_network_group_ipranges_regenerated_for_cidr(self):
resp = self._create_network_group()
new_ng = resp.json_body
new_cidr = "10.3.0.1/20"
new_ng['cidr'] = new_cidr
new_ng['name'] = 'test'
new_ng['meta']['use_gateway'] = True
generated_range = IPNetwork("10.3.0.1/20")
new_ip_range = [str(generated_range[2]), str(generated_range[-2])]
self.app.put(
reverse(
'NetworkGroupHandler',
kwargs={'obj_id': new_ng['id']}
),
jsonutils.dumps(new_ng),
headers=self.default_headers
)
ip_range = self.db.query(models.IPAddrRange)\
.filter_by(network_group_id=new_ng["id"])\
.one()
self.assertEqual(ip_range.first, new_ip_range[0])
self.assertEqual(ip_range.last, new_ip_range[1])
def test_duplicate_network_name_on_creation(self):
resp = self.env._create_network_group()
self.assertEqual(201, resp.status_code)

22
nailgun/nailgun/test/unit/test_objects.py
View File

@ -1190,3 +1190,25 @@ class TestClusterObjectGetNetworkManager(BaseTestCase):
self.env.clusters[0].release.version = '2014.2.2-7.0'
nm = objects.Cluster.get_network_manager(self.env.clusters[0])
self.assertEqual(nm, NeutronManager70)
class TestNetworkGroup(BaseTestCase):
def test_upgrade_range_mask_from_cidr(self):
cluster = self.env.create_cluster(api=False)
ng = cluster.network_groups[0]
objects.NetworkGroup._update_range_from_cidr(
ng, "192.168.10.0/24")
ip_range = ng.ip_ranges[0]
self.assertEqual("192.168.10.1", ip_range.first)
self.assertEqual("192.168.10.254", ip_range.last)
def test_upgrade_range_mask_from_cidr_use_gateway(self):
cluster = self.env.create_cluster(api=False)
ng = cluster.network_groups[0]
objects.NetworkGroup._update_range_from_cidr(
ng, "192.168.10.0/24",
use_gateway=True)
ip_range = ng.ip_ranges[0]
self.assertEqual("192.168.10.2", ip_range.first)
self.assertEqual("192.168.10.254", ip_range.last)