[RFE] l3_agent should separate router_info creation logic

[Yang Youseok]

Currently, l3-agent has tightly coupled with router_info creation logic, so there is no way to make a big change of default router's behaviors. Even there are already many diverse routers (dvr, dvrha, dvr_snat...), every routers depend detailed implementation rather than structured interfaces. So it makes hard to add new feature.

I found a majority of networking-* with l3 implementation just override core L3NatAgent and add a little tweaking functions to change default l3-agent behavior. IMHO, if there is clear interface in L3 agent for specific router, they just add their router business logic rather than did not override core agent class. I noticed they did not change RPC mechanism a lot in agent side, and want to just make few tweak like us.

What I suggest is make abstract class for router with minimal functions (initialize(), process()..), and add create router logic with dynamically configured router_info class. It decouples the creation and RPC controller so that plugin developer can easily change the default router behavior.

To provide more specific use case, what we have done was to add linux VRF feature instead of namespace and East-West traffic is enabled by default so that there is no need to add GW / internal port at all. We do not need majority of functions except floating IP, but every kind of API / RPC is useful at the same time from server side.

[YAMAMOTO Takashi (yamamoto)]

in case you have a concrete proposal, i guess it's nice to have a spec.

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/620348

[OpenStack Infra (hudson-openstack)]

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

[Yang Youseok (ileixe)]

@YAMAMOTO

Hello YAMAMOTO. Thanks for reply.

I did not notice your reply, so I just initial commits for the patch, Sorry for that.
Since I am newbie for this project, may I ask you what to do for spec? I think spec you said
seems to 'https://specs.openstack.org/openstack/neutron-specs/' though, but I'm not sure.

[Miguel Lavalle (minsel)]

This RFE was discussed and approved today. Since the proposed enhancement has the potential to impact not only the Neutron reference implementation but also other networking related projects, a spec is required as a next step. Please create the spec here: https://github.com/openstack/neutron-specs/tree/master/specs/stein

[Yang Youseok (ileixe)]

@Miguel Thanks for the approve. I will be glad to make the spec to follow your request.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/625647
Committed: https://git.openstack.org/cgit/openstack/neutron-specs/commit/?id=bd23545778016827e7f520488c224a7943b59310
Submitter: Zuul
Branch: master

commit bd23545778016827e7f520488c224a7943b59310
Author: Yang Youseok <email address hidden>
Date: Tue Dec 18 01:21:05 2018 +0900

    Registering RouterInfo by L3 extention API

    Currently, most plugin implementations releated L3 override the
    L3NatAgent class itself for their own logic since there is no proper
    interface to extend RouterInfo class. This adds unnecessary complexity
    for developers who just want to extend agent mechanism instead of
    whole RPC related to L3 functinalities.

    This spec introduces RouterFactory class which acts on the factory for
    creating RouterInfo class, and add new l3 extension API which enable to
    dynamically add RouterInfo to the factory. Now plugin developers can
    use new extension API for their own specific router.

    Change-Id: Ic3486830e449f6ee8dbe19db614179b2077bcf7b
    Related-Bug: #1804634
    Implements: blueprint router-factory-with-l3-extension

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/620348
Committed: https://git.openstack.org/cgit/openstack/neutron-lib/commit/?id=176c09fa192df4b42570d1478039607108fad6d1
Submitter: Zuul
Branch: master

commit 176c09fa192df4b42570d1478039607108fad6d1
Author: Yang Youseok <email address hidden>
Date: Fri Nov 23 15:41:20 2018 +0900

    Add RouterNotFoundInRouterFactory exception

    Add new exception for l3-agent. It occurs when there is no valid
    router_info class which registered in router_factory.

    Related-Bug: #1804634
    Change-Id: I55a1cb60898ab87ffe137f8c4b0b2f2803bfb219

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/620349
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=ec875b42b6e92300093c895668532966de9e1327
Submitter: Zuul
Branch: master

commit ec875b42b6e92300093c895668532966de9e1327
Author: Yang Youseok <email address hidden>
Date: Fri Nov 23 15:55:02 2018 +0900

    Add router_factory to l3-agent and L3 extension API

    Currently, most implementations override the L3NatAgent class itself
    for their own logic since there is no proper interface to extend
    RouterInfo class. This adds unnecessary complexity for developers
    who just want to extend router mechanism instead of whole RPC.

    Add a RouterFactory class that developer can registers RouterInfo class
    and delegate it for RouterInfo creation. Seperate functions and variables
    which currently used externally to abstract class from RouterInfo, so that
    extension can use the basic interface.

    Provide the router registration function to the l3 extension API so that
    extension can extend RouterInfo itself which correspond to each features
    (ha, distribtued, ha + distributed)

    Depends-On: https://review.openstack.org/#/c/620348/
    Closes-Bug: #1804634
    Partially-Implements: blueprint openflow-based-dvr
    Change-Id: I1eff726900a8e67596814ca9a5f392938f154d7b

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 15.0.0.0b1 development milestone.

[Nate Johnston (nate-johnston)]

FYI this caused a problem for neutron-fwaas since it was not adjusted to include the new required arguments. See: https://bugs.launchpad.net/neutron/+bug/1845227 - fix in progress.