You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stratos.apache.org by Akila Ravihansa Perera <ra...@wso2.com> on 2015/09/10 23:28:40 UTC
Few issues in master branch related to tenant isolation
Hi,
I have been working on uplifting automation engine framework version to
latest (v4.4.3) and migrating the existing test cases. I completed this
task on stratos-4.1.x branch without any issues. But while doing the same
on master branch I noticed that couple of integration tests have been
commented out. When I ran those tests I noticed lot of NPEs in Stratos side
when creating cluster monitors and tenant isolation related tests were
failing too. Not sure whether this is a problem with the integration test.
While going through the test cases I noticed that existing test cases for
super admin user have been altered with tenant users. I think we should
have created separate test cases for tenant isolation rather than modifying
existing ones. Now the problem is we do not know whether super admin flow
is broken with tenant isolation.
I noticed that there are few changes for scaling.drl file between master
branch and stratos-4.1.x branch. We need to check why those two got
deviated [1]. Apparently "delegator.delegateSpawn" method signature has
been changed. Any idea?
I think we should replace integration tests completely from stratos-4.1.x
to master and create a separate test class with only tenant isolation
related assertions. Group scaling, app bursting, termination order, startup
order should be covered in integration tests for super admin, tenant admins
and tenant users, regardless of tenant isolation. wdyt?
[1] https://www.diffnow.com/?report=wqlfa
Thanks.
--
Akila Ravihansa Perera
WSO2 Inc.; http://wso2.com/
Blog: http://ravihansa3000.blogspot.com
Re: Few issues in master branch related to tenant isolation
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
+1. Thanks for pointing out Gayan. It makes sense that we will need to
modify the existing test cases in order to support it..
Thanks,
Reka
On Mon, Sep 14, 2015 at 11:57 AM, Gayan Gunarathne <ga...@wso2.com> wrote:
> Hi Reka,
>
> We can't directly shift the existing test cases for tenants isolation. As
> an example we are checking the status of the application with in a test
> cases. In that case we need to pass the application name + tenant id to
> retrieve the application related to that tenant.We can't directly use the
> application name.
>
> Application application =
>
> ApplicationManager.getApplications().getApplicationByTenant(applicationName,
> tenantId);
>
> So we need to change the test cases according to the tenant isolation.We
> can include some additional test cases for tenancy related stuff.
>
> Thanks,
> Gayan
>
>
> On Mon, Sep 14, 2015 at 11:29 AM, Reka Thirunavukkarasu <re...@wso2.com>
> wrote:
>
>> Hi
>>
>> On Fri, Sep 11, 2015 at 11:07 AM, Gayan Gunarathne <ga...@wso2.com>
>> wrote:
>>
>>> Hi,
>>>
>>> On Fri, Sep 11, 2015 at 2:58 AM, Akila Ravihansa Perera <
>>> ravihansa@wso2.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> I have been working on uplifting automation engine framework version to
>>>> latest (v4.4.3) and migrating the existing test cases. I completed this
>>>> task on stratos-4.1.x branch without any issues. But while doing the same
>>>> on master branch I noticed that couple of integration tests have been
>>>> commented out.
>>>>
>>> When I ran those tests I noticed lot of NPEs in Stratos side when
>>>> creating cluster monitors and tenant isolation related tests were failing
>>>> too. Not sure whether this is a problem with the integration test.
>>>>
>>> I don't see any commented integration tests in master branch[1].What are
>>> the test cases getting failure?
>>>
>>>
>>>> While going through the test cases I noticed that existing test cases
>>>> for super admin user have been altered with tenant users. I think we should
>>>> have created separate test cases for tenant isolation rather than modifying
>>>> existing ones. Now the problem is we do not know whether super admin flow
>>>> is broken with tenant isolation.
>>>>
>>>
>>> There won't be a big difference for single tenant applications . So I
>>> don't see specific reason to maintain the separate test cases for super
>>> admin. For the multi tenant applications, we can have separate test cases
>>> for super admin and tenant users.
>>>
>>>
>>>>
>>>> I noticed that there are few changes for scaling.drl file between
>>>> master branch and stratos-4.1.x branch. We need to check why those two got
>>>> deviated [1]. Apparently "delegator.delegateSpawn" method signature has
>>>> been changed. Any idea?
>>>>
>>>
>>> This has been changed for the publish health statistics related date
>>> publishing purposes.Those changes are also merged to the master branch also.
>>> @Thanuja, Can you please confirm those
>>>
>>> I think we should replace integration tests completely from
>>>> stratos-4.1.x to master and create a separate test class with only tenant
>>>> isolation related assertions. Group scaling, app bursting, termination
>>>> order, startup order should be covered in integration tests for super
>>>> admin, tenant admins and tenant users, regardless of tenant isolation. wdyt?
>>>>
>>> For multi tenant applications IMO also we may need to introduced
>>> seperate test cases for super admins and tenant. But for single tenant we
>>> may don't need a separate test case. Also please note all the servers
>>> running in the super admin space for all the tenant users.
>>>
>>
>> IMO, we don't need to alter the existing individual test cases. Rather we
>> can let super admin and tenant users to execute these test cases again in
>> their own space. In addition to that, we can write some tenancy related
>> test cases to verify the tenant isolation separately. In that way, we can
>> add more stuffs and it could be extensible as well. WDYT?
>>
>> Thanks,
>> Reka
>>
>>
>>>
>>>> [1] https://www.diffnow.com/?report=wqlfa
>>>>
>>>> Thanks.
>>>>
>>>
>>> [1]
>>> https://github.com/apache/stratos/blob/master/products/stratos/modules/integration/src/test/resources/stratos-testing.xml
>>>
>>>
>>>>
>>>> --
>>>> Akila Ravihansa Perera
>>>> WSO2 Inc.; http://wso2.com/
>>>>
>>>> Blog: http://ravihansa3000.blogspot.com
>>>>
>>>
>>>
>>>
>>> --
>>>
>>> Gayan Gunarathne
>>> Technical Lead, WSO2 Inc. (http://wso2.com)
>>> Committer & PMC Member, Apache Stratos
>>> email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>
>>>
>>>
>>>
>>
>>
>>
>> --
>> Reka Thirunavukkarasu
>> Senior Software Engineer,
>> WSO2, Inc.:http://wso2.com,
>> Mobile: +94776442007
>>
>>
>>
>
>
> --
>
> Gayan Gunarathne
> Technical Lead, WSO2 Inc. (http://wso2.com)
> Committer & PMC Member, Apache Stratos
> email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>
>
>
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007
Re: Few issues in master branch related to tenant isolation
Posted by Gayan Gunarathne <ga...@wso2.com>.
Hi Reka,
We can't directly shift the existing test cases for tenants isolation. As
an example we are checking the status of the application with in a test
cases. In that case we need to pass the application name + tenant id to
retrieve the application related to that tenant.We can't directly use the
application name.
Application application =
ApplicationManager.getApplications().getApplicationByTenant(applicationName,
tenantId);
So we need to change the test cases according to the tenant isolation.We
can include some additional test cases for tenancy related stuff.
Thanks,
Gayan
On Mon, Sep 14, 2015 at 11:29 AM, Reka Thirunavukkarasu <re...@wso2.com>
wrote:
> Hi
>
> On Fri, Sep 11, 2015 at 11:07 AM, Gayan Gunarathne <ga...@wso2.com>
> wrote:
>
>> Hi,
>>
>> On Fri, Sep 11, 2015 at 2:58 AM, Akila Ravihansa Perera <
>> ravihansa@wso2.com> wrote:
>>
>>> Hi,
>>>
>>> I have been working on uplifting automation engine framework version to
>>> latest (v4.4.3) and migrating the existing test cases. I completed this
>>> task on stratos-4.1.x branch without any issues. But while doing the same
>>> on master branch I noticed that couple of integration tests have been
>>> commented out.
>>>
>> When I ran those tests I noticed lot of NPEs in Stratos side when
>>> creating cluster monitors and tenant isolation related tests were failing
>>> too. Not sure whether this is a problem with the integration test.
>>>
>> I don't see any commented integration tests in master branch[1].What are
>> the test cases getting failure?
>>
>>
>>> While going through the test cases I noticed that existing test cases
>>> for super admin user have been altered with tenant users. I think we should
>>> have created separate test cases for tenant isolation rather than modifying
>>> existing ones. Now the problem is we do not know whether super admin flow
>>> is broken with tenant isolation.
>>>
>>
>> There won't be a big difference for single tenant applications . So I
>> don't see specific reason to maintain the separate test cases for super
>> admin. For the multi tenant applications, we can have separate test cases
>> for super admin and tenant users.
>>
>>
>>>
>>> I noticed that there are few changes for scaling.drl file between master
>>> branch and stratos-4.1.x branch. We need to check why those two got
>>> deviated [1]. Apparently "delegator.delegateSpawn" method signature has
>>> been changed. Any idea?
>>>
>>
>> This has been changed for the publish health statistics related date
>> publishing purposes.Those changes are also merged to the master branch also.
>> @Thanuja, Can you please confirm those
>>
>> I think we should replace integration tests completely from stratos-4.1.x
>>> to master and create a separate test class with only tenant isolation
>>> related assertions. Group scaling, app bursting, termination order, startup
>>> order should be covered in integration tests for super admin, tenant admins
>>> and tenant users, regardless of tenant isolation. wdyt?
>>>
>> For multi tenant applications IMO also we may need to introduced seperate
>> test cases for super admins and tenant. But for single tenant we may don't
>> need a separate test case. Also please note all the servers running in the
>> super admin space for all the tenant users.
>>
>
> IMO, we don't need to alter the existing individual test cases. Rather we
> can let super admin and tenant users to execute these test cases again in
> their own space. In addition to that, we can write some tenancy related
> test cases to verify the tenant isolation separately. In that way, we can
> add more stuffs and it could be extensible as well. WDYT?
>
> Thanks,
> Reka
>
>
>>
>>> [1] https://www.diffnow.com/?report=wqlfa
>>>
>>> Thanks.
>>>
>>
>> [1]
>> https://github.com/apache/stratos/blob/master/products/stratos/modules/integration/src/test/resources/stratos-testing.xml
>>
>>
>>>
>>> --
>>> Akila Ravihansa Perera
>>> WSO2 Inc.; http://wso2.com/
>>>
>>> Blog: http://ravihansa3000.blogspot.com
>>>
>>
>>
>>
>> --
>>
>> Gayan Gunarathne
>> Technical Lead, WSO2 Inc. (http://wso2.com)
>> Committer & PMC Member, Apache Stratos
>> email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>
>>
>>
>>
>
>
>
> --
> Reka Thirunavukkarasu
> Senior Software Engineer,
> WSO2, Inc.:http://wso2.com,
> Mobile: +94776442007
>
>
>
--
Gayan Gunarathne
Technical Lead, WSO2 Inc. (http://wso2.com)
Committer & PMC Member, Apache Stratos
email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>
Re: Few issues in master branch related to tenant isolation
Posted by Reka Thirunavukkarasu <re...@wso2.com>.
Hi
On Fri, Sep 11, 2015 at 11:07 AM, Gayan Gunarathne <ga...@wso2.com> wrote:
> Hi,
>
> On Fri, Sep 11, 2015 at 2:58 AM, Akila Ravihansa Perera <
> ravihansa@wso2.com> wrote:
>
>> Hi,
>>
>> I have been working on uplifting automation engine framework version to
>> latest (v4.4.3) and migrating the existing test cases. I completed this
>> task on stratos-4.1.x branch without any issues. But while doing the same
>> on master branch I noticed that couple of integration tests have been
>> commented out.
>>
> When I ran those tests I noticed lot of NPEs in Stratos side when creating
>> cluster monitors and tenant isolation related tests were failing too. Not
>> sure whether this is a problem with the integration test.
>>
> I don't see any commented integration tests in master branch[1].What are
> the test cases getting failure?
>
>
>> While going through the test cases I noticed that existing test cases for
>> super admin user have been altered with tenant users. I think we should
>> have created separate test cases for tenant isolation rather than modifying
>> existing ones. Now the problem is we do not know whether super admin flow
>> is broken with tenant isolation.
>>
>
> There won't be a big difference for single tenant applications . So I
> don't see specific reason to maintain the separate test cases for super
> admin. For the multi tenant applications, we can have separate test cases
> for super admin and tenant users.
>
>
>>
>> I noticed that there are few changes for scaling.drl file between master
>> branch and stratos-4.1.x branch. We need to check why those two got
>> deviated [1]. Apparently "delegator.delegateSpawn" method signature has
>> been changed. Any idea?
>>
>
> This has been changed for the publish health statistics related date
> publishing purposes.Those changes are also merged to the master branch also.
> @Thanuja, Can you please confirm those
>
> I think we should replace integration tests completely from stratos-4.1.x
>> to master and create a separate test class with only tenant isolation
>> related assertions. Group scaling, app bursting, termination order, startup
>> order should be covered in integration tests for super admin, tenant admins
>> and tenant users, regardless of tenant isolation. wdyt?
>>
> For multi tenant applications IMO also we may need to introduced seperate
> test cases for super admins and tenant. But for single tenant we may don't
> need a separate test case. Also please note all the servers running in the
> super admin space for all the tenant users.
>
IMO, we don't need to alter the existing individual test cases. Rather we
can let super admin and tenant users to execute these test cases again in
their own space. In addition to that, we can write some tenancy related
test cases to verify the tenant isolation separately. In that way, we can
add more stuffs and it could be extensible as well. WDYT?
Thanks,
Reka
>
>> [1] https://www.diffnow.com/?report=wqlfa
>>
>> Thanks.
>>
>
> [1]
> https://github.com/apache/stratos/blob/master/products/stratos/modules/integration/src/test/resources/stratos-testing.xml
>
>
>>
>> --
>> Akila Ravihansa Perera
>> WSO2 Inc.; http://wso2.com/
>>
>> Blog: http://ravihansa3000.blogspot.com
>>
>
>
>
> --
>
> Gayan Gunarathne
> Technical Lead, WSO2 Inc. (http://wso2.com)
> Committer & PMC Member, Apache Stratos
> email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>
>
>
>
--
Reka Thirunavukkarasu
Senior Software Engineer,
WSO2, Inc.:http://wso2.com,
Mobile: +94776442007
Re: Few issues in master branch related to tenant isolation
Posted by Gayan Gunarathne <ga...@wso2.com>.
Hi,
On Fri, Sep 11, 2015 at 2:58 AM, Akila Ravihansa Perera <ra...@wso2.com>
wrote:
> Hi,
>
> I have been working on uplifting automation engine framework version to
> latest (v4.4.3) and migrating the existing test cases. I completed this
> task on stratos-4.1.x branch without any issues. But while doing the same
> on master branch I noticed that couple of integration tests have been
> commented out.
>
When I ran those tests I noticed lot of NPEs in Stratos side when creating
> cluster monitors and tenant isolation related tests were failing too. Not
> sure whether this is a problem with the integration test.
>
I don't see any commented integration tests in master branch[1].What are
the test cases getting failure?
> While going through the test cases I noticed that existing test cases for
> super admin user have been altered with tenant users. I think we should
> have created separate test cases for tenant isolation rather than modifying
> existing ones. Now the problem is we do not know whether super admin flow
> is broken with tenant isolation.
>
There won't be a big difference for single tenant applications . So I don't
see specific reason to maintain the separate test cases for super admin.
For the multi tenant applications, we can have separate test cases for
super admin and tenant users.
>
> I noticed that there are few changes for scaling.drl file between master
> branch and stratos-4.1.x branch. We need to check why those two got
> deviated [1]. Apparently "delegator.delegateSpawn" method signature has
> been changed. Any idea?
>
This has been changed for the publish health statistics related date
publishing purposes.Those changes are also merged to the master branch also.
@Thanuja, Can you please confirm those
I think we should replace integration tests completely from stratos-4.1.x
> to master and create a separate test class with only tenant isolation
> related assertions. Group scaling, app bursting, termination order, startup
> order should be covered in integration tests for super admin, tenant admins
> and tenant users, regardless of tenant isolation. wdyt?
>
For multi tenant applications IMO also we may need to introduced seperate
test cases for super admins and tenant. But for single tenant we may don't
need a separate test case. Also please note all the servers running in the
super admin space for all the tenant users.
>
> [1] https://www.diffnow.com/?report=wqlfa
>
> Thanks.
>
[1]
https://github.com/apache/stratos/blob/master/products/stratos/modules/integration/src/test/resources/stratos-testing.xml
>
> --
> Akila Ravihansa Perera
> WSO2 Inc.; http://wso2.com/
>
> Blog: http://ravihansa3000.blogspot.com
>
--
Gayan Gunarathne
Technical Lead, WSO2 Inc. (http://wso2.com)
Committer & PMC Member, Apache Stratos
email : gayang@wso2.com | mobile : +94 775030545 <%2B94%20766819985>