[openstack-dev] [qa] [tempest] [patrole] Service client duplication between Tempest and Tempest plugins

Ghanshyam Mann gmann at ghanshyammann.com
Tue Jul 24 02:38:30 UTC 2018

 ---- On Tue, 24 Jul 2018 04:22:47 +0900 MONTEIRO, FELIPE C <fm577c at att.com> wrote ---- 
 >       Hi,
 >  ** Intention **
 >  Intention is to expand Patrole testing to some service clients that already exist in some Tempest plugins, for core services only.
 >  ** Background **
 >  Digging through Neutron testing, it seems like there is currently a lot of test duplication between neutron-tempest-plugin and Tempest [1]. Under some circumstances it seems OK to have redundant testing/parallel  testing: “Having potential duplication between testing is not a big deal especially compared to the alternative of removing something which is actually providing value and is actively catching bugs, or blocking incorrect patches from landing” [2].

We really need to minimize the test duplication. If there is test in tempest plugin for core services then, we do not need to add those in Tempest repo  until it is interop requirement. This is for new tests so we can avoid the duplication in future. I will write this in Tempest reviewer guide.
For existing duplicate tests, as per bug you mentioned[1] we need to cleanup the duplicate tests and they should live in their respective repo(either in neutron tempest plugin or tempest) which is categorized in etherpad[7]. How many tests are duplicated now? I will plan this as one of cleanup working item in stein. 

 >  This leads me to the following question: If API test duplication is OK, what about service client duplication? Patches like [3] and [4]  promote service client duplication with neutron-tempest-plugin. As far as I can tell, Neutron builds out some of its service clients dynamically here: [5]. Which includes segments service client (proposed as an addition to tempest.lib in [4]) here: [6].

Yeah, they are very dynamic in neutron plugins and its because of old legacy code. That is because when neutron tempest plugin was forked from Tempest as it is. These dynamic generation of service clients are really hard to debug and maintain. This can easily lead to backward incompatible changes if we make those service clients stable interface to consume outside. For those reason, we did fixed those in Tempest 3 years back [8] and made them  static and consistent service client methods like other services clients. 

 >  This leads to a situation where if we want to offer RBAC testing for these APIs (to validate their policy enforcement), we can’t really do so without adding the service client to Tempest, unless  we rely on the neutron-tempest-plugin (for example) in Patrole’s .zuul.yaml.
 >  ** Path Forward **
 >  Option #1: For the core services, most service clients should live in tempest.lib for standardization/governance around documentation and stability for those clients. Service client duplication  should try to be minimized as much as possible. API testing related to some service clients, though, should remain in the Tempest plugins.
 >  Option #2: Proceed with service client duplication, either by adding the service client to Tempest (or as yet another alternative, Patrole). This leads to maintenance overhead: have to maintain  service clients in the plugins and Tempest itself.
 >  Option #3: Don’t offer RBAC testing in Patrole plugin for those APIs.

We need to share the service clients among Tempest plugins. And each service clients which are being shared across repo has to be declared as stable interface like Tempest does. Idea here is service clients will live in the repo where their original tests were added or going to be added. For example in case of neutron tempest plugin, if rbac-policy API tests are in neutron then its service client needs to be owned by neutron-tempest-plugin. further rbac-policy service client can be consumed by Patrole. It is same case for congress tempest plugin, where they consume mistral service client. I recommended the same in that thread also of using service client from Mistral and Mistral make the service client as stable interface [9]. Which is being done in congress[10]

Here are the general recommendation for Tempest Plugins for service clients :
- Tempest Plugins should make their service clients as stable interface which gives 2 advantage:
  1. By this you make sure that you are not allowing to change the API calling interface(service clietns) which indirectly means you are not allowing to change the APIs. Makes your tempest plugin testing more reliable.

   2. Your service clients can be used in other Tempest plugins to avoid duplicate code/interface. If any other plugins use you service clients means, they also test your project so it is good to help them by providing the required interface as stable.

Initial idea of owning the service clients in their respective plugins was to share them among plugins for integrated testing of more then one openstack service.

- Usage of service clients across repo, Tempest provide a better way to do so than importing them directly [11]. You can see the example for Manila's tempest plugin [12]. This gives an advantage of discovering your registered service clients in other Tempest plugins automatically. 

I think its wroth to write as Doc in Tempest for Recommendation to Tempest Plugins.  I will write one later this week. 

Now back to current question of Patrole, Let's check with neutron tempest plugin team about implementing the above recommendation and use the service client from there instead of duplicating it in Tempest.  We should consume the service clients from neutron plugin and tempest where ever they live.

How about below plan:
Step 1. Neutron tempest plugin team declaring service client as stable interface which means no backward incompatible change. 
Step 2. Patrole import those service clients from neutron plugin as of now and proceed with testing. 
Step 3. Later neutron tempest plugin expose service clients via service client registration so that their service clients can be discovered automatically than importing them. Same way Tempest does.  

[7] https://etherpad.openstack.org/p/neutron-tempest-defork
[8] https://review.openstack.org/#/q/status:merged+project:openstack/tempest+branch:master+topic:refactor_neutron_client
[9] http://lists.openstack.org/pipermail/openstack-dev/2018-March/128483.html
[10] https://github.com/openstack/congress-tempest-plugin/blob/master/congress_tempest_plugin/tests/scenario/manager_congress.py#L85
[11] https://docs.openstack.org/tempest/latest/plugin.html#get_service_clients()
[12] https://review.openstack.org/#/c/334596/


 >  Thanks, 
 >  Felipe
 >  [1]  https://bugs.launchpad.net/neutron/+bug/1552960 
 >  [2]  https://docs.openstack.org/tempest/latest/test_removal.html 
 >  [3]  https://review.openstack.org/#/c/482395/ 
 >  [4]  https://review.openstack.org/#/c/582340/ 
 >  [5]  http://git.openstack.org/cgit/openstack/neutron-tempest-plugin/tree/neutron_tempest_plugin/services/network/json/network_client.py
 >  [6]  http://git.openstack.org/cgit/openstack/neutron-tempest-plugin/tree/neutron_tempest_plugin/api/test_timestamp.py 
 >     __________________________________________________________________________
 > OpenStack Development Mailing List (not for usage questions)
 > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
 > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

More information about the OpenStack-dev mailing list