From f21d8510bb3f55b2b76aab251b0427dbfa69c5d9 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 7 Sep 2010 14:34:27 +0200 Subject: Add a clean-traffic filterref to the libvirt templates to prevent spoofing and snooping attacks from the guests. --- nova/virt/libvirt.qemu.xml.template | 3 +++ nova/virt/libvirt.uml.xml.template | 3 +++ 2 files changed, 6 insertions(+) diff --git a/nova/virt/libvirt.qemu.xml.template b/nova/virt/libvirt.qemu.xml.template index 307f9d03a..3de1e5009 100644 --- a/nova/virt/libvirt.qemu.xml.template +++ b/nova/virt/libvirt.qemu.xml.template @@ -20,6 +20,9 @@ + + + diff --git a/nova/virt/libvirt.uml.xml.template b/nova/virt/libvirt.uml.xml.template index 6f4290f98..e64b172d8 100644 --- a/nova/virt/libvirt.uml.xml.template +++ b/nova/virt/libvirt.uml.xml.template @@ -14,6 +14,9 @@ + + + -- cgit From 62dad8422532af4257769bbb0e68120b3393739a Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 7 Sep 2010 14:52:38 +0200 Subject: Add stubbed out handler for AuthorizeSecurityGroupIngress EC2 API call. --- nova/endpoint/cloud.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 8e2beb1e3..89402896c 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -240,6 +240,10 @@ class CloudController(object): # Stubbed for now to unblock other things. return groups + @rbac.allow('netadmin') + def authorize_security_group_ingress(self, context, group_name, **kwargs): + return True + @rbac.allow('netadmin') def create_security_group(self, context, group_name, **kwargs): return True -- cgit From bd07d6b3b3e9ed3ef3e65e99b628c8b1aaf2f82c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 12:35:46 +0200 Subject: Alright, first hole poked all the way through. We can now create security groups and read them back. --- nova/auth/manager.py | 6 +++++ nova/db/api.py | 22 ++++++++++++++++++ nova/db/sqlalchemy/api.py | 38 +++++++++++++++++++++++++++++++ nova/db/sqlalchemy/models.py | 54 +++++++++++++++++++++++++++++++++++++++++++- nova/endpoint/cloud.py | 14 ++++++++---- nova/tests/api_unittest.py | 34 +++++++++++++++++++++++++--- 6 files changed, 160 insertions(+), 8 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index d5fbec7c5..6aa5721c8 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -640,11 +640,17 @@ class AuthManager(object): with self.driver() as drv: user_dict = drv.create_user(name, access, secret, admin) if user_dict: + db.security_group_create(context={}, + values={ 'name' : 'default', + 'description' : 'default', + 'user_id' : name }) return User(**user_dict) def delete_user(self, user): """Deletes a user""" with self.driver() as drv: + for security_group in db.security_group_get_by_user(context = {}, user_id=user.id): + db.security_group_destroy({}, security_group.id) drv.delete_user(User.safe_id(user)) def generate_key_pair(self, user, key_name): diff --git a/nova/db/api.py b/nova/db/api.py index b49707392..b67e3afe0 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -442,3 +442,25 @@ def volume_update(context, volume_id, values): """ return IMPL.volume_update(context, volume_id, values) + +#################### + + +def security_group_create(context, values): + """Create a new security group""" + return IMPL.security_group_create(context, values) + + +def security_group_get_by_instance(context, instance_id): + """Get security groups to which the instance is assigned""" + return IMPL.security_group_get_by_instance(context, instance_id) + + +def security_group_get_by_user(context, user_id): + """Get security groups owned by the given user""" + return IMPL.security_group_get_by_user(context, user_id) + + +def security_group_destroy(context, security_group_id): + """Deletes a security group""" + return IMPL.security_group_destroy(context, security_group_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 5172b87b3..d790d3fac 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -581,3 +581,41 @@ def volume_update(context, volume_id, values): for (key, value) in values.iteritems(): volume_ref[key] = value volume_ref.save() + + +################### + + +def security_group_create(_context, values): + security_group_ref = models.SecurityGroup() + for (key, value) in values.iteritems(): + security_group_ref[key] = value + security_group_ref.save() + return security_group_ref + + +def security_group_get_by_instance(_context, instance_id): + with managed_session() as session: + return session.query(models.Instance) \ + .get(instance_id) \ + .security_groups \ + .all() + + +def security_group_get_by_user(_context, user_id): + with managed_session() as session: + return session.query(models.SecurityGroup) \ + .filter_by(user_id=user_id) \ + .filter_by(deleted=False) \ + .all() + +def security_group_destroy(_context, security_group_id): + with managed_session() as session: + security_group = session.query(models.SecurityGroup) \ + .get(security_group_id) + security_group.delete(session=session) + +def security_group_get_all(_context): + return models.SecurityGroup.all() + + diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 310d4640e..28c25bfbc 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -26,7 +26,7 @@ import datetime # TODO(vish): clean up these imports from sqlalchemy.orm import relationship, backref, validates, exc from sqlalchemy.sql import func -from sqlalchemy import Column, Integer, String +from sqlalchemy import Column, Integer, String, Table from sqlalchemy import ForeignKey, DateTime, Boolean, Text from sqlalchemy.ext.declarative import declarative_base @@ -292,6 +292,58 @@ class ExportDevice(BASE, NovaBase): uselist=False)) +security_group_instance_association = Table('security_group_instance_association', + BASE.metadata, + Column('security_group_id', Integer, + ForeignKey('security_group.id')), + Column('instance_id', Integer, + ForeignKey('instances.id'))) + +class SecurityGroup(BASE, NovaBase): + """Represents a security group""" + __tablename__ = 'security_group' + id = Column(Integer, primary_key=True) + + name = Column(String(255)) + description = Column(String(255)) + + user_id = Column(String(255)) + project_id = Column(String(255)) + + instances = relationship(Instance, + secondary=security_group_instance_association, + backref='security_groups') + + @property + def user(self): + return auth.manager.AuthManager().get_user(self.user_id) + + @property + def project(self): + return auth.manager.AuthManager().get_project(self.project_id) + + +class SecurityGroupIngressRule(BASE, NovaBase): + """Represents a rule in a security group""" + __tablename__ = 'security_group_rules' + id = Column(Integer, primary_key=True) + + parent_security_group = Column(Integer, ForeignKey('security_group.id')) + protocol = Column(String(5)) # "tcp", "udp", or "icmp" + fromport = Column(Integer) + toport = Column(Integer) + + # Note: This is not the parent SecurityGroup's owner. It's the owner of + # the SecurityGroup we're granting access. + user_id = Column(String(255)) + group_id = Column(Integer, ForeignKey('security_group.id')) + + @property + def user(self): + return auth.manager.AuthManager().get_user(self.user_id) + + cidr = Column(String(255)) + class Network(BASE, NovaBase): """Represents a network""" __tablename__ = 'networks' diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 44997be59..7df8bd081 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -212,10 +212,12 @@ class CloudController(object): return True @rbac.allow('all') - def describe_security_groups(self, context, group_names, **kwargs): - groups = {'securityGroupSet': []} + def describe_security_groups(self, context, **kwargs): + groups = {'securityGroupSet': + [{ 'groupDescription': group.description, + 'groupName' : group.name, + 'ownerId': context.user.id } for group in db.security_group_get_by_user(context, context.user.id) ] } - # Stubbed for now to unblock other things. return groups @rbac.allow('netadmin') @@ -223,7 +225,11 @@ class CloudController(object): return True @rbac.allow('netadmin') - def create_security_group(self, context, group_name, **kwargs): + def create_security_group(self, context, group_name, group_description): + db.security_group_create(context, + values = { 'user_id' : context.user.id, + 'name': group_name, + 'description': group_description }) return True @rbac.allow('netadmin') diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 462d1b295..87d99607d 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -185,6 +185,9 @@ class ApiEc2TestCase(test.BaseTestCase): self.host = '127.0.0.1' self.app = api.APIServerApplication({'Cloud': self.cloud}) + + def expect_http(self, host=None, is_secure=False): + """Returns a new EC2 connection""" self.ec2 = boto.connect_ec2( aws_access_key_id='fake', aws_secret_access_key='fake', @@ -194,9 +197,6 @@ class ApiEc2TestCase(test.BaseTestCase): path='/services/Cloud') self.mox.StubOutWithMock(self.ec2, 'new_http_connection') - - def expect_http(self, host=None, is_secure=False): - """Returns a new EC2 connection""" http = FakeHttplibConnection( self.app, '%s:%d' % (self.host, FLAGS.cc_port), False) # pylint: disable-msg=E1103 @@ -231,3 +231,31 @@ class ApiEc2TestCase(test.BaseTestCase): self.assertEquals(len(results), 1) self.manager.delete_project(project) self.manager.delete_user(user) + + def test_get_all_security_groups(self): + """Test that operations on security groups stick""" + self.expect_http() + self.mox.ReplayAll() + security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ + for x in range(random.randint(4, 8))) + user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + project = self.manager.create_project('fake', 'fake', 'fake') + + rv = self.ec2.get_all_security_groups() + self.assertEquals(len(rv), 1) + self.assertEquals(rv[0].name, 'default') + + self.expect_http() + self.mox.ReplayAll() + + self.ec2.create_security_group(security_group_name, 'test group') + + self.expect_http() + self.mox.ReplayAll() + + rv = self.ec2.get_all_security_groups() + self.assertEquals(len(rv), 2) + self.assertTrue(security_group_name in [group.name for group in rv]) + + self.manager.delete_project(project) + self.manager.delete_user(user) -- cgit From bd7ac72b9774a181e51dde5dff09ed4c47b556a7 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 15:13:04 +0200 Subject: AuthorizeSecurityGroupIngress now works. --- nova/db/api.py | 13 +++++++ nova/db/sqlalchemy/api.py | 19 ++++++++++ nova/db/sqlalchemy/models.py | 9 +++-- nova/endpoint/cloud.py | 50 +++++++++++++++++++++++--- nova/tests/api_unittest.py | 83 +++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 161 insertions(+), 13 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index b67e3afe0..af574d6de 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -461,6 +461,19 @@ def security_group_get_by_user(context, user_id): return IMPL.security_group_get_by_user(context, user_id) +def security_group_get_by_user_and_name(context, user_id, name): + """Get user's named security group""" + return IMPL.security_group_get_by_user_and_name(context, user_id, name) + + def security_group_destroy(context, security_group_id): """Deletes a security group""" return IMPL.security_group_destroy(context, security_group_id) + + +#################### + + +def security_group_rule_create(context, values): + """Create a new security group""" + return IMPL.security_group_rule_create(context, values) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index d790d3fac..c8d852f9d 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -609,6 +609,14 @@ def security_group_get_by_user(_context, user_id): .filter_by(deleted=False) \ .all() +def security_group_get_by_user_and_name(_context, user_id, name): + with managed_session() as session: + return session.query(models.SecurityGroup) \ + .filter_by(user_id=user_id) \ + .filter_by(name=name) \ + .filter_by(deleted=False) \ + .one() + def security_group_destroy(_context, security_group_id): with managed_session() as session: security_group = session.query(models.SecurityGroup) \ @@ -619,3 +627,14 @@ def security_group_get_all(_context): return models.SecurityGroup.all() + + +################### + + +def security_group_rule_create(_context, values): + security_group_rule_ref = models.SecurityGroupIngressRule() + for (key, value) in values.iteritems(): + security_group_rule_ref[key] = value + security_group_rule_ref.save() + return security_group_rule_ref diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 28c25bfbc..330262a88 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -330,12 +330,11 @@ class SecurityGroupIngressRule(BASE, NovaBase): parent_security_group = Column(Integer, ForeignKey('security_group.id')) protocol = Column(String(5)) # "tcp", "udp", or "icmp" - fromport = Column(Integer) - toport = Column(Integer) + from_port = Column(Integer) + to_port = Column(Integer) - # Note: This is not the parent SecurityGroup's owner. It's the owner of - # the SecurityGroup we're granting access. - user_id = Column(String(255)) + # Note: This is not the parent SecurityGroup. It's SecurityGroup we're + # granting access for. group_id = Column(Integer, ForeignKey('security_group.id')) @property diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 7df8bd081..0a929b865 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -214,14 +214,54 @@ class CloudController(object): @rbac.allow('all') def describe_security_groups(self, context, **kwargs): groups = {'securityGroupSet': - [{ 'groupDescription': group.description, - 'groupName' : group.name, - 'ownerId': context.user.id } for group in db.security_group_get_by_user(context, context.user.id) ] } + [{ 'groupDescription': group.description, + 'groupName' : group.name, + 'ownerId': context.user.id } for group in \ + db.security_group_get_by_user(context, + context.user.id) ] } return groups @rbac.allow('netadmin') - def authorize_security_group_ingress(self, context, group_name, **kwargs): + def authorize_security_group_ingress(self, context, group_name, + to_port=None, from_port=None, + ip_protocol=None, cidr_ip=None, + user_id=None, + source_security_group_name=None, + source_security_group_owner_id=None): + security_group = db.security_group_get_by_user_and_name(context, + context.user.id, + group_name) + values = { 'parent_security_group' : security_group.id } + + # Aw, crap. + if source_security_group_name: + if source_security_group_owner_id: + other_user_id = source_security_group_owner_id + else: + other_user_id = context.user.id + + foreign_security_group = \ + db.security_group_get_by_user_and_name(context, + other_user_id, + source_security_group_name) + values['group_id'] = foreign_security_group.id + elif cidr_ip: + values['cidr'] = cidr_ip + else: + return { 'return': False } + + if ip_protocol and from_port and to_port: + values['protocol'] = ip_protocol + values['from_port'] = from_port + values['to_port'] = to_port + else: + # If cidr based filtering, protocol and ports are mandatory + if 'cidr' in values: + print values + return None + + security_group_rule = db.security_group_rule_create(context, values) return True @rbac.allow('netadmin') @@ -234,6 +274,8 @@ class CloudController(object): @rbac.allow('netadmin') def delete_security_group(self, context, group_name, **kwargs): + security_group = db.security_group_get_by_user_and_name(context, context.user.id, group_name) + security_group.delete() return True @rbac.allow('projectmanager', 'sysadmin') diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 87d99607d..6cd59541f 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -233,20 +233,29 @@ class ApiEc2TestCase(test.BaseTestCase): self.manager.delete_user(user) def test_get_all_security_groups(self): - """Test that operations on security groups stick""" + """Test that we can retrieve security groups""" self.expect_http() self.mox.ReplayAll() - security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ - for x in range(random.randint(4, 8))) user = self.manager.create_user('fake', 'fake', 'fake', admin=True) project = self.manager.create_project('fake', 'fake', 'fake') rv = self.ec2.get_all_security_groups() + self.assertEquals(len(rv), 1) - self.assertEquals(rv[0].name, 'default') + self.assertEquals(rv[0].name, 'default') + + self.manager.delete_project(project) + self.manager.delete_user(user) + def test_create_delete_security_group(self): + """Test that we can create a security group""" self.expect_http() self.mox.ReplayAll() + user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + project = self.manager.create_project('fake', 'fake', 'fake') + + security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ + for x in range(random.randint(4, 8))) self.ec2.create_security_group(security_group_name, 'test group') @@ -257,5 +266,71 @@ class ApiEc2TestCase(test.BaseTestCase): self.assertEquals(len(rv), 2) self.assertTrue(security_group_name in [group.name for group in rv]) + self.expect_http() + self.mox.ReplayAll() + + self.ec2.delete_security_group(security_group_name) + self.manager.delete_project(project) self.manager.delete_user(user) + + def test_authorize_security_group_cidr(self): + """Test that we can add rules to a security group""" + self.expect_http() + self.mox.ReplayAll() + user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + project = self.manager.create_project('fake', 'fake', 'fake') + + security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ + for x in range(random.randint(4, 8))) + + group = self.ec2.create_security_group(security_group_name, 'test group') + + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + + group.authorize('tcp', 80, 80, '0.0.0.0/0') + + self.expect_http() + self.mox.ReplayAll() + + self.ec2.delete_security_group(security_group_name) + + self.manager.delete_project(project) + self.manager.delete_user(user) + + return + + def test_authorize_security_group_foreign_group(self): + """Test that we can grant another security group access to a security group""" + self.expect_http() + self.mox.ReplayAll() + user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + project = self.manager.create_project('fake', 'fake', 'fake') + + security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ + for x in range(random.randint(4, 8))) + + group = self.ec2.create_security_group(security_group_name, 'test group') + + self.expect_http() + self.mox.ReplayAll() + + other_group = self.ec2.create_security_group('appserver', 'The application tier') + + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + + group.authorize(src_group=other_group) + + self.expect_http() + self.mox.ReplayAll() + + self.ec2.delete_security_group(security_group_name) + + self.manager.delete_project(project) + self.manager.delete_user(user) + + return -- cgit From 59a959299d7883c48626d8d5630974d718194960 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 17:35:02 +0200 Subject: Authorize and Revoke access now works. --- nova/db/api.py | 9 ++++++++ nova/db/sqlalchemy/api.py | 8 +++++++ nova/db/sqlalchemy/models.py | 7 ++++-- nova/endpoint/cloud.py | 51 +++++++++++++++++++++++++++++++++++++++++--- nova/tests/api_unittest.py | 26 ++++++++++++++++++---- 5 files changed, 92 insertions(+), 9 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index af574d6de..63ead04e0 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -477,3 +477,12 @@ def security_group_destroy(context, security_group_id): def security_group_rule_create(context, values): """Create a new security group""" return IMPL.security_group_rule_create(context, values) + + +def security_group_rule_get_by_security_group(context, security_group_id): + """Get all rules for a a given security group""" + return IMPL.security_group_rule_get_by_security_group(context, security_group_id) + +def security_group_rule_destroy(context, security_group_rule_id): + """Deletes a security group rule""" + return IMPL.security_group_rule_destroy(context, security_group_rule_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index c8d852f9d..2db876154 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -25,6 +25,7 @@ from nova import flags from nova.db.sqlalchemy import models from nova.db.sqlalchemy.session import managed_session from sqlalchemy import or_ +from sqlalchemy.orm import eagerload FLAGS = flags.FLAGS @@ -615,6 +616,7 @@ def security_group_get_by_user_and_name(_context, user_id, name): .filter_by(user_id=user_id) \ .filter_by(name=name) \ .filter_by(deleted=False) \ + .options(eagerload('rules')) \ .one() def security_group_destroy(_context, security_group_id): @@ -638,3 +640,9 @@ def security_group_rule_create(_context, values): security_group_rule_ref[key] = value security_group_rule_ref.save() return security_group_rule_ref + +def security_group_rule_destroy(_context, security_group_rule_id): + with managed_session() as session: + security_group_rule = session.query(models.SecurityGroupIngressRule) \ + .get(security_group_rule_id) + security_group_rule.delete(session=session) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 330262a88..d177688d8 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -328,14 +328,17 @@ class SecurityGroupIngressRule(BASE, NovaBase): __tablename__ = 'security_group_rules' id = Column(Integer, primary_key=True) - parent_security_group = Column(Integer, ForeignKey('security_group.id')) + parent_group_id = Column(Integer, ForeignKey('security_group.id')) + parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id) +# primaryjoin=SecurityGroup().id==parent_group_id) + protocol = Column(String(5)) # "tcp", "udp", or "icmp" from_port = Column(Integer) to_port = Column(Integer) # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. - group_id = Column(Integer, ForeignKey('security_group.id')) +# group_id = Column(Integer, ForeignKey('security_group.id')) @property def user(self): diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 0a929b865..6e32a945b 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -222,6 +222,52 @@ class CloudController(object): return groups + @rbac.allow('netadmin') + def revoke_security_group_ingress(self, context, group_name, + to_port=None, from_port=None, + ip_protocol=None, cidr_ip=None, + user_id=None, + source_security_group_name=None, + source_security_group_owner_id=None): + security_group = db.security_group_get_by_user_and_name(context, + context.user.id, + group_name) + + criteria = {} + + if source_security_group_name: + if source_security_group_owner_id: + other_user_id = source_security_group_owner_id + else: + other_user_id = context.user.id + + foreign_security_group = \ + db.security_group_get_by_user_and_name(context, + other_user_id, + source_security_group_name) + criteria['group_id'] = foreign_security_group.id + elif cidr_ip: + criteria['cidr'] = cidr_ip + else: + return { 'return': False } + + if ip_protocol and from_port and to_port: + criteria['protocol'] = ip_protocol + criteria['from_port'] = from_port + criteria['to_port'] = to_port + else: + # If cidr based filtering, protocol and ports are mandatory + if 'cidr' in criteria: + return { 'return': False } + + for rule in security_group.rules: + for (k,v) in criteria.iteritems(): + if getattr(rule, k, False) != v: + break + # If we make it here, we have a match + db.security_group_rule_destroy(context, rule.id) + return True + @rbac.allow('netadmin') def authorize_security_group_ingress(self, context, group_name, to_port=None, from_port=None, @@ -232,13 +278,12 @@ class CloudController(object): security_group = db.security_group_get_by_user_and_name(context, context.user.id, group_name) - values = { 'parent_security_group' : security_group.id } + values = { 'parent_group_id' : security_group.id } - # Aw, crap. if source_security_group_name: if source_security_group_owner_id: other_user_id = source_security_group_owner_id - else: + else: other_user_id = context.user.id foreign_security_group = \ diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 6cd59541f..f25e377d0 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -274,8 +274,11 @@ class ApiEc2TestCase(test.BaseTestCase): self.manager.delete_project(project) self.manager.delete_user(user) - def test_authorize_security_group_cidr(self): - """Test that we can add rules to a security group""" + def test_authorize_revoke_security_group_cidr(self): + """ + Test that we can add and remove CIDR based rules + to a security group + """ self.expect_http() self.mox.ReplayAll() user = self.manager.create_user('fake', 'fake', 'fake', admin=True) @@ -292,6 +295,12 @@ class ApiEc2TestCase(test.BaseTestCase): group.authorize('tcp', 80, 80, '0.0.0.0/0') + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + + group.revoke('tcp', 80, 80, '0.0.0.0/0') + self.expect_http() self.mox.ReplayAll() @@ -302,8 +311,11 @@ class ApiEc2TestCase(test.BaseTestCase): return - def test_authorize_security_group_foreign_group(self): - """Test that we can grant another security group access to a security group""" + def test_authorize_revoke_security_group_foreign_group(self): + """ + Test that we can grant and revoke another security group access + to a security group + """ self.expect_http() self.mox.ReplayAll() user = self.manager.create_user('fake', 'fake', 'fake', admin=True) @@ -325,6 +337,12 @@ class ApiEc2TestCase(test.BaseTestCase): group.authorize(src_group=other_group) + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + + group.revoke(src_group=other_group) + self.expect_http() self.mox.ReplayAll() -- cgit From ecbbfa343edf0ca0e82b35dc655fa23701bbdf22 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 11:47:06 +0200 Subject: Create and delete security groups works. Adding and revoking rules works. DescribeSecurityGroups returns the groups and rules. So, the API seems to be done. Yay. --- nova/db/api.py | 5 ++++ nova/db/sqlalchemy/api.py | 7 ++++++ nova/db/sqlalchemy/models.py | 6 ++--- nova/endpoint/api.py | 1 + nova/endpoint/cloud.py | 43 ++++++++++++++++++++++++++------ nova/tests/api_unittest.py | 58 ++++++++++++++++++++++++++++++++++++++++---- 6 files changed, 104 insertions(+), 16 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index 63ead04e0..c7a6da183 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -451,6 +451,11 @@ def security_group_create(context, values): return IMPL.security_group_create(context, values) +def security_group_get_by_id(context, security_group_id): + """Get security group by its internal id""" + return IMPL.security_group_get_by_id(context, security_group_id) + + def security_group_get_by_instance(context, instance_id): """Get security groups to which the instance is assigned""" return IMPL.security_group_get_by_instance(context, instance_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 2db876154..4027e901c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -595,6 +595,12 @@ def security_group_create(_context, values): return security_group_ref +def security_group_get_by_id(_context, security_group_id): + with managed_session() as session: + return session.query(models.SecurityGroup) \ + .get(security_group_id) + + def security_group_get_by_instance(_context, instance_id): with managed_session() as session: return session.query(models.Instance) \ @@ -608,6 +614,7 @@ def security_group_get_by_user(_context, user_id): return session.query(models.SecurityGroup) \ .filter_by(user_id=user_id) \ .filter_by(deleted=False) \ + .options(eagerload('rules')) \ .all() def security_group_get_by_user_and_name(_context, user_id, name): diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index d177688d8..27c8e4d4c 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -329,8 +329,8 @@ class SecurityGroupIngressRule(BASE, NovaBase): id = Column(Integer, primary_key=True) parent_group_id = Column(Integer, ForeignKey('security_group.id')) - parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id) -# primaryjoin=SecurityGroup().id==parent_group_id) + parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id, + primaryjoin=parent_group_id==SecurityGroup.id) protocol = Column(String(5)) # "tcp", "udp", or "icmp" from_port = Column(Integer) @@ -338,7 +338,7 @@ class SecurityGroupIngressRule(BASE, NovaBase): # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. -# group_id = Column(Integer, ForeignKey('security_group.id')) + group_id = Column(Integer, ForeignKey('security_group.id')) @property def user(self): diff --git a/nova/endpoint/api.py b/nova/endpoint/api.py index 40be00bb7..1f37aeb02 100755 --- a/nova/endpoint/api.py +++ b/nova/endpoint/api.py @@ -135,6 +135,7 @@ class APIRequest(object): response = xml.toxml() xml.unlink() +# print response _log.debug(response) return response diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 6e32a945b..e6eca9850 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -213,14 +213,41 @@ class CloudController(object): @rbac.allow('all') def describe_security_groups(self, context, **kwargs): - groups = {'securityGroupSet': - [{ 'groupDescription': group.description, - 'groupName' : group.name, - 'ownerId': context.user.id } for group in \ - db.security_group_get_by_user(context, - context.user.id) ] } - - return groups + groups = [] + for group in db.security_group_get_by_user(context, context.user.id): + group_dict = {} + group_dict['groupDescription'] = group.description + group_dict['groupName'] = group.name + group_dict['ownerId'] = context.user.id + group_dict['ipPermissions'] = [] + for rule in group.rules: + rule_dict = {} + rule_dict['ipProtocol'] = rule.protocol + rule_dict['fromPort'] = rule.from_port + rule_dict['toPort'] = rule.to_port + rule_dict['groups'] = [] + rule_dict['ipRanges'] = [] + if rule.group_id: + foreign_group = db.security_group_get_by_id({}, rule.group_id) + rule_dict['groups'] += [ { 'groupName': foreign_group.name, + 'userId': foreign_group.user_id } ] + else: + rule_dict['ipRanges'] += [ { 'cidrIp': rule.cidr } ] + group_dict['ipPermissions'] += [ rule_dict ] + groups += [ group_dict ] + + return {'securityGroupInfo': groups } +# +# [{ 'groupDescription': group.description, +# 'groupName' : group.name, +# 'ownerId': context.user.id, +# 'ipPermissions' : [ +# { 'ipProtocol' : rule.protocol, +# 'fromPort' : rule.from_port, +# 'toPort' : rule.to_port, +# 'ipRanges' : [ { 'cidrIp' : rule.cidr } ] } for rule in group.rules ] } for group in \ +# +# return groups @rbac.allow('netadmin') def revoke_security_group_ingress(self, context, group_name, diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index f25e377d0..7e914e6f5 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -293,19 +293,43 @@ class ApiEc2TestCase(test.BaseTestCase): self.mox.ReplayAll() group.connection = self.ec2 - group.authorize('tcp', 80, 80, '0.0.0.0/0') + group.authorize('tcp', 80, 81, '0.0.0.0/0') + + self.expect_http() + self.mox.ReplayAll() + + rv = self.ec2.get_all_security_groups() + # I don't bother checkng that we actually find it here, + # because the create/delete unit test further up should + # be good enough for that. + for group in rv: + if group.name == security_group_name: + self.assertEquals(len(group.rules), 1) + self.assertEquals(int(group.rules[0].from_port), 80) + self.assertEquals(int(group.rules[0].to_port), 81) + self.assertEquals(len(group.rules[0].grants), 1) + self.assertEquals(str(group.rules[0].grants[0]), '0.0.0.0/0') self.expect_http() self.mox.ReplayAll() group.connection = self.ec2 - group.revoke('tcp', 80, 80, '0.0.0.0/0') + group.revoke('tcp', 80, 81, '0.0.0.0/0') self.expect_http() self.mox.ReplayAll() self.ec2.delete_security_group(security_group_name) + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + + rv = self.ec2.get_all_security_groups() + + self.assertEqual(len(rv), 1) + self.assertEqual(rv[0].name, 'default') + self.manager.delete_project(project) self.manager.delete_user(user) @@ -323,13 +347,16 @@ class ApiEc2TestCase(test.BaseTestCase): security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ for x in range(random.randint(4, 8))) + other_security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \ + for x in range(random.randint(4, 8))) group = self.ec2.create_security_group(security_group_name, 'test group') self.expect_http() self.mox.ReplayAll() - other_group = self.ec2.create_security_group('appserver', 'The application tier') + other_group = self.ec2.create_security_group(other_security_group_name, + 'some other group') self.expect_http() self.mox.ReplayAll() @@ -339,9 +366,30 @@ class ApiEc2TestCase(test.BaseTestCase): self.expect_http() self.mox.ReplayAll() - group.connection = self.ec2 - group.revoke(src_group=other_group) + rv = self.ec2.get_all_security_groups() + # I don't bother checkng that we actually find it here, + # because the create/delete unit test further up should + # be good enough for that. + for group in rv: + if group.name == security_group_name: + self.assertEquals(len(group.rules), 1) + self.assertEquals(len(group.rules[0].grants), 1) + self.assertEquals(str(group.rules[0].grants[0]), + '%s-%s' % (other_security_group_name, 'fake')) + + + self.expect_http() + self.mox.ReplayAll() + + rv = self.ec2.get_all_security_groups() + + for group in rv: + if group.name == security_group_name: + self.expect_http() + self.mox.ReplayAll() + group.connection = self.ec2 + group.revoke(src_group=other_group) self.expect_http() self.mox.ReplayAll() -- cgit From c3dd0aa79d982d8f34172e6023d4b632ea23f2b9 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 14:56:36 +0200 Subject: First pass of nwfilter based security group implementation. It is not where it is supposed to be and it does not actually do anything yet. --- nova/auth/manager.py | 2 +- nova/db/sqlalchemy/api.py | 1 + nova/endpoint/cloud.py | 1 - nova/tests/virt_unittest.py | 50 ++++++++++++++++++++++++++++++++--- nova/virt/libvirt_conn.py | 63 +++++++++++++++++++++++++++++++++++++++++++++ run_tests.py | 1 + 6 files changed, 113 insertions(+), 5 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 6aa5721c8..281e2d8f0 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -649,7 +649,7 @@ class AuthManager(object): def delete_user(self, user): """Deletes a user""" with self.driver() as drv: - for security_group in db.security_group_get_by_user(context = {}, user_id=user.id): + for security_group in db.security_group_get_by_user(context = {}, user_id=User.safe_id(user)): db.security_group_destroy({}, security_group.id) drv.delete_user(User.safe_id(user)) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 4027e901c..622e76cd7 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -598,6 +598,7 @@ def security_group_create(_context, values): def security_group_get_by_id(_context, security_group_id): with managed_session() as session: return session.query(models.SecurityGroup) \ + .options(eagerload('rules')) \ .get(security_group_id) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index e6eca9850..5e5ed6c5e 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -299,7 +299,6 @@ class CloudController(object): def authorize_security_group_ingress(self, context, group_name, to_port=None, from_port=None, ip_protocol=None, cidr_ip=None, - user_id=None, source_security_group_name=None, source_security_group_owner_id=None): security_group = db.security_group_get_by_user_and_name(context, diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 2aab16809..b8dcec12b 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -14,23 +14,30 @@ # License for the specific language governing permissions and limitations # under the License. +from xml.dom.minidom import parseString + from nova import flags from nova import test +from nova.endpoint import cloud from nova.virt import libvirt_conn FLAGS = flags.FLAGS class LibvirtConnTestCase(test.TrialTestCase): - def test_get_uri_and_template(self): + def bitrot_test_get_uri_and_template(self): class MockDataModel(object): + def __getitem__(self, name): + return self.datamodel[name] + def __init__(self): self.datamodel = { 'name' : 'i-cafebabe', 'memory_kb' : '1024000', 'basepath' : '/some/path', 'bridge_name' : 'br100', 'mac_address' : '02:12:34:46:56:67', - 'vcpus' : 2 } + 'vcpus' : 2, + 'project_id' : None } type_uri_map = { 'qemu' : ('qemu:///system', [lambda s: '' in s, @@ -53,7 +60,7 @@ class LibvirtConnTestCase(test.TrialTestCase): self.assertEquals(uri, expected_uri) for i, check in enumerate(checks): - xml = conn.toXml(MockDataModel()) + xml = conn.to_xml(MockDataModel()) self.assertTrue(check(xml), '%s failed check %d' % (xml, i)) # Deliberately not just assigning this string to FLAGS.libvirt_uri and @@ -67,3 +74,40 @@ class LibvirtConnTestCase(test.TrialTestCase): uri, template = conn.get_uri_and_template() self.assertEquals(uri, testuri) + +class NWFilterTestCase(test.TrialTestCase): + def test_stuff(self): + cloud_controller = cloud.CloudController() + class FakeContext(object): + pass + + context = FakeContext() + context.user = FakeContext() + context.user.id = 'fake' + context.user.is_superuser = lambda:True + cloud_controller.create_security_group(context, 'testgroup', 'test group description') + cloud_controller.authorize_security_group_ingress(context, 'testgroup', from_port='80', + to_port='81', ip_protocol='tcp', + cidr_ip='0.0.0.0/0') + + fw = libvirt_conn.NWFilterFirewall() + xml = fw.security_group_to_nwfilter_xml(1) + + dom = parseString(xml) + self.assertEqual(dom.firstChild.tagName, 'filter') + + rules = dom.getElementsByTagName('rule') + self.assertEqual(len(rules), 1) + + # It's supposed to allow inbound traffic. + self.assertEqual(rules[0].getAttribute('action'), 'allow') + self.assertEqual(rules[0].getAttribute('direction'), 'in') + + # Must be lower priority than the base filter (which blocks everything) + self.assertTrue(int(rules[0].getAttribute('priority')) < 1000) + + ip_conditions = rules[0].getElementsByTagName('ip') + self.assertEqual(len(ip_conditions), 1) + self.assertEqual(ip_conditions[0].getAttribute('protocol'), 'tcp') + self.assertEqual(ip_conditions[0].getAttribute('dstportstart'), '80') + self.assertEqual(ip_conditions[0].getAttribute('dstportend'), '81') diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index e26030158..7bf2a68b1 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -426,3 +426,66 @@ class LibvirtConnection(object): """ domain = self._conn.lookupByName(instance_name) return domain.interfaceStats(interface) + + +class NWFilterFirewall(object): + """ + This class implements a network filtering mechanism versatile + enough for EC2 style Security Group filtering by leveraging + libvirt's nwfilter. + + First, all instances get a filter ("nova-base-filter") applied. + This filter drops all incoming ipv4 and ipv6 connections. + Outgoing connections are never blocked. + + Second, every security group maps to a nwfilter filter(*). + NWFilters can be updated at runtime and changes are applied + immediately, so changes to security groups can be applied at + runtime (as mandated by the spec). + + Security group rules are named "nova-secgroup-" where + is the internal id of the security group. They're applied only on + hosts that have instances in the security group in question. + + Updates to security groups are done by updating the data model + (in response to API calls) followed by a request sent to all + the nodes with instances in the security group to refresh the + security group. + + Outstanding questions: + + The name is unique, so would there be any good reason to sync + the uuid across the nodes (by assigning it from the datamodel)? + + + (*) This sentence brought to you by the redundancy department of + redundancy. + """ + + def __init__(self): + pass + + def nova_base_filter(self): + return ''' + 26717364-50cf-42d1-8185-29bf893ab110 + + + + + + +''' + + def security_group_to_nwfilter_xml(self, security_group_id): + security_group = db.security_group_get_by_id({}, security_group_id) + rule_xml = "" + for rule in security_group.rules: + rule_xml += "" + if rule.cidr: + rule_xml += ("") % \ + (rule.cidr, rule.protocol, + rule.from_port, rule.to_port) + rule_xml += "" + xml = '''%s''' % (security_group_id, rule_xml,) + return xml diff --git a/run_tests.py b/run_tests.py index d5dc5f934..75ab561a1 100644 --- a/run_tests.py +++ b/run_tests.py @@ -62,6 +62,7 @@ from nova.tests.rpc_unittest import * from nova.tests.service_unittest import * from nova.tests.validator_unittest import * from nova.tests.volume_unittest import * +from nova.tests.virt_unittest import * FLAGS = flags.FLAGS -- cgit From fffa02ac32055650b2bfffff090ec7d52c86291a Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 15:32:56 +0200 Subject: Adjust a few things to make the unit tests happy again. --- nova/endpoint/cloud.py | 2 +- nova/tests/virt_unittest.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 3334f09af..930274aed 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -348,7 +348,7 @@ class CloudController(object): @rbac.allow('netadmin') def delete_security_group(self, context, group_name, **kwargs): security_group = db.security_group_get_by_user_and_name(context, context.user.id, group_name) - security_group.delete() + db.security_group_destroy(context, security_group.id) return True @rbac.allow('projectmanager', 'sysadmin') diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index b8dcec12b..1f573c463 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -16,6 +16,7 @@ from xml.dom.minidom import parseString +from nova import db from nova import flags from nova import test from nova.endpoint import cloud @@ -91,7 +92,10 @@ class NWFilterTestCase(test.TrialTestCase): cidr_ip='0.0.0.0/0') fw = libvirt_conn.NWFilterFirewall() - xml = fw.security_group_to_nwfilter_xml(1) + + security_group = db.security_group_get_by_user_and_name({}, 'fake', 'testgroup') + + xml = fw.security_group_to_nwfilter_xml(security_group.id) dom = parseString(xml) self.assertEqual(dom.firstChild.tagName, 'filter') -- cgit From e53676bb32b70ff01ca27c310e558b651590be3d Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Fri, 10 Sep 2010 15:26:13 -0700 Subject: Refactored to security group api to support projects --- nova/auth/manager.py | 2 -- nova/db/api.py | 34 ++++++++++-------- nova/db/sqlalchemy/api.py | 76 ++++++++++++++++++++++++---------------- nova/db/sqlalchemy/models.py | 22 ++++++------ nova/endpoint/cloud.py | 83 +++++++++++++++++++++++++++----------------- nova/tests/api_unittest.py | 1 + nova/tests/virt_unittest.py | 4 ++- nova/virt/libvirt_conn.py | 2 +- 8 files changed, 133 insertions(+), 91 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 281e2d8f0..34aa73bf6 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -649,8 +649,6 @@ class AuthManager(object): def delete_user(self, user): """Deletes a user""" with self.driver() as drv: - for security_group in db.security_group_get_by_user(context = {}, user_id=User.safe_id(user)): - db.security_group_destroy({}, security_group.id) drv.delete_user(User.safe_id(user)) def generate_key_pair(self, user, key_name): diff --git a/nova/db/api.py b/nova/db/api.py index 2bcf0bd2b..cdbd15486 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -442,33 +442,39 @@ def volume_update(context, volume_id, values): """ return IMPL.volume_update(context, volume_id, values) + #################### -def security_group_create(context, values): - """Create a new security group""" - return IMPL.security_group_create(context, values) +def security_group_get_all(context): + """Get all security groups""" + return IMPL.security_group_get_all(context) -def security_group_get_by_id(context, security_group_id): +def security_group_get(context, security_group_id): """Get security group by its internal id""" - return IMPL.security_group_get_by_id(context, security_group_id) + return IMPL.security_group_get(context, security_group_id) -def security_group_get_by_instance(context, instance_id): - """Get security groups to which the instance is assigned""" - return IMPL.security_group_get_by_instance(context, instance_id) +def security_group_get_by_name(context, project_id, group_name): + """Returns a security group with the specified name from a project""" + return IMPL.securitygroup_get_by_name(context, project_id, group_name) -def security_group_get_by_user(context, user_id): - """Get security groups owned by the given user""" - return IMPL.security_group_get_by_user(context, user_id) +def security_group_get_by_project(context, project_id): + """Get all security groups belonging to a project""" + return IMPL.securitygroup_get_by_project(context, project_id) -def security_group_get_by_user_and_name(context, user_id, name): - """Get user's named security group""" - return IMPL.security_group_get_by_user_and_name(context, user_id, name) +def security_group_get_by_instance(context, instance_id): + """Get security groups to which the instance is assigned""" + return IMPL.security_group_get_by_instance(context, instance_id) + +def security_group_create(context, values): + """Create a new security group""" + return IMPL.security_group_create(context, values) + def security_group_destroy(context, security_group_id): """Deletes a security group""" diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 1c95efd83..61d733940 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -616,20 +616,45 @@ def volume_update(_context, volume_id, values): ################### -def security_group_create(_context, values): - security_group_ref = models.SecurityGroup() - for (key, value) in values.iteritems(): - security_group_ref[key] = value - security_group_ref.save() - return security_group_ref +def security_group_get_all(_context): + session = get_session() + return session.query(models.SecurityGroup + ).options(eagerload('rules') + ).filter_by(deleted=False + ).all() -def security_group_get_by_id(_context, security_group_id): +def security_group_get(_context, security_group_id): session = get_session() with session.begin(): return session.query(models.SecurityGroup + ).options(eagerload('rules') + ).get(security_group_id) + + +def securitygroup_get_by_name(context, project_id, group_name): + session = get_session() + group_ref = session.query(models.SecurityGroup ).options(eagerload('rules') - ).get(security_group_id) + ).filter_by(project_id=project_id + ).filter_by(name=group_name + ).filter_by(deleted=False + ).first() + if not group_ref: + raise exception.NotFound( + 'No security group named %s for project: %s' \ + % (group_name, project_id)) + + return group_ref + + +def securitygroup_get_by_project(_context, project_id): + session = get_session() + return session.query(models.SecurityGroup + ).options(eagerload('rules') + ).filter_by(project_id=project_id + ).filter_by(deleted=False + ).all() def security_group_get_by_instance(_context, instance_id): @@ -638,34 +663,27 @@ def security_group_get_by_instance(_context, instance_id): return session.query(models.Instance ).get(instance_id ).security_groups \ - .all() + .filter_by(deleted=False + ).all() -def security_group_get_by_user(_context, user_id): - session = get_session() - with session.begin(): - return session.query(models.SecurityGroup - ).filter_by(user_id=user_id - ).filter_by(deleted=False - ).options(eagerload('rules') - ).all() +def security_group_create(_context, values): + security_group_ref = models.SecurityGroup() + for (key, value) in values.iteritems(): + security_group_ref[key] = value + security_group_ref.save() + return security_group_ref -def security_group_get_by_user_and_name(_context, user_id, name): - session = get_session() - with session.begin(): - return session.query(models.SecurityGroup - ).filter_by(user_id=user_id - ).filter_by(name=name - ).filter_by(deleted=False - ).options(eagerload('rules') - ).one() def security_group_destroy(_context, security_group_id): session = get_session() with session.begin(): - security_group = session.query(models.SecurityGroup - ).get(security_group_id) - security_group.delete(session=session) + # TODO(vish): do we have to use sql here? + session.execute('update security_group set deleted=1 where id=:id', + {'id': security_group_id}) + session.execute('update security_group_rule set deleted=1 ' + 'where group_id=:id', + {'id': security_group_id}) ################### diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index f27520aa8..3c4b9ddd7 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -306,26 +306,23 @@ class SecurityGroup(BASE, NovaBase): class SecurityGroupIngressRule(BASE, NovaBase): """Represents a rule in a security group""" - __tablename__ = 'security_group_rules' + __tablename__ = 'security_group_rule' id = Column(Integer, primary_key=True) - parent_group_id = Column(Integer, ForeignKey('security_group.id')) - parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id, - primaryjoin=parent_group_id==SecurityGroup.id) + group_id = Column(Integer, ForeignKey('security_group.id')) + group = relationship("SecurityGroup", backref="rules", + foreign_keys=group_id, + primaryjoin=group_id==SecurityGroup.id) protocol = Column(String(5)) # "tcp", "udp", or "icmp" from_port = Column(Integer) to_port = Column(Integer) + cidr = Column(String(255)) # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. - group_id = Column(Integer, ForeignKey('security_group.id')) - - @property - def user(self): - return auth.manager.AuthManager().get_user(self.user_id) + source_group_id = Column(Integer, ForeignKey('security_group.id')) - cidr = Column(String(255)) class Network(BASE, NovaBase): """Represents a network""" @@ -430,8 +427,9 @@ class FloatingIp(BASE, NovaBase): def register_models(): """Register Models and create metadata""" from sqlalchemy import create_engine - models = (Service, Instance, Volume, ExportDevice, - FixedIp, FloatingIp, Network, NetworkIndex) # , Image, Host) + models = (Service, Instance, Volume, ExportDevice, FixedIp, FloatingIp, + Network, NetworkIndex, SecurityGroup, SecurityGroupIngressRule) + # , Image, Host engine = create_engine(FLAGS.sql_connection, echo=False) for model in models: model.metadata.create_all(engine) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 930274aed..4cb09bedb 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -216,7 +216,8 @@ class CloudController(object): @rbac.allow('all') def describe_security_groups(self, context, **kwargs): groups = [] - for group in db.security_group_get_by_user(context, context.user.id): + for group in db.security_group_get_by_project(context, + context.project.id): group_dict = {} group_dict['groupDescription'] = group.description group_dict['groupName'] = group.name @@ -229,10 +230,11 @@ class CloudController(object): rule_dict['toPort'] = rule.to_port rule_dict['groups'] = [] rule_dict['ipRanges'] = [] + import pdb; pdb.set_trace() if rule.group_id: - foreign_group = db.security_group_get_by_id({}, rule.group_id) - rule_dict['groups'] += [ { 'groupName': foreign_group.name, - 'userId': foreign_group.user_id } ] + source_group = db.security_group_get(context, rule.group_id) + rule_dict['groups'] += [ { 'groupName': source_group.name, + 'userId': source_group.user_id } ] else: rule_dict['ipRanges'] += [ { 'cidrIp': rule.cidr } ] group_dict['ipPermissions'] += [ rule_dict ] @@ -258,23 +260,22 @@ class CloudController(object): user_id=None, source_security_group_name=None, source_security_group_owner_id=None): - security_group = db.security_group_get_by_user_and_name(context, - context.user.id, - group_name) + security_group = db.security_group_get_by_name(context, + context.project.id, + group_name) criteria = {} if source_security_group_name: - if source_security_group_owner_id: - other_user_id = source_security_group_owner_id - else: - other_user_id = context.user.id - - foreign_security_group = \ - db.security_group_get_by_user_and_name(context, - other_user_id, - source_security_group_name) - criteria['group_id'] = foreign_security_group.id + source_project_id = self._get_source_project_id(context, + source_security_group_owner_id) + + source_security_group = \ + db.security_group_get_by_name(context, + source_project_id, + source_security_group_name) + + criteria['group_id'] = source_security_group.id elif cidr_ip: criteria['cidr'] = cidr_ip else: @@ -303,22 +304,20 @@ class CloudController(object): ip_protocol=None, cidr_ip=None, source_security_group_name=None, source_security_group_owner_id=None): - security_group = db.security_group_get_by_user_and_name(context, - context.user.id, - group_name) - values = { 'parent_group_id' : security_group.id } + security_group = db.security_group_get_by_name(context, + context.project.id, + group_name) + values = { 'group_id' : security_group.id } if source_security_group_name: - if source_security_group_owner_id: - other_user_id = source_security_group_owner_id - else: - other_user_id = context.user.id - - foreign_security_group = \ - db.security_group_get_by_user_and_name(context, - other_user_id, - source_security_group_name) - values['group_id'] = foreign_security_group.id + source_project_id = self._get_source_project_id(context, + source_security_group_owner_id) + + source_security_group = \ + db.security_group_get_by_name(context, + source_project_id, + source_security_group_name) + values['source_group_id'] = source_security_group.id elif cidr_ip: values['cidr'] = cidr_ip else: @@ -336,18 +335,38 @@ class CloudController(object): security_group_rule = db.security_group_rule_create(context, values) return True + + def _get_source_project_id(self, context, source_security_group_owner_id): + if source_security_group_owner_id: + # Parse user:project for source group. + source_parts = source_security_group_owner_id.split(':') + + # If no project name specified, assume it's same as user name. + # Since we're looking up by project name, the user name is not + # used here. It's only read for EC2 API compatibility. + if len(source_parts) == 2: + source_project_id = parts[1] + else: + source_project_id = parts[0] + else: + source_project_id = context.project.id + + return source_project_id @rbac.allow('netadmin') def create_security_group(self, context, group_name, group_description): db.security_group_create(context, values = { 'user_id' : context.user.id, + 'project_id': context.project.id, 'name': group_name, 'description': group_description }) return True @rbac.allow('netadmin') def delete_security_group(self, context, group_name, **kwargs): - security_group = db.security_group_get_by_user_and_name(context, context.user.id, group_name) + security_group = db.security_group_get_by_name(context, + context.project.id, + group_name) db.security_group_destroy(context, security_group.id) return True diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 7e914e6f5..55b7cb4d8 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -304,6 +304,7 @@ class ApiEc2TestCase(test.BaseTestCase): # be good enough for that. for group in rv: if group.name == security_group_name: + import pdb; pdb.set_trace() self.assertEquals(len(group.rules), 1) self.assertEquals(int(group.rules[0].from_port), 80) self.assertEquals(int(group.rules[0].to_port), 81) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 1f573c463..dceced3a9 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -86,6 +86,8 @@ class NWFilterTestCase(test.TrialTestCase): context.user = FakeContext() context.user.id = 'fake' context.user.is_superuser = lambda:True + context.project = FakeContext() + context.project.id = 'fake' cloud_controller.create_security_group(context, 'testgroup', 'test group description') cloud_controller.authorize_security_group_ingress(context, 'testgroup', from_port='80', to_port='81', ip_protocol='tcp', @@ -93,7 +95,7 @@ class NWFilterTestCase(test.TrialTestCase): fw = libvirt_conn.NWFilterFirewall() - security_group = db.security_group_get_by_user_and_name({}, 'fake', 'testgroup') + security_group = db.security_group_get_by_name({}, 'fake', 'testgroup') xml = fw.security_group_to_nwfilter_xml(security_group.id) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 6f708bb80..09c94577c 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -492,7 +492,7 @@ class NWFilterFirewall(object): ''' def security_group_to_nwfilter_xml(self, security_group_id): - security_group = db.security_group_get_by_id({}, security_group_id) + security_group = db.security_group_get({}, security_group_id) rule_xml = "" for rule in security_group.rules: rule_xml += "" -- cgit From 60b6b06d15ed620cf990db10277c4126b686de80 Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Fri, 10 Sep 2010 19:19:08 -0700 Subject: Finished security group / project refactor --- nova/auth/manager.py | 20 ++++++++++++++++---- nova/db/sqlalchemy/api.py | 2 +- nova/db/sqlalchemy/models.py | 12 ++++++------ nova/endpoint/cloud.py | 5 ++--- nova/tests/api_unittest.py | 2 +- 5 files changed, 26 insertions(+), 15 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 34aa73bf6..48d314ae6 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -531,6 +531,12 @@ class AuthManager(object): except: drv.delete_project(project.id) raise + + db.security_group_create(context={}, + values={ 'name': 'default', + 'description': 'default', + 'user_id': manager_user, + 'project_id': project.id }) return project def add_to_project(self, user, project): @@ -586,6 +592,16 @@ class AuthManager(object): except: logging.exception('Could not destroy network for %s', project) + try: + project_id = Project.safe_id(project) + groups = db.security_group_get_by_project(context={}, + project_id=project_id) + for group in groups: + db.security_group_destroy({}, group.id) + except: + logging.exception('Could not destroy security groups for %s', + project) + with self.driver() as drv: drv.delete_project(Project.safe_id(project)) @@ -640,10 +656,6 @@ class AuthManager(object): with self.driver() as drv: user_dict = drv.create_user(name, access, secret, admin) if user_dict: - db.security_group_create(context={}, - values={ 'name' : 'default', - 'description' : 'default', - 'user_id' : name }) return User(**user_dict) def delete_user(self, user): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 61d733940..f3d4b68c4 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -681,7 +681,7 @@ def security_group_destroy(_context, security_group_id): # TODO(vish): do we have to use sql here? session.execute('update security_group set deleted=1 where id=:id', {'id': security_group_id}) - session.execute('update security_group_rule set deleted=1 ' + session.execute('update security_group_rules set deleted=1 ' 'where group_id=:id', {'id': security_group_id}) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 3c4b9ddd7..e79a0415b 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -306,13 +306,13 @@ class SecurityGroup(BASE, NovaBase): class SecurityGroupIngressRule(BASE, NovaBase): """Represents a rule in a security group""" - __tablename__ = 'security_group_rule' + __tablename__ = 'security_group_rules' id = Column(Integer, primary_key=True) - group_id = Column(Integer, ForeignKey('security_group.id')) - group = relationship("SecurityGroup", backref="rules", - foreign_keys=group_id, - primaryjoin=group_id==SecurityGroup.id) + parent_group_id = Column(Integer, ForeignKey('security_group.id')) + parent_group = relationship("SecurityGroup", backref="rules", + foreign_keys=parent_group_id, + primaryjoin=parent_group_id==SecurityGroup.id) protocol = Column(String(5)) # "tcp", "udp", or "icmp" from_port = Column(Integer) @@ -321,7 +321,7 @@ class SecurityGroupIngressRule(BASE, NovaBase): # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. - source_group_id = Column(Integer, ForeignKey('security_group.id')) + group_id = Column(Integer, ForeignKey('security_group.id')) class Network(BASE, NovaBase): diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 4cb09bedb..a26f90753 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -230,7 +230,6 @@ class CloudController(object): rule_dict['toPort'] = rule.to_port rule_dict['groups'] = [] rule_dict['ipRanges'] = [] - import pdb; pdb.set_trace() if rule.group_id: source_group = db.security_group_get(context, rule.group_id) rule_dict['groups'] += [ { 'groupName': source_group.name, @@ -307,7 +306,7 @@ class CloudController(object): security_group = db.security_group_get_by_name(context, context.project.id, group_name) - values = { 'group_id' : security_group.id } + values = { 'parent_group_id' : security_group.id } if source_security_group_name: source_project_id = self._get_source_project_id(context, @@ -317,7 +316,7 @@ class CloudController(object): db.security_group_get_by_name(context, source_project_id, source_security_group_name) - values['source_group_id'] = source_security_group.id + values['group_id'] = source_security_group.id elif cidr_ip: values['cidr'] = cidr_ip else: diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py index 55b7cb4d8..70669206c 100644 --- a/nova/tests/api_unittest.py +++ b/nova/tests/api_unittest.py @@ -304,7 +304,6 @@ class ApiEc2TestCase(test.BaseTestCase): # be good enough for that. for group in rv: if group.name == security_group_name: - import pdb; pdb.set_trace() self.assertEquals(len(group.rules), 1) self.assertEquals(int(group.rules[0].from_port), 80) self.assertEquals(int(group.rules[0].to_port), 81) @@ -369,6 +368,7 @@ class ApiEc2TestCase(test.BaseTestCase): self.mox.ReplayAll() rv = self.ec2.get_all_security_groups() + # I don't bother checkng that we actually find it here, # because the create/delete unit test further up should # be good enough for that. -- cgit From edccf3f6cf95a4869d7900032a5a6c8eaa65cd18 Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Sat, 11 Sep 2010 02:35:25 +0000 Subject: Fixed manager_user reference in create_project --- nova/auth/manager.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 48d314ae6..5529515a6 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -531,12 +531,12 @@ class AuthManager(object): except: drv.delete_project(project.id) raise - - db.security_group_create(context={}, - values={ 'name': 'default', - 'description': 'default', - 'user_id': manager_user, - 'project_id': project.id }) + + values = {'name': 'default', + 'description': 'default', + 'user_id': User.safe_id(manager_user), + 'project_id': project.id} + db.security_group_create({}, values) return project def add_to_project(self, user, project): -- cgit From f24f20948cf7e6cc0e14c2b1fc41a61d8d2fa34c Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Sat, 11 Sep 2010 11:19:22 -0700 Subject: Security Group API layer cleanup --- nova/db/api.py | 5 +++ nova/db/sqlalchemy/api.py | 11 +++++++ nova/endpoint/cloud.py | 84 ++++++++++++++++++++++++----------------------- 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index cdbd15486..cf39438c2 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -471,6 +471,11 @@ def security_group_get_by_instance(context, instance_id): return IMPL.security_group_get_by_instance(context, instance_id) +def securitygroup_exists(context, project_id, group_name): + """Indicates if a group name exists in a project""" + return IMPL.securitygroup_exists(context, project_id, group_name) + + def security_group_create(context, values): """Create a new security group""" return IMPL.security_group_create(context, values) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index f3d4b68c4..513b47bc9 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -667,8 +667,19 @@ def security_group_get_by_instance(_context, instance_id): ).all() +def securitygroup_exists(_context, project_id, group_name): + try: + group = securitygroup_get_by_name(_context, project_id, group_name) + return group != None + except exception.NotFound: + return False + + def security_group_create(_context, values): security_group_ref = models.SecurityGroup() + # FIXME(devcamcar): Unless I do this, rules fails with lazy load exception + # once save() is called. This will get cleaned up in next orm pass. + security_group_ref.rules for (key, value) in values.iteritems(): security_group_ref[key] = value security_group_ref.save() diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index a26f90753..7408e02e9 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -214,43 +214,40 @@ class CloudController(object): return True @rbac.allow('all') - def describe_security_groups(self, context, **kwargs): - groups = [] - for group in db.security_group_get_by_project(context, - context.project.id): - group_dict = {} - group_dict['groupDescription'] = group.description - group_dict['groupName'] = group.name - group_dict['ownerId'] = context.user.id - group_dict['ipPermissions'] = [] - for rule in group.rules: - rule_dict = {} - rule_dict['ipProtocol'] = rule.protocol - rule_dict['fromPort'] = rule.from_port - rule_dict['toPort'] = rule.to_port - rule_dict['groups'] = [] - rule_dict['ipRanges'] = [] - if rule.group_id: - source_group = db.security_group_get(context, rule.group_id) - rule_dict['groups'] += [ { 'groupName': source_group.name, - 'userId': source_group.user_id } ] - else: - rule_dict['ipRanges'] += [ { 'cidrIp': rule.cidr } ] - group_dict['ipPermissions'] += [ rule_dict ] - groups += [ group_dict ] + def describe_security_groups(self, context, group_name=None, **kwargs): + if context.user.is_admin(): + groups = db.security_group_get_all(context) + else: + groups = db.security_group_get_by_project(context, + context.project.id) + groups = [self._format_security_group(context, g) for g in groups] + if not group_name is None: + groups = [g for g in groups if g.name in group_name] return {'securityGroupInfo': groups } -# -# [{ 'groupDescription': group.description, -# 'groupName' : group.name, -# 'ownerId': context.user.id, -# 'ipPermissions' : [ -# { 'ipProtocol' : rule.protocol, -# 'fromPort' : rule.from_port, -# 'toPort' : rule.to_port, -# 'ipRanges' : [ { 'cidrIp' : rule.cidr } ] } for rule in group.rules ] } for group in \ -# -# return groups + + def _format_security_group(self, context, group): + g = {} + g['groupDescription'] = group.description + g['groupName'] = group.name + g['ownerId'] = context.user.id + g['ipPermissions'] = [] + for rule in group.rules: + r = {} + r['ipProtocol'] = rule.protocol + r['fromPort'] = rule.from_port + r['toPort'] = rule.to_port + r['groups'] = [] + r['ipRanges'] = [] + if rule.group_id: + source_group = db.security_group_get(context, rule.group_id) + r['groups'] += [{'groupName': source_group.name, + 'userId': source_group.user_id}] + else: + r['ipRanges'] += [{'cidrIp': rule.cidr}] + g['ipPermissions'] += [r] + return g + @rbac.allow('netadmin') def revoke_security_group_ingress(self, context, group_name, @@ -354,12 +351,17 @@ class CloudController(object): @rbac.allow('netadmin') def create_security_group(self, context, group_name, group_description): - db.security_group_create(context, - values = { 'user_id' : context.user.id, - 'project_id': context.project.id, - 'name': group_name, - 'description': group_description }) - return True + if db.securitygroup_exists(context, context.project.id, group_name): + raise exception.ApiError('group %s already exists' % group_name) + + group = {'user_id' : context.user.id, + 'project_id': context.project.id, + 'name': group_name, + 'description': group_description} + group_ref = db.security_group_create(context, group) + + return {'securityGroupSet': [self._format_security_group(context, + group_ref)]} @rbac.allow('netadmin') def delete_security_group(self, context, group_name, **kwargs): -- cgit From 2a782110bc51f147bdb35264445badac3b3e8e65 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 11:45:28 +0200 Subject: Filters all get defined when running an instance. --- nova/db/api.py | 5 ++ nova/db/sqlalchemy/api.py | 16 +++++- nova/db/sqlalchemy/models.py | 1 - nova/tests/virt_unittest.py | 101 +++++++++++++++++++++++++++++++----- nova/virt/libvirt.qemu.xml.template | 2 +- nova/virt/libvirt.uml.xml.template | 2 +- nova/virt/libvirt_conn.py | 74 ++++++++++++++++++++++++-- 7 files changed, 179 insertions(+), 22 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index cf39438c2..1d10b1987 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -244,6 +244,11 @@ def instance_update(context, instance_id, values): return IMPL.instance_update(context, instance_id, values) +def instance_add_security_group(context, instance_id, security_group_id): + """Associate the given security group with the given instance""" + return IMPL.instance_add_security_group(context, instance_id, security_group_id) + + #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 513b47bc9..11779e30c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -238,7 +238,10 @@ def instance_destroy(_context, instance_id): def instance_get(_context, instance_id): - return models.Instance.find(instance_id) + session = get_session() + return session.query(models.Instance + ).options(eagerload('security_groups') + ).get(instance_id) def instance_get_all(_context): @@ -317,6 +320,17 @@ def instance_update(_context, instance_id, values): instance_ref.save(session=session) +def instance_add_security_group(context, instance_id, security_group_id): + """Associate the given security group with the given instance""" + session = get_session() + with session.begin(): + instance_ref = models.Instance.find(instance_id, session=session) + security_group_ref = models.SecurityGroup.find(security_group_id, + session=session) + instance_ref.security_groups += [security_group_ref] + instance_ref.save(session=session) + + ################### diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index e79a0415b..424906c1f 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -215,7 +215,6 @@ class Instance(BASE, NovaBase): launch_index = Column(Integer) key_name = Column(String(255)) key_data = Column(Text) - security_group = Column(String(255)) state = Column(Integer) state_description = Column(String(255)) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index dceced3a9..a61849a72 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -77,27 +77,39 @@ class LibvirtConnTestCase(test.TrialTestCase): class NWFilterTestCase(test.TrialTestCase): - def test_stuff(self): - cloud_controller = cloud.CloudController() - class FakeContext(object): + def setUp(self): + super(NWFilterTestCase, self).setUp() + + class Mock(object): pass - context = FakeContext() - context.user = FakeContext() - context.user.id = 'fake' - context.user.is_superuser = lambda:True - context.project = FakeContext() - context.project.id = 'fake' - cloud_controller.create_security_group(context, 'testgroup', 'test group description') - cloud_controller.authorize_security_group_ingress(context, 'testgroup', from_port='80', - to_port='81', ip_protocol='tcp', + self.context = Mock() + self.context.user = Mock() + self.context.user.id = 'fake' + self.context.user.is_superuser = lambda:True + self.context.project = Mock() + self.context.project.id = 'fake' + + self.fake_libvirt_connection = Mock() + + self.fw = libvirt_conn.NWFilterFirewall(self.fake_libvirt_connection) + + def test_cidr_rule_nwfilter_xml(self): + cloud_controller = cloud.CloudController() + cloud_controller.create_security_group(self.context, + 'testgroup', + 'test group description') + cloud_controller.authorize_security_group_ingress(self.context, + 'testgroup', + from_port='80', + to_port='81', + ip_protocol='tcp', cidr_ip='0.0.0.0/0') - fw = libvirt_conn.NWFilterFirewall() security_group = db.security_group_get_by_name({}, 'fake', 'testgroup') - xml = fw.security_group_to_nwfilter_xml(security_group.id) + xml = self.fw.security_group_to_nwfilter_xml(security_group.id) dom = parseString(xml) self.assertEqual(dom.firstChild.tagName, 'filter') @@ -117,3 +129,64 @@ class NWFilterTestCase(test.TrialTestCase): self.assertEqual(ip_conditions[0].getAttribute('protocol'), 'tcp') self.assertEqual(ip_conditions[0].getAttribute('dstportstart'), '80') self.assertEqual(ip_conditions[0].getAttribute('dstportend'), '81') + + + self.teardown_security_group() + + def teardown_security_group(self): + cloud_controller = cloud.CloudController() + cloud_controller.delete_security_group(self.context, 'testgroup') + + + def setup_and_return_security_group(self): + cloud_controller = cloud.CloudController() + cloud_controller.create_security_group(self.context, + 'testgroup', + 'test group description') + cloud_controller.authorize_security_group_ingress(self.context, + 'testgroup', + from_port='80', + to_port='81', + ip_protocol='tcp', + cidr_ip='0.0.0.0/0') + + return db.security_group_get_by_name({}, 'fake', 'testgroup') + + def test_creates_base_rule_first(self): + self.defined_filters = [] + self.fake_libvirt_connection.listNWFilter = lambda:self.defined_filters + self.base_filter_defined = False + self.i = 0 + def _filterDefineXMLMock(xml): + dom = parseString(xml) + name = dom.firstChild.getAttribute('name') + if self.i == 0: + self.assertEqual(dom.firstChild.getAttribute('name'), + 'nova-base-filter') + elif self.i == 1: + self.assertTrue(name.startswith('nova-secgroup-'), + 'unexpected name: %s' % name) + elif self.i == 2: + self.assertTrue(name.startswith('nova-instance-'), + 'unexpected name: %s' % name) + + self.defined_filters.append(name) + self.i += 1 + return True + + def _ensure_all_called(_): + self.assertEqual(self.i, 3) + + self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock + + inst_id = db.instance_create({}, { 'user_id' : 'fake', 'project_id' : 'fake' }) + security_group = self.setup_and_return_security_group() + + db.instance_add_security_group({}, inst_id, security_group.id) + instance = db.instance_get({}, inst_id) + + d = self.fw.setup_nwfilters_for_instance(instance) + d.addCallback(_ensure_all_called) + d.addCallback(lambda _:self.teardown_security_group()) + + return d diff --git a/nova/virt/libvirt.qemu.xml.template b/nova/virt/libvirt.qemu.xml.template index 5d3755b65..cbf501f9c 100644 --- a/nova/virt/libvirt.qemu.xml.template +++ b/nova/virt/libvirt.qemu.xml.template @@ -20,7 +20,7 @@ - + diff --git a/nova/virt/libvirt.uml.xml.template b/nova/virt/libvirt.uml.xml.template index 1000da5ab..2030b87d2 100644 --- a/nova/virt/libvirt.uml.xml.template +++ b/nova/virt/libvirt.uml.xml.template @@ -14,7 +14,7 @@ - + diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 09c94577c..89ede1d1a 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -27,6 +27,7 @@ import shutil from twisted.internet import defer from twisted.internet import task +from twisted.internet import threads from nova import db from nova import exception @@ -216,6 +217,7 @@ class LibvirtConnection(object): instance['id'], power_state.NOSTATE, 'launching') + yield NWFilterFirewall(self._conn).setup_nwfilters_for_instance(instance) yield self._create_image(instance, xml) yield self._conn.createXML(xml, 0) # TODO(termie): this should actually register @@ -442,7 +444,6 @@ class LibvirtConnection(object): domain = self._conn.lookupByName(instance_name) return domain.interfaceStats(interface) - class NWFilterFirewall(object): """ This class implements a network filtering mechanism versatile @@ -467,6 +468,14 @@ class NWFilterFirewall(object): the nodes with instances in the security group to refresh the security group. + Each instance has its own NWFilter, which references the above + mentioned security group NWFilters. This was done because + interfaces can only reference one filter while filters can + reference multiple other filters. This has the added benefit of + actually being able to add and remove security groups from an + instance at run time. This functionality is not exposed anywhere, + though. + Outstanding questions: The name is unique, so would there be any good reason to sync @@ -477,12 +486,14 @@ class NWFilterFirewall(object): redundancy. """ - def __init__(self): - pass + def __init__(self, get_connection): + self._conn = get_connection + def nova_base_filter(self): return ''' 26717364-50cf-42d1-8185-29bf893ab110 + @@ -491,6 +502,60 @@ class NWFilterFirewall(object): ''' + + def setup_nwfilters_for_instance(self, instance): + """ + Creates an NWFilter for the given instance. In the process, + it makes sure the filters for the security groups as well as + the base filter are all in place. + """ + + d = self.ensure_base_filter() + + nwfilter_xml = ("\n" + + " \n" + ) % instance['name'] + + for security_group in instance.security_groups: + d.addCallback(lambda _:self.ensure_security_group_filter(security_group.id)) + + nwfilter_xml += (" \n" + ) % security_group.id + nwfilter_xml += "" + + d.addCallback(lambda _: threads.deferToThread( + self._conn.nwfilterDefineXML, + nwfilter_xml)) + return d + + + def _nwfilter_name_for_security_group(self, security_group_id): + return 'nova-secgroup-%d' % (security_group_id,) + + + def ensure_filter(self, name, xml_generator): + def _already_exists_check(filterlist, filter): + return filter in filterlist + def _define_if_not_exists(exists, xml_generator): + if not exists: + xml = xml_generator() + return threads.deferToThread(self._conn.nwfilterDefineXML, xml) + d = threads.deferToThread(self._conn.listNWFilter) + d.addCallback(_already_exists_check, name) + d.addCallback(_define_if_not_exists, xml_generator) + return d + + + def ensure_base_filter(self): + return self.ensure_filter('nova-base-filter', self.nova_base_filter) + + + def ensure_security_group_filter(self, security_group_id): + return self.ensure_filter( + self._nwfilter_name_for_security_group(security_group_id), + lambda:self.security_group_to_nwfilter_xml(security_group_id)) + + def security_group_to_nwfilter_xml(self, security_group_id): security_group = db.security_group_get({}, security_group_id) rule_xml = "" @@ -498,7 +563,8 @@ class NWFilterFirewall(object): rule_xml += "" if rule.cidr: rule_xml += ("") % \ + "dstportstart='%s' dstportend='%s' />" + + "priority='900'\n") % \ (rule.cidr, rule.protocol, rule.from_port, rule.to_port) rule_xml += "" -- cgit From 077fc783c4f94de427da98818d262aeb09a31044 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 12:04:06 +0200 Subject: (Untested) Make changes to security group rules propagate to the relevant compute nodes. --- nova/compute/manager.py | 5 +++++ nova/endpoint/cloud.py | 20 +++++++++++++++++--- nova/virt/libvirt_conn.py | 37 ++++++++++++++++++++++++------------- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5f7a94106..a00fd9baa 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -61,6 +61,11 @@ class ComputeManager(manager.Manager): state = self.driver.get_info(instance_ref.name)['state'] self.db.instance_set_state(context, instance_id, state) + @defer.inlineCallbacks + @exception.wrap_exception + def refresh_security_group(self, context, security_group_id, **_kwargs): + self.driver.refresh_security_group(security_group_id) + @defer.inlineCallbacks @exception.wrap_exception def run_instance(self, context, instance_id, **_kwargs): diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 7408e02e9..1403a62f6 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -93,6 +93,14 @@ class CloudController(object): result[instance['key_name']] = [line] return result + def _refresh_security_group(self, security_group): + nodes = set([instance.host for instance in security_group.instances]) + for node in nodes: + rpc.call('%s.%s' % (FLAGS.compute_topic, node), + { "method": "refresh_security_group", + "args": { "context": None, + "security_group_id": security_group.id}}) + def get_metadata(self, address): instance_ref = db.fixed_ip_get_instance(None, address) if instance_ref is None: @@ -265,12 +273,12 @@ class CloudController(object): if source_security_group_name: source_project_id = self._get_source_project_id(context, source_security_group_owner_id) - + source_security_group = \ db.security_group_get_by_name(context, source_project_id, source_security_group_name) - + criteria['group_id'] = source_security_group.id elif cidr_ip: criteria['cidr'] = cidr_ip @@ -292,6 +300,9 @@ class CloudController(object): break # If we make it here, we have a match db.security_group_rule_destroy(context, rule.id) + + self._refresh_security_group(security_group) + return True @rbac.allow('netadmin') @@ -330,8 +341,11 @@ class CloudController(object): return None security_group_rule = db.security_group_rule_create(context, values) + + self._refresh_security_group(security_group) + return True - + def _get_source_project_id(self, context, source_security_group_owner_id): if source_security_group_owner_id: # Parse user:project for source group. diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 89ede1d1a..a343267dc 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -444,6 +444,12 @@ class LibvirtConnection(object): domain = self._conn.lookupByName(instance_name) return domain.interfaceStats(interface) + + def refresh_security_group(self, security_group_id): + fw = self.NWFilterFirewall(self._conn) + fw.ensure_security_group_filter(security_group_id, override=True) + + class NWFilterFirewall(object): """ This class implements a network filtering mechanism versatile @@ -533,27 +539,32 @@ class NWFilterFirewall(object): return 'nova-secgroup-%d' % (security_group_id,) - def ensure_filter(self, name, xml_generator): - def _already_exists_check(filterlist, filter): - return filter in filterlist - def _define_if_not_exists(exists, xml_generator): - if not exists: - xml = xml_generator() - return threads.deferToThread(self._conn.nwfilterDefineXML, xml) - d = threads.deferToThread(self._conn.listNWFilter) - d.addCallback(_already_exists_check, name) + def define_filter(self, name, xml_generator, override=False): + if not override: + def _already_exists_check(filterlist, filter): + return filter in filterlist + def _define_if_not_exists(exists, xml_generator): + if not exists: + xml = xml_generator() + return threads.deferToThread(self._conn.nwfilterDefineXML, xml) + d = threads.deferToThread(self._conn.listNWFilter) + d.addCallback(_already_exists_check, name) + else: + # Pretend we looked it up and it wasn't defined + d = defer.succeed(False) d.addCallback(_define_if_not_exists, xml_generator) return d def ensure_base_filter(self): - return self.ensure_filter('nova-base-filter', self.nova_base_filter) + return self.define_filter('nova-base-filter', self.nova_base_filter) - def ensure_security_group_filter(self, security_group_id): - return self.ensure_filter( + def ensure_security_group_filter(self, security_group_id, override=False): + return self.define_filter( self._nwfilter_name_for_security_group(security_group_id), - lambda:self.security_group_to_nwfilter_xml(security_group_id)) + lambda:self.security_group_to_nwfilter_xml(security_group_id), + override=override) def security_group_to_nwfilter_xml(self, security_group_id): -- cgit From b15bde79b71e474d96674c8eae4108ac9c063731 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 14:18:08 +0200 Subject: Fix call to listNWFilters --- nova/tests/virt_unittest.py | 2 +- nova/virt/libvirt_conn.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index a61849a72..8cafa778e 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -154,7 +154,7 @@ class NWFilterTestCase(test.TrialTestCase): def test_creates_base_rule_first(self): self.defined_filters = [] - self.fake_libvirt_connection.listNWFilter = lambda:self.defined_filters + self.fake_libvirt_connection.listNWFilters = lambda:self.defined_filters self.base_filter_defined = False self.i = 0 def _filterDefineXMLMock(xml): diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index a343267dc..2e1dfcefc 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -547,7 +547,7 @@ class NWFilterFirewall(object): if not exists: xml = xml_generator() return threads.deferToThread(self._conn.nwfilterDefineXML, xml) - d = threads.deferToThread(self._conn.listNWFilter) + d = threads.deferToThread(self._conn.listNWFilters) d.addCallback(_already_exists_check, name) else: # Pretend we looked it up and it wasn't defined -- cgit From 9c4b6612e65d548542b1bf37373200e4e6abc98d Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 14:20:32 +0200 Subject: Correctly pass ip_address to templates. --- nova/virt/libvirt.qemu.xml.template | 4 ++-- nova/virt/libvirt.uml.xml.template | 4 ++-- nova/virt/libvirt_conn.py | 4 +++- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/nova/virt/libvirt.qemu.xml.template b/nova/virt/libvirt.qemu.xml.template index cbf501f9c..d02aa9114 100644 --- a/nova/virt/libvirt.qemu.xml.template +++ b/nova/virt/libvirt.qemu.xml.template @@ -20,8 +20,8 @@ - - + + diff --git a/nova/virt/libvirt.uml.xml.template b/nova/virt/libvirt.uml.xml.template index 2030b87d2..bf3f2f86a 100644 --- a/nova/virt/libvirt.uml.xml.template +++ b/nova/virt/libvirt.uml.xml.template @@ -14,8 +14,8 @@ - - + + diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 2e1dfcefc..00a80989f 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -322,6 +322,7 @@ class LibvirtConnection(object): network = db.project_get_network(None, instance['project_id']) # FIXME(vish): stick this in db instance_type = instance_types.INSTANCE_TYPES[instance['instance_type']] + ip_address = db.instance_get_fixed_address({}, instance['id']) xml_info = {'type': FLAGS.libvirt_type, 'name': instance['name'], 'basepath': os.path.join(FLAGS.instances_path, @@ -329,7 +330,8 @@ class LibvirtConnection(object): 'memory_kb': instance_type['memory_mb'] * 1024, 'vcpus': instance_type['vcpus'], 'bridge_name': network['bridge'], - 'mac_address': instance['mac_address']} + 'mac_address': instance['mac_address'], + 'ip_address': ip_address } libvirt_xml = self.libvirt_xml % xml_info logging.debug('instance %s: finished toXML method', instance['name']) -- cgit From 3fbbc09cbe2594e816803796e22ef39bcf02b029 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 13:01:57 +0200 Subject: Multiple security group support. --- nova/endpoint/cloud.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 1403a62f6..715470f30 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -279,7 +279,7 @@ class CloudController(object): source_project_id, source_security_group_name) - criteria['group_id'] = source_security_group.id + criteria['group_id'] = source_security_group elif cidr_ip: criteria['cidr'] = cidr_ip else: @@ -682,8 +682,16 @@ class CloudController(object): kwargs['key_name']) key_data = key_pair.public_key - # TODO: Get the real security group of launch in here - security_group = "default" + security_group_arg = kwargs.get('security_group', ["default"]) + if not type(security_group_arg) is list: + security_group_arg = [security_group_arg] + + security_groups = [] + for security_group_name in security_group_arg: + group = db.security_group_get_by_project(context, + context.project.id, + security_group_name) + security_groups.append(group) reservation_id = utils.generate_uid('r') base_options = {} @@ -697,7 +705,7 @@ class CloudController(object): base_options['project_id'] = context.project.id base_options['user_data'] = kwargs.get('user_data', '') base_options['instance_type'] = kwargs.get('instance_type', 'm1.small') - base_options['security_group'] = security_group + base_options['security_groups'] = security_groups for num in range(int(kwargs['max_count'])): inst_id = db.instance_create(context, base_options) -- cgit From 757088eb394552b0aaee61673b0af5094f01c356 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 13:22:17 +0200 Subject: Add a bunch of TODO's to the API implementation. --- nova/endpoint/cloud.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 715470f30..5dd1bd340 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -305,6 +305,18 @@ class CloudController(object): return True + # TODO(soren): Lots and lots of input validation. We're accepting + # strings here (such as ipProtocol), which is put into + # filter rules verbatim. + # TODO(soren): Dupe detection. Adding the same rule twice actually + # adds the same rule twice to the rule set, which is + # pointless. + # TODO(soren): This has only been tested with Boto as the client. + # Unfortunately, it seems Boto is using an old API + # for these operations, so support for newer API versions + # is sketchy. + # TODO(soren): De-duplicate the turning method arguments into dict stuff. + # revoke_security_group_ingress uses the exact same logic. @rbac.allow('netadmin') def authorize_security_group_ingress(self, context, group_name, to_port=None, from_port=None, @@ -350,7 +362,7 @@ class CloudController(object): if source_security_group_owner_id: # Parse user:project for source group. source_parts = source_security_group_owner_id.split(':') - + # If no project name specified, assume it's same as user name. # Since we're looking up by project name, the user name is not # used here. It's only read for EC2 API compatibility. @@ -360,14 +372,14 @@ class CloudController(object): source_project_id = parts[0] else: source_project_id = context.project.id - + return source_project_id @rbac.allow('netadmin') def create_security_group(self, context, group_name, group_description): if db.securitygroup_exists(context, context.project.id, group_name): raise exception.ApiError('group %s already exists' % group_name) - + group = {'user_id' : context.user.id, 'project_id': context.project.id, 'name': group_name, -- cgit From 01a041dd732ae9c56533f6eac25f08c34917d733 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:17:52 +0200 Subject: Fix up rule generation. It turns out nwfilter gets very, very wonky indeed if you mix rules and rules. Setting a TCP rule adds an early rule to ebtables that ends up overriding the rules which are last in that table. --- nova/db/sqlalchemy/session.py | 11 ++++++----- nova/endpoint/cloud.py | 3 +-- nova/tests/virt_unittest.py | 6 +++--- nova/virt/libvirt_conn.py | 41 +++++++++++++++++++++++++++-------------- 4 files changed, 37 insertions(+), 24 deletions(-) diff --git a/nova/db/sqlalchemy/session.py b/nova/db/sqlalchemy/session.py index 69a205378..fffbd3443 100644 --- a/nova/db/sqlalchemy/session.py +++ b/nova/db/sqlalchemy/session.py @@ -20,7 +20,7 @@ Session Handling for SQLAlchemy backend """ from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import sessionmaker, scoped_session from nova import flags @@ -36,7 +36,8 @@ def get_session(autocommit=True, expire_on_commit=False): if not _MAKER: if not _ENGINE: _ENGINE = create_engine(FLAGS.sql_connection, echo=False) - _MAKER = sessionmaker(bind=_ENGINE, - autocommit=autocommit, - expire_on_commit=expire_on_commit) - return _MAKER() + _MAKER = scoped_session(sessionmaker(bind=_ENGINE, + autocommit=autocommit, + expire_on_commit=expire_on_commit)) + session = _MAKER() + return session diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 5dd1bd340..fc83a9d1c 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -326,7 +326,7 @@ class CloudController(object): security_group = db.security_group_get_by_name(context, context.project.id, group_name) - values = { 'parent_group_id' : security_group.id } + values = { 'parent_group' : security_group } if source_security_group_name: source_project_id = self._get_source_project_id(context, @@ -349,7 +349,6 @@ class CloudController(object): else: # If cidr based filtering, protocol and ports are mandatory if 'cidr' in values: - print values return None security_group_rule = db.security_group_rule_create(context, values) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 8cafa778e..d5a6d11f8 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -118,15 +118,15 @@ class NWFilterTestCase(test.TrialTestCase): self.assertEqual(len(rules), 1) # It's supposed to allow inbound traffic. - self.assertEqual(rules[0].getAttribute('action'), 'allow') + self.assertEqual(rules[0].getAttribute('action'), 'accept') self.assertEqual(rules[0].getAttribute('direction'), 'in') # Must be lower priority than the base filter (which blocks everything) self.assertTrue(int(rules[0].getAttribute('priority')) < 1000) - ip_conditions = rules[0].getElementsByTagName('ip') + ip_conditions = rules[0].getElementsByTagName('tcp') self.assertEqual(len(ip_conditions), 1) - self.assertEqual(ip_conditions[0].getAttribute('protocol'), 'tcp') + self.assertEqual(ip_conditions[0].getAttribute('srcipaddr'), '0.0.0.0/0') self.assertEqual(ip_conditions[0].getAttribute('dstportstart'), '80') self.assertEqual(ip_conditions[0].getAttribute('dstportend'), '81') diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 00a80989f..aaa2c69b6 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -290,7 +290,6 @@ class LibvirtConnection(object): address = db.instance_get_fixed_address(None, inst['id']) with open(FLAGS.injected_network_template) as f: net = f.read() % {'address': address, - 'network': network_ref['network'], 'netmask': network_ref['netmask'], 'gateway': network_ref['gateway'], 'broadcast': network_ref['broadcast'], @@ -448,7 +447,7 @@ class LibvirtConnection(object): def refresh_security_group(self, security_group_id): - fw = self.NWFilterFirewall(self._conn) + fw = NWFilterFirewall(self._conn) fw.ensure_security_group_filter(security_group_id, override=True) @@ -541,19 +540,26 @@ class NWFilterFirewall(object): return 'nova-secgroup-%d' % (security_group_id,) + # TODO(soren): Should override be the default (and should it even + # be optional? We save a bit of processing time in + # libvirt by only defining this conditionally, but + # we still have to go and ask libvirt if the group + # is already defined, and there's the off chance of + # of inconsitencies having snuck in which would get + # fixed by just redefining the filter. def define_filter(self, name, xml_generator, override=False): if not override: def _already_exists_check(filterlist, filter): return filter in filterlist - def _define_if_not_exists(exists, xml_generator): - if not exists: - xml = xml_generator() - return threads.deferToThread(self._conn.nwfilterDefineXML, xml) d = threads.deferToThread(self._conn.listNWFilters) d.addCallback(_already_exists_check, name) else: # Pretend we looked it up and it wasn't defined d = defer.succeed(False) + def _define_if_not_exists(exists, xml_generator): + if not exists: + xml = xml_generator() + return threads.deferToThread(self._conn.nwfilterDefineXML, xml) d.addCallback(_define_if_not_exists, xml_generator) return d @@ -573,13 +579,20 @@ class NWFilterFirewall(object): security_group = db.security_group_get({}, security_group_id) rule_xml = "" for rule in security_group.rules: - rule_xml += "" + rule_xml += "" if rule.cidr: - rule_xml += ("" + - "priority='900'\n") % \ - (rule.cidr, rule.protocol, - rule.from_port, rule.to_port) - rule_xml += "" - xml = '''%s''' % (security_group_id, rule_xml,) + rule_xml += "<%s srcipaddr='%s' " % (rule.protocol, rule.cidr) + if rule.protocol in ['tcp', 'udp']: + rule_xml += "dstportstart='%s' dstportend='%s' " % \ + (rule.from_port, rule.to_port) + elif rule.protocol == 'icmp': + logging.info('rule.protocol: %r, rule.from_port: %r, rule.to_port: %r' % (rule.protocol, rule.from_port, rule.to_port)) + if rule.from_port != -1: + rule_xml += "type='%s' " % rule.from_port + if rule.to_port != -1: + rule_xml += "code='%s' " % rule.to_port + + rule_xml += '/>\n' + rule_xml += "\n" + xml = '''%s''' % (security_group_id, rule_xml,) return xml -- cgit From 65113c4aa92fa5e803bbe1ab56f7facf57753962 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:20:08 +0200 Subject: Make refresh_security_groups play well with inlineCallbacks. --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index a00fd9baa..1f3a181ff 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -64,7 +64,7 @@ class ComputeManager(manager.Manager): @defer.inlineCallbacks @exception.wrap_exception def refresh_security_group(self, context, security_group_id, **_kwargs): - self.driver.refresh_security_group(security_group_id) + yield self.driver.refresh_security_group(security_group_id) @defer.inlineCallbacks @exception.wrap_exception -- cgit From 85dbf6162d7b22991389db397f9aa1871464737f Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:22:56 +0200 Subject: Cast process input to a str. It must not be unicode, but stuff that comes out of the database might very well be unicode, so using such a value in a template makes the whole thing unicode. --- nova/process.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/process.py b/nova/process.py index 74725c157..bda8147d5 100644 --- a/nova/process.py +++ b/nova/process.py @@ -113,7 +113,7 @@ class BackRelayWithInput(protocol.ProcessProtocol): if self.started_deferred: self.started_deferred.callback(self) if self.process_input: - self.transport.write(self.process_input) + self.transport.write(str(self.process_input)) self.transport.closeStdin() def get_process_output(executable, args=None, env=None, path=None, -- cgit From b6932a9553e45c122af8a71f6300ac62381efb94 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:23:29 +0200 Subject: Network model has network_str attribute. --- nova/network/manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/network/manager.py b/nova/network/manager.py index 83de5d023..bca3217f0 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -193,7 +193,6 @@ class FlatManager(NetworkManager): # in the datastore? net = {} net['injected'] = True - net['network_str'] = FLAGS.flat_network_network net['netmask'] = FLAGS.flat_network_netmask net['bridge'] = FLAGS.flat_network_bridge net['gateway'] = FLAGS.flat_network_gateway -- cgit From 587b21cc00919cc29e2f815fc9de3e3ad6e6fa30 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:23:58 +0200 Subject: Leave out the network setting from the interfaces template. It does not get passed anymore. --- nova/virt/interfaces.template | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/virt/interfaces.template b/nova/virt/interfaces.template index 11df301f6..87b92b84a 100644 --- a/nova/virt/interfaces.template +++ b/nova/virt/interfaces.template @@ -10,7 +10,6 @@ auto eth0 iface eth0 inet static address %(address)s netmask %(netmask)s - network %(network)s broadcast %(broadcast)s gateway %(gateway)s dns-nameservers %(dns)s -- cgit From faebe1ecd4aec4e2050a12f191266beadc468134 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 12:01:08 +0200 Subject: Clean up use of objects coming out of the ORM. --- nova/auth/manager.py | 12 ++++++------ nova/endpoint/api.py | 1 - nova/endpoint/cloud.py | 18 +++++++++--------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 5529515a6..c4f964b80 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -531,11 +531,11 @@ class AuthManager(object): except: drv.delete_project(project.id) raise - - values = {'name': 'default', - 'description': 'default', - 'user_id': User.safe_id(manager_user), - 'project_id': project.id} + + values = { 'name' : 'default', + 'description' : 'default', + 'user_id' : User.safe_id(manager_user), + 'project_id' : project['id'] } db.security_group_create({}, values) return project @@ -597,7 +597,7 @@ class AuthManager(object): groups = db.security_group_get_by_project(context={}, project_id=project_id) for group in groups: - db.security_group_destroy({}, group.id) + db.security_group_destroy({}, group['id']) except: logging.exception('Could not destroy security groups for %s', project) diff --git a/nova/endpoint/api.py b/nova/endpoint/api.py index 1f37aeb02..40be00bb7 100755 --- a/nova/endpoint/api.py +++ b/nova/endpoint/api.py @@ -135,7 +135,6 @@ class APIRequest(object): response = xml.toxml() xml.unlink() -# print response _log.debug(response) return response diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index fc83a9d1c..0289de285 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -93,7 +93,7 @@ class CloudController(object): result[instance['key_name']] = [line] return result - def _refresh_security_group(self, security_group): + def _trigger_refresh_security_group(self, security_group): nodes = set([instance.host for instance in security_group.instances]) for node in nodes: rpc.call('%s.%s' % (FLAGS.compute_topic, node), @@ -227,7 +227,7 @@ class CloudController(object): groups = db.security_group_get_all(context) else: groups = db.security_group_get_by_project(context, - context.project.id) + context.project['id']) groups = [self._format_security_group(context, g) for g in groups] if not group_name is None: groups = [g for g in groups if g.name in group_name] @@ -265,7 +265,7 @@ class CloudController(object): source_security_group_name=None, source_security_group_owner_id=None): security_group = db.security_group_get_by_name(context, - context.project.id, + context.project['id'], group_name) criteria = {} @@ -301,12 +301,12 @@ class CloudController(object): # If we make it here, we have a match db.security_group_rule_destroy(context, rule.id) - self._refresh_security_group(security_group) + self._trigger_refresh_security_group(security_group) return True # TODO(soren): Lots and lots of input validation. We're accepting - # strings here (such as ipProtocol), which is put into + # strings here (such as ipProtocol), which are put into # filter rules verbatim. # TODO(soren): Dupe detection. Adding the same rule twice actually # adds the same rule twice to the rule set, which is @@ -324,7 +324,7 @@ class CloudController(object): source_security_group_name=None, source_security_group_owner_id=None): security_group = db.security_group_get_by_name(context, - context.project.id, + context.project['id'], group_name) values = { 'parent_group' : security_group } @@ -366,11 +366,11 @@ class CloudController(object): # Since we're looking up by project name, the user name is not # used here. It's only read for EC2 API compatibility. if len(source_parts) == 2: - source_project_id = parts[1] + source_project_id = source_parts[1] else: - source_project_id = parts[0] + source_project_id = source_parts[0] else: - source_project_id = context.project.id + source_project_id = context.project['id'] return source_project_id -- cgit From 62871e83ba9b7bd8b17a7c457d8af7feb18853ea Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 12:05:37 +0200 Subject: More ORM object cleanup. --- nova/endpoint/cloud.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 0289de285..32732e9d5 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -700,9 +700,9 @@ class CloudController(object): security_groups = [] for security_group_name in security_group_arg: group = db.security_group_get_by_project(context, - context.project.id, + context.project['id'], security_group_name) - security_groups.append(group) + security_groups.append(group['id']) reservation_id = utils.generate_uid('r') base_options = {} @@ -716,11 +716,14 @@ class CloudController(object): base_options['project_id'] = context.project.id base_options['user_data'] = kwargs.get('user_data', '') base_options['instance_type'] = kwargs.get('instance_type', 'm1.small') - base_options['security_groups'] = security_groups for num in range(int(kwargs['max_count'])): inst_id = db.instance_create(context, base_options) + for security_group_id in security_groups: + db.instance_add_security_group(context, inst_id, + security_group_id) + inst = {} inst['mac_address'] = utils.generate_mac() inst['launch_index'] = num -- cgit From 0cb25fddcad2626ce617f5c2472cea1c02f1d961 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 13:56:17 +0200 Subject: Roll back my slightly over-zealous clean up work. --- nova/auth/manager.py | 4 ++-- nova/endpoint/cloud.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/nova/auth/manager.py b/nova/auth/manager.py index c4f964b80..323c48dd0 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -535,7 +535,7 @@ class AuthManager(object): values = { 'name' : 'default', 'description' : 'default', 'user_id' : User.safe_id(manager_user), - 'project_id' : project['id'] } + 'project_id' : project.id } db.security_group_create({}, values) return project @@ -601,7 +601,7 @@ class AuthManager(object): except: logging.exception('Could not destroy security groups for %s', project) - + with self.driver() as drv: drv.delete_project(Project.safe_id(project)) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 32732e9d5..ab3f5b2d9 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -227,7 +227,7 @@ class CloudController(object): groups = db.security_group_get_all(context) else: groups = db.security_group_get_by_project(context, - context.project['id']) + context.project.id) groups = [self._format_security_group(context, g) for g in groups] if not group_name is None: groups = [g for g in groups if g.name in group_name] @@ -265,7 +265,7 @@ class CloudController(object): source_security_group_name=None, source_security_group_owner_id=None): security_group = db.security_group_get_by_name(context, - context.project['id'], + context.project.id, group_name) criteria = {} @@ -324,7 +324,7 @@ class CloudController(object): source_security_group_name=None, source_security_group_owner_id=None): security_group = db.security_group_get_by_name(context, - context.project['id'], + context.project.id, group_name) values = { 'parent_group' : security_group } @@ -370,7 +370,7 @@ class CloudController(object): else: source_project_id = source_parts[0] else: - source_project_id = context.project['id'] + source_project_id = context.project.id return source_project_id @@ -700,7 +700,7 @@ class CloudController(object): security_groups = [] for security_group_name in security_group_arg: group = db.security_group_get_by_project(context, - context.project['id'], + context.project.id, security_group_name) security_groups.append(group['id']) -- cgit From 9196b74080d5effd8dcfacce9de7d2dd37fcba1b Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 14:04:07 +0200 Subject: Clean up use of ORM to remove the need for scoped_session. --- nova/db/api.py | 6 +++--- nova/db/sqlalchemy/api.py | 9 +++++---- nova/db/sqlalchemy/session.py | 4 ++-- nova/endpoint/cloud.py | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index 1d10b1987..fa937dab2 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -463,12 +463,12 @@ def security_group_get(context, security_group_id): def security_group_get_by_name(context, project_id, group_name): """Returns a security group with the specified name from a project""" - return IMPL.securitygroup_get_by_name(context, project_id, group_name) + return IMPL.security_group_get_by_name(context, project_id, group_name) def security_group_get_by_project(context, project_id): """Get all security groups belonging to a project""" - return IMPL.securitygroup_get_by_project(context, project_id) + return IMPL.security_group_get_by_project(context, project_id) def security_group_get_by_instance(context, instance_id): @@ -478,7 +478,7 @@ def security_group_get_by_instance(context, instance_id): def securitygroup_exists(context, project_id, group_name): """Indicates if a group name exists in a project""" - return IMPL.securitygroup_exists(context, project_id, group_name) + return IMPL.security_group_exists(context, project_id, group_name) def security_group_create(context, values): diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 11779e30c..038bb7f23 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -646,10 +646,11 @@ def security_group_get(_context, security_group_id): ).get(security_group_id) -def securitygroup_get_by_name(context, project_id, group_name): +def security_group_get_by_name(context, project_id, group_name): session = get_session() group_ref = session.query(models.SecurityGroup ).options(eagerload('rules') + ).options(eagerload('instances') ).filter_by(project_id=project_id ).filter_by(name=group_name ).filter_by(deleted=False @@ -662,7 +663,7 @@ def securitygroup_get_by_name(context, project_id, group_name): return group_ref -def securitygroup_get_by_project(_context, project_id): +def security_group_get_by_project(_context, project_id): session = get_session() return session.query(models.SecurityGroup ).options(eagerload('rules') @@ -681,9 +682,9 @@ def security_group_get_by_instance(_context, instance_id): ).all() -def securitygroup_exists(_context, project_id, group_name): +def security_group_exists(_context, project_id, group_name): try: - group = securitygroup_get_by_name(_context, project_id, group_name) + group = security_group_get_by_name(_context, project_id, group_name) return group != None except exception.NotFound: return False diff --git a/nova/db/sqlalchemy/session.py b/nova/db/sqlalchemy/session.py index fffbd3443..826754f6a 100644 --- a/nova/db/sqlalchemy/session.py +++ b/nova/db/sqlalchemy/session.py @@ -20,7 +20,7 @@ Session Handling for SQLAlchemy backend """ from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker, scoped_session +from sqlalchemy.orm import sessionmaker from nova import flags @@ -36,7 +36,7 @@ def get_session(autocommit=True, expire_on_commit=False): if not _MAKER: if not _ENGINE: _ENGINE = create_engine(FLAGS.sql_connection, echo=False) - _MAKER = scoped_session(sessionmaker(bind=_ENGINE, + _MAKER = (sessionmaker(bind=_ENGINE, autocommit=autocommit, expire_on_commit=expire_on_commit)) session = _MAKER() diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index ab3f5b2d9..d2606e3a7 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -326,7 +326,7 @@ class CloudController(object): security_group = db.security_group_get_by_name(context, context.project.id, group_name) - values = { 'parent_group' : security_group } + values = { 'parent_group_id' : security_group.id } if source_security_group_name: source_project_id = self._get_source_project_id(context, @@ -353,7 +353,7 @@ class CloudController(object): security_group_rule = db.security_group_rule_create(context, values) - self._refresh_security_group(security_group) + self._trigger_refresh_security_group(security_group) return True -- cgit From 28336ed41e0d44d7600588a6014f6253e4b87a42 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 14:27:34 +0200 Subject: Address a couple of the TODO's: We now have half-decent input validation for AuthorizeSecurityGroupIngress and RevokeDitto. --- nova/endpoint/cloud.py | 95 +++++++++++++++++++++++--------------------------- nova/exception.py | 3 ++ 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index d2606e3a7..d1ccf24ff 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -27,6 +27,8 @@ import logging import os import time +import IPy + from twisted.internet import defer from nova import db @@ -43,6 +45,7 @@ from nova.endpoint import images FLAGS = flags.FLAGS flags.DECLARE('storage_availability_zone', 'nova.volume.manager') +InvalidInputException = exception.InvalidInputException def _gen_key(user_id, key_name): """ Tuck this into AuthManager """ @@ -257,18 +260,14 @@ class CloudController(object): return g - @rbac.allow('netadmin') - def revoke_security_group_ingress(self, context, group_name, - to_port=None, from_port=None, - ip_protocol=None, cidr_ip=None, - user_id=None, - source_security_group_name=None, - source_security_group_owner_id=None): - security_group = db.security_group_get_by_name(context, - context.project.id, - group_name) + def _authorize_revoke_rule_args_to_dict(self, context, + to_port=None, from_port=None, + ip_protocol=None, cidr_ip=None, + user_id=None, + source_security_group_name=None, + source_security_group_owner_id=None): - criteria = {} + values = {} if source_security_group_name: source_project_id = self._get_source_project_id(context, @@ -278,21 +277,43 @@ class CloudController(object): db.security_group_get_by_name(context, source_project_id, source_security_group_name) - - criteria['group_id'] = source_security_group + values['group_id'] = source_security_group.id elif cidr_ip: - criteria['cidr'] = cidr_ip + # If this fails, it throws an exception. This is what we want. + IPy.IP(cidr_ip) + values['cidr'] = cidr_ip else: return { 'return': False } if ip_protocol and from_port and to_port: - criteria['protocol'] = ip_protocol - criteria['from_port'] = from_port - criteria['to_port'] = to_port + from_port = int(from_port) + to_port = int(to_port) + ip_protocol = str(ip_protocol) + + if ip_protocol.upper() not in ['TCP','UDP','ICMP']: + raise InvalidInputException('%s is not a valid ipProtocol' % + (ip_protocol,)) + if ((min(from_port, to_port) < -1) or + (max(from_port, to_port) > 65535)): + raise InvalidInputException('Invalid port range') + + values['protocol'] = ip_protocol + values['from_port'] = from_port + values['to_port'] = to_port else: # If cidr based filtering, protocol and ports are mandatory - if 'cidr' in criteria: - return { 'return': False } + if 'cidr' in values: + return None + + return values + + @rbac.allow('netadmin') + def revoke_security_group_ingress(self, context, group_name, **kwargs): + security_group = db.security_group_get_by_name(context, + context.project.id, + group_name) + + criteria = self._authorize_revoke_rule_args_to_dict(context, **kwargs) for rule in security_group.rules: for (k,v) in criteria.iteritems(): @@ -305,9 +326,6 @@ class CloudController(object): return True - # TODO(soren): Lots and lots of input validation. We're accepting - # strings here (such as ipProtocol), which are put into - # filter rules verbatim. # TODO(soren): Dupe detection. Adding the same rule twice actually # adds the same rule twice to the rule set, which is # pointless. @@ -315,41 +333,14 @@ class CloudController(object): # Unfortunately, it seems Boto is using an old API # for these operations, so support for newer API versions # is sketchy. - # TODO(soren): De-duplicate the turning method arguments into dict stuff. - # revoke_security_group_ingress uses the exact same logic. @rbac.allow('netadmin') - def authorize_security_group_ingress(self, context, group_name, - to_port=None, from_port=None, - ip_protocol=None, cidr_ip=None, - source_security_group_name=None, - source_security_group_owner_id=None): + def authorize_security_group_ingress(self, context, group_name, **kwargs): security_group = db.security_group_get_by_name(context, context.project.id, group_name) - values = { 'parent_group_id' : security_group.id } - if source_security_group_name: - source_project_id = self._get_source_project_id(context, - source_security_group_owner_id) - - source_security_group = \ - db.security_group_get_by_name(context, - source_project_id, - source_security_group_name) - values['group_id'] = source_security_group.id - elif cidr_ip: - values['cidr'] = cidr_ip - else: - return { 'return': False } - - if ip_protocol and from_port and to_port: - values['protocol'] = ip_protocol - values['from_port'] = from_port - values['to_port'] = to_port - else: - # If cidr based filtering, protocol and ports are mandatory - if 'cidr' in values: - return None + values = self._authorize_revoke_rule_args_to_dict(context, **kwargs) + values['parent_group_id'] = security_group.id security_group_rule = db.security_group_rule_create(context, values) diff --git a/nova/exception.py b/nova/exception.py index 29bcb17f8..43e5c36c6 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -57,6 +57,9 @@ class NotEmpty(Error): class Invalid(Error): pass +class InvalidInputException(Error): + pass + def wrap_exception(f): def _wrap(*args, **kw): -- cgit From 75a1815aa2724d64d1f487996265ba9136017029 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 20 Sep 2010 11:33:35 +0200 Subject: Add Xen template and use it by default if libvirt_type=xen. --- nova/virt/libvirt.xen.xml.template | 30 ++++++++++++++++++++++++++++++ nova/virt/libvirt_conn.py | 8 +++++++- 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 nova/virt/libvirt.xen.xml.template diff --git a/nova/virt/libvirt.xen.xml.template b/nova/virt/libvirt.xen.xml.template new file mode 100644 index 000000000..9677902c6 --- /dev/null +++ b/nova/virt/libvirt.xen.xml.template @@ -0,0 +1,30 @@ + + %(name)s + + linux + %(basepath)s/kernel + %(basepath)s/ramdisk + /dev/xvda1 + ro + + + + + %(memory_kb)s + %(vcpus)s + + + + + + + + + + + + + + + + diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index d868e083c..09f178c2a 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -47,6 +47,9 @@ FLAGS = flags.FLAGS flags.DEFINE_string('libvirt_xml_template', utils.abspath('virt/libvirt.qemu.xml.template'), 'Libvirt XML Template for QEmu/KVM') +flags.DEFINE_string('libvirt_xen_xml_template', + utils.abspath('virt/libvirt.xen.xml.template'), + 'Libvirt XML Template for Xen') flags.DEFINE_string('libvirt_uml_xml_template', utils.abspath('virt/libvirt.uml.xml.template'), 'Libvirt XML Template for user-mode-linux') @@ -55,7 +58,7 @@ flags.DEFINE_string('injected_network_template', 'Template file for injected network') flags.DEFINE_string('libvirt_type', 'kvm', - 'Libvirt domain type (valid options are: kvm, qemu, uml)') + 'Libvirt domain type (valid options are: kvm, qemu, uml, xen)') flags.DEFINE_string('libvirt_uri', '', 'Override the default libvirt URI (which is dependent' @@ -104,6 +107,9 @@ class LibvirtConnection(object): if FLAGS.libvirt_type == 'uml': uri = FLAGS.libvirt_uri or 'uml:///system' template_file = FLAGS.libvirt_uml_xml_template + elif FLAGS.libvirt_type == 'xen': + uri = FLAGS.libvirt_uri or 'xen:///' + template_file = FLAGS.libvirt_xen_xml_template else: uri = FLAGS.libvirt_uri or 'qemu:///system' template_file = FLAGS.libvirt_xml_template -- cgit From a3d003d7ec92f3ae23a667954a790c71efdbfdbe Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 20 Sep 2010 11:46:18 +0200 Subject: Move the code that extracts the console output into the virt drivers. Move the code that formats it up into the API layer. Add support for Xen console. --- nova/compute/manager.py | 18 +----------------- nova/endpoint/cloud.py | 15 ++++++++++----- nova/virt/fake.py | 2 ++ nova/virt/libvirt_conn.py | 40 ++++++++++++++++++++++++++++++++++++++++ nova/virt/xenapi.py | 3 +++ 5 files changed, 56 insertions(+), 22 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ae7099812..9f5674be2 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -144,26 +144,10 @@ class ComputeManager(manager.Manager): @exception.wrap_exception def get_console_output(self, context, instance_id): """Send the console output for an instance.""" - # TODO(vish): Move this into the driver layer - logging.debug("instance %s: getting console output", instance_id) instance_ref = self.db.instance_get(context, instance_id) - if FLAGS.connection_type == 'libvirt': - fname = os.path.abspath(os.path.join(FLAGS.instances_path, - instance_ref['str_id'], - 'console.log')) - with open(fname, 'r') as f: - output = f.read() - else: - output = 'FAKE CONSOLE OUTPUT' - - # TODO(termie): this stuff belongs in the API layer, no need to - # munge the data we send to ourselves - output = {"InstanceId": instance_id, - "Timestamp": "2", - "output": base64.b64encode(output)} - return output + return self.driver.get_console_output(instance_ref) @defer.inlineCallbacks @exception.wrap_exception diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py index 622b4e2a4..a55c00a0d 100644 --- a/nova/endpoint/cloud.py +++ b/nova/endpoint/cloud.py @@ -233,11 +233,16 @@ class CloudController(object): def get_console_output(self, context, instance_id, **kwargs): # instance_id is passed in as a list of instances instance_ref = db.instance_get_by_str(context, instance_id[0]) - return rpc.call('%s.%s' % (FLAGS.compute_topic, - instance_ref['host']), - {"method": "get_console_output", - "args": {"context": None, - "instance_id": instance_ref['id']}}) + d = rpc.call('%s.%s' % (FLAGS.compute_topic, + instance_ref['host']), + { "method" : "get_console_output", + "args" : { "context": None, + "instance_id": instance_ref['id']}}) + + d.addCallback(lambda output: { "InstanceId": instance_id, + "Timestamp": "2", + "output": base64.b64encode(output)}) + return d @rbac.allow('projectmanager', 'sysadmin') def describe_volumes(self, context, **kwargs): diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 4ae6afcc4..dc6112f20 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -223,6 +223,8 @@ class FakeConnection(object): """ return [0L, 0L, 0L, 0L, 0L, 0L, 0L, 0L] + def get_console_output(self, instance): + return 'FAKE CONSOLE OUTPUT' class FakeInstance(object): def __init__(self): diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 09f178c2a..addd9c997 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -248,6 +248,46 @@ class LibvirtConnection(object): timer.start(interval=0.5, now=True) yield local_d + def _flush_xen_console(self, virsh_output): + logging.info('virsh said: %r' % (virsh_output,)) + virsh_output = virsh_output[0].strip() + + if virsh_output.startswith('/dev/'): + logging.info('cool, it\'s a device') + d = process.simple_execute("sudo dd if=%s iflag=nonblock" % virsh_output, check_exit_code=False) + d.addCallback(lambda r:r[0]) + return d + else: + return '' + + def _append_to_file(self, data, fpath): + logging.info('data: %r, fpath: %r' % (data, fpath)) + fp = open(fpath, 'a+') + fp.write(data) + return fpath + + def _dump_file(self, fpath): + fp = open(fpath, 'r+') + contents = fp.read() + logging.info('Contents: %r' % (contents,)) + return contents + + @exception.wrap_exception + def get_console_output(self, instance): + console_log = os.path.join(FLAGS.instances_path, instance['name'], 'console.log') + logging.info('console_log: %s' % console_log) + logging.info('FLAGS.libvirt_type: %s' % FLAGS.libvirt_type) + if FLAGS.libvirt_type == 'xen': + # Xen is spethial + d = process.simple_execute("virsh ttyconsole %s" % instance['name']) + d.addCallback(self._flush_xen_console) + d.addCallback(self._append_to_file, console_log) + else: + d = defer.succeed(console_log) + d.addCallback(self._dump_file) + return d + + @defer.inlineCallbacks def _create_image(self, inst, libvirt_xml): # syntactic nicety diff --git a/nova/virt/xenapi.py b/nova/virt/xenapi.py index 1c6de4403..5fdd2b9fc 100644 --- a/nova/virt/xenapi.py +++ b/nova/virt/xenapi.py @@ -299,6 +299,9 @@ class XenAPIConnection(object): 'num_cpu': rec['VCPUs_max'], 'cpu_time': 0} + def get_console_output(self, instance): + return 'FAKE CONSOLE OUTPUT' + @utils.deferredToThread def _lookup(self, i): return self._lookup_blocking(i) -- cgit From 169ac33d89e0721c3e5229f2c58b799b64f1b51d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 21 Sep 2010 16:32:07 -0700 Subject: typo in instance_get --- nova/db/sqlalchemy/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 5ceb4c814..90c7938df 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -380,7 +380,7 @@ def instance_destroy(_context, instance_id): def instance_get(context, instance_id): session = get_session() - result = session.query(models.FixedIp + result = session.query(models.Instance ).options(eagerload('security_groups') ).filter_by(instance_id=instance_id ).filter_by(deleted=_deleted(context) -- cgit From e78273f72640eb9cbd1797d8d66dc41dcb96bee0 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 21 Sep 2010 16:43:32 -0700 Subject: typo in instance_get --- nova/db/sqlalchemy/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 90c7938df..420fd4a4a 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -382,7 +382,7 @@ def instance_get(context, instance_id): session = get_session() result = session.query(models.Instance ).options(eagerload('security_groups') - ).filter_by(instance_id=instance_id + ).filter_by(id=instance_id ).filter_by(deleted=_deleted(context) ).first() if not result: -- cgit From fed57c47da49a0457fce8fec3b59c9142e62785e Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 23 Sep 2010 13:59:33 +0200 Subject: Address Vishy's comments. --- nova/api/ec2/cloud.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 046aee14a..0f0aa327c 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -725,9 +725,9 @@ class CloudController(object): security_groups = [] for security_group_name in security_group_arg: - group = db.security_group_get_by_project(context, - context.project.id, - security_group_name) + group = db.security_group_get_by_name(context, + context.project.id, + security_group_name) security_groups.append(group['id']) reservation_id = utils.generate_uid('r') @@ -744,6 +744,7 @@ class CloudController(object): base_options['user_data'] = kwargs.get('user_data', '') type_data = INSTANCE_TYPES[instance_type] + base_options['instance_type'] = instance_type base_options['memory_mb'] = type_data['memory_mb'] base_options['vcpus'] = type_data['vcpus'] base_options['local_gb'] = type_data['local_gb'] -- cgit From 9dbdca83a8233110e94356415629ab9589b580d5 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 13:13:29 +0200 Subject: Allow DHCP requests through, pass the IP of the gateway as the dhcp server. --- nova/virt/libvirt.qemu.xml.template | 1 + nova/virt/libvirt.uml.xml.template | 1 + nova/virt/libvirt_conn.py | 6 +++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/nova/virt/libvirt.qemu.xml.template b/nova/virt/libvirt.qemu.xml.template index d02aa9114..2538b1ade 100644 --- a/nova/virt/libvirt.qemu.xml.template +++ b/nova/virt/libvirt.qemu.xml.template @@ -22,6 +22,7 @@ + diff --git a/nova/virt/libvirt.uml.xml.template b/nova/virt/libvirt.uml.xml.template index bf3f2f86a..bb8b47911 100644 --- a/nova/virt/libvirt.uml.xml.template +++ b/nova/virt/libvirt.uml.xml.template @@ -16,6 +16,7 @@ + diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 4c4c7980b..93f6977d4 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -319,6 +319,8 @@ class LibvirtConnection(object): # FIXME(vish): stick this in db instance_type = instance_types.INSTANCE_TYPES[instance['instance_type']] ip_address = db.instance_get_fixed_address({}, instance['id']) + # Assume that the gateway also acts as the dhcp server. + dhcp_server = network['gateway'] xml_info = {'type': FLAGS.libvirt_type, 'name': instance['name'], 'basepath': os.path.join(FLAGS.instances_path, @@ -327,7 +329,8 @@ class LibvirtConnection(object): 'vcpus': instance_type['vcpus'], 'bridge_name': network['bridge'], 'mac_address': instance['mac_address'], - 'ip_address': ip_address } + 'ip_address': ip_address, + 'dhcp_server': dhcp_server } libvirt_xml = self.libvirt_xml % xml_info logging.debug('instance %s: finished toXML method', instance['name']) @@ -498,6 +501,7 @@ class NWFilterFirewall(object): return ''' 26717364-50cf-42d1-8185-29bf893ab110 + -- cgit From e6ada2403cb83070c270a96c7e371513d21e27f4 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 15:13:11 +0200 Subject: If an instance never got scheduled for whatever reason, its host will turn up as None. Filter those out to make sure refresh works. --- nova/api/ec2/cloud.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 0f0aa327c..7330967fa 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -116,7 +116,8 @@ class CloudController(object): return result def _trigger_refresh_security_group(self, security_group): - nodes = set([instance.host for instance in security_group.instances]) + nodes = set([instance['host'] for instance in security_group.instances + if instance['host'] is not None]) for node in nodes: rpc.call('%s.%s' % (FLAGS.compute_topic, node), { "method": "refresh_security_group", -- cgit From 523f1c95ac12ed4782476c3273b337601ad8b6ae Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 21:49:24 +0200 Subject: If neither a security group nor a cidr has been passed, assume cidr=0.0.0.0/0 --- nova/api/ec2/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 7330967fa..4cf2666a5 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -301,7 +301,7 @@ class CloudController(object): IPy.IP(cidr_ip) values['cidr'] = cidr_ip else: - return { 'return': False } + values['cidr'] = '0.0.0.0/0' if ip_protocol and from_port and to_port: from_port = int(from_port) -- cgit From ab31fa628f4d9148aae8d42bbb41d721716c18e3 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 21:49:53 +0200 Subject: Clean up nwfilter code. Move our filters into the ipv4 chain. --- nova/virt/libvirt_conn.py | 99 ++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 61 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 93f6977d4..558854c38 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -497,20 +497,36 @@ class NWFilterFirewall(object): self._conn = get_connection - def nova_base_filter(self): - return ''' - 26717364-50cf-42d1-8185-29bf893ab110 - - - - - - - - -''' + nova_base_filter = ''' + 26717364-50cf-42d1-8185-29bf893ab110 + + + + + + + ''' + + nova_base_ipv4_filter = ''' + + ''' + + + nova_base_ipv6_filter = ''' + + ''' + + + def _define_filter(self, xml): + if callable(xml): + xml = xml() + d = threads.deferToThread(self._conn.nwfilterDefineXML, xml) + return d + @defer.inlineCallbacks def setup_nwfilters_for_instance(self, instance): """ Creates an NWFilter for the given instance. In the process, @@ -518,63 +534,24 @@ class NWFilterFirewall(object): the base filter are all in place. """ - d = self.ensure_base_filter() + yield self._define_filter(self.nova_base_ipv4_filter) + yield self._define_filter(self.nova_base_ipv6_filter) + yield self._define_filter(self.nova_base_filter) nwfilter_xml = ("\n" + - " \n" + " \n" ) % instance['name'] for security_group in instance.security_groups: - d.addCallback(lambda _:self.ensure_security_group_filter(security_group.id)) + yield self._define_filter( + self.security_group_to_nwfilter_xml(security_group['id'])) nwfilter_xml += (" \n" ) % security_group.id nwfilter_xml += "" - d.addCallback(lambda _: threads.deferToThread( - self._conn.nwfilterDefineXML, - nwfilter_xml)) - return d - - - def _nwfilter_name_for_security_group(self, security_group_id): - return 'nova-secgroup-%d' % (security_group_id,) - - - # TODO(soren): Should override be the default (and should it even - # be optional? We save a bit of processing time in - # libvirt by only defining this conditionally, but - # we still have to go and ask libvirt if the group - # is already defined, and there's the off chance of - # of inconsitencies having snuck in which would get - # fixed by just redefining the filter. - def define_filter(self, name, xml_generator, override=False): - if not override: - def _already_exists_check(filterlist, filter): - return filter in filterlist - d = threads.deferToThread(self._conn.listNWFilters) - d.addCallback(_already_exists_check, name) - else: - # Pretend we looked it up and it wasn't defined - d = defer.succeed(False) - def _define_if_not_exists(exists, xml_generator): - if not exists: - xml = xml_generator() - return threads.deferToThread(self._conn.nwfilterDefineXML, xml) - d.addCallback(_define_if_not_exists, xml_generator) - return d - - - def ensure_base_filter(self): - return self.define_filter('nova-base-filter', self.nova_base_filter) - - - def ensure_security_group_filter(self, security_group_id, override=False): - return self.define_filter( - self._nwfilter_name_for_security_group(security_group_id), - lambda:self.security_group_to_nwfilter_xml(security_group_id), - override=override) - + yield self._define_filter(nwfilter_xml) + return def security_group_to_nwfilter_xml(self, security_group_id): security_group = db.security_group_get({}, security_group_id) @@ -593,7 +570,7 @@ class NWFilterFirewall(object): if rule.to_port != -1: rule_xml += "code='%s' " % rule.to_port - rule_xml += '/>\n' + rule_xml += '/>\n' rule_xml += "\n" - xml = '''%s''' % (security_group_id, rule_xml,) + xml = '''%s''' % (security_group_id, rule_xml,) return xml -- cgit From e705b666679ecccfc3e91c8029f2c646849509ee Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 21:57:13 +0200 Subject: Recreate ensure_security_group_filter. Needed for refresh. --- nova/virt/libvirt_conn.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 558854c38..a7370e036 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -448,7 +448,7 @@ class LibvirtConnection(object): def refresh_security_group(self, security_group_id): fw = NWFilterFirewall(self._conn) - fw.ensure_security_group_filter(security_group_id, override=True) + fw.ensure_security_group_filter(security_group_id) class NWFilterFirewall(object): @@ -543,16 +543,20 @@ class NWFilterFirewall(object): ) % instance['name'] for security_group in instance.security_groups: - yield self._define_filter( - self.security_group_to_nwfilter_xml(security_group['id'])) + yield self.ensure_security_group_filter(security_group['id']) nwfilter_xml += (" \n" - ) % security_group.id + ) % security_group['id'] nwfilter_xml += "" yield self._define_filter(nwfilter_xml) return + def ensure_security_group_filter(self, security_group_id): + return self._define_filter( + self.security_group_to_nwfilter_xml(security_group_id)) + + def security_group_to_nwfilter_xml(self, security_group_id): security_group = db.security_group_get({}, security_group_id) rule_xml = "" -- cgit From 9140cd991e5507f65ff1d6a608bd8fd4c9956dbf Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 27 Sep 2010 22:00:17 +0200 Subject: Set priority of security group rules to 300 to make sure they override the defaults. --- nova/virt/libvirt_conn.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index a7370e036..d90853084 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -561,7 +561,7 @@ class NWFilterFirewall(object): security_group = db.security_group_get({}, security_group_id) rule_xml = "" for rule in security_group.rules: - rule_xml += "" + rule_xml += "" if rule.cidr: rule_xml += "<%s srcipaddr='%s' " % (rule.protocol, rule.cidr) if rule.protocol in ['tcp', 'udp']: -- cgit From 574aa4bb03c6e79c204d73a8f2a146460cbdb848 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 28 Sep 2010 00:21:36 +0200 Subject: This is getting ridiculous. --- nova/virt/libvirt_conn.py | 50 +++++++++++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index d90853084..854fa6761 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -503,20 +503,49 @@ class NWFilterFirewall(object): + ''' - nova_base_ipv4_filter = ''' - - ''' - - - nova_base_ipv6_filter = ''' - - ''' + nova_dhcp_filter = ''' + 891e4787-e5c0-d59b-cbd6-41bc3c6b36fc + + + + + + + ''' + + def nova_base_ipv4_filter(self): + retval = "" + for protocol in ['tcp', 'udp', 'icmp']: + for direction,action in [('out','accept'), + ('in','drop')]: + retval += """ + <%s /> + """ % (action, direction, protocol) + retval += '' + return retval + + + def nova_base_ipv6_filter(self): + retval = "" + for protocol in ['tcp', 'udp', 'icmp']: + for direction,action in [('out','accept'), + ('in','drop')]: + retval += """ + <%s-ipv6 /> + """ % (action, direction, protocol) + retval += '' + return retval def _define_filter(self, xml): @@ -536,6 +565,7 @@ class NWFilterFirewall(object): yield self._define_filter(self.nova_base_ipv4_filter) yield self._define_filter(self.nova_base_ipv6_filter) + yield self._define_filter(self.nova_dhcp_filter) yield self._define_filter(self.nova_base_filter) nwfilter_xml = ("\n" + -- cgit From 886534ba4d0281afc0d169546a8d55d3a5c8ece9 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 28 Sep 2010 09:07:48 +0200 Subject: Make the incoming blocking rules take precedence over the output accept rules. --- nova/virt/libvirt_conn.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 854fa6761..40a921743 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -527,11 +527,11 @@ class NWFilterFirewall(object): def nova_base_ipv4_filter(self): retval = "" for protocol in ['tcp', 'udp', 'icmp']: - for direction,action in [('out','accept'), - ('in','drop')]: - retval += """ + for direction,action,priority in [('out','accept', 400), + ('in','drop', 399)]: + retval += """ <%s /> - """ % (action, direction, protocol) + """ % (action, direction, protocol, priority) retval += '' return retval @@ -539,11 +539,12 @@ class NWFilterFirewall(object): def nova_base_ipv6_filter(self): retval = "" for protocol in ['tcp', 'udp', 'icmp']: - for direction,action in [('out','accept'), - ('in','drop')]: - retval += """ + for direction,action,priority in [('out','accept',400), + ('in','drop',399)]: + retval += """ <%s-ipv6 /> - """ % (action, direction, protocol) + """ % (action, direction, + protocol, priority) retval += '' return retval -- cgit From 0dcf2e7e593cce4be1654fb4923ec4bb4524198f Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 28 Sep 2010 09:47:25 +0200 Subject: Make sure arguments to string format are in the correct order. --- nova/virt/libvirt_conn.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 40a921743..c86f3ffb7 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -531,7 +531,8 @@ class NWFilterFirewall(object): ('in','drop', 399)]: retval += """ <%s /> - """ % (action, direction, protocol, priority) + """ % (action, direction, + priority, protocol) retval += '' return retval @@ -544,7 +545,7 @@ class NWFilterFirewall(object): retval += """ <%s-ipv6 /> """ % (action, direction, - protocol, priority) + priority, protocol) retval += '' return retval -- cgit From f09fa50fd31ded3f2f31e020b54f2d3d2b380a35 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 28 Sep 2010 10:26:29 +0200 Subject: Improve unit tests for network filtering. It now tracks recursive filter dependencies, so even if we change the filter layering, it still correctly checks for the presence of the arp, mac, and ip spoofing filters. --- nova/tests/virt_unittest.py | 45 ++++++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 985236edf..f9ff0f71f 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -153,37 +153,48 @@ class NWFilterTestCase(test.TrialTestCase): return db.security_group_get_by_name({}, 'fake', 'testgroup') def test_creates_base_rule_first(self): - self.defined_filters = [] - self.fake_libvirt_connection.listNWFilters = lambda:self.defined_filters - self.base_filter_defined = False - self.i = 0 + # These come pre-defined by libvirt + self.defined_filters = ['no-mac-spoofing', + 'no-ip-spoofing', + 'no-arp-spoofing', + 'allow-dhcp-server'] + + self.recursive_depends = {} + for f in self.defined_filters: + self.recursive_depends[f] = [] + def _filterDefineXMLMock(xml): dom = parseString(xml) name = dom.firstChild.getAttribute('name') - if self.i == 0: - self.assertEqual(dom.firstChild.getAttribute('name'), - 'nova-base-filter') - elif self.i == 1: - self.assertTrue(name.startswith('nova-secgroup-'), - 'unexpected name: %s' % name) - elif self.i == 2: - self.assertTrue(name.startswith('nova-instance-'), - 'unexpected name: %s' % name) + self.recursive_depends[name] = [] + for f in dom.getElementsByTagName('filterref'): + ref = f.getAttribute('filter') + self.assertTrue(ref in self.defined_filters, + ('%s referenced filter that does ' + + 'not yet exist: %s') % (name, ref)) + dependencies = [ref] + self.recursive_depends[ref] + self.recursive_depends[name] += dependencies self.defined_filters.append(name) - self.i += 1 return True def _ensure_all_called(_): - self.assertEqual(self.i, 3) + instance_filter = 'nova-instance-i-1' + secgroup_filter = 'nova-secgroup-%s' % self.security_group['id'] + for required in [secgroup_filter, 'allow-dhcp-server', + 'no-arp-spoofing', 'no-ip-spoofing', + 'no-mac-spoofing']: + self.assertTrue(required in self.recursive_depends[instance_filter], + "Instance's filter does not include %s" % required) self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock inst_id = db.instance_create({}, {'user_id': 'fake', 'project_id': 'fake'})['id'] - security_group = self.setup_and_return_security_group() - db.instance_add_security_group({}, inst_id, security_group.id) + self.security_group = self.setup_and_return_security_group() + + db.instance_add_security_group({}, inst_id, self.security_group.id) instance = db.instance_get({}, inst_id) d = self.fw.setup_nwfilters_for_instance(instance) -- cgit From e4cb0d3a93ddc4cae40c4a8c570c7e7d2a0061ff Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 10:34:32 -0700 Subject: get rid of network indexes and make networks into a pool --- bin/nova-manage | 27 +++++++++++-- nova/auth/manager.py | 13 ------ nova/db/api.py | 35 ++++++++-------- nova/db/sqlalchemy/api.py | 91 +++++++++++++++++++----------------------- nova/db/sqlalchemy/models.py | 33 ++++++--------- nova/network/manager.py | 73 ++++++++++++++++++--------------- nova/test.py | 10 +++++ nova/tests/network_unittest.py | 15 +++++-- 8 files changed, 158 insertions(+), 139 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index baa1cb4db..40d690e30 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -72,6 +72,7 @@ from nova import flags from nova import quota from nova import utils from nova.auth import manager +from nova.network import manager as network_manager from nova.cloudpipe import pipelib @@ -321,6 +322,25 @@ class FloatingIpCommands(object): floating_ip['address'], instance) +class NetworkCommands(object): + """Class for managing networks.""" + + def create(self, num_networks=None, network_size=None, + vlan_start=None, vpn_start=None): + """Creates floating ips for host by range + arguments: [num_networks=FLAG], [network_size=FLAG], + [vlan_start=FLAG, vpn_start=FLAG]""" + if not num_networks: + num_networks = FLAGS.num_networks + if not network_size: + network_size = FLAGS.network_size + if not vlan_start: + vlan_start = FLAGS.vlan_start + if not vpn_start: + vpn_start = FLAGS.vpn_start + net_manager = network_manager.VlanManager() + net_manager.create_networks(None, int(num_networks), int(network_size), + int(vlan_start), int(vpn_start)) CATEGORIES = [ ('user', UserCommands), @@ -328,7 +348,8 @@ CATEGORIES = [ ('role', RoleCommands), ('shell', ShellCommands), ('vpn', VpnCommands), - ('floating', FloatingIpCommands) + ('floating', FloatingIpCommands), + ('network', NetworkCommands) ] @@ -394,9 +415,9 @@ def main(): fn(*argv) sys.exit(0) except TypeError: - print "Wrong number of arguments supplied" + print "Possible wrong number of arguments supplied" print "%s %s: %s" % (category, action, fn.__doc__) - sys.exit(2) + raise if __name__ == '__main__': main() diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 55fbf42aa..8cccfe306 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -484,12 +484,6 @@ class AuthManager(object): member_users) if project_dict: project = Project(**project_dict) - try: - self.network_manager.allocate_network(context, - project.id) - except: - drv.delete_project(project.id) - raise return project def modify_project(self, project, manager_user=None, description=None): @@ -558,13 +552,6 @@ class AuthManager(object): def delete_project(self, project, context=None): """Deletes a project""" - try: - network_ref = db.project_get_network(context, - Project.safe_id(project)) - db.network_destroy(context, network_ref['id']) - except: - logging.exception('Could not destroy network for %s', - project) with self.driver() as drv: drv.delete_project(Project.safe_id(project)) diff --git a/nova/db/api.py b/nova/db/api.py index 848f5febf..1e0a2a5c8 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -337,6 +337,11 @@ def key_pair_get_all_by_user(context, user_id): #################### +def network_associate(context, project_id): + """Associate a free network to a project.""" + return IMPL.network_associate(context, project_id) + + def network_count(context): """Return the number of networks.""" return IMPL.network_count(context) @@ -357,9 +362,12 @@ def network_count_reserved_ips(context, network_id): return IMPL.network_count_reserved_ips(context, network_id) -def network_create(context, values): - """Create a network from the values dictionary.""" - return IMPL.network_create(context, values) +def network_create_safe(context, values): + """Create a network from the values dict + + The network is only returned if the create succeeds. If the create violates + constraints because the network already exists, no exception is raised.""" + return IMPL.network_create_safe(context, values) def network_create_fixed_ips(context, network_id, num_vpn_clients): @@ -367,9 +375,14 @@ def network_create_fixed_ips(context, network_id, num_vpn_clients): return IMPL.network_create_fixed_ips(context, network_id, num_vpn_clients) -def network_destroy(context, network_id): - """Destroy the network or raise if it does not exist.""" - return IMPL.network_destroy(context, network_id) +def network_disassociate(context, network_id): + """Disassociate the network from project or raise if it does not exist.""" + return IMPL.network_disassociate(context, network_id) + + +def network_disassociate_all(context): + """Disassociate all networks from projects.""" + return IMPL.network_disassociate_all(context) def network_get(context, network_id): @@ -398,16 +411,6 @@ def network_get_vpn_ip(context, network_id): return IMPL.network_get_vpn_ip(context, network_id) -def network_index_count(context): - """Return count of network indexes""" - return IMPL.network_index_count(context) - - -def network_index_create(context, values): - """Create a network index from the values dict""" - return IMPL.network_index_create(context, values) - - def network_set_cidr(context, network_id, cidr): """Set the Classless Inner Domain Routing for the network""" return IMPL.network_set_cidr(context, network_id, cidr) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index b5caae940..dc7001b93 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -19,14 +19,13 @@ Implementation of SQLAlchemy backend """ -import sys - from nova import db from nova import exception from nova import flags from nova.db.sqlalchemy import models from nova.db.sqlalchemy.session import get_session from sqlalchemy import or_ +from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import joinedload_all from sqlalchemy.sql import func @@ -536,6 +535,24 @@ def key_pair_get_all_by_user(_context, user_id): ################### +def network_associate(_context, project_id): + session = get_session() + with session.begin(): + network_ref = session.query(models.Network + ).filter_by(deleted=False + ).filter_by(project_id=None + ).with_lockmode('update' + ).first() + # NOTE(vish): if with_lockmode isn't supported, as in sqlite, + # then this has concurrency issues + if not network_ref: + raise db.NoMoreNetworks() + network_ref['project_id'] = project_id + session.add(network_ref) + return network_ref + + + def network_count(_context): return models.Network.count() @@ -568,30 +585,24 @@ def network_count_reserved_ips(_context, network_id): ).count() -def network_create(_context, values): +def network_create_safe(_context, values): network_ref = models.Network() for (key, value) in values.iteritems(): network_ref[key] = value - network_ref.save() - return network_ref + try: + network_ref.save() + return network_ref + except IntegrityError: + return None + + +def network_disassociate(context, network_id): + db.network.update(context, network_id, {'project_id': None}) -def network_destroy(_context, network_id): +def network_disassociate_all(context): session = get_session() - with session.begin(): - # TODO(vish): do we have to use sql here? - session.execute('update networks set deleted=1 where id=:id', - {'id': network_id}) - session.execute('update fixed_ips set deleted=1 where network_id=:id', - {'id': network_id}) - session.execute('update floating_ips set deleted=1 ' - 'where fixed_ip_id in ' - '(select id from fixed_ips ' - 'where network_id=:id)', - {'id': network_id}) - session.execute('update network_indexes set network_id=NULL ' - 'where network_id=:id', - {'id': network_id}) + session.execute('update networks set project_id=NULL') def network_get(_context, network_id): @@ -622,33 +633,6 @@ def network_get_by_bridge(_context, bridge): return rv -def network_get_index(_context, network_id): - session = get_session() - with session.begin(): - network_index = session.query(models.NetworkIndex - ).filter_by(network_id=None - ).filter_by(deleted=False - ).with_lockmode('update' - ).first() - if not network_index: - raise db.NoMoreNetworks() - network_index['network'] = models.Network.find(network_id, - session=session) - session.add(network_index) - return network_index['index'] - - -def network_index_count(_context): - return models.NetworkIndex.count() - - -def network_index_create(_context, values): - network_index_ref = models.NetworkIndex() - for (key, value) in values.iteritems(): - network_index_ref[key] = value - network_index_ref.save() - - def network_set_host(_context, network_id, host_id): session = get_session() with session.begin(): @@ -680,14 +664,23 @@ def network_update(_context, network_id, values): ################### -def project_get_network(_context, project_id): +def project_get_network(context, project_id): session = get_session() rv = session.query(models.Network ).filter_by(project_id=project_id ).filter_by(deleted=False ).first() if not rv: - raise exception.NotFound('No network for project: %s' % project_id) + try: + return db.network_associate(context, project_id) + except IntegrityError: + # NOTE(vish): We hit this if there is a race and two + # processes are attempting to allocate the + # network at the same time + rv = session.query(models.Network + ).filter_by(project_id=project_id + ).filter_by(deleted=False + ).first() return rv diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index f6ba7953f..cfbe474c6 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -25,7 +25,7 @@ import datetime # TODO(vish): clean up these imports from sqlalchemy.orm import relationship, backref, exc, object_mapper -from sqlalchemy import Column, Integer, String +from sqlalchemy import Column, Integer, String, schema from sqlalchemy import ForeignKey, DateTime, Boolean, Text from sqlalchemy.ext.declarative import declarative_base @@ -363,10 +363,13 @@ class KeyPair(BASE, NovaBase): class Network(BASE, NovaBase): """Represents a network""" __tablename__ = 'networks' + __table_args__ = (schema.UniqueConstraint("vpn_public_address", + "vpn_public_port"), + {'mysql_engine': 'InnoDB'}) id = Column(Integer, primary_key=True) injected = Column(Boolean, default=False) - cidr = Column(String(255)) + cidr = Column(String(255), unique=True) netmask = Column(String(255)) bridge = Column(String(255)) gateway = Column(String(255)) @@ -379,27 +382,16 @@ class Network(BASE, NovaBase): vpn_private_address = Column(String(255)) dhcp_start = Column(String(255)) - project_id = Column(String(255)) + # NOTE(vish): The unique constraint below helps avoid a race condition + # when associating a network, but it also means that we + # can't associate two networks with one project. + project_id = Column(String(255), unique=True) host = Column(String(255)) # , ForeignKey('hosts.id')) -class NetworkIndex(BASE, NovaBase): - """Represents a unique offset for a network - - Currently vlan number, vpn port, and fixed ip ranges are keyed off of - this index. These may ultimately need to be converted to separate - pools. - """ - __tablename__ = 'network_indexes' - id = Column(Integer, primary_key=True) - index = Column(Integer) - network_id = Column(Integer, ForeignKey('networks.id'), nullable=True) - network = relationship(Network, backref=backref('network_index', - uselist=False)) - class AuthToken(BASE, NovaBase): - """Represents an authorization token for all API transactions. Fields - are a string representing the actual token and a user id for mapping + """Represents an authorization token for all API transactions. Fields + are a string representing the actual token and a user id for mapping to the actual user""" __tablename__ = 'auth_tokens' token_hash = Column(String(255), primary_key=True) @@ -409,7 +401,6 @@ class AuthToken(BASE, NovaBase): cdn_management_url = Column(String(255)) - # TODO(vish): can these both come from the same baseclass? class FixedIp(BASE, NovaBase): """Represents a fixed ip for an instance""" @@ -476,7 +467,7 @@ def register_models(): """Register Models and create metadata""" from sqlalchemy import create_engine models = (Service, Instance, Volume, ExportDevice, - FixedIp, FloatingIp, Network, NetworkIndex, + FixedIp, FloatingIp, Network, AuthToken) # , Image, Host) engine = create_engine(FLAGS.sql_connection, echo=False) for model in models: diff --git a/nova/network/manager.py b/nova/network/manager.py index b4c4a53b4..761a035a2 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -157,7 +157,7 @@ class NetworkManager(manager.Manager): def _create_fixed_ips(self, context, network_id): """Create all fixed ips for network""" network_ref = self.db.network_get(context, network_id) - # NOTE(vish): should these be properties of the network as opposed + # NOTE(vish): Should these be properties of the network as opposed # to properties of the manager class? bottom_reserved = self._bottom_reserved_ips top_reserved = self._top_reserved_ips @@ -308,35 +308,19 @@ class VlanManager(NetworkManager): network_ref = self.db.fixed_ip_get_network(context, address) self.driver.update_dhcp(context, network_ref['id']) - def allocate_network(self, context, project_id): - """Set up the network""" - self._ensure_indexes(context) - network_ref = db.network_create(context, {'project_id': project_id}) - network_id = network_ref['id'] - private_net = IPy.IP(FLAGS.private_range) - index = db.network_get_index(context, network_id) - vlan = FLAGS.vlan_start + index - start = index * FLAGS.network_size - significant_bits = 32 - int(math.log(FLAGS.network_size, 2)) - cidr = "%s/%s" % (private_net[start], significant_bits) - project_net = IPy.IP(cidr) - net = {} - net['cidr'] = cidr - # NOTE(vish): we could turn these into properties - net['netmask'] = str(project_net.netmask()) - net['gateway'] = str(project_net[1]) - net['broadcast'] = str(project_net.broadcast()) - net['vpn_private_address'] = str(project_net[2]) - net['dhcp_start'] = str(project_net[3]) - net['vlan'] = vlan - net['bridge'] = 'br%s' % vlan - net['vpn_public_address'] = FLAGS.vpn_ip - net['vpn_public_port'] = FLAGS.vpn_start + index - db.network_update(context, network_id, net) - self._create_fixed_ips(context, network_id) + def associate_network(self, context, project_id): + """Associates a network to a project""" + network_ref = db.network_associate(context, project_id) + network_id = network_ref['id'] return network_id + + def disassociate_network(self, context, network_id): + """Disassociates a newtwork from a project""" + db.network_disassocate(context, network_id) + + def setup_compute_network(self, context, project_id): """Sets up matching network for compute hosts""" network_ref = self.db.project_get_network(context, project_id) @@ -348,17 +332,40 @@ class VlanManager(NetworkManager): # TODO(vish): Implement this pass - def _ensure_indexes(self, context): - """Ensure the indexes for the network exist - This could use a manage command instead of keying off of a flag""" - if not self.db.network_index_count(context): - for index in range(FLAGS.num_networks): - self.db.network_index_create(context, {'index': index}) + def create_networks(self, context, num_networks, network_size, + vlan_start, vpn_start): + """Create networks based on parameters""" + for index in range(num_networks): + private_net = IPy.IP(FLAGS.private_range) + vlan = vlan_start + index + start = index * network_size + significant_bits = 32 - int(math.log(network_size, 2)) + cidr = "%s/%s" % (private_net[start], significant_bits) + project_net = IPy.IP(cidr) + net = {} + net['cidr'] = cidr + net['netmask'] = str(project_net.netmask()) + net['gateway'] = str(project_net[1]) + net['broadcast'] = str(project_net.broadcast()) + net['vpn_private_address'] = str(project_net[2]) + net['dhcp_start'] = str(project_net[3]) + net['vlan'] = vlan + net['bridge'] = 'br%s' % vlan + # NOTE(vish): This makes ports unique accross the cloud, a more + # robust solution would be to make them unique per ip + net['vpn_public_port'] = vpn_start + index + network_ref = self.db.network_create_safe(context, net) + if network_ref: + self._create_fixed_ips(context, network_ref['id']) + def _on_set_network_host(self, context, network_id): """Called when this host becomes the host for a project""" network_ref = self.db.network_get(context, network_id) + net = {} + net['vpn_public_address'] = FLAGS.vpn_ip + db.network_update(context, network_id, net) self.driver.ensure_vlan_bridge(network_ref['vlan'], network_ref['bridge'], network_ref) diff --git a/nova/test.py b/nova/test.py index c392c8a84..a491c2902 100644 --- a/nova/test.py +++ b/nova/test.py @@ -31,8 +31,10 @@ from tornado import ioloop from twisted.internet import defer from twisted.trial import unittest +from nova import db from nova import fakerabbit from nova import flags +from nova.network import manager as network_manager FLAGS = flags.FLAGS @@ -56,6 +58,13 @@ class TrialTestCase(unittest.TestCase): def setUp(self): # pylint: disable-msg=C0103 """Run before each test method to initialize test environment""" super(TrialTestCase, self).setUp() + # NOTE(vish): We need a better method for creating fixtures for tests + # now that we have some required db setup for the system + # to work properly. + if db.network_count(None) != 5: + network_manager.VlanManager().create_networks(None, 5, 16, + FLAGS.vlan_start, + FLAGS.vpn_start) # emulate some of the mox stuff, we can't use the metaclass # because it screws with our generators @@ -71,6 +80,7 @@ class TrialTestCase(unittest.TestCase): self.stubs.UnsetAll() self.stubs.SmartUnsetAll() self.mox.VerifyAll() + db.network_disassociate_all(None) if FLAGS.fake_rabbit: fakerabbit.reset_all() diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index da65b50a2..5ceb336ec 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -94,6 +94,7 @@ class NetworkTestCase(test.TrialTestCase): float_addr = self.network.allocate_floating_ip(self.context, self.projects[0].id) fix_addr = self._create_address(0) + lease_ip(fix_addr) self.assertEqual(float_addr, str(pubnet[0])) self.network.associate_floating_ip(self.context, float_addr, fix_addr) address = db.instance_get_floating_address(None, self.instance_id) @@ -103,6 +104,7 @@ class NetworkTestCase(test.TrialTestCase): self.assertEqual(address, None) self.network.deallocate_floating_ip(self.context, float_addr) self.network.deallocate_fixed_ip(self.context, fix_addr) + release_ip(fix_addr) def test_allocate_deallocate_fixed_ip(self): """Makes sure that we can allocate and deallocate a fixed ip""" @@ -180,8 +182,8 @@ class NetworkTestCase(test.TrialTestCase): release_ip(address3) for instance_id in instance_ids: db.instance_destroy(None, instance_id) - release_ip(first) self.network.deallocate_fixed_ip(self.context, first) + release_ip(first) def test_vpn_ip_and_port_looks_valid(self): """Ensure the vpn ip and port are reasonable""" @@ -197,10 +199,13 @@ class NetworkTestCase(test.TrialTestCase): for i in range(networks_left): project = self.manager.create_project('many%s' % i, self.user) projects.append(project) + db.project_get_network(None, project.id) + project = self.manager.create_project('last', self.user) + projects.append(project) self.assertRaises(db.NoMoreNetworks, - self.manager.create_project, - 'boom', - self.user) + db.project_get_network, + None, + project.id) for project in projects: self.manager.delete_project(project) @@ -213,7 +218,9 @@ class NetworkTestCase(test.TrialTestCase): address2 = self._create_address(0) self.assertEqual(address, address2) + lease_ip(address) self.network.deallocate_fixed_ip(self.context, address2) + release_ip(address) def test_available_ips(self): """Make sure the number of available ips for the network is correct -- cgit From 84fbad82d65b837d43f138e7a5acd24f182499e2 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 12:09:17 -0700 Subject: move default group creation to api --- nova/api/ec2/cloud.py | 17 +++++++++++++++++ nova/auth/manager.py | 14 -------------- nova/db/api.py | 5 +++++ nova/db/sqlalchemy/api.py | 6 ++++++ nova/test.py | 2 ++ 5 files changed, 30 insertions(+), 14 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 4cf2666a5..d54562ec6 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -244,6 +244,7 @@ class CloudController(object): return True def describe_security_groups(self, context, group_name=None, **kwargs): + self._ensure_default_security_group(context) if context.user.is_admin(): groups = db.security_group_get_all(context) else: @@ -326,6 +327,7 @@ class CloudController(object): return values def revoke_security_group_ingress(self, context, group_name, **kwargs): + self._ensure_default_security_group(context) security_group = db.security_group_get_by_name(context, context.project.id, group_name) @@ -351,6 +353,7 @@ class CloudController(object): # for these operations, so support for newer API versions # is sketchy. def authorize_security_group_ingress(self, context, group_name, **kwargs): + self._ensure_default_security_group(context) security_group = db.security_group_get_by_name(context, context.project.id, group_name) @@ -383,6 +386,7 @@ class CloudController(object): def create_security_group(self, context, group_name, group_description): + self._ensure_default_security_group(context) if db.securitygroup_exists(context, context.project.id, group_name): raise exception.ApiError('group %s already exists' % group_name) @@ -673,6 +677,18 @@ class CloudController(object): "project_id": context.project.id}}) return db.queue_get_for(context, FLAGS.network_topic, host) + def _ensure_default_security_group(self, context): + try: + db.security_group_get_by_name(context, + context.project.id, + 'default') + except exception.NotFound: + values = { 'name' : 'default', + 'description' : 'default', + 'user_id' : context.user.id, + 'project_id' : context.project.id } + group = db.security_group_create({}, values) + def run_instances(self, context, **kwargs): instance_type = kwargs.get('instance_type', 'm1.small') if instance_type not in INSTANCE_TYPES: @@ -725,6 +741,7 @@ class CloudController(object): security_group_arg = [security_group_arg] security_groups = [] + self._ensure_default_security_group(context) for security_group_name in security_group_arg: group = db.security_group_get_by_name(context, context.project.id, diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 7075070cf..bea4c7933 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -491,11 +491,6 @@ class AuthManager(object): drv.delete_project(project.id) raise - values = { 'name' : 'default', - 'description' : 'default', - 'user_id' : User.safe_id(manager_user), - 'project_id' : project.id } - db.security_group_create({}, values) return project def modify_project(self, project, manager_user=None, description=None): @@ -571,15 +566,6 @@ class AuthManager(object): except: logging.exception('Could not destroy network for %s', project) - try: - project_id = Project.safe_id(project) - groups = db.security_group_get_by_project(context={}, - project_id=project_id) - for group in groups: - db.security_group_destroy({}, group['id']) - except: - logging.exception('Could not destroy security groups for %s', - project) with self.driver() as drv: drv.delete_project(Project.safe_id(project)) diff --git a/nova/db/api.py b/nova/db/api.py index 602c3cf09..5e033b59d 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -604,6 +604,11 @@ def security_group_destroy(context, security_group_id): return IMPL.security_group_destroy(context, security_group_id) +def security_group_destroy_all(context): + """Deletes a security group""" + return IMPL.security_group_destroy_all(context) + + #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index d2847506e..07ea5d145 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -947,6 +947,12 @@ def security_group_destroy(_context, security_group_id): 'where group_id=:id', {'id': security_group_id}) +def security_group_destroy_all(_context): + session = get_session() + with session.begin(): + # TODO(vish): do we have to use sql here? + session.execute('update security_group set deleted=1') + session.execute('update security_group_rules set deleted=1') ################### diff --git a/nova/test.py b/nova/test.py index c392c8a84..5ed0c73d3 100644 --- a/nova/test.py +++ b/nova/test.py @@ -31,6 +31,7 @@ from tornado import ioloop from twisted.internet import defer from twisted.trial import unittest +from nova import db from nova import fakerabbit from nova import flags @@ -74,6 +75,7 @@ class TrialTestCase(unittest.TestCase): if FLAGS.fake_rabbit: fakerabbit.reset_all() + db.security_group_destroy_all(None) def flags(self, **kw): """Override flag variables for a test""" -- cgit From c53af2fc9d9803cebc7f4078b8f772476a09df81 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 18:47:47 -0700 Subject: fix security group revoke --- nova/api/ec2/cloud.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 4cf2666a5..6eea95f84 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -295,7 +295,7 @@ class CloudController(object): db.security_group_get_by_name(context, source_project_id, source_security_group_name) - values['group_id'] = source_security_group.id + values['group_id'] = source_security_group['id'] elif cidr_ip: # If this fails, it throws an exception. This is what we want. IPy.IP(cidr_ip) @@ -331,17 +331,19 @@ class CloudController(object): group_name) criteria = self._authorize_revoke_rule_args_to_dict(context, **kwargs) + if criteria == None: + raise exception.ApiError("No rule for the specified parameters.") for rule in security_group.rules: + match = True for (k,v) in criteria.iteritems(): if getattr(rule, k, False) != v: - break - # If we make it here, we have a match - db.security_group_rule_destroy(context, rule.id) + match = False + if match: + db.security_group_rule_destroy(context, rule['id']) + self._trigger_refresh_security_group(security_group) - self._trigger_refresh_security_group(security_group) - - return True + raise exception.ApiError("No rule for the specified parameters.") # TODO(soren): Dupe detection. Adding the same rule twice actually # adds the same rule twice to the rule set, which is -- cgit From d9855ba51f53a27f5475b3c0b7f669b378ccc006 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 20:53:24 -0700 Subject: fix eagerload to be joins that filter by deleted == False --- nova/db/sqlalchemy/api.py | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index d2847506e..8b754c78e 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -25,7 +25,7 @@ from nova import flags from nova.db.sqlalchemy import models from nova.db.sqlalchemy.session import get_session from sqlalchemy import or_ -from sqlalchemy.orm import eagerload, joinedload_all +from sqlalchemy.orm import contains_eager, eagerload, joinedload_all from sqlalchemy.sql import func FLAGS = flags.FLAGS @@ -711,7 +711,7 @@ def auth_create_token(_context, token): tk[k] = v tk.save() return tk - + ################### @@ -868,7 +868,9 @@ def volume_update(_context, volume_id, values): def security_group_get_all(_context): session = get_session() return session.query(models.SecurityGroup - ).options(eagerload('rules') + ).join(models.SecurityGroupIngressRule + ).options(contains_eager(models.SecurityGroup.rules) + ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(deleted=False ).all() @@ -876,7 +878,11 @@ def security_group_get_all(_context): def security_group_get(_context, security_group_id): session = get_session() result = session.query(models.SecurityGroup - ).options(eagerload('rules') + ).join(models.SecurityGroupIngressRule + ).options(contains_eager(models.SecurityGroup.rules) + ).filter(models.SecurityGroupIngressRule.deleted == False + ).filter_by(deleted=False + ).filter_by(id=security_group_id ).get(security_group_id) if not result: raise exception.NotFound("No secuity group with id %s" % @@ -887,8 +893,11 @@ def security_group_get(_context, security_group_id): def security_group_get_by_name(context, project_id, group_name): session = get_session() group_ref = session.query(models.SecurityGroup - ).options(eagerload('rules') - ).options(eagerload('instances') + ).join(models.SecurityGroupIngressRule + ).join(models.Instances + ).options(contains_eager(models.SecurityGroup.rules) + ).options(contains_eager(models.SecurityGroup.instances) + ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(project_id=project_id ).filter_by(name=group_name ).filter_by(deleted=False @@ -903,7 +912,9 @@ def security_group_get_by_name(context, project_id, group_name): def security_group_get_by_project(_context, project_id): session = get_session() return session.query(models.SecurityGroup - ).options(eagerload('rules') + ).join(models.SecurityGroupIngressRule + ).options(contains_eager(models.SecurityGroup.rules) + ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(project_id=project_id ).filter_by(deleted=False ).all() -- cgit From 3124cf70c6ab2bcab570f0ffbcbe31672a9556f8 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 21:03:45 -0700 Subject: fix join and misnamed method --- nova/api/ec2/cloud.py | 2 +- nova/db/api.py | 2 +- nova/db/sqlalchemy/api.py | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 6eea95f84..a1a3960f6 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -385,7 +385,7 @@ class CloudController(object): def create_security_group(self, context, group_name, group_description): - if db.securitygroup_exists(context, context.project.id, group_name): + if db.security_group_exists(context, context.project.id, group_name): raise exception.ApiError('group %s already exists' % group_name) group = {'user_id' : context.user.id, diff --git a/nova/db/api.py b/nova/db/api.py index 602c3cf09..1e2738b99 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -589,7 +589,7 @@ def security_group_get_by_instance(context, instance_id): return IMPL.security_group_get_by_instance(context, instance_id) -def securitygroup_exists(context, project_id, group_name): +def security_group_exists(context, project_id, group_name): """Indicates if a group name exists in a project""" return IMPL.security_group_exists(context, project_id, group_name) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 8b754c78e..fcdf945eb 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -868,7 +868,7 @@ def volume_update(_context, volume_id, values): def security_group_get_all(_context): session = get_session() return session.query(models.SecurityGroup - ).join(models.SecurityGroupIngressRule + ).join(models.SecurityGroup.rules ).options(contains_eager(models.SecurityGroup.rules) ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(deleted=False @@ -878,7 +878,7 @@ def security_group_get_all(_context): def security_group_get(_context, security_group_id): session = get_session() result = session.query(models.SecurityGroup - ).join(models.SecurityGroupIngressRule + ).join(models.SecurityGroup.rules ).options(contains_eager(models.SecurityGroup.rules) ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(deleted=False @@ -893,7 +893,7 @@ def security_group_get(_context, security_group_id): def security_group_get_by_name(context, project_id, group_name): session = get_session() group_ref = session.query(models.SecurityGroup - ).join(models.SecurityGroupIngressRule + ).join(models.SecurityGroup.rules ).join(models.Instances ).options(contains_eager(models.SecurityGroup.rules) ).options(contains_eager(models.SecurityGroup.instances) @@ -912,7 +912,7 @@ def security_group_get_by_name(context, project_id, group_name): def security_group_get_by_project(_context, project_id): session = get_session() return session.query(models.SecurityGroup - ).join(models.SecurityGroupIngressRule + ).join(models.SecurityGroup.rules ).options(contains_eager(models.SecurityGroup.rules) ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(project_id=project_id -- cgit From b952e1ef61a6ed73e34c6dd0318cd4d52faf47dc Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 21:07:26 -0700 Subject: patch for test --- nova/tests/virt_unittest.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index f9ff0f71f..5e9505374 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -178,8 +178,14 @@ class NWFilterTestCase(test.TrialTestCase): self.defined_filters.append(name) return True + self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock + + instance_ref = db.instance_create({}, {'user_id': 'fake', + 'project_id': 'fake'}) + inst_id = instance_ref['id'] + def _ensure_all_called(_): - instance_filter = 'nova-instance-i-1' + instance_filter = 'nova-instance-%s' % instance_ref['str_id'] secgroup_filter = 'nova-secgroup-%s' % self.security_group['id'] for required in [secgroup_filter, 'allow-dhcp-server', 'no-arp-spoofing', 'no-ip-spoofing', @@ -187,11 +193,6 @@ class NWFilterTestCase(test.TrialTestCase): self.assertTrue(required in self.recursive_depends[instance_filter], "Instance's filter does not include %s" % required) - self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock - - inst_id = db.instance_create({}, {'user_id': 'fake', - 'project_id': 'fake'})['id'] - self.security_group = self.setup_and_return_security_group() db.instance_add_security_group({}, inst_id, self.security_group.id) -- cgit From 970114e1729c35ebcc05930659bb5dfaf5b59d3d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 29 Sep 2010 00:30:35 -0700 Subject: fix loading to ignore deleted items --- nova/api/ec2/cloud.py | 2 +- nova/db/sqlalchemy/api.py | 65 ++++++++++++++++++++++++++------------------ nova/db/sqlalchemy/models.py | 21 ++++++++------ 3 files changed, 53 insertions(+), 35 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 4c27440dc..d85b8512a 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -342,7 +342,7 @@ class CloudController(object): if match: db.security_group_rule_destroy(context, rule['id']) self._trigger_refresh_security_group(security_group) - + return True raise exception.ApiError("No rule for the specified parameters.") # TODO(soren): Dupe detection. Adding the same rule twice actually diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index dad544cdb..fee50ec9c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -29,7 +29,7 @@ from nova.db.sqlalchemy import models from nova.db.sqlalchemy.session import get_session from sqlalchemy import or_ from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm import contains_eager, eagerload, joinedload_all +from sqlalchemy.orm import contains_eager, joinedload_all from sqlalchemy.sql import exists, func FLAGS = flags.FLAGS @@ -410,8 +410,17 @@ def instance_destroy(_context, instance_id): def instance_get(context, instance_id): - return models.Instance().find(instance_id, deleted=_deleted(context), - options=eagerload('security_groups')) + session = get_session() + instance_ref = session.query(models.Instance + ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload_all('security_groups') + ).filter_by(id=instance_id + ).filter_by(deleted=_deleted(context) + ).first() + if not instance_ref: + raise exception.NotFound('Instance %s not found' % (instance_id)) + + return instance_ref def instance_get_all(context): @@ -942,25 +951,29 @@ def volume_update(_context, volume_id, values): ################### +INSTANCES_OR = or_(models.Instance.deleted == False, + models.Instance.deleted == None) + + +RULES_OR = or_(models.SecurityGroupIngressRule.deleted == False, + models.SecurityGroupIngressRule.deleted == None) + + def security_group_get_all(_context): session = get_session() return session.query(models.SecurityGroup - ).join(models.SecurityGroup.rules - ).options(contains_eager(models.SecurityGroup.rules) - ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(deleted=False + ).options(joinedload_all('rules') ).all() def security_group_get(_context, security_group_id): session = get_session() result = session.query(models.SecurityGroup - ).join(models.SecurityGroup.rules - ).options(contains_eager(models.SecurityGroup.rules) - ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(deleted=False ).filter_by(id=security_group_id - ).get(security_group_id) + ).options(joinedload_all('rules') + ).first() if not result: raise exception.NotFound("No secuity group with id %s" % security_group_id) @@ -969,41 +982,41 @@ def security_group_get(_context, security_group_id): def security_group_get_by_name(context, project_id, group_name): session = get_session() - group_ref = session.query(models.SecurityGroup - ).join(models.SecurityGroup.rules - ).join(models.Instances - ).options(contains_eager(models.SecurityGroup.rules) - ).options(contains_eager(models.SecurityGroup.instances) - ).filter(models.SecurityGroupIngressRule.deleted == False + result = session.query(models.SecurityGroup ).filter_by(project_id=project_id ).filter_by(name=group_name ).filter_by(deleted=False + ).options(joinedload_all('rules') + ).options(joinedload_all('instances') ).first() - if not group_ref: + if not result: raise exception.NotFound( 'No security group named %s for project: %s' \ % (group_name, project_id)) - return group_ref + return result def security_group_get_by_project(_context, project_id): session = get_session() return session.query(models.SecurityGroup - ).join(models.SecurityGroup.rules - ).options(contains_eager(models.SecurityGroup.rules) - ).filter(models.SecurityGroupIngressRule.deleted == False ).filter_by(project_id=project_id ).filter_by(deleted=False + ).options(joinedload_all('rules') + ).outerjoin(models.SecurityGroup.rules + ).options(contains_eager(models.SecurityGroup.rules) + ).filter(RULES_OR ).all() def security_group_get_by_instance(_context, instance_id): session = get_session() - with session.begin(): - return session.query(models.Instance - ).join(models.Instance.security_groups - ).filter_by(deleted=False - ).all() + return session.query(models.SecurityGroup + ).filter_by(deleted=False + ).options(joinedload_all('rules') + ).join(models.SecurityGroup.instances + ).filter_by(id=instance_id + ).filter_by(deleted=False + ).all() def security_group_exists(_context, project_id, group_name): diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index d4caf0b52..b89616ddb 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -340,12 +340,12 @@ class ExportDevice(BASE, NovaBase): uselist=False)) -security_group_instance_association = Table('security_group_instance_association', - BASE.metadata, - Column('security_group_id', Integer, - ForeignKey('security_group.id')), - Column('instance_id', Integer, - ForeignKey('instances.id'))) +class SecurityGroupInstanceAssociation(BASE, NovaBase): + __tablename__ = 'security_group_instance_association' + id = Column(Integer, primary_key=True) + security_group_id = Column(Integer, ForeignKey('security_group.id')) + instance_id = Column(Integer, ForeignKey('instances.id')) + class SecurityGroup(BASE, NovaBase): """Represents a security group""" @@ -358,7 +358,11 @@ class SecurityGroup(BASE, NovaBase): project_id = Column(String(255)) instances = relationship(Instance, - secondary=security_group_instance_association, + secondary="security_group_instance_association", + secondaryjoin="and_(SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id," + "Instance.id == SecurityGroupInstanceAssociation.instance_id," + "SecurityGroup.deleted == False," + "Instance.deleted == False)", backref='security_groups') @property @@ -378,7 +382,8 @@ class SecurityGroupIngressRule(BASE, NovaBase): parent_group_id = Column(Integer, ForeignKey('security_group.id')) parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id, - primaryjoin=parent_group_id==SecurityGroup.id) + primaryjoin="and_(SecurityGroupIngressRule.parent_group_id == SecurityGroup.id," + "SecurityGroupIngressRule.deleted == False)") protocol = Column(String(5)) # "tcp", "udp", or "icmp" from_port = Column(Integer) -- cgit From c0abb5cd45314e072096e173830b2e3d379bf3e7 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 29 Sep 2010 00:42:18 -0700 Subject: removed a few extra items --- nova/db/sqlalchemy/api.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index e823bf15e..7ef92cad5 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -29,7 +29,7 @@ from nova.db.sqlalchemy import models from nova.db.sqlalchemy.session import get_session from sqlalchemy import or_ from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm import contains_eager, joinedload_all +from sqlalchemy.orm import joinedload_all from sqlalchemy.sql import exists, func FLAGS = flags.FLAGS @@ -951,14 +951,6 @@ def volume_update(_context, volume_id, values): ################### -INSTANCES_OR = or_(models.Instance.deleted == False, - models.Instance.deleted == None) - - -RULES_OR = or_(models.SecurityGroupIngressRule.deleted == False, - models.SecurityGroupIngressRule.deleted == None) - - def security_group_get_all(_context): session = get_session() return session.query(models.SecurityGroup @@ -1002,9 +994,6 @@ def security_group_get_by_project(_context, project_id): ).filter_by(project_id=project_id ).filter_by(deleted=False ).options(joinedload_all('rules') - ).outerjoin(models.SecurityGroup.rules - ).options(contains_eager(models.SecurityGroup.rules) - ).filter(RULES_OR ).all() -- cgit From bfb01ef2e2960803feffb2a3998810b0966e1e79 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 29 Sep 2010 09:46:37 +0200 Subject: Apply patch from Vish to fix a hardcoded id in the unit tests. --- nova/tests/virt_unittest.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index f9ff0f71f..5e9505374 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -178,8 +178,14 @@ class NWFilterTestCase(test.TrialTestCase): self.defined_filters.append(name) return True + self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock + + instance_ref = db.instance_create({}, {'user_id': 'fake', + 'project_id': 'fake'}) + inst_id = instance_ref['id'] + def _ensure_all_called(_): - instance_filter = 'nova-instance-i-1' + instance_filter = 'nova-instance-%s' % instance_ref['str_id'] secgroup_filter = 'nova-secgroup-%s' % self.security_group['id'] for required in [secgroup_filter, 'allow-dhcp-server', 'no-arp-spoofing', 'no-ip-spoofing', @@ -187,11 +193,6 @@ class NWFilterTestCase(test.TrialTestCase): self.assertTrue(required in self.recursive_depends[instance_filter], "Instance's filter does not include %s" % required) - self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock - - inst_id = db.instance_create({}, {'user_id': 'fake', - 'project_id': 'fake'})['id'] - self.security_group = self.setup_and_return_security_group() db.instance_add_security_group({}, inst_id, self.security_group.id) -- cgit From 793516d14630a82bb3592f626b753736e63955ec Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 29 Sep 2010 01:33:30 -0700 Subject: autocreate the models and use security_groups --- nova/db/sqlalchemy/api.py | 4 ++-- nova/db/sqlalchemy/models.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 7ef92cad5..200fb3b3c 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -1031,7 +1031,7 @@ def security_group_destroy(_context, security_group_id): session = get_session() with session.begin(): # TODO(vish): do we have to use sql here? - session.execute('update security_group set deleted=1 where id=:id', + session.execute('update security_groups set deleted=1 where id=:id', {'id': security_group_id}) session.execute('update security_group_rules set deleted=1 ' 'where group_id=:id', @@ -1041,7 +1041,7 @@ def security_group_destroy_all(_context): session = get_session() with session.begin(): # TODO(vish): do we have to use sql here? - session.execute('update security_group set deleted=1') + session.execute('update security_groups set deleted=1') session.execute('update security_group_rules set deleted=1') ################### diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index b89616ddb..c2dbf2345 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -25,7 +25,7 @@ import datetime # TODO(vish): clean up these imports from sqlalchemy.orm import relationship, backref, exc, object_mapper -from sqlalchemy import Column, Integer, String, Table +from sqlalchemy import Column, Integer, String from sqlalchemy import ForeignKey, DateTime, Boolean, Text from sqlalchemy.ext.declarative import declarative_base @@ -343,13 +343,13 @@ class ExportDevice(BASE, NovaBase): class SecurityGroupInstanceAssociation(BASE, NovaBase): __tablename__ = 'security_group_instance_association' id = Column(Integer, primary_key=True) - security_group_id = Column(Integer, ForeignKey('security_group.id')) + security_group_id = Column(Integer, ForeignKey('security_groups.id')) instance_id = Column(Integer, ForeignKey('instances.id')) class SecurityGroup(BASE, NovaBase): """Represents a security group""" - __tablename__ = 'security_group' + __tablename__ = 'security_groups' id = Column(Integer, primary_key=True) name = Column(String(255)) @@ -379,7 +379,7 @@ class SecurityGroupIngressRule(BASE, NovaBase): __tablename__ = 'security_group_rules' id = Column(Integer, primary_key=True) - parent_group_id = Column(Integer, ForeignKey('security_group.id')) + parent_group_id = Column(Integer, ForeignKey('security_groups.id')) parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id, primaryjoin="and_(SecurityGroupIngressRule.parent_group_id == SecurityGroup.id," @@ -392,7 +392,7 @@ class SecurityGroupIngressRule(BASE, NovaBase): # Note: This is not the parent SecurityGroup. It's SecurityGroup we're # granting access for. - group_id = Column(Integer, ForeignKey('security_group.id')) + group_id = Column(Integer, ForeignKey('security_groups.id')) class KeyPair(BASE, NovaBase): @@ -546,7 +546,7 @@ def register_models(): from sqlalchemy import create_engine models = (Service, Instance, Volume, ExportDevice, FixedIp, FloatingIp, Network, NetworkIndex, SecurityGroup, SecurityGroupIngressRule, - AuthToken) # , Image, Host + SecurityGroupInstanceAssociation, AuthToken) # , Image, Host engine = create_engine(FLAGS.sql_connection, echo=False) for model in models: model.metadata.create_all(engine) -- cgit From 5fa5a0b0b9e13f8f44b257eac0385730c959b92f Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 29 Sep 2010 01:58:19 -0700 Subject: fix the primary and secondary join --- nova/db/sqlalchemy/api.py | 4 ++-- nova/db/sqlalchemy/models.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 200fb3b3c..447d20b25 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -412,10 +412,10 @@ def instance_destroy(_context, instance_id): def instance_get(context, instance_id): session = get_session() instance_ref = session.query(models.Instance - ).options(joinedload_all('fixed_ip.floating_ips') - ).options(joinedload_all('security_groups') ).filter_by(id=instance_id ).filter_by(deleted=_deleted(context) + ).options(joinedload_all('security_groups') + ).options(joinedload_all('fixed_ip.floating_ips') ).first() if not instance_ref: raise exception.NotFound('Instance %s not found' % (instance_id)) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index c2dbf2345..67142ad78 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -359,9 +359,9 @@ class SecurityGroup(BASE, NovaBase): instances = relationship(Instance, secondary="security_group_instance_association", - secondaryjoin="and_(SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id," - "Instance.id == SecurityGroupInstanceAssociation.instance_id," - "SecurityGroup.deleted == False," + primaryjoin="and_(SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id," + "SecurityGroup.deleted == False)", + secondaryjoin="and_(SecurityGroupInstanceAssociation.instance_id == Instance.id," "Instance.deleted == False)", backref='security_groups') -- cgit From a86507b3224eb051fea97f65bd5653758fa91668 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Wed, 29 Sep 2010 06:17:39 -0700 Subject: fix ordering of rules to actually allow out and drop in --- nova/virt/libvirt_conn.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index c86f3ffb7..9d889cf29 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -527,8 +527,8 @@ class NWFilterFirewall(object): def nova_base_ipv4_filter(self): retval = "" for protocol in ['tcp', 'udp', 'icmp']: - for direction,action,priority in [('out','accept', 400), - ('in','drop', 399)]: + for direction,action,priority in [('out','accept', 399), + ('inout','drop', 400)]: retval += """ <%s /> """ % (action, direction, @@ -540,8 +540,8 @@ class NWFilterFirewall(object): def nova_base_ipv6_filter(self): retval = "" for protocol in ['tcp', 'udp', 'icmp']: - for direction,action,priority in [('out','accept',400), - ('in','drop',399)]: + for direction,action,priority in [('out','accept',399), + ('inout','drop',400)]: retval += """ <%s-ipv6 /> """ % (action, direction, -- cgit From 8cb9e732115d531d80c2ae13bc21a48458cd5f2a Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 30 Sep 2010 19:21:50 -0700 Subject: create a new manager for flat networking including dhcp --- nova/network/linux_net.py | 22 ++++--- nova/network/manager.py | 164 +++++++++++++++++++++++++--------------------- 2 files changed, 101 insertions(+), 85 deletions(-) diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 709195ba4..1628fbded 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -172,10 +172,10 @@ def update_dhcp(context, network_id): signal causing it to reload, otherwise spawn a new instance """ network_ref = db.network_get(context, network_id) - with open(_dhcp_file(network_ref['vlan'], 'conf'), 'w') as f: + with open(_dhcp_file(network_ref['bridge'], 'conf'), 'w') as f: f.write(get_dhcp_hosts(context, network_id)) - pid = _dnsmasq_pid_for(network_ref['vlan']) + pid = _dnsmasq_pid_for(network_ref['bridge']) # if dnsmasq is already running, then tell it to reload if pid: @@ -238,11 +238,11 @@ def _dnsmasq_cmd(net): ' --strict-order', ' --bind-interfaces', ' --conf-file=', - ' --pid-file=%s' % _dhcp_file(net['vlan'], 'pid'), + ' --pid-file=%s' % _dhcp_file(net['bridge'], 'pid'), ' --listen-address=%s' % net['gateway'], ' --except-interface=lo', ' --dhcp-range=%s,static,120s' % net['dhcp_start'], - ' --dhcp-hostsfile=%s' % _dhcp_file(net['vlan'], 'conf'), + ' --dhcp-hostsfile=%s' % _dhcp_file(net['bridge'], 'conf'), ' --dhcp-script=%s' % _bin_file('nova-dhcpbridge'), ' --leasefile-ro'] return ''.join(cmd) @@ -259,10 +259,12 @@ def _stop_dnsmasq(network): logging.debug("Killing dnsmasq threw %s", exc) -def _dhcp_file(vlan, kind): - """Return path to a pid, leases or conf file for a vlan""" +def _dhcp_file(bridge, kind): + """Return path to a pid, leases or conf file for a bridge""" - return os.path.abspath("%s/nova-%s.%s" % (FLAGS.networks_path, vlan, kind)) + return os.path.abspath("%s/nova-%s.%s" % (FLAGS.networks_path, + bridge, + kind)) def _bin_file(script): @@ -270,15 +272,15 @@ def _bin_file(script): return os.path.abspath(os.path.join(__file__, "../../../bin", script)) -def _dnsmasq_pid_for(vlan): - """Returns he pid for prior dnsmasq instance for a vlan +def _dnsmasq_pid_for(bridge): + """Returns he pid for prior dnsmasq instance for a bridge Returns None if no pid file exists If machine has rebooted pid might be incorrect (caller should check) """ - pid_file = _dhcp_file(vlan, 'pid') + pid_file = _dhcp_file(bridge, 'pid') if os.path.exists(pid_file): with open(pid_file, 'r') as f: diff --git a/nova/network/manager.py b/nova/network/manager.py index 1325c300b..4267dc9a9 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -50,6 +50,8 @@ flags.DEFINE_string('flat_network_broadcast', '192.168.0.255', 'Broadcast for simple network') flags.DEFINE_string('flat_network_dns', '8.8.4.4', 'Dns for simple network') +flags.DEFINE_string('flat_network_dhcp_start', '192.168.0.2', + 'Dhcp start for FlatDhcp') flags.DEFINE_integer('vlan_start', 100, 'First VLAN for private networks') flags.DEFINE_integer('num_networks', 1000, 'Number of networks to support') flags.DEFINE_string('vpn_ip', utils.get_my_ip(), @@ -98,14 +100,6 @@ class NetworkManager(manager.Manager): self._on_set_network_host(context, network_id) return host - def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): - """Gets a fixed ip from the pool""" - raise NotImplementedError() - - def deallocate_fixed_ip(self, context, instance_id, *args, **kwargs): - """Returns a fixed ip to the pool""" - raise NotImplementedError() - def setup_fixed_ip(self, context, address): """Sets up rules for fixed ip""" raise NotImplementedError() @@ -144,6 +138,61 @@ class NetworkManager(manager.Manager): """Returns an floating ip to the pool""" self.db.floating_ip_deallocate(context, floating_address) + def lease_fixed_ip(self, context, mac, address): + """Called by dhcp-bridge when ip is leased""" + logging.debug("Leasing IP %s", address) + fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) + instance_ref = fixed_ip_ref['instance'] + if not instance_ref: + raise exception.Error("IP %s leased that isn't associated" % + address) + if instance_ref['mac_address'] != mac: + raise exception.Error("IP %s leased to bad mac %s vs %s" % + (address, instance_ref['mac_address'], mac)) + self.db.fixed_ip_update(context, + fixed_ip_ref['address'], + {'leased': True}) + if not fixed_ip_ref['allocated']: + logging.warn("IP %s leased that was already deallocated", address) + + def release_fixed_ip(self, context, mac, address): + """Called by dhcp-bridge when ip is released""" + logging.debug("Releasing IP %s", address) + fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) + instance_ref = fixed_ip_ref['instance'] + if not instance_ref: + raise exception.Error("IP %s released that isn't associated" % + address) + if instance_ref['mac_address'] != mac: + raise exception.Error("IP %s released from bad mac %s vs %s" % + (address, instance_ref['mac_address'], mac)) + if not fixed_ip_ref['leased']: + logging.warn("IP %s released that was not leased", address) + self.db.fixed_ip_update(context, + fixed_ip_ref['str_id'], + {'leased': False}) + if not fixed_ip_ref['allocated']: + self.db.fixed_ip_disassociate(context, address) + # NOTE(vish): dhcp server isn't updated until next setup, this + # means there will stale entries in the conf file + # the code below will update the file if necessary + if FLAGS.update_dhcp_on_disassociate: + network_ref = self.db.fixed_ip_get_network(context, address) + self.driver.update_dhcp(context, network_ref['id']) + + def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): + """Gets a fixed ip from the pool""" + network_ref = self.db.project_get_network(context, context.project.id) + address = self.db.fixed_ip_associate_pool(context, + network_ref['id'], + instance_id) + self.db.fixed_ip_update(context, address, {'allocated': True}) + return address + + def deallocate_fixed_ip(self, context, address, *args, **kwargs): + """Returns a fixed ip to the pool""" + self.db.fixed_ip_update(context, address, {'allocated': False}) + @property def _bottom_reserved_ips(self): # pylint: disable-msg=R0201 """Number of reserved ips at the bottom of the range""" @@ -177,20 +226,6 @@ class NetworkManager(manager.Manager): class FlatManager(NetworkManager): """Basic network where no vlans are used""" - def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): - """Gets a fixed ip from the pool""" - network_ref = self.db.project_get_network(context, context.project.id) - address = self.db.fixed_ip_associate_pool(context, - network_ref['id'], - instance_id) - self.db.fixed_ip_update(context, address, {'allocated': True}) - return address - - def deallocate_fixed_ip(self, context, address, *args, **kwargs): - """Returns a fixed ip to the pool""" - self.db.fixed_ip_update(context, address, {'allocated': False}) - self.db.fixed_ip_disassociate(context, address) - def setup_compute_network(self, context, project_id): """Network is created manually""" pass @@ -199,6 +234,11 @@ class FlatManager(NetworkManager): """Currently no setup""" pass + def deallocate_fixed_ip(self, context, address, *args, **kwargs): + """Returns a fixed ip to the pool""" + self.db.fixed_ip_update(context, address, {'allocated': False}) + self.db.fixed_ip_disassociate(context, address) + def _on_set_network_host(self, context, network_id): """Called when this host becomes the host for a project""" # NOTE(vish): should there be two types of network objects @@ -220,6 +260,26 @@ class FlatManager(NetworkManager): self.db.fixed_ip_create(context, {'address': address}) +class FlatDHCPManager(NetworkManager): + """Flat networking with dhcp""" + + def setup_fixed_ip(self, context, address): + """Setup dhcp for this network""" + network_ref = db.fixed_ip_get_by_address(context, address) + self.driver.update_dhcp(context, network_ref['id']) + + def _on_set_network_host(self, context, network_id): + """Called when this host becomes the host for a project""" + super(FlatDHCPManager, self)._on_set_network_host(context, network_id) + network_ref = self.db.network_get(context, network_id) + self.db.network_update(context, + network_id, + {'dhcp_start': FLAGS.flat_network_dhcp_start}) + self.driver.ensure_bridge(network_ref['bridge'], + FLAGS.bridge_dev, + network_ref) + + class VlanManager(NetworkManager): """Vlan network with dhcp""" @@ -244,23 +304,19 @@ class VlanManager(NetworkManager): def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): """Gets a fixed ip from the pool""" - network_ref = self.db.project_get_network(context, context.project.id) if kwargs.get('vpn', None): + network_ref = self.db.project_get_network(context, + context.project.id) address = network_ref['vpn_private_address'] self.db.fixed_ip_associate(context, address, instance_id) + self.db.fixed_ip_update(context, address, {'allocated': True}) else: - address = self.db.fixed_ip_associate_pool(context, - network_ref['id'], - instance_id) - self.db.fixed_ip_update(context, address, {'allocated': True}) + address = super(VlanManager, self).allocate_fixed_ip(context, + instance_id, + *args, + **kwargs) return address - def deallocate_fixed_ip(self, context, address, *args, **kwargs): - """Returns a fixed ip to the pool""" - self.db.fixed_ip_update(context, address, {'allocated': False}) - fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) - - def setup_fixed_ip(self, context, address): """Sets forwarding rules and dhcp for fixed ip""" fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) @@ -271,48 +327,6 @@ class VlanManager(NetworkManager): network_ref['vpn_private_address']) self.driver.update_dhcp(context, network_ref['id']) - def lease_fixed_ip(self, context, mac, address): - """Called by dhcp-bridge when ip is leased""" - logging.debug("Leasing IP %s", address) - fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) - instance_ref = fixed_ip_ref['instance'] - if not instance_ref: - raise exception.Error("IP %s leased that isn't associated" % - address) - if instance_ref['mac_address'] != mac: - raise exception.Error("IP %s leased to bad mac %s vs %s" % - (address, instance_ref['mac_address'], mac)) - self.db.fixed_ip_update(context, - fixed_ip_ref['address'], - {'leased': True}) - if not fixed_ip_ref['allocated']: - logging.warn("IP %s leased that was already deallocated", address) - - def release_fixed_ip(self, context, mac, address): - """Called by dhcp-bridge when ip is released""" - logging.debug("Releasing IP %s", address) - fixed_ip_ref = self.db.fixed_ip_get_by_address(context, address) - instance_ref = fixed_ip_ref['instance'] - if not instance_ref: - raise exception.Error("IP %s released that isn't associated" % - address) - if instance_ref['mac_address'] != mac: - raise exception.Error("IP %s released from bad mac %s vs %s" % - (address, instance_ref['mac_address'], mac)) - if not fixed_ip_ref['leased']: - logging.warn("IP %s released that was not leased", address) - self.db.fixed_ip_update(context, - fixed_ip_ref['str_id'], - {'leased': False}) - if not fixed_ip_ref['allocated']: - self.db.fixed_ip_disassociate(context, address) - # NOTE(vish): dhcp server isn't updated until next setup, this - # means there will stale entries in the conf file - # the code below will update the file if necessary - if FLAGS.update_dhcp_on_disassociate: - network_ref = self.db.fixed_ip_get_network(context, address) - self.driver.update_dhcp(context, network_ref['id']) - def allocate_network(self, context, project_id): """Set up the network""" self._ensure_indexes(context) -- cgit From ddaaebb28649811d723f93a89ee46d69cc3ecabc Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 30 Sep 2010 20:24:42 -0700 Subject: show project ids for groups instead of user ids --- nova/api/ec2/cloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 6c67db28d..8aa76a787 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -260,7 +260,7 @@ class CloudController(object): g = {} g['groupDescription'] = group.description g['groupName'] = group.name - g['ownerId'] = context.user.id + g['ownerId'] = group.project_id g['ipPermissions'] = [] for rule in group.rules: r = {} @@ -272,7 +272,7 @@ class CloudController(object): if rule.group_id: source_group = db.security_group_get(context, rule.group_id) r['groups'] += [{'groupName': source_group.name, - 'userId': source_group.user_id}] + 'userId': source_group.project_id}] else: r['ipRanges'] += [{'cidrIp': rule.cidr}] g['ipPermissions'] += [r] -- cgit From c9d2b8bcb365f326a47df93920c11be2ca054b18 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Thu, 30 Sep 2010 23:04:53 -0700 Subject: Fixed flat network manager with network index gone. Both managers use ips created through nova manage. Use of project_get_network is minimized to make way for managers that would prefer to use cluste or host based ips instead of project based ips. --- bin/nova-manage | 20 ++++--- nova/api/ec2/cloud.py | 11 ++-- nova/api/rackspace/servers.py | 37 ++++++------ nova/db/api.py | 7 ++- nova/db/sqlalchemy/api.py | 14 ++++- nova/network/linux_net.py | 6 +- nova/network/manager.py | 121 +++++++++++++++++++++------------------ nova/test.py | 4 +- nova/tests/compute_unittest.py | 3 +- nova/tests/network_unittest.py | 13 +++-- nova/tests/scheduler_unittest.py | 1 + nova/virt/libvirt_conn.py | 4 +- 12 files changed, 140 insertions(+), 101 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index d421b997a..6da2efe95 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -375,11 +375,14 @@ class FloatingIpCommands(object): class NetworkCommands(object): """Class for managing networks.""" - def create(self, num_networks=None, network_size=None, - vlan_start=None, vpn_start=None): - """Creates floating ips for host by range - arguments: [num_networks=FLAG], [network_size=FLAG], - [vlan_start=FLAG, vpn_start=FLAG]""" + def create(self, fixed_range=None, num_networks=None, + network_size=None, vlan_start=None, vpn_start=None): + """Creates fixed ips for host by range + arguments: [fixed_range=FLAG], [num_networks=FLAG], + [network_size=FLAG], [vlan_start=FLAG], + [vpn_start=FLAG]""" + if not fixed_range: + fixed_range = FLAGS.fixed_range if not num_networks: num_networks = FLAGS.num_networks if not network_size: @@ -388,9 +391,10 @@ class NetworkCommands(object): vlan_start = FLAGS.vlan_start if not vpn_start: vpn_start = FLAGS.vpn_start - net_manager = network_manager.VlanManager() - net_manager.create_networks(None, int(num_networks), int(network_size), - int(vlan_start), int(vpn_start)) + net_manager = utils.import_object(FLAGS.network_manager) + net_manager.create_networks(None, fixed_range, int(num_networks), + int(network_size), int(vlan_start), + int(vpn_start)) CATEGORIES = [ ('user', UserCommands), diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 79c95788b..d8462f7a0 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -522,13 +522,13 @@ class CloudController(object): def _get_network_topic(self, context): """Retrieves the network host for a project""" - network_ref = db.project_get_network(context, context.project.id) + network_ref = self.network_manager.get_network(context) host = network_ref['host'] if not host: host = rpc.call(FLAGS.network_topic, {"method": "set_network_host", "args": {"context": None, - "project_id": context.project.id}}) + "network_id": network_ref['id']}}) return db.queue_get_for(context, FLAGS.network_topic, host) def run_instances(self, context, **kwargs): @@ -612,12 +612,13 @@ class CloudController(object): inst['launch_index'] = num inst['hostname'] = instance_ref['ec2_id'] db.instance_update(context, inst_id, inst) + # TODO(vish): This probably should be done in the scheduler + # or in compute as a call. The network should be + # allocated after the host is assigned and setup + # can happen at the same time. address = self.network_manager.allocate_fixed_ip(context, inst_id, vpn) - - # TODO(vish): This probably should be done in the scheduler - # network is setup when host is assigned network_topic = self._get_network_topic(context) rpc.call(network_topic, {"method": "setup_fixed_ip", diff --git a/nova/api/rackspace/servers.py b/nova/api/rackspace/servers.py index 11efd8aef..0606d14bb 100644 --- a/nova/api/rackspace/servers.py +++ b/nova/api/rackspace/servers.py @@ -64,8 +64,8 @@ def _entity_list(entities): def _entity_detail(inst): """ Maps everything to Rackspace-like attributes for return""" - power_mapping = { - power_state.NOSTATE: 'build', + power_mapping = { + power_state.NOSTATE: 'build', power_state.RUNNING: 'active', power_state.BLOCKED: 'active', power_state.PAUSED: 'suspended', @@ -75,7 +75,7 @@ def _entity_detail(inst): } inst_dict = {} - mapped_keys = dict(status='state', imageId='image_id', + mapped_keys = dict(status='state', imageId='image_id', flavorId='instance_type', name='server_name', id='id') for k, v in mapped_keys.iteritems(): @@ -98,7 +98,7 @@ class Controller(wsgi.Controller): _serialization_metadata = { 'application/xml': { "attributes": { - "server": [ "id", "imageId", "name", "flavorId", "hostId", + "server": [ "id", "imageId", "name", "flavorId", "hostId", "status", "progress", "progress" ] } } @@ -178,7 +178,7 @@ class Controller(wsgi.Controller): user_id = req.environ['nova.context']['user']['id'] inst_dict = self._deserialize(req.body, req) - + if not inst_dict: return faults.Fault(exc.HTTPUnprocessableEntity()) @@ -186,12 +186,12 @@ class Controller(wsgi.Controller): if not instance or instance.user_id != user_id: return faults.Fault(exc.HTTPNotFound()) - self.db_driver.instance_update(None, id, + self.db_driver.instance_update(None, id, _filter_params(inst_dict['server'])) return faults.Fault(exc.HTTPNoContent()) def action(self, req, id): - """ multi-purpose method used to reboot, rebuild, and + """ multi-purpose method used to reboot, rebuild, and resize a server """ input_dict = self._deserialize(req.body, req) try: @@ -217,13 +217,13 @@ class Controller(wsgi.Controller): if v['flavorid'] == flavor_id][0] image_id = env['server']['imageId'] - + img_service, image_id_trans = _image_service() - opaque_image_id = image_id_trans.to_rs_id(image_id) + opaque_image_id = image_id_trans.to_rs_id(image_id) image = img_service.show(opaque_image_id) - if not image: + if not image: raise Exception, "Image not found" inst['server_name'] = env['server']['name'] @@ -259,15 +259,15 @@ class Controller(wsgi.Controller): ref = self.db_driver.instance_create(None, inst) inst['id'] = inst_id_trans.to_rs_id(ref.ec2_id) - + # TODO(dietz): this isn't explicitly necessary, but the networking # calls depend on an object with a project_id property, and therefore # should be cleaned up later api_context = context.APIRequestContext(user_id) - + inst['mac_address'] = utils.generate_mac() - - #TODO(dietz) is this necessary? + + #TODO(dietz) is this necessary? inst['launch_index'] = 0 inst['hostname'] = ref.ec2_id @@ -279,21 +279,20 @@ class Controller(wsgi.Controller): # TODO(vish): This probably should be done in the scheduler # network is setup when host is assigned - network_topic = self._get_network_topic(user_id) + network_topic = self._get_network_topic(None) rpc.call(network_topic, {"method": "setup_fixed_ip", "args": {"context": None, "address": address}}) return inst - def _get_network_topic(self, user_id): + def _get_network_topic(self, context): """Retrieves the network host for a project""" - network_ref = self.db_driver.project_get_network(None, - user_id) + network_ref = self.network_manager.get_network(context) host = network_ref['host'] if not host: host = rpc.call(FLAGS.network_topic, {"method": "set_network_host", "args": {"context": None, - "project_id": user_id}}) + "network_id": network_ref['id']}}) return self.db_driver.queue_get_for(None, FLAGS.network_topic, host) diff --git a/nova/db/api.py b/nova/db/api.py index 9ba994909..1eeee524b 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -400,10 +400,15 @@ def network_get_associated_fixed_ips(context, network_id): def network_get_by_bridge(context, bridge): - """Get an network or raise if it does not exist.""" + """Get a network by bridge or raise if it does not exist.""" return IMPL.network_get_by_bridge(context, bridge) +def network_get_by_instance(context, instance_id): + """Get a network by instance id or raise if it does not exist.""" + return IMPL.network_get_by_instance(context, instance_id) + + def network_get_index(context, network_id): """Get non-conflicting index for network""" return IMPL.network_get_index(context, network_id) diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9d17ccec4..c8099c83e 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -28,7 +28,6 @@ from nova.db.sqlalchemy.session import get_session from sqlalchemy import or_ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm import joinedload_all -from sqlalchemy.orm.exc import NoResultFound from sqlalchemy.sql import exists, func FLAGS = flags.FLAGS @@ -664,6 +663,19 @@ def network_get_by_bridge(_context, bridge): return rv +def network_get_by_instance(_context, instance_id): + session = get_session() + rv = session.query(models.Network + ).filter_by(deleted=False + ).join(models.Network.fixed_ips + ).filter_by(instance_id=instance_id + ).filter_by(deleted=False + ).first() + if not rv: + raise exception.NotFound('No network for instance %s' % instance_id) + return rv + + def network_set_host(_context, network_id, host_id): session = get_session() with session.begin(): diff --git a/nova/network/linux_net.py b/nova/network/linux_net.py index 709195ba4..a3a0d9a37 100644 --- a/nova/network/linux_net.py +++ b/nova/network/linux_net.py @@ -58,12 +58,12 @@ def init_host(): # SNAT rule for outbound traffic. _confirm_rule("POSTROUTING", "-t nat -s %s " "-j SNAT --to-source %s" - % (FLAGS.private_range, FLAGS.routing_source_ip)) + % (FLAGS.fixed_range, FLAGS.routing_source_ip)) _confirm_rule("POSTROUTING", "-t nat -s %s -j MASQUERADE" % - FLAGS.private_range) + FLAGS.fixed_range) _confirm_rule("POSTROUTING", "-t nat -s %(range)s -d %(range)s -j ACCEPT" % - {'range': FLAGS.private_range}) + {'range': FLAGS.fixed_range}) def bind_floating_ip(floating_ip): """Bind ip to public interface""" diff --git a/nova/network/manager.py b/nova/network/manager.py index 0a5acf17a..71f915707 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -37,17 +37,6 @@ from nova import utils FLAGS = flags.FLAGS flags.DEFINE_string('flat_network_bridge', 'br100', 'Bridge for simple network instances') -flags.DEFINE_list('flat_network_ips', - ['192.168.0.2', '192.168.0.3', '192.168.0.4'], - 'Available ips for simple network') -flags.DEFINE_string('flat_network_network', '192.168.0.0', - 'Network for simple network') -flags.DEFINE_string('flat_network_netmask', '255.255.255.0', - 'Netmask for simple network') -flags.DEFINE_string('flat_network_gateway', '192.168.0.1', - 'Broadcast for simple network') -flags.DEFINE_string('flat_network_broadcast', '192.168.0.255', - 'Broadcast for simple network') flags.DEFINE_string('flat_network_dns', '8.8.4.4', 'Dns for simple network') flags.DEFINE_integer('vlan_start', 100, 'First VLAN for private networks') @@ -57,8 +46,8 @@ flags.DEFINE_string('vpn_ip', utils.get_my_ip(), flags.DEFINE_integer('vpn_start', 1000, 'First Vpn port for private networks') flags.DEFINE_integer('network_size', 256, 'Number of addresses in each private subnet') -flags.DEFINE_string('public_range', '4.4.4.0/24', 'Public IP address block') -flags.DEFINE_string('private_range', '10.0.0.0/8', 'Private IP address block') +flags.DEFINE_string('floating_range', '4.4.4.0/24', 'Floating IP address block') +flags.DEFINE_string('fixed_range', '10.0.0.0/8', 'Fixed IP address block') flags.DEFINE_integer('cnt_vpn_clients', 5, 'Number of addresses reserved for vpn clients') flags.DEFINE_string('network_driver', 'nova.network.linux_net', @@ -85,13 +74,9 @@ class NetworkManager(manager.Manager): self.driver = utils.import_object(network_driver) super(NetworkManager, self).__init__(*args, **kwargs) - def set_network_host(self, context, project_id): - """Safely sets the host of the projects network""" + def set_network_host(self, context, network_id): + """Safely sets the host of the network""" logging.debug("setting network host") - network_ref = self.db.project_get_network(context, project_id) - # TODO(vish): can we minimize db access by just getting the - # id here instead of the ref? - network_id = network_ref['id'] host = self.db.network_set_host(context, network_id, self.host) @@ -111,10 +96,10 @@ class NetworkManager(manager.Manager): raise NotImplementedError() def _on_set_network_host(self, context, network_id): - """Called when this host becomes the host for a project""" + """Called when this host becomes the host for a network""" raise NotImplementedError() - def setup_compute_network(self, context, project_id): + def setup_compute_network(self, context, instance_id): """Sets up matching network for compute hosts""" raise NotImplementedError() @@ -144,6 +129,16 @@ class NetworkManager(manager.Manager): """Returns an floating ip to the pool""" self.db.floating_ip_deallocate(context, floating_address) + def get_network(self, context): + """Get the network for the current context""" + raise NotImplementedError() + + def create_networks(self, context, num_networks, network_size, + *args, **kwargs): + """Create networks based on parameters""" + raise NotImplementedError() + + @property def _bottom_reserved_ips(self): # pylint: disable-msg=R0201 """Number of reserved ips at the bottom of the range""" @@ -179,7 +174,12 @@ class FlatManager(NetworkManager): def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): """Gets a fixed ip from the pool""" - network_ref = self.db.project_get_network(context, context.project.id) + # TODO(vish): when this is called by compute, we can associate compute + # with a network, or a cluster of computes with a network + # and use that network here with a method like + # network_get_by_compute_host + network_ref = self.db.network_get_by_bridge(context, + FLAGS.flat_network_bridge) address = self.db.fixed_ip_associate_pool(context, network_ref['id'], instance_id) @@ -191,7 +191,7 @@ class FlatManager(NetworkManager): self.db.fixed_ip_update(context, address, {'allocated': False}) self.db.fixed_ip_disassociate(context, address) - def setup_compute_network(self, context, project_id): + def setup_compute_network(self, context, instance_id): """Network is created manually""" pass @@ -199,25 +199,42 @@ class FlatManager(NetworkManager): """Currently no setup""" pass + def create_networks(self, context, cidr, num_networks, network_size, + *args, **kwargs): + """Create networks based on parameters""" + fixed_net = IPy.IP(cidr) + for index in range(num_networks): + start = index * network_size + significant_bits = 32 - int(math.log(network_size, 2)) + cidr = "%s/%s" % (fixed_net[start], significant_bits) + project_net = IPy.IP(cidr) + net = {} + net['cidr'] = cidr + net['netmask'] = str(project_net.netmask()) + net['gateway'] = str(project_net[1]) + net['broadcast'] = str(project_net.broadcast()) + net['dhcp_start'] = str(project_net[2]) + network_ref = self.db.network_create_safe(context, net) + if network_ref: + self._create_fixed_ips(context, network_ref['id']) + + def get_network(self, context): + """Get the network for the current context""" + # NOTE(vish): To support mutilple network hosts, This could randomly + # select from multiple networks instead of just + # returning the one. It could also potentially be done + # in the scheduler. + return self.db.network_get_by_bridge(context, + FLAGS.flat_network_bridge) + def _on_set_network_host(self, context, network_id): - """Called when this host becomes the host for a project""" - # NOTE(vish): should there be two types of network objects - # in the datastore? + """Called when this host becomes the host for a network""" net = {} net['injected'] = True - net['network_str'] = FLAGS.flat_network_network - net['netmask'] = FLAGS.flat_network_netmask net['bridge'] = FLAGS.flat_network_bridge - net['gateway'] = FLAGS.flat_network_gateway - net['broadcast'] = FLAGS.flat_network_broadcast net['dns'] = FLAGS.flat_network_dns self.db.network_update(context, network_id, net) - # NOTE(vish): Rignt now we are putting all of the fixed ips in - # one large pool, but ultimately it may be better to - # have each network manager have its own network that - # it is responsible for and its own pool of ips. - for address in FLAGS.flat_network_ips: - self.db.fixed_ip_create(context, {'address': address}) + class VlanManager(NetworkManager): @@ -244,6 +261,9 @@ class VlanManager(NetworkManager): def allocate_fixed_ip(self, context, instance_id, *args, **kwargs): """Gets a fixed ip from the pool""" + # TODO(vish): This should probably be getting project_id from + # the instance, but it is another trip to the db. + # Perhaps this method should take an instance_ref. network_ref = self.db.project_get_network(context, context.project.id) if kwargs.get('vpn', None): address = network_ref['vpn_private_address'] @@ -313,22 +333,9 @@ class VlanManager(NetworkManager): network_ref = self.db.fixed_ip_get_network(context, address) self.driver.update_dhcp(context, network_ref['id']) - - def associate_network(self, context, project_id): - """Associates a network to a project""" - network_ref = db.network_associate(context, project_id) - network_id = network_ref['id'] - return network_id - - - def disassociate_network(self, context, network_id): - """Disassociates a newtwork from a project""" - db.network_disassocate(context, network_id) - - - def setup_compute_network(self, context, project_id): + def setup_compute_network(self, context, instance_id): """Sets up matching network for compute hosts""" - network_ref = self.db.project_get_network(context, project_id) + network_ref = db.network_get_by_instance(context, instance_id) self.driver.ensure_vlan_bridge(network_ref['vlan'], network_ref['bridge']) @@ -337,15 +344,15 @@ class VlanManager(NetworkManager): # TODO(vish): Implement this pass - def create_networks(self, context, num_networks, network_size, + def create_networks(self, context, cidr, num_networks, network_size, vlan_start, vpn_start): """Create networks based on parameters""" + fixed_net = IPy.IP(cidr) for index in range(num_networks): - private_net = IPy.IP(FLAGS.private_range) vlan = vlan_start + index start = index * network_size significant_bits = 32 - int(math.log(network_size, 2)) - cidr = "%s/%s" % (private_net[start], significant_bits) + cidr = "%s/%s" % (fixed_net[start], significant_bits) project_net = IPy.IP(cidr) net = {} net['cidr'] = cidr @@ -363,8 +370,12 @@ class VlanManager(NetworkManager): if network_ref: self._create_fixed_ips(context, network_ref['id']) + def get_network(self, context): + """Get the network for the current context""" + return self.db.project_get_network(context, context.project.id) + def _on_set_network_host(self, context, network_id): - """Called when this host becomes the host for a project""" + """Called when this host becomes the host for a network""" network_ref = self.db.network_get(context, network_id) net = {} net['vpn_public_address'] = FLAGS.vpn_ip diff --git a/nova/test.py b/nova/test.py index 91668f9b0..493754e83 100644 --- a/nova/test.py +++ b/nova/test.py @@ -63,7 +63,9 @@ class TrialTestCase(unittest.TestCase): # now that we have some required db setup for the system # to work properly. if db.network_count(None) != 5: - network_manager.VlanManager().create_networks(None, 5, 16, + network_manager.VlanManager().create_networks(None, + FLAGS.fixed_range, + 5, 16, FLAGS.vlan_start, FLAGS.vpn_start) diff --git a/nova/tests/compute_unittest.py b/nova/tests/compute_unittest.py index f5c0f1c09..e695d8fba 100644 --- a/nova/tests/compute_unittest.py +++ b/nova/tests/compute_unittest.py @@ -40,7 +40,8 @@ class ComputeTestCase(test.TrialTestCase): def setUp(self): # pylint: disable-msg=C0103 logging.getLogger().setLevel(logging.DEBUG) super(ComputeTestCase, self).setUp() - self.flags(connection_type='fake') + self.flags(connection_type='fake', + network_manager='nova.network.manager.FlatManager') self.compute = utils.import_object(FLAGS.compute_manager) self.manager = manager.AuthManager() self.user = self.manager.create_user('fake', 'fake', 'fake') diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 5ceb336ec..8bd2bb2fd 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -52,11 +52,14 @@ class NetworkTestCase(test.TrialTestCase): self.context = context.APIRequestContext(project=None, user=self.user) for i in range(5): name = 'project%s' % i - self.projects.append(self.manager.create_project(name, - 'netuser', - name)) + project = self.manager.create_project(name, 'netuser', name) + self.projects.append(project) # create the necessary network data for the project - self.network.set_network_host(self.context, self.projects[i].id) + self.context.project = project + network_ref = self.network.get_network(self.context) + if not network_ref['host']: + self.network.set_network_host(self.context, network_ref['id']) + self.context.project = None instance_ref = db.instance_create(None, {'mac_address': utils.generate_mac()}) self.instance_id = instance_ref['id'] @@ -84,7 +87,7 @@ class NetworkTestCase(test.TrialTestCase): def test_public_network_association(self): """Makes sure that we can allocaate a public ip""" # TODO(vish): better way of adding floating ips - pubnet = IPy.IP(flags.FLAGS.public_range) + pubnet = IPy.IP(flags.FLAGS.floating_range) address = str(pubnet[0]) try: db.floating_ip_get_by_address(None, address) diff --git a/nova/tests/scheduler_unittest.py b/nova/tests/scheduler_unittest.py index fde30f81e..f6ee19756 100644 --- a/nova/tests/scheduler_unittest.py +++ b/nova/tests/scheduler_unittest.py @@ -75,6 +75,7 @@ class SimpleDriverTestCase(test.TrialTestCase): self.flags(connection_type='fake', max_cores=4, max_gigabytes=4, + network_manager='nova.network.manager.FlatManager', volume_driver='nova.volume.driver.FakeAOEDriver', scheduler_driver='nova.scheduler.simple.SimpleScheduler') self.scheduler = manager.SchedulerManager() diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index d868e083c..f6d8aace6 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -280,7 +280,7 @@ class LibvirtConnection(object): key = str(inst['key_data']) net = None - network_ref = db.project_get_network(None, project.id) + network_ref = db.network_get_by_instance(None, inst['id']) if network_ref['injected']: address = db.instance_get_fixed_address(None, inst['id']) with open(FLAGS.injected_network_template) as f: @@ -314,7 +314,7 @@ class LibvirtConnection(object): def to_xml(self, instance): # TODO(termie): cache? logging.debug('instance %s: starting toXML method', instance['name']) - network = db.project_get_network(None, instance['project_id']) + network = db.instance_get_fixed_by_instance(None, inst['id']) # FIXME(vish): stick this in db instance_type = instance_types.INSTANCE_TYPES[instance['instance_type']] xml_info = {'type': FLAGS.libvirt_type, -- cgit From 48ff601a3ab2d72275061135cac56557042e8e9d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Sat, 2 Oct 2010 12:46:12 -0700 Subject: fix typo in setup_compute_network --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index f370ede8b..4c6d2f06f 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -71,7 +71,7 @@ class ComputeManager(manager.Manager): raise exception.Error("Instance has already been created") logging.debug("instance %s: starting...", instance_id) project_id = instance_ref['project_id'] - self.network_manager.setup_compute_network(context, project_id) + self.network_manager.setup_compute_network(context, instance_id) self.db.instance_update(context, instance_id, {'host': self.host}) -- cgit From a4720c03a8260fb920035d072799d3ecc478db99 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 4 Oct 2010 21:58:22 +0200 Subject: Merge security group related changes from lp:~anso/nova/deploy --- nova/api/ec2/cloud.py | 31 ++++++++++--- nova/db/sqlalchemy/api.py | 105 +++++++++++++++++++++++++++++++++----------- nova/tests/virt_unittest.py | 33 +++++++++----- nova/virt/libvirt_conn.py | 39 ++++++++++++++-- 4 files changed, 162 insertions(+), 46 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 839b84b4e..4cd4c78ae 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -327,6 +327,26 @@ class CloudController(object): return values + + def _security_group_rule_exists(self, security_group, values): + """Indicates whether the specified rule values are already + defined in the given security group. + """ + for rule in security_group.rules: + if 'group_id' in values: + if rule['group_id'] == values['group_id']: + return True + else: + is_duplicate = True + for key in ('cidr', 'from_port', 'to_port', 'protocol'): + if rule[key] != values[key]: + is_duplicate = False + break + if is_duplicate: + return True + return False + + def revoke_security_group_ingress(self, context, group_name, **kwargs): self._ensure_default_security_group(context) security_group = db.security_group_get_by_name(context, @@ -348,9 +368,6 @@ class CloudController(object): return True raise exception.ApiError("No rule for the specified parameters.") - # TODO(soren): Dupe detection. Adding the same rule twice actually - # adds the same rule twice to the rule set, which is - # pointless. # TODO(soren): This has only been tested with Boto as the client. # Unfortunately, it seems Boto is using an old API # for these operations, so support for newer API versions @@ -364,6 +381,10 @@ class CloudController(object): values = self._authorize_revoke_rule_args_to_dict(context, **kwargs) values['parent_group_id'] = security_group.id + if self._security_group_rule_exists(security_group, values): + raise exception.ApiError('This rule already exists in group %s' % + group_name) + security_group_rule = db.security_group_rule_create(context, values) self._trigger_refresh_security_group(security_group) @@ -709,7 +730,7 @@ class CloudController(object): 'description' : 'default', 'user_id' : context.user.id, 'project_id' : context.project.id } - group = db.security_group_create({}, values) + group = db.security_group_create(context, values) def run_instances(self, context, **kwargs): instance_type = kwargs.get('instance_type', 'm1.small') @@ -797,7 +818,7 @@ class CloudController(object): inst_id = instance_ref['id'] for security_group_id in security_groups: - db.instance_add_security_group(context, inst_id, + db.instance_add_security_group(context.admin(), inst_id, security_group_id) inst = {} diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index d395b7e2c..bc5ef5a9b 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -572,11 +572,13 @@ def instance_get(context, instance_id, session=None): if is_admin_context(context): result = session.query(models.Instance + ).options(joinedload('security_groups') ).filter_by(id=instance_id ).filter_by(deleted=can_read_deleted(context) ).first() elif is_user_context(context): result = session.query(models.Instance + ).options(joinedload('security_groups') ).filter_by(project_id=context.project.id ).filter_by(id=instance_id ).filter_by(deleted=False @@ -592,6 +594,7 @@ def instance_get_all(context): session = get_session() return session.query(models.Instance ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload('security_groups') ).filter_by(deleted=can_read_deleted(context) ).all() @@ -601,6 +604,7 @@ def instance_get_all_by_user(context, user_id): session = get_session() return session.query(models.Instance ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload('security_groups') ).filter_by(deleted=can_read_deleted(context) ).filter_by(user_id=user_id ).all() @@ -613,6 +617,7 @@ def instance_get_all_by_project(context, project_id): session = get_session() return session.query(models.Instance ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload('security_groups') ).filter_by(project_id=project_id ).filter_by(deleted=can_read_deleted(context) ).all() @@ -625,12 +630,14 @@ def instance_get_all_by_reservation(context, reservation_id): if is_admin_context(context): return session.query(models.Instance ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload('security_groups') ).filter_by(reservation_id=reservation_id ).filter_by(deleted=can_read_deleted(context) ).all() elif is_user_context(context): return session.query(models.Instance ).options(joinedload_all('fixed_ip.floating_ips') + ).options(joinedload('security_groups') ).filter_by(project_id=context.project.id ).filter_by(reservation_id=reservation_id ).filter_by(deleted=False @@ -643,11 +650,13 @@ def instance_get_by_ec2_id(context, ec2_id): if is_admin_context(context): result = session.query(models.Instance + ).options(joinedload('security_groups') ).filter_by(ec2_id=ec2_id ).filter_by(deleted=can_read_deleted(context) ).first() elif is_user_context(context): result = session.query(models.Instance + ).options(joinedload('security_groups') ).filter_by(project_id=context.project.id ).filter_by(ec2_id=ec2_id ).filter_by(deleted=False @@ -721,9 +730,10 @@ def instance_add_security_group(context, instance_id, security_group_id): """Associate the given security group with the given instance""" session = get_session() with session.begin(): - instance_ref = models.Instance.find(instance_id, session=session) - security_group_ref = models.SecurityGroup.find(security_group_id, - session=session) + instance_ref = instance_get(context, instance_id, session=session) + security_group_ref = security_group_get(context, + security_group_id, + session=session) instance_ref.security_groups += [security_group_ref] instance_ref.save(session=session) @@ -1202,6 +1212,7 @@ def volume_get(context, volume_id, session=None): @require_admin_context def volume_get_all(context): + session = get_session() return session.query(models.Volume ).filter_by(deleted=can_read_deleted(context) ).all() @@ -1292,27 +1303,39 @@ def volume_update(context, volume_id, values): ################### -def security_group_get_all(_context): +@require_context +def security_group_get_all(context): session = get_session() return session.query(models.SecurityGroup - ).filter_by(deleted=False + ).filter_by(deleted=can_read_deleted(context) ).options(joinedload_all('rules') ).all() -def security_group_get(_context, security_group_id): - session = get_session() - result = session.query(models.SecurityGroup - ).filter_by(deleted=False - ).filter_by(id=security_group_id - ).options(joinedload_all('rules') - ).first() +@require_context +def security_group_get(context, security_group_id, session=None): + if not session: + session = get_session() + if is_admin_context(context): + result = session.query(models.SecurityGroup + ).filter_by(deleted=can_read_deleted(context), + ).filter_by(id=security_group_id + ).options(joinedload_all('rules') + ).first() + else: + result = session.query(models.SecurityGroup + ).filter_by(deleted=False + ).filter_by(id=security_group_id + ).filter_by(project_id=context.project_id + ).options(joinedload_all('rules') + ).first() if not result: raise exception.NotFound("No secuity group with id %s" % security_group_id) return result +@require_context def security_group_get_by_name(context, project_id, group_name): session = get_session() result = session.query(models.SecurityGroup @@ -1329,7 +1352,8 @@ def security_group_get_by_name(context, project_id, group_name): return result -def security_group_get_by_project(_context, project_id): +@require_context +def security_group_get_by_project(context, project_id): session = get_session() return session.query(models.SecurityGroup ).filter_by(project_id=project_id @@ -1338,7 +1362,8 @@ def security_group_get_by_project(_context, project_id): ).all() -def security_group_get_by_instance(_context, instance_id): +@require_context +def security_group_get_by_instance(context, instance_id): session = get_session() return session.query(models.SecurityGroup ).filter_by(deleted=False @@ -1349,15 +1374,17 @@ def security_group_get_by_instance(_context, instance_id): ).all() -def security_group_exists(_context, project_id, group_name): +@require_context +def security_group_exists(context, project_id, group_name): try: - group = security_group_get_by_name(_context, project_id, group_name) + group = security_group_get_by_name(context, project_id, group_name) return group != None except exception.NotFound: return False -def security_group_create(_context, values): +@require_context +def security_group_create(context, values): security_group_ref = models.SecurityGroup() # FIXME(devcamcar): Unless I do this, rules fails with lazy load exception # once save() is called. This will get cleaned up in next orm pass. @@ -1368,7 +1395,8 @@ def security_group_create(_context, values): return security_group_ref -def security_group_destroy(_context, security_group_id): +@require_context +def security_group_destroy(context, security_group_id): session = get_session() with session.begin(): # TODO(vish): do we have to use sql here? @@ -1378,35 +1406,62 @@ def security_group_destroy(_context, security_group_id): 'where group_id=:id', {'id': security_group_id}) -def security_group_destroy_all(_context): - session = get_session() +@require_context +def security_group_destroy_all(context, session=None): + if not session: + session = get_session() with session.begin(): # TODO(vish): do we have to use sql here? session.execute('update security_groups set deleted=1') session.execute('update security_group_rules set deleted=1') + ################### -def security_group_rule_create(_context, values): +@require_context +def security_group_rule_get(context, security_group_rule_id, session=None): + if not session: + session = get_session() + if is_admin_context(context): + result = session.query(models.SecurityGroupIngressRule + ).filter_by(deleted=can_read_deleted(context) + ).filter_by(id=security_group_rule_id + ).first() + else: + # TODO(vish): Join to group and check for project_id + result = session.query(models.SecurityGroupIngressRule + ).filter_by(deleted=False + ).filter_by(id=security_group_rule_id + ).first() + if not result: + raise exception.NotFound("No secuity group rule with id %s" % + security_group_rule_id) + return result + + +@require_context +def security_group_rule_create(context, values): security_group_rule_ref = models.SecurityGroupIngressRule() for (key, value) in values.iteritems(): security_group_rule_ref[key] = value security_group_rule_ref.save() return security_group_rule_ref -def security_group_rule_destroy(_context, security_group_rule_id): +@require_context +def security_group_rule_destroy(context, security_group_rule_id): session = get_session() with session.begin(): - model = models.SecurityGroupIngressRule - security_group_rule = model.find(security_group_rule_id, - session=session) + security_group_rule = security_group_rule_get(context, + security_group_rule_id, + session=session) security_group_rule.delete(session=session) ################### +@require_admin_context def host_get_networks(context, host): session = get_session() with session.begin(): diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 7fa8e52ac..8b0de6c29 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -19,7 +19,9 @@ from xml.dom.minidom import parseString from nova import db from nova import flags from nova import test +from nova.api import context from nova.api.ec2 import cloud +from nova.auth import manager from nova.virt import libvirt_conn FLAGS = flags.FLAGS @@ -83,17 +85,20 @@ class NWFilterTestCase(test.TrialTestCase): class Mock(object): pass - self.context = Mock() - self.context.user = Mock() - self.context.user.id = 'fake' - self.context.user.is_superuser = lambda:True - self.context.project = Mock() - self.context.project.id = 'fake' + self.manager = manager.AuthManager() + self.user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + self.project = self.manager.create_project('fake', 'fake', 'fake') + self.context = context.APIRequestContext(self.user, self.project) self.fake_libvirt_connection = Mock() self.fw = libvirt_conn.NWFilterFirewall(self.fake_libvirt_connection) + def tearDown(self): + self.manager.delete_project(self.project) + self.manager.delete_user(self.user) + + def test_cidr_rule_nwfilter_xml(self): cloud_controller = cloud.CloudController() cloud_controller.create_security_group(self.context, @@ -107,7 +112,9 @@ class NWFilterTestCase(test.TrialTestCase): cidr_ip='0.0.0.0/0') - security_group = db.security_group_get_by_name({}, 'fake', 'testgroup') + security_group = db.security_group_get_by_name(self.context, + 'fake', + 'testgroup') xml = self.fw.security_group_to_nwfilter_xml(security_group.id) @@ -126,7 +133,8 @@ class NWFilterTestCase(test.TrialTestCase): ip_conditions = rules[0].getElementsByTagName('tcp') self.assertEqual(len(ip_conditions), 1) - self.assertEqual(ip_conditions[0].getAttribute('srcipaddr'), '0.0.0.0/0') + self.assertEqual(ip_conditions[0].getAttribute('srcipaddr'), '0.0.0.0') + self.assertEqual(ip_conditions[0].getAttribute('srcipmask'), '0.0.0.0') self.assertEqual(ip_conditions[0].getAttribute('dstportstart'), '80') self.assertEqual(ip_conditions[0].getAttribute('dstportend'), '81') @@ -150,7 +158,7 @@ class NWFilterTestCase(test.TrialTestCase): ip_protocol='tcp', cidr_ip='0.0.0.0/0') - return db.security_group_get_by_name({}, 'fake', 'testgroup') + return db.security_group_get_by_name(self.context, 'fake', 'testgroup') def test_creates_base_rule_first(self): # These come pre-defined by libvirt @@ -180,7 +188,8 @@ class NWFilterTestCase(test.TrialTestCase): self.fake_libvirt_connection.nwfilterDefineXML = _filterDefineXMLMock - instance_ref = db.instance_create({}, {'user_id': 'fake', + instance_ref = db.instance_create(self.context, + {'user_id': 'fake', 'project_id': 'fake'}) inst_id = instance_ref['id'] @@ -195,8 +204,8 @@ class NWFilterTestCase(test.TrialTestCase): self.security_group = self.setup_and_return_security_group() - db.instance_add_security_group({}, inst_id, self.security_group.id) - instance = db.instance_get({}, inst_id) + db.instance_add_security_group(self.context, inst_id, self.security_group.id) + instance = db.instance_get(self.context, inst_id) d = self.fw.setup_nwfilters_for_instance(instance) d.addCallback(_ensure_all_called) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 9d889cf29..319f7d2af 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -25,6 +25,7 @@ import logging import os import shutil +import IPy from twisted.internet import defer from twisted.internet import task from twisted.internet import threads @@ -34,6 +35,7 @@ from nova import exception from nova import flags from nova import process from nova import utils +#from nova.api import context from nova.auth import manager from nova.compute import disk from nova.compute import instance_types @@ -61,6 +63,9 @@ flags.DEFINE_string('libvirt_uri', '', 'Override the default libvirt URI (which is dependent' ' on libvirt_type)') +flags.DEFINE_bool('allow_project_net_traffic', + True, + 'Whether to allow in project network traffic') def get_connection(read_only): @@ -135,7 +140,7 @@ class LibvirtConnection(object): d.addCallback(lambda _: self._cleanup(instance)) # FIXME: What does this comment mean? # TODO(termie): short-circuit me for tests - # WE'LL save this for when we do shutdown, + # WE'LL save this for when we do shutdown, # instead of destroy - but destroy returns immediately timer = task.LoopingCall(f=None) def _wait_for_shutdown(): @@ -550,6 +555,16 @@ class NWFilterFirewall(object): return retval + def nova_project_filter(self, project, net, mask): + retval = "" % project + for protocol in ['tcp', 'udp', 'icmp']: + retval += """ + <%s srcipaddr='%s' srcipmask='%s' /> + """ % (protocol, net, mask) + retval += '' + return retval + + def _define_filter(self, xml): if callable(xml): xml = xml() @@ -557,6 +572,11 @@ class NWFilterFirewall(object): return d + @staticmethod + def _get_net_and_mask(cidr): + net = IPy.IP(cidr) + return str(net.net()), str(net.netmask()) + @defer.inlineCallbacks def setup_nwfilters_for_instance(self, instance): """ @@ -570,9 +590,19 @@ class NWFilterFirewall(object): yield self._define_filter(self.nova_dhcp_filter) yield self._define_filter(self.nova_base_filter) - nwfilter_xml = ("\n" + + nwfilter_xml = ("\n" + " \n" - ) % instance['name'] + ) % instance['name'] + + if FLAGS.allow_project_net_traffic: + network_ref = db.project_get_network({}, instance['project_id']) + net, mask = self._get_net_and_mask(network_ref['cidr']) + project_filter = self.nova_project_filter(instance['project_id'], + net, mask) + yield self._define_filter(project_filter) + + nwfilter_xml += (" \n" + ) % instance['project_id'] for security_group in instance.security_groups: yield self.ensure_security_group_filter(security_group['id']) @@ -595,7 +625,8 @@ class NWFilterFirewall(object): for rule in security_group.rules: rule_xml += "" if rule.cidr: - rule_xml += "<%s srcipaddr='%s' " % (rule.protocol, rule.cidr) + net, mask = self._get_net_and_mask(rule.cidr) + rule_xml += "<%s srcipaddr='%s' srcipmask='%s' " % (rule.protocol, net, mask) if rule.protocol in ['tcp', 'udp']: rule_xml += "dstportstart='%s' dstportend='%s' " % \ (rule.from_port, rule.to_port) -- cgit From 1158e1817b7d39e9655b219ede865f301153e713 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 11:17:03 +0200 Subject: Un-twistedify get_console_ouptut. --- nova/api/ec2/cloud.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 1f01731ae..e7147ec05 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -247,16 +247,16 @@ class CloudController(object): def get_console_output(self, context, instance_id, **kwargs): # instance_id is passed in as a list of instances instance_ref = db.instance_get_by_ec2_id(context, instance_id[0]) - d = rpc.call('%s.%s' % (FLAGS.compute_topic, + output = rpc.call('%s.%s' % (FLAGS.compute_topic, instance_ref['host']), { "method" : "get_console_output", "args" : { "context": None, "instance_id": instance_ref['id']}}) - d.addCallback(lambda output: { "InstanceId": instance_id, - "Timestamp": "2", - "output": base64.b64encode(output)}) - return d + now = datetime.datetime.utcnow() + return { "InstanceId" : instance_id, + "Timestamp" : now, + "output" : base64.b64encode(output) } def describe_volumes(self, context, **kwargs): if context.user.is_admin(): -- cgit From 4b9be67025c3aff4b7f5f9b31a74eb14924885cb Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 15:20:23 +0200 Subject: Add a connect_to_eventlet method. --- nova/rpc.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/nova/rpc.py b/nova/rpc.py index fe52ad35f..782c76765 100644 --- a/nova/rpc.py +++ b/nova/rpc.py @@ -28,6 +28,7 @@ import uuid from carrot import connection as carrot_connection from carrot import messaging +from eventlet import greenthread from twisted.internet import defer from twisted.internet import task @@ -107,6 +108,13 @@ class Consumer(messaging.Consumer): logging.exception("Failed to fetch message from queue") self.failed_connection = True + def attach_to_eventlet(self): + def fetch_repeatedly(): + while True: + self.fetch(enable_callbacks=True) + greenthread.sleep(0.1) + greenthread.spawn(fetch_repeatedly) + def attach_to_twisted(self): """Attach a callback to twisted that fires 10 times a second""" loop = task.LoopingCall(self.fetch, enable_callbacks=True) -- cgit From 394c5ae180ed25f7e617f6c43f9a88f003e5d2ea Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 15:21:08 +0200 Subject: Make rpc calls work in unit tests by adding extra declare_consumer and consume methods on the FakeRabbit backend. --- nova/fakerabbit.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/nova/fakerabbit.py b/nova/fakerabbit.py index 068025249..6679f8dab 100644 --- a/nova/fakerabbit.py +++ b/nova/fakerabbit.py @@ -38,6 +38,7 @@ class Exchange(object): def publish(self, message, routing_key=None): logging.debug('(%s) publish (key: %s) %s', self.name, routing_key, message) + routing_key = routing_key.split('.')[0] if routing_key in self._routes: for f in self._routes[routing_key]: logging.debug('Publishing to route %s', f) @@ -94,6 +95,20 @@ class Backend(object): self._exchanges[exchange].bind(self._queues[queue].push, routing_key) + def declare_consumer(self, queue, callback, *args, **kwargs): + print 'declare_consumer', queue, callback + self.current_queue = queue + self.current_callback = callback + + def consume(self, *args, **kwargs): + from eventlet import greenthread + while True: + item = self.get(self.current_queue) + if item: + self.current_callback(item) + raise StopIteration() + greenthread.sleep(0) + def get(self, queue, no_ack=False): if not queue in self._queues or not self._queues[queue].size(): return None @@ -102,6 +117,7 @@ class Backend(object): message = Message(backend=self, body=message_data, content_type=content_type, content_encoding=content_encoding) + message.result = True logging.debug('Getting from %s: %s', queue, message) return message -- cgit From 10bbf9f638b5c8c9182984cc7e22f732b194476f Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 15:21:31 +0200 Subject: Stub out ec2.images.list() for unit tests. --- nova/api/ec2/images.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nova/api/ec2/images.py b/nova/api/ec2/images.py index cb54cdda2..f0a43dad6 100644 --- a/nova/api/ec2/images.py +++ b/nova/api/ec2/images.py @@ -69,6 +69,9 @@ def list(context, filter_list=[]): optionally filtered by a list of image_id """ + if FLAGS.connection_type == 'fake': + return [{ 'imageId' : 'bar'}] + # FIXME: send along the list of only_images to check for response = conn(context).make_request( method='GET', -- cgit From 83430481760250e633275182d5cf4eb826f65ea2 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 15:21:56 +0200 Subject: Make (some) cloud unit tests run without a full-blown set up. --- nova/tests/cloud_unittest.py | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index ae7dea1db..f67eff7c2 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -16,6 +16,7 @@ # License for the specific language governing permissions and limitations # under the License. +from base64 import b64decode import json import logging from M2Crypto import BIO @@ -63,11 +64,16 @@ class CloudTestCase(test.TrialTestCase): self.cloud = cloud.CloudController() # set up a service - self.compute = utils.import_class(FLAGS.compute_manager) + self.compute = utils.import_class(FLAGS.compute_manager)() self.compute_consumer = rpc.AdapterConsumer(connection=self.conn, topic=FLAGS.compute_topic, proxy=self.compute) - self.compute_consumer.attach_to_twisted() + self.compute_consumer.attach_to_eventlet() + self.network = utils.import_class(FLAGS.network_manager)() + self.network_consumer = rpc.AdapterConsumer(connection=self.conn, + topic=FLAGS.network_topic, + proxy=self.network) + self.network_consumer.attach_to_eventlet() self.manager = manager.AuthManager() self.user = self.manager.create_user('admin', 'admin', 'admin', True) @@ -85,15 +91,17 @@ class CloudTestCase(test.TrialTestCase): return cloud._gen_key(self.context, self.context.user.id, name) def test_console_output(self): - if FLAGS.connection_type == 'fake': - logging.debug("Can't test instances without a real virtual env.") - return - instance_id = 'foo' - inst = yield self.compute.run_instance(instance_id) + image_id = FLAGS.default_image + instance_type = FLAGS.default_instance_type + max_count = 1 + kwargs = {'image_id': image_id, + 'instance_type': instance_type, + 'max_count': max_count } + rv = yield self.cloud.run_instances(self.context, **kwargs) + instance_id = rv['instancesSet'][0]['instanceId'] output = yield self.cloud.get_console_output(self.context, [instance_id]) - logging.debug(output) - self.assert_(output) - rv = yield self.compute.terminate_instance(instance_id) + self.assertEquals(b64decode(output['output']), 'FAKE CONSOLE OUTPUT') + rv = yield self.cloud.terminate_instances(self.context, [instance_id]) def test_key_generation(self): -- cgit From 5a5da05a966dcdd3113a074468b37e12d406b350 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 15:25:50 +0200 Subject: Remove debugging code, and move import to the top. --- nova/fakerabbit.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/nova/fakerabbit.py b/nova/fakerabbit.py index 6679f8dab..df5e61e6e 100644 --- a/nova/fakerabbit.py +++ b/nova/fakerabbit.py @@ -22,6 +22,7 @@ import logging import Queue as queue from carrot.backends import base +from eventlet import greenthread class Message(base.BaseMessage): @@ -96,12 +97,10 @@ class Backend(object): routing_key) def declare_consumer(self, queue, callback, *args, **kwargs): - print 'declare_consumer', queue, callback self.current_queue = queue self.current_callback = callback def consume(self, *args, **kwargs): - from eventlet import greenthread while True: item = self.get(self.current_queue) if item: -- cgit From da7fa3f388a45b3afca16dba6a59b68ea8804f7a Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 12 Oct 2010 09:24:33 +0200 Subject: APIRequestContext.admin is no more.. --- nova/api/ec2/cloud.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 555518448..7839dc92c 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -841,7 +841,7 @@ class CloudController(object): inst_id = instance_ref['id'] for security_group_id in security_groups: - db.instance_add_security_group(context.admin(), inst_id, + db.instance_add_security_group(context, inst_id, security_group_id) inst = {} -- cgit From ac1dfd25c4b356c1725339709e535d4147feda3c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 12 Oct 2010 14:29:57 +0200 Subject: Remove spurious project_id addition to KeyPair model. --- nova/db/sqlalchemy/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 584214deb..85b7c0aae 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -351,7 +351,6 @@ class KeyPair(BASE, NovaBase): name = Column(String(255)) user_id = Column(String(255)) - project_id = Column(String(255)) fingerprint = Column(String(255)) public_key = Column(Text) -- cgit From 3894e22d517447fb3d5e9c367ffd2e67162f4b0f Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 12 Oct 2010 13:09:35 -0400 Subject: Fix bug 659330 --- nova/db/sqlalchemy/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index ebcb73413..fc99a535d 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -169,7 +169,7 @@ class Instance(BASE, NovaBase): @property def name(self): - return self.internal_id + return "instance-%d" % self.internal_id image_id = Column(String(255)) kernel_id = Column(String(255)) -- cgit From 32ea289d13a7ec9d273a57d2bf30484b80bfebec Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 12 Oct 2010 13:42:43 -0400 Subject: Now that the ec2 id is not the same as the name of the instance, don't compare internal_id [nee ec2_id] to instance names provided by the virtualization driver. Compare names directly instead. --- nova/compute/manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 131fac406..99705d3a9 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -67,7 +67,7 @@ class ComputeManager(manager.Manager): def run_instance(self, context, instance_id, **_kwargs): """Launch a new instance with specified options.""" instance_ref = self.db.instance_get(context, instance_id) - if instance_ref['internal_id'] in self.driver.list_instances(): + if instance_ref['name'] in self.driver.list_instances(): raise exception.Error("Instance has already been created") logging.debug("instance %s: starting...", instance_id) project_id = instance_ref['project_id'] -- cgit From aa92c017ab91d7fb0ec9c2cd5fd420e625ce2dbd Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 12 Oct 2010 18:27:59 -0400 Subject: Revert 64 bit storage and use 32 bit again. I didn't notice that we verify that randomly created uids don't already exist in the DB, so the chance of collision isn't really an issue until we get to tens of thousands of machines. Even then we should only expect a few retries before finding a free ID. --- nova/db/sqlalchemy/models.py | 4 ++-- nova/utils.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 9809eb7a7..fc99a535d 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -25,7 +25,7 @@ import datetime # TODO(vish): clean up these imports from sqlalchemy.orm import relationship, backref, exc, object_mapper -from sqlalchemy import Column, PickleType, Integer, String +from sqlalchemy import Column, Integer, String from sqlalchemy import ForeignKey, DateTime, Boolean, Text from sqlalchemy.exc import IntegrityError from sqlalchemy.ext.declarative import declarative_base @@ -152,7 +152,7 @@ class Instance(BASE, NovaBase): __tablename__ = 'instances' __prefix__ = 'i' id = Column(Integer, primary_key=True) - internal_id = Column(PickleType(mutable=False), unique=True) + internal_id = Column(Integer, unique=True) admin_pass = Column(String(255)) diff --git a/nova/utils.py b/nova/utils.py index 12afd388f..10b27ffec 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -128,7 +128,7 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): if topic == "i": # Instances have integer internal ids. - return random.randint(0, 2**64-1) + return random.randint(0, 2**32-1) else: characters = '01234567890abcdefghijklmnopqrstuvwxyz' choices = [random.choice(characters) for x in xrange(size)] -- cgit From b2a95bc859b3b52adb71efc4445924e1dbbdd06a Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 12 Oct 2010 19:27:48 -0700 Subject: fix tests --- nova/tests/network_unittest.py | 3 ++- nova/tests/virt_unittest.py | 15 ++++++++++----- nova/virt/libvirt_conn.py | 3 ++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index c81f93bb3..3afb4d19e 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -58,7 +58,8 @@ class NetworkTestCase(test.TrialTestCase): user_context = context.APIRequestContext(project=self.projects[i], user=self.user) network_ref = self.network.get_network(user_context) - self.network.set_network_host(user_context, network_ref['id']) + self.network.set_network_host(context.get_admin_context(), + network_ref['id']) instance_ref = self._create_instance(0) self.instance_id = instance_ref['id'] instance_ref = self._create_instance(1) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 684347473..13f976715 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -20,21 +20,22 @@ from xml.dom.minidom import parseString as xml_to_dom from nova import db from nova import flags from nova import test +from nova import utils from nova.api import context from nova.api.ec2 import cloud from nova.auth import manager - -# Needed to get FLAGS.instances_path defined: -from nova.compute import manager as compute_manager from nova.virt import libvirt_conn FLAGS = flags.FLAGS +flags.DECLARE('instances_path', 'nova.compute.manager') class LibvirtConnTestCase(test.TrialTestCase): def setUp(self): + super(LibvirtConnTestCase, self).setUp() self.manager = manager.AuthManager() self.user = self.manager.create_user('fake', 'fake', 'fake', admin=True) self.project = self.manager.create_project('fake', 'fake', 'fake') + self.network = utils.import_object(FLAGS.network_manager) FLAGS.instances_path = '' def test_get_uri_and_template(self): @@ -51,11 +52,15 @@ class LibvirtConnTestCase(test.TrialTestCase): 'instance_type' : 'm1.small'} instance_ref = db.instance_create(None, instance) - network_ref = db.project_get_network(None, self.project.id) + user_context = context.APIRequestContext(project=self.project, + user=self.user) + network_ref = self.network.get_network(user_context) + self.network.set_network_host(context.get_admin_context(), + network_ref['id']) fixed_ip = { 'address' : ip, 'network_id' : network_ref['id'] } - + fixed_ip_ref = db.fixed_ip_create(None, fixed_ip) db.fixed_ip_update(None, ip, { 'allocated' : True, 'instance_id' : instance_ref['id'] }) diff --git a/nova/virt/libvirt_conn.py b/nova/virt/libvirt_conn.py index 6ef5aa472..94a36a530 100644 --- a/nova/virt/libvirt_conn.py +++ b/nova/virt/libvirt_conn.py @@ -320,7 +320,8 @@ class LibvirtConnection(object): def to_xml(self, instance): # TODO(termie): cache? logging.debug('instance %s: starting toXML method', instance['name']) - network = db.instance_get_fixed_by_instance(None, inst['id']) + network = db.project_get_network(None, + instance['project_id']) # FIXME(vish): stick this in db instance_type = instance_types.INSTANCE_TYPES[instance['instance_type']] ip_address = db.instance_get_fixed_address({}, instance['id']) -- cgit From b9dc877303c13d43fb6b3e1105e9c0bd7161219d Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 12 Oct 2010 19:37:09 -0700 Subject: super teardown --- nova/tests/virt_unittest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 13f976715..edcdba425 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -118,6 +118,7 @@ class LibvirtConnTestCase(test.TrialTestCase): def tearDown(self): + super(LibvirtConnTestCase, self).tearDown() self.manager.delete_project(self.project) self.manager.delete_user(self.user) -- cgit From d533d6414041437e642f2bbfbc7a86daa2527a65 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 12 Oct 2010 20:13:06 -0700 Subject: cleanup leftover addresses --- nova/test.py | 4 ++++ nova/tests/cloud_unittest.py | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/nova/test.py b/nova/test.py index 5cf2abd53..f6485377d 100644 --- a/nova/test.py +++ b/nova/test.py @@ -24,6 +24,7 @@ and some black magic for inline callbacks. import sys import time +import datetime import mox import stubout @@ -62,6 +63,7 @@ class TrialTestCase(unittest.TestCase): # NOTE(vish): We need a better method for creating fixtures for tests # now that we have some required db setup for the system # to work properly. + self.start = datetime.datetime.utcnow() if db.network_count(None) != 5: network_manager.VlanManager().create_networks(None, FLAGS.fixed_range, @@ -84,6 +86,8 @@ class TrialTestCase(unittest.TestCase): self.stubs.UnsetAll() self.stubs.SmartUnsetAll() self.mox.VerifyAll() + # NOTE(vish): Clean up any ips associated during the test. + db.fixed_ip_disassociate_all_by_timeout(None, FLAGS.host, self.start) db.network_disassociate_all(None) rpc.Consumer.attach_to_twisted = self.originalAttach for x in self.injected: diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index 8e5881edb..a880237f2 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -64,12 +64,12 @@ class CloudTestCase(test.TrialTestCase): self.cloud = cloud.CloudController() # set up a service - self.compute = utils.import_class(FLAGS.compute_manager)() + self.compute = utils.import_object(FLAGS.compute_manager) self.compute_consumer = rpc.AdapterConsumer(connection=self.conn, topic=FLAGS.compute_topic, proxy=self.compute) self.compute_consumer.attach_to_eventlet() - self.network = utils.import_class(FLAGS.network_manager)() + self.network = utils.import_object(FLAGS.network_manager) self.network_consumer = rpc.AdapterConsumer(connection=self.conn, topic=FLAGS.network_topic, proxy=self.network) -- cgit