[neutron] [dev] ml2plugin doesn't use new security group rpc api

Just FooBar just.foobar42 at gmail.com
Thu Mar 14 09:09:56 UTC 2019


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_updated to 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_updated
from 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?

[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> 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>
> 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!
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-discuss/attachments/20190314/3a30f2fe/attachment.html>


More information about the openstack-discuss mailing list