---- On Tue, 24 Jul 2018 04:22:47 +0900 MONTEIRO, FELIPE C <fm5...@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/

-gmann

 >   
 >  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-requ...@lists.openstack.org?subject:unsubscribe
 > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
 > 



__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to