RPC callback should not use mixin approach

Bug #1359416 reported by Akihiro Motoki
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Fix Released
High
Akihiro Motoki

Bug Description

RPC is versioned per feature, but all of callback classes are implemented as mixin classes
and they are inherited into a single class.
This prevents RPC callback class from versioning properly.

This kind of chaos leads to bugs like bug 1353309.

This bug focuses on fixes of the server side of this problem.

# Agent side implementation uses neutron.manager.Manager and
# Manager requires callback as a top level method. It leads to mixin approach.

The following are the current status of the server side.

[l3-agent RPC callback]

- 1.1: NEC, Cisco N1KV, oneconvergence, ryu
- 1.2: Brocade, Hyper-V, LinuxBridge, MLNX, OVS
     version is bumped due to L2-plugin-agent RPC callback change
- 1.3: L3 Router plugin

All plugins above use l3-agent and RPC version should match.

[dhcp-agent RPC callback]

DHCP RPC version itself is 1.1.

The versions of all plugins which use dhcp-agent are:

- 1.1: nec, bigswitch, cisco n1kv, midonet, oneconvergence, vmware nsx
- 1.2: brocade, hyper-v, linuxbridge, mlnx, ovs
     the version is bumped due to L2 plugin agent RPC callback change
- 1.3: ml2
    the version is bumped due to RPC callback changes in L2 plugin agent and DVR support.

Tags: neutron-core
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

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

Changed in neutron:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Changed in neutron:
assignee: Akihiro Motoki (amotoki) → Miguel Angel Ajo (mangelajo)
Changed in neutron:
assignee: Miguel Angel Ajo (mangelajo) → Akihiro Motoki (amotoki)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on neutron (master)

Change abandoned by Akihiro Motoki (<email address hidden>) on branch: master
Review: https://review.openstack.org/117061
Reason: I made a mistake when squashing commits....

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/115798
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=73719a80bf4ea036a52617bffdf6d5bb725e4a90
Submitter: Jenkins
Branch: master

commit 73719a80bf4ea036a52617bffdf6d5bb725e4a90
Author: Akihiro Motoki <email address hidden>
Date: Tue Aug 19 03:49:30 2014 +0900

    Make L3RpcCallback a separate callback class

    RPC has a version of itself. In Neutron a plugin implements
    several RPC interface, so a single RPC version doesn't work.
    In Mixin callback class approach, RPC versioning depends on
    each plugin implementation and it makes harder to maintain
    RPC version appropriately. This patch series replaces mixin
    RPC callback of server side with a separate class.

    This commit handles server-side callback of L3-agent RPC interface.
    L3-agent server-side callback class is moved from db/ to
    api/rpc/handlers because it doesn't involve any db operations
    and defining all RPC interfaces in a single place sounds reasonable.

    Note that moving other L3-agent related RPC interface class
    to api/rpc/handlers will be done in a separate patch as this patch
    focuses on reorganizing the server-side RPC callback class.

    Partial-Bug: #1359416
    Change-Id: Ie3f2c9b2ad907a1110e05fe94d42e41e93fbcaa7

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/115799
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=42a8770a49e0342acffa7fdde22655c98e783184
Submitter: Jenkins
Branch: master

commit 42a8770a49e0342acffa7fdde22655c98e783184
Author: Akihiro Motoki <email address hidden>
Date: Tue Aug 19 04:14:31 2014 +0900

    Make DhcpRpcCallback a separate callback class

    RPC has a version of itself. In Neutron a plugin implements
    several RPC interface, so a single RPC version doesn't work.
    In Mixin callback class approach, RPC versioning depends on
    each plugin implementation and it makes harder to maintain
    RPC version appropriately. This patch series replaces mixin
    RPC callback of server side with a separate class.

    This commit handles server-side callback of dhcp-agent RPC interface.
    DHCP-agent server-side callback class is moved from db/ to
    api/rpc/handlers because it doesn't involve any db operations
    and defining all RPC interfaces in a single place sounds reasonable.

    Note that moving other DHCP-agent related RPC interface class
    to api/rpc/handlers can be done in a separate patch as this patch
    focuses on reorganizing the server-side RPC callback class.

    Partial-Bug: #1359416
    Change-Id: Ifb2a1bc0b7971995aae2856c9d4cd88c6dbc22d6

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/115800
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=f872bbd4f4bab5582ab4cb8e449e8afe61577563
Submitter: Jenkins
Branch: master

commit f872bbd4f4bab5582ab4cb8e449e8afe61577563
Author: Akihiro Motoki <email address hidden>
Date: Tue Aug 19 21:16:19 2014 +0900

    Make DvrServerRpcCallback a separate callback class

    RPC has a version of itself. In Neutron a plugin implements
    several RPC interface, so a single RPC version doesn't work.
    In Mixin callback class approach, RPC versioning depends on
    each plugin implementation and it makes harder to maintain
    RPC version appropriately. This patch series replaces mixin
    RPC callback of server side with a separate class.

    This commit handles server-side callback of DVR ML2 RPC interface.

    Partial-Bug: #1359416
    Change-Id: I1b6383f7b0af5d9aed18eda3a15f21d3504d0347

Changed in neutron:
assignee: Akihiro Motoki (amotoki) → shihanzhang (shihanzhang)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/116312
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=2781fce238e79690bf707a3df4823aec422687bd
Submitter: Jenkins
Branch: master

commit 2781fce238e79690bf707a3df4823aec422687bd
Author: Akihiro Motoki <email address hidden>
Date: Sat Aug 23 18:16:18 2014 +0900

    Make SecurityGroupsRpcCallback a separate callback class

    RPC has a version of itself. In Neutron a plugin implements
    several RPC interface, so a single RPC version doesn't work.
    In Mixin callback class approach, RPC versioning depends on
    each plugin implementation and it makes harder to maintain
    RPC version appropriately. This patch series replaces mixin
    RPC callback of server side with a separate class.

    This commit handles server-side callback of security group
    RPC interface.
    * The server-side callback of Security group RPC is moved to
      api/rpc/handler and db/securitygroups_rpc_base now only
      contains a mixin class to add agent-based security group
      implementation with db operations.
    * get_port_from_device method in server-side callback class
      is moved to a mixin class of plugin implementation
      (SecurityGroupServerRpcMixin) because it involves DB lookup
      and is tightly coupled with plugin implementation rather
      than RPC interface definition.

    Most unit tests for SGServerRpcCallBackTestCase were skipped
    in the base class before, but now they are no longer skipped.

    The following items will be planned in later patches
    to avoid drastic changes in a single patch.
    * Merge security group RPC API and agent callback classes in
      agent/securitygroups_rpc into api/rpc/handlers/securitygroup_rpc
    * Remove completely duplicated db access code in get_port_from_device
      and get_port_and_sgs

    Partial-Bug: #1359416
    Change-Id: Ia6535217d2e3b849a95667c1b53dd09675002892

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to neutron (master)

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

Changed in neutron:
assignee: shihanzhang (shihanzhang) → Akihiro Motoki (amotoki)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to neutron (master)

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

Thierry Carrez (ttx)
Changed in neutron:
milestone: juno-3 → juno-rc1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to neutron (master)

Reviewed: https://review.openstack.org/118165
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=d3ed330ca81d9663b1280a0386b572421ec48496
Submitter: Jenkins
Branch: master

commit d3ed330ca81d9663b1280a0386b572421ec48496
Author: Akihiro Motoki <email address hidden>
Date: Mon Sep 1 22:24:54 2014 +0900

    Fix a bug in Mellanox plugin RPC caused by secgroup RPC refactoring

    SecurityGroupsRpcCallback RPC refactoring patch (commit 2781fce238)
    moves get_port_from_device method from plugin RPC callback class to
    plugin layer, but Mellanox plugin RPC callback was forgot to update
    and as a result RPC calls from agents to Mellanox plugin fails.

    Change-Id: I1aa82ac00c16a53a59a93087f0ca4ef281ee3f2b
    Related-Bug: #1359416

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to neutron (master)

Reviewed: https://review.openstack.org/118161
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=8fb175bffd1b02262447478215d81b2a3edbe6a1
Submitter: Jenkins
Branch: master

commit 8fb175bffd1b02262447478215d81b2a3edbe6a1
Author: Akihiro Motoki <email address hidden>
Date: Sun Aug 31 01:47:24 2014 +0900

    Fix comments in api.rpc.handlers

    Follow-up patch of RPC refactoring of bug 1359416.
    It addresses minor comments in the above patch series.

    Change-Id: I2d6268db777f0f73fda61a5a7d0967a91bcb292b
    Closes-Bug: #1359416

Changed in neutron:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in neutron:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in neutron:
milestone: juno-rc1 → 2014.2
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.