From e3b0ada2477ace68433cd48158003d4a1248c7bf Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 7 Sep 2010 14:52:38 +0200 Subject: [PATCH 01/52] 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 From c550b04e909dfc9a4106330d0419d1c46c8e520b Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 12:35:46 +0200 Subject: [PATCH 02/52] Alright, first hole poked all the way through. We can now create security groups and read them back. --- nova/auth/manager.py | 6 ++++++ nova/endpoint/cloud.py | 14 ++++++++++---- nova/tests/api_unittest.py | 34 +++++++++++++++++++++++++++++++--- 3 files changed, 47 insertions(+), 7 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/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) From f2a06487c19d0939874c8b6f7f3ec8a0eb103711 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 15:13:04 +0200 Subject: [PATCH 03/52] AuthorizeSecurityGroupIngress now works. --- nova/endpoint/cloud.py | 50 ++++++++++++++++++++-- nova/tests/api_unittest.py | 85 +++++++++++++++++++++++++++++++++++--- 2 files changed, 126 insertions(+), 9 deletions(-) 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(len(rv), 1) + 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 From f0cde2a54fd4e20d7c9f1f3ec890d5cef69f83ea Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Thu, 9 Sep 2010 17:35:02 +0200 Subject: [PATCH 04/52] Authorize and Revoke access now works. --- nova/endpoint/cloud.py | 51 +++++++++++++++++++++++++++++++++++--- nova/tests/api_unittest.py | 26 ++++++++++++++++--- 2 files changed, 70 insertions(+), 7 deletions(-) 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() From 4d6454a32a8b8e5db67309e3bbfdd058f789af0c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 11:47:06 +0200 Subject: [PATCH 05/52] 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/endpoint/api.py | 1 + nova/endpoint/cloud.py | 41 ++++++++++++++++++++++----- nova/tests/api_unittest.py | 58 ++++++++++++++++++++++++++++++++++---- 3 files changed, 88 insertions(+), 12 deletions(-) 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) ] } + 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 groups + 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() From 52349bf85bca00370d81e97b951086a9638e384c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 14:56:36 +0200 Subject: [PATCH 06/52] 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/endpoint/cloud.py | 1 - nova/tests/virt_unittest.py | 50 ++++++++++++++++++++++++++++++++++--- run_tests.py | 1 + 4 files changed, 49 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/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/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 From 94feb9fb4a1f8ded63dff1e41de34d04a27522eb Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 10 Sep 2010 15:32:56 +0200 Subject: [PATCH 07/52] 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') From 25bb032a254c9ea1882cf04d76acdd6a7b7a45ce Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Fri, 10 Sep 2010 15:26:13 -0700 Subject: [PATCH 08/52] Refactored to security group api to support projects --- nova/auth/manager.py | 2 - nova/endpoint/cloud.py | 81 +++++++++++++++++++++++-------------- nova/tests/api_unittest.py | 1 + nova/tests/virt_unittest.py | 4 +- 4 files changed, 54 insertions(+), 34 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/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 + source_project_id = self._get_source_project_id(context, + source_security_group_owner_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_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) From 9686c52833aebcc567246f4323cbc910e74a525e Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Fri, 10 Sep 2010 19:19:08 -0700 Subject: [PATCH 09/52] Finished security group / project refactor --- nova/auth/manager.py | 20 ++++++++++++++++---- nova/endpoint/cloud.py | 5 ++--- nova/tests/api_unittest.py | 2 +- 3 files changed, 19 insertions(+), 8 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/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. From 13a41efe2f8e106d3b2a19645693c1fd4b698724 Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Sat, 11 Sep 2010 02:35:25 +0000 Subject: [PATCH 10/52] 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): From 1d3c2214604183b8e603edf52b01f97396271863 Mon Sep 17 00:00:00 2001 From: Devin Carlen Date: Sat, 11 Sep 2010 11:19:22 -0700 Subject: [PATCH 11/52] Security Group API layer cleanup --- nova/endpoint/cloud.py | 84 +++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 41 deletions(-) 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): From f50e0df56404c30b82363e7ad03af44c8b57d609 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 11:45:28 +0200 Subject: [PATCH 12/52] Filters all get defined when running an instance. --- nova/tests/virt_unittest.py | 101 +++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 14 deletions(-) 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 From 5db82803b59a769f4816023b1f34e20b87685dae Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 12:04:06 +0200 Subject: [PATCH 13/52] (Untested) Make changes to security group rules propagate to the relevant compute nodes. --- nova/endpoint/cloud.py | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) 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. From 743758bf09f52be65ccfafce796c1db6207935d8 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 13 Sep 2010 14:18:08 +0200 Subject: [PATCH 14/52] Fix call to listNWFilters --- nova/tests/virt_unittest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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): From 3b0248e8751e5f8165d88a027c4e4306d1a52543 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 13:01:57 +0200 Subject: [PATCH 15/52] 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) From d5d4fc8718b96b5c26e439abe3d024afd41cb4ca Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 13:22:17 +0200 Subject: [PATCH 16/52] 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, From fffe510f2597905ed2e37ec345fe9918f6249e37 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:17:52 +0200 Subject: [PATCH 17/52] 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/endpoint/cloud.py | 3 +-- nova/tests/virt_unittest.py | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) 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') From 9375314e926a0023f2630650fec551e17effc4f5 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 14 Sep 2010 15:22:56 +0200 Subject: [PATCH 18/52] 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, From de7bc6eb5c019d3e9fdf237a88d82ec4dca4a52a Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 12:01:08 +0200 Subject: [PATCH 19/52] 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 From 6d3a1dc052b04048b7c18b79e85e40e540f6da12 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 12:05:37 +0200 Subject: [PATCH 20/52] 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 From 4882c910004dea463bc0a217e6f67b58afd4055c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 13:56:17 +0200 Subject: [PATCH 21/52] 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']) From c12ecb16e7e5187b288fb241ee11ff3229e30e2e Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 14:04:07 +0200 Subject: [PATCH 22/52] Clean up use of ORM to remove the need for scoped_session. --- nova/endpoint/cloud.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 From 84dcc4b89b13abf914b62e54617938a7c5821a06 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 15 Sep 2010 14:27:34 +0200 Subject: [PATCH 23/52] 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): From 47d66b6b696bc71bdcd834b97bfbe64ecb61c165 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 28 Sep 2010 10:26:29 +0200 Subject: [PATCH 24/52] 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) From a3e532853f3561dd1b788d0db1c79df5acbe0bed Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Tue, 28 Sep 2010 14:48:03 -0400 Subject: [PATCH 25/52] Adds --force option to run_tests.sh to clear virtualenv. Useful when dependencies change --- run_tests.sh | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index 6ea40d95e..ec727d094 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -6,6 +6,7 @@ function usage { echo "" echo " -V, --virtual-env Always use virtualenv. Install automatically if not present" echo " -N, --no-virtual-env Don't use virtualenv. Run tests in local environment" + echo " -f, --force Force a clean re-build of the virtual environment. Useful when dependencies have been added." echo " -h, --help Print this usage message" echo "" echo "Note: with no options specified, the script will try to run the tests in a virtual environment," @@ -14,20 +15,12 @@ function usage { exit } -function process_options { - array=$1 - elements=${#array[@]} - for (( x=0;x<$elements;x++)); do - process_option ${array[${x}]} - done -} - function process_option { - option=$1 - case $option in + case "$1" in -h|--help) usage;; -V|--virtual-env) let always_venv=1; let never_venv=0;; -N|--no-virtual-env) let always_venv=0; let never_venv=1;; + -f|--force) let force=1;; esac } @@ -35,9 +28,11 @@ venv=.nova-venv with_venv=tools/with_venv.sh always_venv=0 never_venv=0 -options=("$@") +force=0 -process_options $options +for arg in "$@"; do + process_option $arg +done if [ $never_venv -eq 1 ]; then # Just run the test suites in current environment @@ -45,6 +40,12 @@ if [ $never_venv -eq 1 ]; then exit fi +# Remove the virtual environment if --force used +if [ $force -eq 1 ]; then + echo "Cleaning virtualenv..." + rm -rf ${venv} +fi + if [ -e ${venv} ]; then ${with_venv} python run_tests.py $@ else From 1a26e77d1bdb9e2eba79a28ab05e53368127b44c Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 12:09:17 -0700 Subject: [PATCH 26/52] move default group creation to api --- nova/auth/manager.py | 14 -------------- nova/test.py | 2 ++ 2 files changed, 2 insertions(+), 14 deletions(-) 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/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""" From 3d09c4988350642658b0fe0c487b5b0210da6947 Mon Sep 17 00:00:00 2001 From: Vishvananda Ishaya Date: Tue, 28 Sep 2010 21:07:26 -0700 Subject: [PATCH 27/52] 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) From ca8714c77636b54cf17a75874c79768e12eb9fda Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Wed, 29 Sep 2010 09:46:37 +0200 Subject: [PATCH 28/52] 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) From a4bc0f2b476c43f1743ace70d13722b76ecfd3c2 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Fri, 1 Oct 2010 00:42:09 +0200 Subject: [PATCH 29/52] Add a DB backend for auth manager. --- nova/auth/dbdriver.py | 236 ++++++++++++++++++++++++++++++++++++ nova/auth/manager.py | 2 +- nova/tests/auth_unittest.py | 9 +- nova/tests/fake_flags.py | 2 +- 4 files changed, 246 insertions(+), 3 deletions(-) create mode 100644 nova/auth/dbdriver.py diff --git a/nova/auth/dbdriver.py b/nova/auth/dbdriver.py new file mode 100644 index 000000000..09d15018b --- /dev/null +++ b/nova/auth/dbdriver.py @@ -0,0 +1,236 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2010 United States Government as represented by the +# Administrator of the National Aeronautics and Space Administration. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Auth driver using the DB as its backend. +""" + +import logging +import sys + +from nova import exception +from nova import db + + +class DbDriver(object): + """DB Auth driver + + Defines enter and exit and therefore supports the with/as syntax. + """ + + def __init__(self): + """Imports the LDAP module""" + pass + db + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_value, traceback): + pass + + def get_user(self, uid): + """Retrieve user by id""" + return self._db_user_to_auth_user(db.user_get({}, uid)) + + def get_user_from_access_key(self, access): + """Retrieve user by access key""" + return self._db_user_to_auth_user(db.user_get_by_access_key({}, access)) + + def get_project(self, pid): + """Retrieve project by id""" + return self._db_project_to_auth_projectuser(db.project_get({}, pid)) + + def get_users(self): + """Retrieve list of users""" + return [self._db_user_to_auth_user(user) for user in db.user_get_all({})] + + def get_projects(self, uid=None): + """Retrieve list of projects""" + if uid: + result = db.project_get_by_user({}, uid) + else: + result = db.project_get_all({}) + return [self._db_project_to_auth_projectuser(proj) for proj in result] + + def create_user(self, name, access_key, secret_key, is_admin): + """Create a user""" + values = { 'id' : name, + 'access_key' : access_key, + 'secret_key' : secret_key, + 'is_admin' : is_admin + } + try: + user_ref = db.user_create({}, values) + return self._db_user_to_auth_user(user_ref) + except exception.Duplicate, e: + raise exception.Duplicate('User %s already exists' % name) + + def _db_user_to_auth_user(self, user_ref): + return { 'id' : user_ref['id'], + 'name' : user_ref['id'], + 'access' : user_ref['access_key'], + 'secret' : user_ref['secret_key'], + 'admin' : user_ref['is_admin'] } + + def _db_project_to_auth_projectuser(self, project_ref): + return { 'id' : project_ref['id'], + 'name' : project_ref['name'], + 'project_manager_id' : project_ref['project_manager'], + 'description' : project_ref['description'], + 'member_ids' : [member['id'] for member in project_ref['members']] } + + def create_project(self, name, manager_uid, + description=None, member_uids=None): + """Create a project""" + manager = db.user_get({}, manager_uid) + if not manager: + raise exception.NotFound("Project can't be created because " + "manager %s doesn't exist" % manager_uid) + + # description is a required attribute + if description is None: + description = name + + # First, we ensure that all the given users exist before we go + # on to create the project. This way we won't have to destroy + # the project again because a user turns out to be invalid. + members = set([manager]) + if member_uids != None: + for member_uid in member_uids: + member = db.user_get({}, member_uid) + if not member: + raise exception.NotFound("Project can't be created " + "because user %s doesn't exist" + % member_uid) + members.add(member) + + values = { 'id' : name, + 'name' : name, + 'project_manager' : manager['id'], + 'description': description } + + try: + project = db.project_create({}, values) + except exception.Duplicate: + raise exception.Duplicate("Project can't be created because " + "project %s already exists" % name) + + for member in members: + db.project_add_member({}, project['id'], member['id']) + + # This looks silly, but ensures that the members element has been + # correctly populated + project_ref = db.project_get({}, project['id']) + return self._db_project_to_auth_projectuser(project_ref) + + def modify_project(self, project_id, manager_uid=None, description=None): + """Modify an existing project""" + if not manager_uid and not description: + return + values = {} + if manager_uid: + manager = db.user_get({}, manager_uid) + if not manager: + raise exception.NotFound("Project can't be modified because " + "manager %s doesn't exist" % + manager_uid) + values['project_manager'] = manager['id'] + if description: + values['description'] = description + + db.project_update({}, project_id, values) + + def add_to_project(self, uid, project_id): + """Add user to project""" + user, project = self._validate_user_and_project(uid, project_id) + db.project_add_member({}, project['id'], user['id']) + + def remove_from_project(self, uid, project_id): + """Remove user from project""" + user, project = self._validate_user_and_project(uid, project_id) + db.project_remove_member({}, project['id'], user['id']) + + def is_in_project(self, uid, project_id): + """Check if user is in project""" + user, project = self._validate_user_and_project(uid, project_id) + return user in project.members + + def has_role(self, uid, role, project_id=None): + """Check if user has role + + If project is specified, it checks for local role, otherwise it + checks for global role + """ + + return role in self.get_user_roles(uid, project_id) + + def add_role(self, uid, role, project_id=None): + """Add role for user (or user and project)""" + if not project_id: + db.user_add_role({}, uid, role) + return + db.user_add_project_role({}, uid, project_id, role) + + def remove_role(self, uid, role, project_id=None): + """Remove role for user (or user and project)""" + if not project_id: + db.user_remove_role({}, uid, role) + return + db.user_remove_project_role({}, uid, project_id, role) + + def get_user_roles(self, uid, project_id=None): + """Retrieve list of roles for user (or user and project)""" + if project_id is None: + roles = db.user_get_roles({}, uid) + return roles + else: + roles = db.user_get_roles_for_project({}, uid, project_id) + return roles + + def delete_user(self, id): + """Delete a user""" + user = db.user_get({}, id) + db.user_delete({}, user['id']) + + def delete_project(self, project_id): + """Delete a project""" + db.project_delete({}, project_id) + + def modify_user(self, uid, access_key=None, secret_key=None, admin=None): + """Modify an existing user""" + if not access_key and not secret_key and admin is None: + return + values = {} + if access_key: + values['access_key'] = access_key + if secret_key: + values['secret_key'] = secret_key + if admin is not None: + values['is_admin'] = admin + db.user_update({}, uid, values) + + def _validate_user_and_project(self, user_id, project_id): + user = db.user_get({}, user_id) + if not user: + raise exception.NotFound('User "%s" not found' % user_id) + project = db.project_get({}, project_id) + if not project: + raise exception.NotFound('Project "%s" not found' % project_id) + return user, project + diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 0bc12c80f..ce8a294df 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -69,7 +69,7 @@ flags.DEFINE_string('credential_cert_subject', '/C=US/ST=California/L=MountainView/O=AnsoLabs/' 'OU=NovaDev/CN=%s-%s', 'Subject for certificate for users') -flags.DEFINE_string('auth_driver', 'nova.auth.ldapdriver.FakeLdapDriver', +flags.DEFINE_string('auth_driver', 'nova.auth.dbdriver.DbDriver', 'Driver that auth manager uses') diff --git a/nova/tests/auth_unittest.py b/nova/tests/auth_unittest.py index 1955bb417..99f7ab599 100644 --- a/nova/tests/auth_unittest.py +++ b/nova/tests/auth_unittest.py @@ -75,8 +75,9 @@ class user_and_project_generator(object): self.manager.delete_user(self.user) self.manager.delete_project(self.project) -class AuthManagerTestCase(test.TrialTestCase): +class AuthManagerTestCase(object): def setUp(self): + FLAGS.auth_driver = self.auth_driver super(AuthManagerTestCase, self).setUp() self.flags(connection_type='fake') self.manager = manager.AuthManager() @@ -320,6 +321,12 @@ class AuthManagerTestCase(test.TrialTestCase): self.assertEqual('secret', user.secret) self.assertTrue(user.is_admin()) +class AuthManagerLdapTestCase(AuthManagerTestCase, test.TrialTestCase): + auth_driver = 'nova.auth.ldapdriver.FakeLdapDriver' + +class AuthManagerDbTestCase(AuthManagerTestCase, test.TrialTestCase): + auth_driver = 'nova.auth.dbdriver.DbDriver' + if __name__ == "__main__": # TODO: Implement use_fake as an option diff --git a/nova/tests/fake_flags.py b/nova/tests/fake_flags.py index 8f4754650..4bbef8832 100644 --- a/nova/tests/fake_flags.py +++ b/nova/tests/fake_flags.py @@ -24,7 +24,7 @@ flags.DECLARE('volume_driver', 'nova.volume.manager') FLAGS.volume_driver = 'nova.volume.driver.FakeAOEDriver' FLAGS.connection_type = 'fake' FLAGS.fake_rabbit = True -FLAGS.auth_driver = 'nova.auth.ldapdriver.FakeLdapDriver' +FLAGS.auth_driver = 'nova.auth.dbdriver.DbDriver' flags.DECLARE('network_size', 'nova.network.manager') flags.DECLARE('num_networks', 'nova.network.manager') flags.DECLARE('fake_network', 'nova.network.manager') From 8988b0e50868f500490e4b5e080d0f00f0ed1bf0 Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 22:05:16 -0400 Subject: [PATCH 30/52] Find other places in the code that used ec2_id or get_instance_by_ec2_id and use internal_id as appropriate --- nova/tests/cloud_unittest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index ae7dea1db..d316db153 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -236,7 +236,7 @@ class CloudTestCase(test.TrialTestCase): def test_update_of_instance_display_fields(self): inst = db.instance_create({}, {}) - self.cloud.update_instance(self.context, inst['ec2_id'], + self.cloud.update_instance(self.context, inst['internal_id'], display_name='c00l 1m4g3') inst = db.instance_get({}, inst['id']) self.assertEqual('c00l 1m4g3', inst['display_name']) From c2f4e407c5db6692da3114a874a1843c4b3dda0b Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Thu, 30 Sep 2010 22:09:46 -0400 Subject: [PATCH 31/52] First attempt at a uuid generator -- but we've lost a 'topic' input so i don't know what that did. --- nova/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nova/utils.py b/nova/utils.py index d18dd9843..86ff3d22e 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,7 +126,9 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) + #TODO(gundlach): we want internal ids to just be ints now. i just dropped + #off a topic prefix, so what have I broken? + return random.randint(0, 2**64-1) def generate_mac(): From e0565c55785a5f1988a09984fc27bf4413975afa Mon Sep 17 00:00:00 2001 From: "jaypipes@gmail.com" <> Date: Fri, 1 Oct 2010 16:06:14 -0400 Subject: [PATCH 32/52] Adds BaseImageService and flag to control image service loading. Adds unit test for local image service. --- nova/flags.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/nova/flags.py b/nova/flags.py index c32cdd7a4..ab80e83fb 100644 --- a/nova/flags.py +++ b/nova/flags.py @@ -222,6 +222,10 @@ DEFINE_string('volume_manager', 'nova.volume.manager.AOEManager', DEFINE_string('scheduler_manager', 'nova.scheduler.manager.SchedulerManager', 'Manager for scheduler') +# The service to use for image search and retrieval +DEFINE_string('image_service', 'nova.image.service.LocalImageService', + 'The service to use for retrieving and searching for images.') + DEFINE_string('host', socket.gethostname(), 'name of this node') From 49eee1b7c4be6200b77c17b6130781c661658656 Mon Sep 17 00:00:00 2001 From: Todd Willey Date: Fri, 1 Oct 2010 21:46:36 -0400 Subject: [PATCH 33/52] Keep handles to loggers open after daemonizing. --- nova/server.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nova/server.py b/nova/server.py index d4563bfe0..c58a15041 100644 --- a/nova/server.py +++ b/nova/server.py @@ -106,6 +106,7 @@ def serve(name, main): def daemonize(args, name, main): """Does the work of daemonizing the process""" logging.getLogger('amqplib').setLevel(logging.WARN) + files_to_keep = [] if FLAGS.daemonize: logger = logging.getLogger() formatter = logging.Formatter( @@ -114,12 +115,14 @@ def daemonize(args, name, main): syslog = logging.handlers.SysLogHandler(address='/dev/log') syslog.setFormatter(formatter) logger.addHandler(syslog) + files_to_keep.append(syslog.socket) else: if not FLAGS.logfile: FLAGS.logfile = '%s.log' % name logfile = logging.FileHandler(FLAGS.logfile) logfile.setFormatter(formatter) logger.addHandler(logfile) + files_to_keep.append(logfile.stream) stdin, stdout, stderr = None, None, None else: stdin, stdout, stderr = sys.stdin, sys.stdout, sys.stderr @@ -139,6 +142,7 @@ def daemonize(args, name, main): stdout=stdout, stderr=stderr, uid=FLAGS.uid, - gid=FLAGS.gid + gid=FLAGS.gid, + files_preserve=files_to_keep ): main(args) From e7ed796d7eb40c5551a583f16a593a2aaff66454 Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Sun, 3 Oct 2010 12:41:07 +0100 Subject: [PATCH 34/52] Bug #654023: nova-manage vpn commands broken, resulting in erroneous "Wrong number of arguments supplied" message Add a context of None to the call to db.instance_get_all. This is deprecated, but it's what all the other calls in this file do, and it's better than exploding, so it will do for now. --- bin/nova-manage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/nova-manage b/bin/nova-manage index bf3c67612..0b5869dfd 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -114,7 +114,7 @@ class VpnCommands(object): def _vpn_for(self, project_id): """Get the VPN instance for a project ID.""" - for instance in db.instance_get_all(): + for instance in db.instance_get_all(None): if (instance['image_id'] == FLAGS.vpn_image_id and not instance['state_description'] in ['shutting_down', 'shutdown'] From 575ceb8e1b893fc286c9d77a351b1448e18506c3 Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Sun, 3 Oct 2010 13:12:32 +0100 Subject: [PATCH 35/52] Bug #654025: nova-manage project zip and nova-manage vpn list broken by change in DB semantics when networks are missing Catch exception.NotFound when getting project VPN data. This is in two places: nova-manage as part of its vpn list command, and auth.manager.AuthManager.get_credentials. Also, document the behaviour of db.api.project_get_network. --- bin/nova-manage | 9 +++++++-- nova/auth/manager.py | 5 ++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/bin/nova-manage b/bin/nova-manage index bf3c67612..5293fc942 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -88,11 +88,16 @@ class VpnCommands(object): def list(self): """Print a listing of the VPNs for all projects.""" print "%-12s\t" % 'project', - print "%-12s\t" % 'ip:port', + print "%-20s\t" % 'ip:port', print "%s" % 'state' for project in self.manager.get_projects(): print "%-12s\t" % project.name, - print "%s:%s\t" % (project.vpn_ip, project.vpn_port), + + try: + s = "%s:%s" % (project.vpn_ip, project.vpn_port) + except exception.NotFound: + s = "None" + print "%-20s\t" % s, vpn = self._vpn_for(project.id) if vpn: diff --git a/nova/auth/manager.py b/nova/auth/manager.py index 0bc12c80f..c30192e20 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -653,7 +653,10 @@ class AuthManager(object): zippy.writestr(FLAGS.credential_key_file, private_key) zippy.writestr(FLAGS.credential_cert_file, signed_cert) - (vpn_ip, vpn_port) = self.get_project_vpn_data(project) + try: + (vpn_ip, vpn_port) = self.get_project_vpn_data(project) + except exception.NotFound: + vpn_ip = None if vpn_ip: configfile = open(FLAGS.vpn_client_template, "r") s = string.Template(configfile.read()) From b45670336e564facdc9fe86bd8fb5113470e5bb5 Mon Sep 17 00:00:00 2001 From: Ewan Mellor Date: Sun, 3 Oct 2010 13:17:20 +0100 Subject: [PATCH 36/52] Bug #654034: nova-manage doesn't honour --verbose flag Honour the --verbose flag by setting the logging level to DEBUG. --- bin/nova-manage | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bin/nova-manage b/bin/nova-manage index bf3c67612..ce87b9437 100755 --- a/bin/nova-manage +++ b/bin/nova-manage @@ -52,6 +52,7 @@ CLI interface for nova management. """ +import logging import os import sys import time @@ -417,6 +418,10 @@ def main(): """Parse options and call the appropriate class/method.""" utils.default_flagfile('/etc/nova/nova-manage.conf') argv = FLAGS(sys.argv) + + if FLAGS.verbose: + logging.getLogger().setLevel(logging.DEBUG) + script_name = argv.pop(0) if len(argv) < 1: print script_name + " category action []" From e491d4215a503e63b6f3bd61bd0a5390b418cfc1 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Sun, 3 Oct 2010 20:22:35 +0200 Subject: [PATCH 37/52] s/APIRequestContext/get_admin_context/ <-- sudo for request contexts. --- nova/tests/network_unittest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nova/tests/network_unittest.py b/nova/tests/network_unittest.py index 5370966d2..59b0a36e4 100644 --- a/nova/tests/network_unittest.py +++ b/nova/tests/network_unittest.py @@ -56,8 +56,8 @@ class NetworkTestCase(test.TrialTestCase): 'netuser', name)) # create the necessary network data for the project - user_context = context.APIRequestContext(project=self.projects[i], - user=self.user) + user_context = context.get_admin_context(user=self.user) + self.network.set_network_host(user_context, self.projects[i].id) instance_ref = self._create_instance(0) self.instance_id = instance_ref['id'] From c2e4f85f5cbc93d45d67acb1e0737223d9f3a4db Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 4 Oct 2010 11:53:27 +0200 Subject: [PATCH 38/52] Move manager_class instantiation and db.service_* calls out of nova.service.Service.__init__ into a new nova.service.Service.startService method which gets called by twisted. This delays opening db connections (and thus sqlite file creation) until after privileges have been shed by twisted. --- nova/service.py | 12 +++++++++--- nova/tests/scheduler_unittest.py | 10 ++++++++++ nova/tests/service_unittest.py | 3 +++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/nova/service.py b/nova/service.py index a6c186896..dadef3c48 100644 --- a/nova/service.py +++ b/nova/service.py @@ -52,11 +52,17 @@ class Service(object, service.Service): self.host = host self.binary = binary self.topic = topic - manager_class = utils.import_class(manager) - self.manager = manager_class(host=host, *args, **kwargs) + self.manager_class_name = manager + super(Service, self).__init__(*args, **kwargs) + self.saved_args, self.saved_kwargs = args, kwargs + + + def startService(self): + manager_class = utils.import_class(self.manager_class_name) + self.manager = manager_class(host=self.host, *self.saved_args, + **self.saved_kwargs) self.manager.init_host() self.model_disconnected = False - super(Service, self).__init__(*args, **kwargs) try: service_ref = db.service_get_by_args(None, self.host, diff --git a/nova/tests/scheduler_unittest.py b/nova/tests/scheduler_unittest.py index fde30f81e..53a8be144 100644 --- a/nova/tests/scheduler_unittest.py +++ b/nova/tests/scheduler_unittest.py @@ -117,10 +117,12 @@ class SimpleDriverTestCase(test.TrialTestCase): 'nova-compute', 'compute', FLAGS.compute_manager) + compute1.startService() compute2 = service.Service('host2', 'nova-compute', 'compute', FLAGS.compute_manager) + compute2.startService() hosts = self.scheduler.driver.hosts_up(self.context, 'compute') self.assertEqual(len(hosts), 2) compute1.kill() @@ -132,10 +134,12 @@ class SimpleDriverTestCase(test.TrialTestCase): 'nova-compute', 'compute', FLAGS.compute_manager) + compute1.startService() compute2 = service.Service('host2', 'nova-compute', 'compute', FLAGS.compute_manager) + compute2.startService() instance_id1 = self._create_instance() compute1.run_instance(self.context, instance_id1) instance_id2 = self._create_instance() @@ -153,10 +157,12 @@ class SimpleDriverTestCase(test.TrialTestCase): 'nova-compute', 'compute', FLAGS.compute_manager) + compute1.startService() compute2 = service.Service('host2', 'nova-compute', 'compute', FLAGS.compute_manager) + compute2.startService() instance_ids1 = [] instance_ids2 = [] for index in xrange(FLAGS.max_cores): @@ -184,10 +190,12 @@ class SimpleDriverTestCase(test.TrialTestCase): 'nova-volume', 'volume', FLAGS.volume_manager) + volume1.startService() volume2 = service.Service('host2', 'nova-volume', 'volume', FLAGS.volume_manager) + volume2.startService() volume_id1 = self._create_volume() volume1.create_volume(self.context, volume_id1) volume_id2 = self._create_volume() @@ -205,10 +213,12 @@ class SimpleDriverTestCase(test.TrialTestCase): 'nova-volume', 'volume', FLAGS.volume_manager) + volume1.startService() volume2 = service.Service('host2', 'nova-volume', 'volume', FLAGS.volume_manager) + volume2.startService() volume_ids1 = [] volume_ids2 = [] for index in xrange(FLAGS.max_gigabytes): diff --git a/nova/tests/service_unittest.py b/nova/tests/service_unittest.py index 06f80e82c..6afeec377 100644 --- a/nova/tests/service_unittest.py +++ b/nova/tests/service_unittest.py @@ -22,6 +22,8 @@ Unit Tests for remote procedure calls using queue import mox +from twisted.application.app import startApplication + from nova import exception from nova import flags from nova import rpc @@ -96,6 +98,7 @@ class ServiceTestCase(test.BaseTestCase): self.mox.ReplayAll() app = service.Service.create(host=host, binary=binary) + startApplication(app, False) self.assert_(app) # We're testing sort of weird behavior in how report_state decides From cd173cd71db7d6bd608e45cefd519a44c19e563e Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 4 Oct 2010 14:26:55 -0400 Subject: [PATCH 39/52] Revert r312 --- nova/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nova/utils.py b/nova/utils.py index 86ff3d22e..5f64b13c4 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,9 +126,8 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - #TODO(gundlach): we want internal ids to just be ints now. i just dropped - #off a topic prefix, so what have I broken? - return random.randint(0, 2**64-1) + return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) + def generate_mac(): From e157d79f952f82918913ee2201b288130fc13b50 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 4 Oct 2010 21:01:31 +0200 Subject: [PATCH 40/52] Add pylint thingamajig for startService (name defined by Twisted). --- nova/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nova/service.py b/nova/service.py index dadef3c48..115e0ff32 100644 --- a/nova/service.py +++ b/nova/service.py @@ -57,7 +57,7 @@ class Service(object, service.Service): self.saved_args, self.saved_kwargs = args, kwargs - def startService(self): + def startService(self): # pylint: disable-msg C0103 manager_class = utils.import_class(self.manager_class_name) self.manager = manager_class(host=self.host, *self.saved_args, **self.saved_kwargs) From a1a0b7f8352dbb43aa3b6fdd15d7e9d8a9f508be Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 4 Oct 2010 21:53:22 +0200 Subject: [PATCH 41/52] Replace the embarrasingly crude string based tests for to_xml with some more sensible ElementTree based stuff. --- nova/tests/virt_unittest.py | 63 ++++++++++++++++++++++++------------- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 2aab16809..998cc07db 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -14,36 +14,49 @@ # License for the specific language governing permissions and limitations # under the License. +from xml.etree.ElementTree import fromstring as parseXml + from nova import flags from nova import test from nova.virt import libvirt_conn FLAGS = flags.FLAGS - class LibvirtConnTestCase(test.TrialTestCase): def test_get_uri_and_template(self): - class MockDataModel(object): - 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 } + instance = { 'name' : 'i-cafebabe', + 'id' : 'i-cafebabe', + 'memory_kb' : '1024000', + 'basepath' : '/some/path', + 'bridge_name' : 'br100', + 'mac_address' : '02:12:34:46:56:67', + 'vcpus' : 2, + 'project_id' : 'fake', + 'ip_address' : '10.11.12.13', + 'bridge' : 'br101', + 'instance_type' : 'm1.small'} type_uri_map = { 'qemu' : ('qemu:///system', - [lambda s: '' in s, - lambda s: 'type>hvm/usr/bin/kvm' not in s]), + [(lambda t: t.find('.').tag, 'domain'), + (lambda t: t.find('.').get('type'), 'qemu'), + (lambda t: t.find('./os/type').text, 'hvm'), + (lambda t: t.find('./devices/emulator'), None)]), 'kvm' : ('qemu:///system', - [lambda s: '' in s, - lambda s: 'type>hvm/usr/bin/qemu<' not in s]), + [(lambda t: t.find('.').tag, 'domain'), + (lambda t: t.find('.').get('type'), 'kvm'), + (lambda t: t.find('./os/type').text, 'hvm'), + (lambda t: t.find('./devices/emulator'), None)]), 'uml' : ('uml:///system', - [lambda s: '' in s, - lambda s: 'type>uml Date: Mon, 4 Oct 2010 21:58:22 +0200 Subject: [PATCH 42/52] Merge security group related changes from lp:~anso/nova/deploy --- nova/tests/virt_unittest.py | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) 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) From 0de48caa33174ad947be2aeb63837ed754a5787f Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Mon, 4 Oct 2010 16:39:05 -0400 Subject: [PATCH 43/52] Fix broken unit tests --- nova/tests/cloud_unittest.py | 3 ++- nova/utils.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index d316db153..615e589cf 100644 --- a/nova/tests/cloud_unittest.py +++ b/nova/tests/cloud_unittest.py @@ -236,7 +236,8 @@ class CloudTestCase(test.TrialTestCase): def test_update_of_instance_display_fields(self): inst = db.instance_create({}, {}) - self.cloud.update_instance(self.context, inst['internal_id'], + ec2_id = cloud.internal_id_to_ec2_id(inst['internal_id']) + self.cloud.update_instance(self.context, ec2_id, display_name='c00l 1m4g3') inst = db.instance_get({}, inst['id']) self.assertEqual('c00l 1m4g3', inst['display_name']) diff --git a/nova/utils.py b/nova/utils.py index 5f64b13c4..b1699bda8 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -126,8 +126,15 @@ def runthis(prompt, cmd, check_exit_code = True): def generate_uid(topic, size=8): - return '%s-%s' % (topic, ''.join([random.choice('01234567890abcdefghijklmnopqrstuvwxyz') for x in xrange(size)])) - + if topic == "i": + # Instances have integer internal ids. + #TODO(gundlach): We should make this more than 32 bits, but we need to + #figure out how to make the DB happy with 64 bit integers. + return random.randint(0, 2**32-1) + else: + characters = '01234567890abcdefghijklmnopqrstuvwxyz' + choices = [random.choice(characters) for x in xrange(size)] + return '%s-%s' % (topic, ''.join(choices)) def generate_mac(): From 94d6c5f509189d85f0d9cb3ea56c2a12aa9ac5c4 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 10:06:54 +0200 Subject: [PATCH 44/52] Run the virt tests by default. --- run_tests.py | 1 + 1 file changed, 1 insertion(+) diff --git a/run_tests.py b/run_tests.py index 4121f4c06..fa1e6f15b 100644 --- a/run_tests.py +++ b/run_tests.py @@ -63,6 +63,7 @@ from nova.tests.rpc_unittest import * from nova.tests.scheduler_unittest import * from nova.tests.service_unittest import * from nova.tests.validator_unittest import * +from nova.tests.virt_unittest import * from nova.tests.volume_unittest import * From aeb698b51208ee431c9ec7aa7901608b4ac2643c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Tue, 5 Oct 2010 10:07:37 +0200 Subject: [PATCH 45/52] Create and destroy user appropriately. Remove security group related tests (since they haven't been merged yet). --- nova/tests/virt_unittest.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/nova/tests/virt_unittest.py b/nova/tests/virt_unittest.py index 998cc07db..730928f39 100644 --- a/nova/tests/virt_unittest.py +++ b/nova/tests/virt_unittest.py @@ -18,11 +18,20 @@ from xml.etree.ElementTree import fromstring as parseXml from nova import flags from nova import test +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 class LibvirtConnTestCase(test.TrialTestCase): + def setUp(self): + self.manager = manager.AuthManager() + self.user = self.manager.create_user('fake', 'fake', 'fake', admin=True) + self.project = self.manager.create_project('fake', 'fake', 'fake') + FLAGS.instances_path = '' + def test_get_uri_and_template(self): instance = { 'name' : 'i-cafebabe', 'id' : 'i-cafebabe', @@ -51,12 +60,6 @@ class LibvirtConnTestCase(test.TrialTestCase): (lambda t: t.find('.').get('type'), 'uml'), (lambda t: t.find('./os/type').text, 'uml')]), } - common_checks = [(lambda t: \ - t.find('./devices/interface/filterref/parameter') \ - .get('name'), 'IP'), - (lambda t: \ - t.find('./devices/interface/filterref/parameter') \ - .get('value'), '10.11.12.13')] for (libvirt_type,(expected_uri, checks)) in type_uri_map.iteritems(): FLAGS.libvirt_type = libvirt_type @@ -72,11 +75,6 @@ class LibvirtConnTestCase(test.TrialTestCase): expected_result, '%s failed check %d' % (xml, i)) - for i, (check, expected_result) in enumerate(common_checks): - self.assertEqual(check(tree), - expected_result, - '%s failed common check %d' % (xml, i)) - # Deliberately not just assigning this string to FLAGS.libvirt_uri and # checking against that later on. This way we make sure the # implementation doesn't fiddle around with the FLAGS. @@ -88,3 +86,7 @@ class LibvirtConnTestCase(test.TrialTestCase): uri, template = conn.get_uri_and_template() self.assertEquals(uri, testuri) + + def tearDown(self): + self.manager.delete_project(self.project) + self.manager.delete_user(self.user) From 9895aca06a89256699ae2102dcb9646b52bdb59c Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 11 Oct 2010 13:39:33 +0200 Subject: [PATCH 49/52] Rename ec2 get_console_output's instance ID argument to 'instance_id'. It's passed as a kwarg, based on key in the http query, so it must be named this way. --- nova/fakerabbit.py | 14 ++++++++++++++ nova/rpc.py | 9 +++++++++ nova/tests/cloud_unittest.py | 31 ++++++++++++++++++++----------- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/nova/fakerabbit.py b/nova/fakerabbit.py index 068025249..835973810 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): @@ -38,6 +39,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 +96,18 @@ class Backend(object): self._exchanges[exchange].bind(self._queues[queue].push, routing_key) + def declare_consumer(self, queue, callback, *args, **kwargs): + self.current_queue = queue + self.current_callback = callback + + def consume(self, *args, **kwargs): + 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 diff --git a/nova/rpc.py b/nova/rpc.py index fe52ad35f..447ad3b93 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,14 @@ class Consumer(messaging.Consumer): logging.exception("Failed to fetch message from queue") self.failed_connection = True + def attach_to_eventlet(self): + """Only needed for unit tests!""" + 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) diff --git a/nova/tests/cloud_unittest.py b/nova/tests/cloud_unittest.py index 615e589cf..8e5881edb 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,17 @@ 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 +92,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) - 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) + 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(context=self.context, instance_id=[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): From 8b5259845f0ee16f99c502ea3d1d73df0747af32 Mon Sep 17 00:00:00 2001 From: Soren Hansen Date: Mon, 11 Oct 2010 14:09:24 +0200 Subject: [PATCH 50/52] If machine manifest includes a kernel and/or ramdisk id, include it in the image's metadata. --- nova/tests/objectstore_unittest.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/nova/tests/objectstore_unittest.py b/nova/tests/objectstore_unittest.py index 5a599ff3a..eb2ee0406 100644 --- a/nova/tests/objectstore_unittest.py +++ b/nova/tests/objectstore_unittest.py @@ -133,13 +133,22 @@ class ObjectStoreTestCase(test.TrialTestCase): self.assertRaises(NotFound, objectstore.bucket.Bucket, 'new_bucket') def test_images(self): + self.do_test_images('1mb.manifest.xml', True, + 'image_bucket1', 'i-testing1') + + def test_images_no_kernel_or_ramdisk(self): + self.do_test_images('1mb.no_kernel_or_ramdisk.manifest.xml', + False, 'image_bucket2', 'i-testing2') + + def do_test_images(self, manifest_file, expect_kernel_and_ramdisk, + image_bucket, image_name): "Test the image API." self.context.user = self.auth_manager.get_user('user1') self.context.project = self.auth_manager.get_project('proj1') # create a bucket for our bundle - objectstore.bucket.Bucket.create('image_bucket', self.context) - bucket = objectstore.bucket.Bucket('image_bucket') + objectstore.bucket.Bucket.create(image_bucket, self.context) + bucket = objectstore.bucket.Bucket(image_bucket) # upload an image manifest/parts bundle_path = os.path.join(os.path.dirname(__file__), 'bundle') @@ -147,18 +156,28 @@ class ObjectStoreTestCase(test.TrialTestCase): bucket[os.path.basename(path)] = open(path, 'rb').read() # register an image - image.Image.register_aws_image('i-testing', - 'image_bucket/1mb.manifest.xml', + image.Image.register_aws_image(image_name, + '%s/%s' % (image_bucket, manifest_file), self.context) # verify image - my_img = image.Image('i-testing') + my_img = image.Image(image_name) result_image_file = os.path.join(my_img.path, 'image') self.assertEqual(os.stat(result_image_file).st_size, 1048576) sha = hashlib.sha1(open(result_image_file).read()).hexdigest() self.assertEqual(sha, '3b71f43ff30f4b15b5cd85dd9e95ebc7e84eb5a3') + if expect_kernel_and_ramdisk: + # Verify the default kernel and ramdisk are set + self.assertEqual(my_img.metadata['kernelId'], 'aki-test') + self.assertEqual(my_img.metadata['ramdiskId'], 'ari-test') + else: + # Verify that the default kernel and ramdisk (the one from FLAGS) + # doesn't get embedded in the metadata + self.assertFalse('kernelId' in my_img.metadata) + self.assertFalse('ramdiskId' in my_img.metadata) + # verify image permissions self.context.user = self.auth_manager.get_user('user2') self.context.project = self.auth_manager.get_project('proj2') From 38bbe0dbf57f8b4b2d55d2959e1ffe1360ae09da Mon Sep 17 00:00:00 2001 From: Michael Gundlach Date: Tue, 12 Oct 2010 18:27:59 -0400 Subject: [PATCH 52/52] 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/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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)]