add host to security group broken

[Simon]

I am running nova/neutron forked from trunk around 12/30/2013. Neutron is configured with openvswitch plugin and security group enabled.

How to reproduce the issue: create a security group SG1; add a rule to allow ingress from SG1 group to port 5000; add host A, B, and C to SG1 in order.

It seems that A can talk to B and C over port 5000, B can talk to C, but C can talk to neither of A and B. I confirmed that the iptables rules are incorrect for A and B. It seems to me that when A is added to the group, nothing changed since no other group member exists. When B and C were added to the group, A's ingress iptables rules were never updated.

[Aaron Rosen (arosen)]

Are you using the iptables implementation in the ovs-agent or in nova-network? What is: firewall_driver = set to in your nova.conf ?

[Simon (xchenum)]

Not sure if that matters..

https://github.com/openstack/nova/blob/master/nova/network/security_group/neutron_driver.py#L390

Here seems to be the problem. Only the ports on the VM with added the security group are updated. I think all VMs within the security group should update their ports.

[Aaron Rosen (arosen)]

why do you think that? I'm still not seeing the problem here sorry :/

[Simon (xchenum)]

Aaron, when you add a VM to a security group, only the ports of the VM in question are updated:
https://github.com/openstack/nova/blob/master/nova/network/security_group/neutron_driver.py#L410

This is problematic: the ports of the existing VMs in that security group should be updated too, otherwise they won't set up proper rules to allow traffic from the new VM in the group, if there are source groups ingress rules.

[Simon (xchenum)]

Actually, this is a bug for openvswitch plugin.. I'll submit a patch in a bit.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/101293

[Simon (xchenum)]

Can someone look at the code review? It's clearly a critical bug that we've hit in production, making the function of "add to security group" unusable.

I have added test cases according to the suggestion of Mark Mcclain. So, please at least take a look, since it's been stalled for long.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/101293
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=e97eea83ae162b9ce0f45f1ff334dbf70608fe3f
Submitter: Jenkins
Branch: master

commit e97eea83ae162b9ce0f45f1ff334dbf70608fe3f
Author: Xu Chen <email address hidden>
Date: Thu Jun 19 15:01:33 2014 -0400

    call security_groups_member_updated in port_update

    When a running VM is added to a security group, all existing VMs (ports)
    in the security group should be notified/updated - otherwise they would
    have incorrect rules, not knowing the new VM/port added.

    The current behavior would only update the port of the added VM. This
    patch forces an security_groups_member_updated() call for all the
    security groups that have ports removed or added.

    Change-Id: Ibdcd74f47043762386b62f3ec0fa1723060446ac
    Closes-Bug: 1316618

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/icehouse
Review: https://review.openstack.org/115038

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/115038
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=3520e66b8745924ebb9bd2978e8e11b60f58a221
Submitter: Jenkins
Branch: stable/icehouse

commit 3520e66b8745924ebb9bd2978e8e11b60f58a221
Author: Xu Chen <email address hidden>
Date: Thu Jun 19 15:01:33 2014 -0400

    call security_groups_member_updated in port_update

    When a running VM is added to a security group, all existing VMs (ports)
    in the security group should be notified/updated - otherwise they would
    have incorrect rules, not knowing the new VM/port added.

    The current behavior would only update the port of the added VM. This
    patch forces an security_groups_member_updated() call for all the
    security groups that have ports removed or added.

    Change-Id: Ibdcd74f47043762386b62f3ec0fa1723060446ac
    Closes-Bug: 1316618
    (cherry picked from commit e97eea83ae162b9ce0f45f1ff334dbf70608fe3f)