Merge lp:~soren/nova/ec2-security-groups into lp:~hudson-openstack/nova/trunk

Heya, needs a quick merge with trunk...there's conflicts.

Soren,

I merged your branch with trunk and fixed a few errors that popped up. Unfortunately libvirt is complaining heartily when i try and launch an instance and not giving very useful error messages. I'm using libvirt from the nova-core ppa
virsh fails too
error: Failed to create domain from libvirt.xml
error: Unknown failure

On Sep 21, 2010, at 3:09 AM, Soren Hansen wrote:

> Soren Hansen has proposed merging lp:~soren/nova/ec2-security-groups into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
>
>
> This patch adds support for EC2 security groups using libvirt's nwfilter mechanism, which in turn uses iptables and ebtables on the individual compute nodes.
> This has a number of benefits:
> * Inter-VM network traffic can take the fastest route through the network without our having to worry about getting it through a central firewall.
> * Not relying on a central firewall also removes a potential SPOF.
> * The filtering load is distributed, offering great scalability.
>
> Caveats:
> * It only works with libvirt and only with libvirt drivers that support nwfilter (qemu (and thus kvm) and uml, at the moment)
> --
> https://code.launchpad.net/~soren/nova/ec2-security-groups/+merge/36119
> Your team Nova Core is requested to review the proposed merge of lp:~soren/nova/ec2-security-groups into lp:nova.
> === modified file 'nova/auth/manager.py'
> --- nova/auth/manager.py 2010-09-21 05:08:31 +0000
> +++ nova/auth/manager.py 2010-09-21 10:09:06 +0000
> @@ -490,6 +490,12 @@
> except:
> 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):
> @@ -565,6 +571,16 @@
> 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))
>
>
> === modified file 'nova/compute/manager.py'
> --- nova/compute/manager.py 2010-09-13 09:15:02 +0000
> +++ nova/compute/manager.py 2010-09-21 10:09:06 +0000
> @@ -64,6 +64,11 @@
>
> @defer.inlineCallbacks
> @exception.wrap_exception
> + def refresh_security_group(self, context, security_group_id, **_kwargs):
> + yield self.driver.refresh_security_group(security_group_id)
> +
> + @defer.inlineCallbacks
> + @exception.wrap...

Erk... that's a lot of fallout. I've got it mostly done, but I have a few unit tests that are acting up. I'm also not feeling very well today, so I may not get it done today after all. :(

Phew. There we go. All cleaned up, and happy test cases again :)

I still can't get libvirt to actually run the instance with this patch in. Is there some configuration i need to do that is left out? I'm using libvirt from nova-core and it chokes badly trying to launch with the filter in the url.

Also, a couple typos in your latest version:

=== modified file 'nova/api/ec2/cloud.py'
--- nova/api/ec2/cloud.py 2010-09-22 20:16:12 +0000
+++ nova/api/ec2/cloud.py 2010-09-22 23:40:30 +0000
@@ -725,9 +725,9 @@

         security_groups = []
         for security_group_name in security_group_arg:
- group = db.security_group_get_by_project(context,
- context.project.id,
- security_group_name)
+ group = db.security_group_get_by_name(context,
+ context.project.id,
+ security_group_name)
             security_groups.append(group['id'])

         reservation_id = utils.generate_uid('r')
@@ -744,6 +744,7 @@
         base_options['user_data'] = kwargs.get('user_data', '')

         type_data = INSTANCE_TYPES[instance_type]
+ base_options['instance_type'] = instance_type
         base_options['memory_mb'] = type_data['memory_mb']
         base_options['vcpus'] = type_data['vcpus']
         base_options['local_gb'] = type_data['local_gb']

On Sep 21, 2010, at 3:09 AM, Soren Hansen wrote:

> Soren Hansen has proposed merging lp:~soren/nova/ec2-security-groups into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
>
>
> This patch adds support for EC2 security groups using libvirt's nwfilter mechanism, which in turn uses iptables and ebtables on the individual compute nodes.
> This has a number of benefits:
> * Inter-VM network traffic can take the fastest route through the network without our having to worry about getting it through a central firewall.
> * Not relying on a central firewall also removes a potential SPOF.
> * The filtering load is distributed, offering great scalability.
>
> Caveats:
> * It only works with libvirt and only with libvirt drivers that support nwfilter (qemu (and thus kvm) and uml, at the moment)
> --
> https://code.launchpad.net/~soren/nova/ec2-security-groups/+merge/36119
> Your team Nova Core is requested to review the proposed merge of lp:~soren/nova/ec2-security-groups into lp:nova.
> === modified file 'nova/auth/manager.py'
> --- nova/auth/manager.py 2010-09-21 05:08:31 +0000
> +++ nova/auth/manager.py 2010-09-21 10:09:06 +0000
> @@ -490,6 +490,12 @@
> except:
> 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):
> @@ -565,6 +571,16 @@
> except:
> logging.exce...

s/in the url/in the xml

On Sep 21, 2010, at 3:09 AM, Soren Hansen wrote:

> Soren Hansen has proposed merging lp:~soren/nova/ec2-security-groups into lp:nova.
>
> Requested reviews:
> Nova Core (nova-core)
>
>
> This patch adds support for EC2 security groups using libvirt's nwfilter mechanism, which in turn uses iptables and ebtables on the individual compute nodes.
> This has a number of benefits:
> * Inter-VM network traffic can take the fastest route through the network without our having to worry about getting it through a central firewall.
> * Not relying on a central firewall also removes a potential SPOF.
> * The filtering load is distributed, offering great scalability.
>
> Caveats:
> * It only works with libvirt and only with libvirt drivers that support nwfilter (qemu (and thus kvm) and uml, at the moment)
> --
> https://code.launchpad.net/~soren/nova/ec2-security-groups/+merge/36119
> Your team Nova Core is requested to review the proposed merge of lp:~soren/nova/ec2-security-groups into lp:nova.
> === modified file 'nova/auth/manager.py'
> --- nova/auth/manager.py 2010-09-21 05:08:31 +0000
> +++ nova/auth/manager.py 2010-09-21 10:09:06 +0000
> @@ -490,6 +490,12 @@
> except:
> 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):
> @@ -565,6 +571,16 @@
> 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))
>
>
> === modified file 'nova/compute/manager.py'
> --- nova/compute/manager.py 2010-09-13 09:15:02 +0000
> +++ nova/compute/manager.py 2010-09-21 10:09:06 +0000
> @@ -64,6 +64,11 @@
>
> @defer.inlineCallbacks
> @exception.wrap_exception
> + def refresh_security_group(self, context, security_group_id, **_kwargs):
> + yield self.driver.refresh_security_group(security_group_id)
> +
> + @defer.inlineCallbacks
> + @exception.wrap_exception
> def run_instance(self, context, instance_id, **_kwargs):
> """Launch a new instance with specified options."""
> instance_ref = self.db.instance_get(context, instance_id)
>
> === modified file 'nova/db/api.py'
> --- nova/db/api.py 2010-09-21 02:17:36 +0000
> +++ nova/db/api....

As for the libvirt problem, it turns out to be apparmour that gets in the way.

Run this on the box, and you should be fine:

sudo aa-complain /usr/sbin/libvirtd

Have fun.

Still not quite there on the security groups. Using your aa-complain fix i still get the following error in lucid:
libvir: Network Filtererror : internal error Could not get access to ACL tech driver 'ebiptables'
ERROR:root:instance i-1: Failed to spawn
Traceback (most recent call last):
  File "/srv/cloud/nova/nova/compute/manager.py", line 91, in run_instance
    yield self.driver.spawn(instance_ref)
  File "/usr/local/lib/python2.6/dist-packages/twisted/internet/defer.py", line 823, in _inlineCallbacks
    result = g.send(result)
  File "/srv/cloud/nova/nova/virt/libvirt_conn.py", line 220, in spawn
    yield self._conn.createXML(xml, 0)
  File "/usr/lib/python2.6/dist-packages/libvirt.py", line 1289, in createXML
    if ret is None:raise libvirtError('virDomainCreateXML() failed', conn=self)
libvirtError: internal error Could not get access to ACL tech driver 'ebiptables'

I tried downloading and installing the maverick dpkg from the nova-repo
I grabbed libnl, libvirt0 and libvirt-bin and they install and run fine. Now I can actually run machines again, but every time it tries to update the rules on the compute host, I get the following:
libvir: Network Filtererror : internal error cannot create rule since iptables tool is missing.
2010-09-25 09:11:49+0000 [-] Unhandled error in Deferred:
2010-09-25 09:11:49+0000 [-] Unhandled Error
        Traceback (most recent call last):
          File "/usr/lib/python2.6/threading.py", line 504, in __bootstrap
            self.__bootstrap_inner()
          File "/usr/lib/python2.6/threading.py", line 532, in __bootstrap_inner
            self.run()
          File "/usr/lib/python2.6/threading.py", line 484, in run
            self.__target(*self.__args, **self.__kwargs)
        --- <exception caught here> ---
          File "/usr/local/lib/python2.6/dist-packages/twisted/python/threadpool.py", line 210, in _worker
            result = context.call(ctx, function, *args, **kwargs)
          File "/usr/local/lib/python2.6/dist-packages/twisted/python/context.py", line 59, in callWithContext
            return self.currentContext().callWithContext(ctx, func, *args, **kw)
          File "/usr/local/lib/python2.6/dist-packages/twisted/python/context.py", line 37, in callWithContext
            return func(*args,**kw)
          File "/usr/lib/python2.6/dist-packages/libvirt.py", line 1646, in nwfilterDefineXML
            if ret is None:raise libvirtError('virNWFilterDefineXML() failed', conn=self)
        libvirt.libvirtError: internal error cannot create rule since iptables tool is missing.

So apparently, I'm missing the 'iptables tool'. Do i need to upgrade another package to a maverick version that wasn't listed as a depend properly in the dpkg?

Or is there some way that I can get a working lucid version?

Also DHCPackets are not coming through at all. I'm thinking that it is due to not being able to set up the rules properly, but I'm a bit surprised the dhcpdiscover isn't getting out, since it is udp.

Any suggestions are most welcome, since we really need to get the security groups tested ASAP.

Thanks,
Vish

On Sep 21, 2010, at 3:09 AM, Soren Hansen wrote:

> Soren Hansen has proposed merging lp...

On 25-09-2010 11:34, vishvananda wrote:
> Still not quite there on the security groups. Using your aa-complain fix i still get the following error in lucid:
> libvir: Network Filtererror : internal error Could not get access to ACL tech driver 'ebiptables'
> ERROR:root:instance i-1: Failed to spawn
> Traceback (most recent call last):
> File "/srv/cloud/nova/nova/compute/manager.py", line 91, in run_instance
> yield self.driver.spawn(instance_ref)
> File "/usr/local/lib/python2.6/dist-packages/twisted/internet/defer.py", line 823, in _inlineCallbacks
> result = g.send(result)
> File "/srv/cloud/nova/nova/virt/libvirt_conn.py", line 220, in spawn
> yield self._conn.createXML(xml, 0)
> File "/usr/lib/python2.6/dist-packages/libvirt.py", line 1289, in createXML
> if ret is None:raise libvirtError('virDomainCreateXML() failed', conn=self)
> libvirtError: internal error Could not get access to ACL tech driver 'ebiptables'

Do you have ebtables installed? This corner of libvirt has horrendous
error reporting, unfortunately. You need gawk, ebtables, and iptables
installed. Well, and bash and grep, but I can't imagine you'd be missing
those.

> I tried downloading and installing the maverick dpkg from the nova-repo
> I grabbed libnl, libvirt0 and libvirt-bin and they install and run fine. Now I can actually run machines again, but every time it tries to update the rules on the compute host, I get the following:
> libvir: Network Filtererror : internal error cannot create rule since iptables tool is missing.

Glancing at libvirt's code, that error code means it either failed to
locate iptables in your $PATH, or running "iptables -L FORWARD" failed.
Do you have iptables installed and in the $PATH of the libvirtd process?
(if you're using debian or ubuntu packages, you should be fine). Same

> So apparently, I'm missing the 'iptables tool'. Do i need to upgrade another package to a maverick version that wasn't listed as a depend properly in the dpkg?

Any reasonably recent version of iptables should be fine. Lucid is
definitely reasonably recent.

> Also DHCPackets are not coming through at all.

Yeah. That was the embarassing thing I was talking about on IRC
yesterday :) When I was attempting to get the networking stuff working
and couldn't get DHCP to work I was actually using this branch, and it
turns out it filters dhcp by default. To get DHCP to work, we can either
add a reference to the allow-dhcp or (preferably) allow-dhcp-server
filter for the instance. If the latter, we need to know the IP of the
DHCP server. If we have that, we can just add
<filterref filter="allow-dhcp-server">
  <parameter name="DHCPSERVER" value="<ip of DHCP server>" />
</filterref>

> I'm thinking that it is due to not being able to set up the rules properly, but I'm a bit surprised the dhcpdiscover isn't getting out, since it is udp.

That's an artefact of the no-ip-spoofing filter (part of the
clean-traffic which is included by nova-base-filter), I think. It
refuses traffic from an IP other than the one designated in the libvirt
XML (including DHCP requests that have a NULL IP, IIRC). Adding the
above mentioned allow-dhcp(-server) filter should ...

Test comment to see if it will subscribe me

problem running multiple tests, it may not be instance i-1. This patch fixes.

=== modified file 'nova/tests/virt_unittest.py'
--- nova/tests/virt_unittest.py 2010-09-28 08:26:29 +0000
+++ nova/tests/virt_unittest.py 2010-09-28 17:47:54 +0000
@@ -178,8 +178,14 @@
             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 @@
                 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)

After that fix, it is an approve from me. But we really need to make sure the lucid package is easily accessible from the ppa and works out of the box. I suggest we make sure that the needed app-armor rules are in, it is the most current libvirt, and that we add dependencies on gawk, iptables, and ebtables.

I've pushed a new libvirt to the nova-core Lucid ppa. It's identical to the one in Ubuntu Maverick, except macvtap has been disabled (Lucid's kernel does not support it). This fixes the AppArmour problems.

I've added iptables, ebtables, and gawk as dependencies of nova-compute.

Approved. This is excellent work. Especially on the test cases.

Approve.

FYI db/sqlalchemy/api.py looks like it could be using dict.update instead of a tight for loop in a few places, in case you missed that.

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/db/sqlalchemy/api.py

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/tests/virt_unittest.py

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/tests/virt_unittest.py

Oh, come on!

Woo hoo!
On Oct 12, 2010 1:28 PM, <email address hidden> wrote:
> The proposal to merge lp:~soren/nova/ec2-security-groups into lp:nova has
been updated.
>
> Status: Approved => Merged
> --
> https://code.launchpad.net/~soren/nova/ec2-security-groups/+merge/36119
> You are subscribed to branch lp:~soren/nova/ec2-security-groups.