<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>Brian,</div><div><br></div><div>Thanks a lot for the clarification of my code understanding!</div><div><br></div><div>Yes, originally I also thought that this was a problem with an old code because I have upgraded the system. I performed upgrade according to the official openstack notes, and neutron-server has been restarted after rpm package update. To be on a safe side, I removed all *.pyc, *.pyo files in the /usr/lib/python2.7/site-packages/neutron directory, compiled them again and restarted the server. The problem still persisted and that's where I've thought that this might be a code bug.<br></div><div><br></div><div>I am not running any third-party plugins, here's plugin configuration from my /etc/neutron/neutron.conf:</div><div><br></div><div># The core plugin Neutron will use (string value)<br>core_plugin = ml2<br># The service plugins Neutron will use (list value)<br>service_plugins = router,neutron_lbaas.services.loadbalancer.plugin.LoadBalancerPluginv2,neutron.services.qos.qos_plugin.QoSPlugin<br><br></div></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 14, 2019 at 8:25 PM Brian Haley <<a href="mailto:haleyb.dev@gmail.com">haleyb.dev@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 3/14/19 5:09 AM, Just FooBar wrote:<br>
> Hi Neil,<br>
> <br>
> Thanks for your reply!<br>
> <br>
> I had a look at the code you mentioned, and while I don't know enough <br>
> about the calico implementation, one thing seems like a good idea for an <br>
> improvement to me.<br>
> In this code, you're patching method security_groups_rule_updatedto be <br>
> able to run mech_driver.send_sg_updates(sgids, context). I think that it <br>
> should be possible to read notifications about <br>
> security_groups_rule_updatedfrom MQ and run the mech_driver's method <br>
> there, in a separate chunk of code. The current implementation of <br>
> AgentNotifierApi is inheriting from SecurityGroupAgentRpcApiMixin and <br>
> you can see the code sending notifications to MQ here [1] It should <br>
> allow you to be more agnostic of AgentNotifierApi implementations and <br>
> get rid of this patching.<br>
> <br>
> Back to my point. I needed clarification about the situation I described <br>
> because I don't know much about internals of neutron. Is it a bug that <br>
> AgentNotifierApi is still inheriting from the class [2] marked for <br>
> removal [3]? Or is it something intended to be and I'm just interpreting <br>
> it wrong?<br>
<br>
In Rocky (13.0.x), all this was cleaned-up, and the moves.moved_class() <br>
code dealing with the moved classes was removed, those classes were <br>
never deprecated, just moved to another location.<br>
<br>
Re-reading the bug I am still confused, it almost seems like there is a <br>
neutron-server running old code and publishing messages to the wrong <br>
exchange.<br>
<br>
Also, is this using the in-tree code or a third-party plugin?<br>
<br>
-Brian<br>
<br>
> <br>
> [1] <br>
> <a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L148" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L148</a><br>
> [2] <br>
> <a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122</a><br>
> [3] <br>
> <a href="https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293</a><br>
> <br>
> Thanks!<br>
> <br>
> On Mon, Mar 11, 2019 at 8:09 PM Neil Jerram <<a href="mailto:neil@tigera.io" target="_blank">neil@tigera.io</a> <br>
> <mailto:<a href="mailto:neil@tigera.io" target="_blank">neil@tigera.io</a>>> wrote:<br>
> <br>
> Hi Just,<br>
> <br>
> I'm interested in what you're saying, because it sounds like it<br>
> relates to the following horrible code in networking-calico [1], which<br>
> I'd love to make less horrible:<br>
> <br>
> # This terrible global variable points to the running instance<br>
> of the<br>
> # Calico Mechanism Driver. This variable relies on the basic<br>
> assertion that<br>
> # any Neutron process, forked or not, should only ever have<br>
> *one* Calico<br>
> # Mechanism Driver in it. It's used by our monkeypatch of the<br>
> # security_groups_rule_updated method below to locate the<br>
> mechanism driver.<br>
> # TODO(nj): Let's not do this any more. Please?<br>
> mech_driver = None<br>
> <br>
> [...]<br>
> <br>
> # This section monkeypatches the<br>
> AgentNotifierApi.security_groups_rule_updated<br>
> # method to ensure that the Calico driver gets told about<br>
> security group<br>
> # updates at all times. This is a deeply unpleasant hack. Please,<br>
> do as I say,<br>
> # not as I do.<br>
> #<br>
> # For more info, please see issues #635 and #641.<br>
> original_sgr_updated =<br>
> rpc.AgentNotifierApi.security_groups_rule_updated<br>
> <br>
> <br>
> def security_groups_rule_updated(self, context, sgids):<br>
> LOG.info("security_groups_rule_updated: %s %s" % (context,<br>
> sgids))<br>
> mech_driver.send_sg_updates(sgids, context)<br>
> original_sgr_updated(self, context, sgids)<br>
> <br>
> <br>
> rpc.AgentNotifierApi.security_groups_rule_updated = (<br>
> security_groups_rule_updated<br>
> )<br>
> <br>
> [1]<br>
> <a href="https://opendev.org/openstack/networking-calico/src/branch/master/networking_calico/plugins/ml2/drivers/calico/mech_calico.py#L148" rel="noreferrer" target="_blank">https://opendev.org/openstack/networking-calico/src/branch/master/networking_calico/plugins/ml2/drivers/calico/mech_calico.py#L148</a><br>
> <br>
> But I'm afraid I'm unclear what your point is. Is it about something<br>
> that could improve the code above?<br>
> <br>
> Thanks,<br>
> Neil<br>
> <br>
> On Fri, Mar 8, 2019 at 3:13 PM Just FooBar <<a href="mailto:just.foobar42@gmail.com" target="_blank">just.foobar42@gmail.com</a><br>
> <mailto:<a href="mailto:just.foobar42@gmail.com" target="_blank">just.foobar42@gmail.com</a>>> wrote:<br>
> ><br>
> > Hello everyone,<br>
> ><br>
> > I'm having problems with Security Group Rule updates not being<br>
> applied to vms on hypervisors and I think I know why this is happening.<br>
> ><br>
> > I see that there's already a bug about that:<br>
> <a href="https://bugs.launchpad.net/neutron/+bug/1814209" rel="noreferrer" target="_blank">https://bugs.launchpad.net/neutron/+bug/1814209</a> but there isn't much<br>
> action going on there. In my case, neutron-server is also using the<br>
> queue q-agent-notifier-security_group-update (as seen from<br>
> neutron-server logs in debug mode). The neutron version is the same,<br>
> 12.0.4.<br>
> ><br>
> > I went to check the code for my version 12.0.4 and I've found<br>
> some suspicious part which might be the cause of this bug. Let me<br>
> explain my understanding of the situation.<br>
> ><br>
> > I started with ovs agent code and found that it is using<br>
> SecurityGroupServerAPIShim<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L374" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L374</a>)<br>
> class for, well, Security Group RPC.<br>
> ><br>
> > Comments in this class definition<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L204" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L204</a>)<br>
> explain that it is a replacement for an older interface,<br>
> SecurityGroupServerRpcApi<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L33" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L33</a>).<br>
> > SecurityGroupServerAPIShim inherits from<br>
> SecurityGroupInfoAPIMixin<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/db/securitygroups_rpc_base.py#L126" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/db/securitygroups_rpc_base.py#L126</a>)<br>
> which is also a parent of the server side Ml2Plugin<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L121" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L121</a>).<br>
> From this I make a conclusion that Ml2Plugin was also switched to<br>
> the new SG RPC interface.<br>
> ><br>
> > Now, there are other details that suggest that Ml2Plugin wasn't<br>
> switched to the new interface entirely and continues to use the<br>
> old-style SG RPC classes.<br>
> ><br>
> > There's a class AgentNotifierApi<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/rpc.py#L376" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/rpc.py#L376</a>)<br>
> used by neutron-server's Ml2Plugin<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L269" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin.py#L269</a>)<br>
> to send notifications (to agents, I suppose). It inherits from the<br>
> class SecurityGroupAgentRpcApiMixin<br>
> (<a href="https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/securitygroups_rpc.py#L122</a>)<br>
> which has been marked for removal starting from Pike 3 years ago in<br>
> this commit:<br>
> <a href="https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573a3432c17#diff-e4d9694fe7cfd3a791360aa215c12db8R293</a>.<br>
> This AgentNotifierApi class wasn't switched to a new Shim RPC<br>
> interface for SG (as it was done for the ovs agent and Ml2Plugin<br>
> itself).<br>
> ><br>
> > All previous links are for 12.0.4 version, the one used in my<br>
> system currently.<br>
> > And here's the same class AgentNotifierApi from the Rocky<br>
> release:<br>
> <a href="https://github.com/openstack/neutron/blob/stable/rocky/neutron/plugins/ml2/rpc.py#L387" rel="noreferrer" target="_blank">https://github.com/openstack/neutron/blob/stable/rocky/neutron/plugins/ml2/rpc.py#L387</a><br>
> . As you can see, it still inherits from the class marked for<br>
> removal and isn't using new style SG RPC API.<br>
> ><br>
> > From all this I conclude that until AgentNotifierApi is using new<br>
> style API or the way that Ml2Plugin is sending notifications isn't<br>
> changed, the bug will still be present.<br>
> ><br>
> > Please let me know if I'm getting this wrong. If I'm right, I'm<br>
> interested in helping to fix the bug.<br>
> ><br>
> > Thank you for your attention!<br>
> <br>
<br>
</blockquote></div>