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/se... [2] https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/se... [3] https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573... Thanks! On Mon, Mar 11, 2019 at 8:09 PM Neil Jerram <neil@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...
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@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...) class for, well, Security Group RPC.
Comments in this class definition (
https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/se...) explain that it is a replacement for an older interface, SecurityGroupServerRpcApi ( https://github.com/openstack/neutron/blob/12.0.4/neutron/api/rpc/handlers/se... ).
SecurityGroupServerAPIShim inherits from SecurityGroupInfoAPIMixin ( https://github.com/openstack/neutron/blob/12.0.4/neutron/db/securitygroups_r...) which is also a parent of the server side Ml2Plugin ( https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin....). 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#...) used by neutron-server's Ml2Plugin ( https://github.com/openstack/neutron/blob/12.0.4/neutron/plugins/ml2/plugin....) 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/se...) which has been marked for removal starting from Pike 3 years ago in this commit: https://github.com/openstack/neutron/commit/97338258967d3b95f382f188ab2ab573.... 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/r... . 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!