cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Date Thu, 29 May 2014 08:55:03 GMT
Hiroki,

Did you have a look at 4.4 or master as well? tell us when it works,
so we can cherry-pick the fix to later versions.

On Thu, May 29, 2014 at 10:25 AM, Hiroki Ohashi <hiroki.soum@gmail.com> wrote:
> sebgoa
>
>
> 2014-05-29 16:52 GMT+09:00 sebgoa <runseb@gmail.com>:
>>
>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hiroki.soum@gmail.com> wrote:
>>
>>> Dear guys
>>>
>>> I found a bug about listUsageRecords API in 4.3 related to a issue
>>> CLOUDSTACK-6472 and made a patch. Would you review the patch and
>>> correct me if I am wrong?
>>>
>>> Sam Schmit push a commit about NPEs/Null UUIDs at May 5th but I think
>>> his commit is not enough.
>>>
>>>    commit e8047e11db7ef2bdc44bbedfdc47e63c55f080c5
>>>    Author: Sam Schmit <sam.schmit@appcore.com>
>>>    Date:   Mon May 5 16:38:23 2014 -0500
>>>
>>>        CLOUDSTACK-6472 (4.3 specific) listUsageRecords: Pull
>>> information from removed items as well, fixing NPEs/Null UUIDs with
>>> usage API calls.
>>>
>>>        Signed-off-by: Sebastien Goasguen <runseb@gmail.com>
>>>
>>> When I called listUsageRecords against CloudStack 4.3 management
>>> server that was build at latest 4.3 branch, usage records about
>>> destroyed instances of usage type 1 and 2 lacked 'virtualmachineid' as
>>> below.
>>>
>>>    {
>>>      "account": "sgcadm",
>>>      "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
>>>      "description": "mycluster-front allocated (ServiceOffering: 13)
>>> (Template: 203)",
>>>      "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
>>>      "enddate": "2014-05-20'T'08:59:59+09:00",
>>>      "name": "mycluster-front",
>>>      "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
>>>      "rawusage": "0.223611",
>>>      "startdate": "2014-05-19'T'09:00:00+09:00",
>>>      "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
>>>      "type": "KVM",
>>>      "usage": "0.223611 Hrs",
>>>      "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>      "usagetype": 2,
>>>      "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
>>>    },
>>>
>>> Hence, it is unable to refer to any destroyed instance id. I patched
>>> createUsageResponse method like this.
>>>
>>>    diff --git a/server/src/com/cloud/api/ApiResponseHelper.java
>>> b/server/src/com/cloud/api/ApiResponseHelper.java
>>>    index 7718fc1..cbf3914 100755
>>>    --- a/server/src/com/cloud/api/ApiResponseHelper.java
>>>    +++ b/server/src/com/cloud/api/ApiResponseHelper.java
>>>    @@ -3304,7 +3304,7 @@ public class ApiResponseHelper implements
>>> ResponseGenerator {
>>>             usageRecResponse.setUsage(usageRecord.getUsageDisplay());
>>>             usageRecResponse.setUsageType(usageRecord.getUsageType());
>>>             if (usageRecord.getVmInstanceId() != null) {
>>>    -            VMInstanceVO vm =
>>> _entityMgr.findById(VMInstanceVO.class,
>>> usageRecord.getVmInstanceId());
>>>    +            VMInstanceVO vm =
>>> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
>>> usageRecord.getVmInstanceId());
>>>                 if (vm != null) {
>>>                     usageRecResponse.setVirtualMachineId(vm.getUuid());
>>>                 }
>>>
>>
>> Looks like an omission to me, I went ahead and patched it:
>>
>> commit 24e03bed560feb959a61126b76c2d6971e77092e
>> Author: Sebastien Goasguen <runseb@gmail.com>
>> Date:   Thu May 29 09:48:46 2014 +0200
>>
>>     Fix usage response, patch by Hiroki Ohashi
>>
>> We can always revert if people don't agree.
>>
>> Can you pull the latest 4.3 and try it.
>>
>
> Thanks! I will try it.
>
>> Thanks for the patch. you can always attach a patch at http://reviews.apache.org
>> and follow the instructions at http://cloudstack.apache.org/developers.html
>
> OK, I will follow the instruction next time.
>
>
>>> After that, listUsageRecords API contains 'virtualmachineid' whether
>>> or not instances are destroyed. Here is a result.
>>>
>>>    {
>>>      "account": "sgcadm",
>>>      "accountid": "f9e4e1ca-69fd-4ae3-b70c-15bbcc13406e",
>>>      "description": "mycluster-front allocated (ServiceOffering: 13)
>>> (Template: 203)",
>>>      "domainid": "84dd635d-fb99-4895-b199-7d777aa144d5",
>>>      "enddate": "2014-05-20'T'08:59:59+09:00",
>>>      "name": "mycluster-front",
>>>      "offeringid": "e5137018-8fca-4e4a-a79e-9e4d1707e079",
>>>      "rawusage": "0.223611",
>>>      "startdate": "2014-05-19'T'09:00:00+09:00",
>>>      "templateid": "1ebdc71e-dcbe-4d3e-82d3-d31897f78d4c",
>>>      "type": "KVM",
>>>      "usage": "0.223611 Hrs",
>>>      "usageid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>      "usagetype": 2,
>>>      "virtualmachineid": "09f3d7d6-3c6a-4326-b3d3-fb51506e669d",
>>>      "zoneid": "c2ba1354-0c15-4284-bdba-4f201767362d"
>>>    },
>>>
>>> I think this is an expectecd behaviour, so please review and test this
>>> patch.
>>>
>>>
>>> Best Regards
>>>
>>> --
>>> Hiroki Ohashi
>>
>
>
> Best Regards
>
> --
> Hiroki Ohashi



-- 
Daan

Mime
View raw message