The fix is already in 4.4
commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
Author: Sam Schmit <[email protected]>
AuthorDate: Fri May 2 13:07:34 2014 -0500
Commit: Daan Hoogland <[email protected]>
CommitDate: Tue May 6 17:46:20 2014 +0200
CLOUDSTACK-6472 listUsageRecords: Pull information from removed items as
well, fixing NPEs/Null UUIDs with usage API calls.
M server/src/com/cloud/api/ApiResponseHelper.java
@sebgoa
i think it would be better to include bug id in the commit messages as thats
the only way to track all the commits for a defect.
~Rajani
On 29-May-2014, at 2:25 pm, Daan Hoogland <[email protected]> wrote:
> 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 <[email protected]> wrote:
>> sebgoa
>>
>>
>> 2014-05-29 16:52 GMT+09:00 sebgoa <[email protected]>:
>>>
>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <[email protected]> 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 <[email protected]>
>>>> 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 <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>> 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
