Please see comments inline. > ---- 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:
In this case we should also expand the Tempest plugin stable interface documentation here (which currently gives people a narrow understanding of what stable interface means) to include stable interfaces in other plugins: https://docs.openstack.org/tempest/latest/plugin.html#stable-tempest-apis-plugins-may-use > 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. Thanks, this is good to know. > > - 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. Ok, so it seems like this is just something that is agreed to in IRC and then formalized using a documentation update saying they will commit to a stable interface in their plugin. I am wondering whether there is/should be a governance tag for saying whether a plugin abides by Tempest's stable interface guidelines. > 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. I will work with Neutron team to see about moving some of their legacy code to stable interface if they agree to this. - Felipe > > > [7] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__etherpad.openstack.org_p_neutron-2Dtempest- > 2Ddefork&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru- > SJ9DRnCxhze-aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=errkCNHIciPwWe-fA2xZ1yN0VisE-YIwV-cpZv-0PKI&e= > [8] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__review.openstack.org_-23_q_status-3Amerged-2Bproject- > 3Aopenstack_tempest-2Bbranch-3Amaster-2Btopic-3Arefactor-5Fneutron- > 5Fclient&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru- > SJ9DRnCxhze-aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=G47gpRDWJUpv-IRZJXTPwn7DzrmIm7XKma_BozhVMOc&e= > [9] https://urldefense.proofpoint.com/v2/url?u=http- > 3A__lists.openstack.org_pipermail_openstack-2Ddev_2018- > 2DMarch_128483.html&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=xuspIDlgBB1uj9BZP_vfNp8KEdzHd_iy1VvBpGe_szM&e= > [10] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__github.com_openstack_congress-2Dtempest- > 2Dplugin_blob_master_congress-5Ftempest- > 5Fplugin_tests_scenario_manager-5Fcongress.py- > 23L85&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=cuEOpCNTzf3TRDkSQjIKkL6cGq6seYBb0ETpSmIM5dM&e= > [11] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__docs.openstack.org_tempest_latest_plugin.html-23get-5Fservice- > 5Fclients-28-29&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru- > SJ9DRnCxhze-aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=4JdQX-W0qSV-T2D99zl9gO6mfZ8KQw8-7GtAq59b9DE&e= > [12] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__review.openstack.org_-23_c_334596_&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=ZTAYLqZ9s4T42S9jUk5bBewSEzGhsDrm76TfVTItGdI&e= > > -gmann > > > > > Thanks, > > > > Felipe > > > > [1] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__bugs.launchpad.net_neutron_-2Bbug_1552960&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=6LxRtrN9LZhGRBXF590Bs6C1wCih14Y6JfM_76Ns30E&e= > > [2] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__docs.openstack.org_tempest_latest_test- > 5Fremoval.html&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru- > SJ9DRnCxhze-aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=qTEukP09Fe68swHryH3J6xCYX0ThWfinmcLKi-SDVLk&e= > > [3] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__review.openstack.org_-23_c_482395_&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=dk1mrXrEQgzUFRx4OvToUouAYN5oi2pqsO6JrwSCMc8&e= > > [4] https://urldefense.proofpoint.com/v2/url?u=https- > 3A__review.openstack.org_-23_c_582340_&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=gnRz4yfHw5DQywEUFkJaIbzTjRKjFMpDwt0wJ8KDGPU&e= > > [5] https://urldefense.proofpoint.com/v2/url?u=http- > 3A__git.openstack.org_cgit_openstack_neutron-2Dtempest- > 2Dplugin_tree_neutron-5Ftempest- > 5Fplugin_services_network_json_network-5Fclient.py&d=DwIGaQ&c=LFYZ- > o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=9uIB2Sj4ia_NxOlVSwBM9aZizpfbCycgS6vN9CpItDM&e= > > [6] https://urldefense.proofpoint.com/v2/url?u=http- > 3A__git.openstack.org_cgit_openstack_neutron-2Dtempest- > 2Dplugin_tree_neutron-5Ftempest-5Fplugin_api_test- > 5Ftimestamp.py&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru- > SJ9DRnCxhze-aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=K2tI5uH8MWQWc73ha7FcZhPZn4yavgstc8kQng6SwRY&e= > > > > > > > ______________________________________________________________ > ____________ > > OpenStack Development Mailing List (not for usage questions) > > Unsubscribe: OpenStack-dev- > requ...@lists.openstack.org?subject:unsubscribe > > https://urldefense.proofpoint.com/v2/url?u=http- > 3A__lists.openstack.org_cgi-2Dbin_mailman_listinfo_openstack- > 2Ddev&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=7qFHngtdEnU2hibbKqDY2DRGMQOIfoaSjYJ0Xrei_tw&e= > > > > > > ______________________________________________________________ > ____________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: OpenStack-dev- > requ...@lists.openstack.org?subject:unsubscribe > https://urldefense.proofpoint.com/v2/url?u=http- > 3A__lists.openstack.org_cgi-2Dbin_mailman_listinfo_openstack- > 2Ddev&d=DwIGaQ&c=LFYZ-o9_HUMeMTSQicvjIg&r=X4GwEru-SJ9DRnCxhze- > aw&m=OvA_eRSYUmntz1eBg1F-r1FDZchsK7u4OtLgezXae- > A&s=7qFHngtdEnU2hibbKqDY2DRGMQOIfoaSjYJ0Xrei_tw&e= __________________________________________________________________________ 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