Open Stack

On 3/14/19 5:09 AM, Just FooBar wrote:
> Hi Neil,
> 
> Thanks for your reply!
> 
> I had a look at the code you mentioned, and while I don't know enough 
> about the calico implementation, one thing seems like a good idea for an 
> improvement to me.
> In this code, you're patching method security_groups_rule_updatedto be 
> able to run mech_driver.send_sg_updates(sgids, context). I think that it 
> should be possible to read notifications about 
> security_groups_rule_updatedfrom MQ and run the mech_driver's method 
> there, in a separate chunk of code. The current implementation of 
> AgentNotifierApi is inheriting from SecurityGroupAgentRpcApiMixin and 
> you can see the code sending notifications to MQ here [1] It should 
> allow you to be more agnostic of AgentNotifierApi implementations and 
> get rid of this patching.
> 
> Back to my point. I needed clarification about the situation I described 
> because I don't know much about internals of neutron. Is it a bug that 
> AgentNotifierApi is still inheriting from the class [2] marked for 
> removal [3]? Or is it something intended to be and I'm just interpreting 
> it wrong?

In Rocky (13.0.x), all this was cleaned-up, and the moves.moved_class() 
code dealing with the moved classes was removed, those classes were 
never deprecated, just moved to another location.

Re-reading the bug I am still confused, it almost seems like there is a 
neutron-server running old code and publishing messages to the wrong 
exchange.

Also, is this using the in-tree code or a third-party plugin?

-Brian

> 
> [1] 
> https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L148
> [2] 
> https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122
> [3] 
> https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293
> 
> Thanks!
> 
> On Mon, Mar 11, 2019 at 8:09 PM Neil Jerram <neil at tigera.io 
> <mailto:neil at tigera.io>> wrote:
> 
>     Hi Just,
> 
>     I'm interested in what you're saying, because it sounds like it
>     relates to the following horrible code in networking-calico [1], which
>     I'd love to make less horrible:
> 
>          # This terrible global variable points to the running instance
>     of the
>          # Calico Mechanism Driver. This variable relies on the basic
>     assertion that
>          # any Neutron process, forked or not, should only ever have
>     *one* Calico
>          # Mechanism Driver in it. It's used by our monkeypatch of the
>          # security_groups_rule_updated method below to locate the
>     mechanism driver.
>          # TODO(nj): Let's not do this any more. Please?
>          mech_driver = None
> 
>          [...]
> 
>          # This section monkeypatches the
>     AgentNotifierApi.security_groups_rule_updated
>          # method to ensure that the Calico driver gets told about
>     security group
>          # updates at all times. This is a deeply unpleasant hack. Please,
>     do as I say,
>          # not as I do.
>          #
>          # For more info, please see issues #635 and #641.
>          original_sgr_updated =
>     rpc.AgentNotifierApi.security_groups_rule_updated
> 
> 
>          def security_groups_rule_updated(self, context, sgids):
>             LOG.info("security_groups_rule_updated: %s %s" % (context,
>     sgids))
>             mech_driver.send_sg_updates(sgids, context)
>             original_sgr_updated(self, context, sgids)
> 
> 
>          rpc.AgentNotifierApi.security_groups_rule_updated = (
>             security_groups_rule_updated
>          )
> 
>     [1]
>     https://opendev.org/openstack/networking-calico/src/branch/master/networking_calico/plugins/ml2/drivers/calico/mech_calico.py#L148
> 
>     But I'm afraid I'm unclear what your point is.  Is it about something
>     that could improve the code above?
> 
>     Thanks,
>           Neil
> 
>     On Fri, Mar 8, 2019 at 3:13 PM Just FooBar <just.foobar42 at gmail.com
>     <mailto:just.foobar42 at gmail.com>> wrote:
>      >
>      > Hello everyone,
>      >
>      > I'm having problems with Security Group Rule updates not being
>     applied to vms on hypervisors and I think I know why this is happening.
>      >
>      > I see that there's already a bug about that:
>     https://bugs.launchpad.net/neutron/+bug/1814209 but there isn't much
>     action going on there. In my case, neutron-server is also using the
>     queue q-agent-notifier-security_group-update (as seen from
>     neutron-server logs in debug mode). The neutron version is the same,
>     12.0.4.
>      >
>      > I went to check the code for my version 12.0.4 and I've found
>     some suspicious part which might be the cause of this bug. Let me
>     explain my understanding of the situation.
>      >
>      > I started with ovs agent code and found that it is using
>     SecurityGroupServerAPIShim
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L374)
>     class for, well, Security Group RPC.
>      >
>      > Comments in this class definition
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L204)
>     explain that it is a replacement for an older interface,
>     SecurityGroupServerRpcApi
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L33).
>      > SecurityGroupServerAPIShim inherits from
>     SecurityGroupInfoAPIMixin
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/db/securitygroups_rpc_base.py#L126)
>     which is also a parent of the server side Ml2Plugin
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L121).
>      From this I make a conclusion that Ml2Plugin was also switched to
>     the new SG RPC interface.
>      >
>      > Now, there are other details that suggest that Ml2Plugin wasn't
>     switched to the new interface entirely and continues to use the
>     old-style SG RPC classes.
>      >
>      > There's a class AgentNotifierApi
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/rpc.py#L376)
>     used by neutron-server's Ml2Plugin
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L269)
>     to send notifications (to agents, I suppose). It inherits from the
>     class SecurityGroupAgentRpcApiMixin
>     (https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122)
>     which has been marked for removal starting from Pike 3 years ago in
>     this commit:
>     https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293.
>     This AgentNotifierApi class wasn't switched to a new Shim RPC
>     interface for SG (as it was done for the ovs agent and Ml2Plugin
>     itself).
>      >
>      > All previous links are for 12.0.4 version, the one used in my
>     system currently.
>      > And here's the same class AgentNotifierApi from the Rocky
>     release:
>     https://github.com/openstack/neutron/blob/stable/rocky/neutron/plugins/ml2/rpc.py#L387
>     . As you can see, it still inherits from the class marked for
>     removal and isn't using new style SG RPC API.
>      >
>      > From all this I conclude that until AgentNotifierApi is using new
>     style API or the way that Ml2Plugin is sending notifications isn't
>     changed, the bug will still be present.
>      >
>      > Please let me know if I'm getting this wrong. If I'm right, I'm
>     interested in helping to fix the bug.
>      >
>      > Thank you for your attention!
>