You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Hiroki Ohashi <hi...@gmail.com> on 2014/05/29 09:26:55 UTC
[ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
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 <sa...@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 <ru...@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());
}
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
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by Daan Hoogland <da...@gmail.com>.
H Hiroki,
those calls are for ServiceOfferingVO not for VMInstanceVO. So they
are not the same issue but that doesn't mean that we shouldn't those
the be counted when deleted either.
On Thu, May 29, 2014 at 12:11 PM, Hiroki Ohashi <hi...@gmail.com> wrote:
> Hi Daan, Rajani
>
> I looked at a code of master and 4.4 branch. findById is used at line
> 3031 of master branch and line 3299 of 4.4 branch, so I think this
> leads to an same problem.
>
> The code snippets of master and 4.4 branch are below.
>
> [master] server/src/com/cloud/api/ApiResponseHelper.java
>
> 2987 @Override
> 2988 public UsageRecordResponse createUsageResponse(Usage usageRecord) {
> 2989 UsageRecordResponse usageRecResponse = new
> UsageRecordResponse();
> 2990
> 2991 Account account =
> ApiDBUtils.findAccountById(usageRecord.getAccountId());
> 2992 if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
> 2993 //find the project
> 2994 Project project =
> ApiDBUtils.findProjectByProjectAccountId(account.getId());
> 2995 usageRecResponse.setProjectId(project.getUuid());
> 2996 usageRecResponse.setProjectName(project.getName());
> 2997 } else {
> 2998 usageRecResponse.setAccountId(account.getUuid());
> 2999 usageRecResponse.setAccountName(account.getAccountName());
> 3000 }
> 3001
> 3002 Domain domain =
> ApiDBUtils.findDomainById(usageRecord.getDomainId());
> 3003 if (domain != null) {
> 3004 usageRecResponse.setDomainId(domain.getUuid());
> 3005 }
> 3006
> 3007 if (usageRecord.getZoneId() != null) {
> 3008 DataCenter zone =
> ApiDBUtils.findZoneById(usageRecord.getZoneId());
> 3009 if (zone != null) {
> 3010 usageRecResponse.setZoneId(zone.getUuid());
> 3011 }
> 3012 }
> 3013 usageRecResponse.setDescription(usageRecord.getDescription());
> 3014 usageRecResponse.setUsage(usageRecord.getUsageDisplay());
> 3015 usageRecResponse.setUsageType(usageRecord.getUsageType());
> 3016 if (usageRecord.getVmInstanceId() != null) {
> 3017 VMInstanceVO vm =
> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
> usageRecord.getVmInstanceId());
> 3018 if (vm != null) {
> 3019 usageRecResponse.setVirtualMachineId(vm.getUuid());
> 3020 }
> 3021 }
> 3022 usageRecResponse.setVmName(usageRecord.getVmName());
> 3023 if (usageRecord.getTemplateId() != null) {
> 3024 VMTemplateVO template =
> ApiDBUtils.findTemplateById(usageRecord.getTemplateId());
> 3025 if (template != null) {
> 3026 usageRecResponse.setTemplateId(template.getUuid());
> 3027 }
> 3028 }
> 3029
> 3030 if (usageRecord.getUsageType() ==
> UsageTypes.RUNNING_VM || usageRecord.getUsageType() ==
> UsageTypes.ALLOCATED_VM) {
> 3031 ServiceOfferingVO svcOffering =
> _entityMgr.findById(ServiceOfferingVO.class,
> usageRecord.getOfferingId().toString());
> 3032 //Service Offering Id
> 3033 usageRecResponse.setOfferingId(svcOffering.getUuid());
> 3034 //VM Instance ID
> 3035 VMInstanceVO vm =
> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
> usageRecord.getUsageId().toString());
> 3036 if (vm != null) {
> 3037 usageRecResponse.setUsageId(vm.getUuid());
> 3038 }
> 3039 //Hypervisor Type
> 3040 usageRecResponse.setType(usageRecord.getType());
> 3041
>
> [ACS 4.4] server/src/com/cloud/api/ApiResponseHelper.java
>
> 3255 @Override
> 3256 public UsageRecordResponse createUsageResponse(Usage usageRecord) {
> 3257 UsageRecordResponse usageRecResponse = new
> UsageRecordResponse();
> 3258
> 3259 Account account =
> ApiDBUtils.findAccountById(usageRecord.getAccountId());
> 3260 if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
> 3261 //find the project
> 3262 Project project =
> ApiDBUtils.findProjectByProjectAccountId(account.getId());
> 3263 usageRecResponse.setProjectId(project.getUuid());
> 3264 usageRecResponse.setProjectName(project.getName());
> 3265 } else {
> 3266 usageRecResponse.setAccountId(account.getUuid());
> 3267 usageRecResponse.setAccountName(account.getAccountName());
> 3268 }
> 3269
> 3270 Domain domain =
> ApiDBUtils.findDomainById(usageRecord.getDomainId());
> 3271 if (domain != null) {
> 3272 usageRecResponse.setDomainId(domain.getUuid());
> 3273 }
> 3274
> 3275 if (usageRecord.getZoneId() != null) {
> 3276 DataCenter zone =
> ApiDBUtils.findZoneById(usageRecord.getZoneId());
> 3277 if (zone != null) {
> 3278 usageRecResponse.setZoneId(zone.getUuid());
> 3279 }
> 3280 }
> 3281 usageRecResponse.setDescription(usageRecord.getDescription());
> 3282 usageRecResponse.setUsage(usageRecord.getUsageDisplay());
> 3283 usageRecResponse.setUsageType(usageRecord.getUsageType());
> 3284 if (usageRecord.getVmInstanceId() != null) {
> 3285 VMInstanceVO vm =
> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
> usageRecord.getVmInstanceId());
> 3286 if (vm != null) {
> 3287 usageRecResponse.setVirtualMachineId(vm.getUuid());
> 3288 }
> 3289 }
> 3290 usageRecResponse.setVmName(usageRecord.getVmName());
> 3291 if (usageRecord.getTemplateId() != null) {
> 3292 VMTemplateVO template =
> ApiDBUtils.findTemplateById(usageRecord.getTemplateId());
> 3293 if (template != null) {
> 3294 usageRecResponse.setTemplateId(template.getUuid());
> 3295 }
> 3296 }
> 3297
> 3298 if (usageRecord.getUsageType() ==
> UsageTypes.RUNNING_VM || usageRecord.getUsageType() ==
> UsageTypes.ALLOCATED_VM) {
> 3299 ServiceOfferingVO svcOffering =
> _entityMgr.findById(ServiceOfferingVO.class,
> usageRecord.getOfferingId().toString());
> 3300 //Service Offering Id
> 3301 usageRecResponse.setOfferingId(svcOffering.getUuid());
> 3302 //VM Instance ID
> 3303 VMInstanceVO vm =
> _entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
> usageRecord.getUsageId().toString());
> 3304 if (vm != null) {
> 3305 usageRecResponse.setUsageId(vm.getUuid());
> 3306 }
> 3307 //Hypervisor Type
> 3308 usageRecResponse.setType(usageRecord.getType());
> 3309
>
> In addition to this, I'm not sure whether findAccountById,
> findProjectByProjectAccountId, findDomainById, findZoneById and
> findTemplateById work correctly when resources are destroyed. Does
> anyone know these methods are OK?
>
>
> 2014-05-29 18:25 GMT+09:00 sebgoa <ru...@gmail.com>:
>>
>> On May 29, 2014, at 11:20 AM, Rajani Karuturi <Ra...@citrix.com> wrote:
>>
>>> The fix is already in 4.4
>>>
>>> commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
>>> Author: Sam Schmit <sa...@appcore.com>
>>> AuthorDate: Fri May 2 13:07:34 2014 -0500
>>> Commit: Daan Hoogland <da...@onecht.net>
>>> 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
>>>
>>
>> still need to check if this fix in 4.4 is complete. Hiroki's change was to 4.3 branch. I did not check 4.4.
>>
>>>
>>> @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.
>>>
>>
>> yes my bad, did not take time to look up the bug id.
>>
>>>
>>> ~Rajani
>>>
>>>
>>>
>>> On 29-May-2014, at 2:25 pm, Daan Hoogland <da...@gmail.com> 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 <hi...@gmail.com> wrote:
>>>>> sebgoa
>>>>>
>>>>>
>>>>> 2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>>>>>>
>>>>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
>>>
>>
>
>
> Best Regards
>
> --
> Hiroki Ohashi
--
Daan
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by Hiroki Ohashi <hi...@gmail.com>.
Hi Daan, Rajani
I looked at a code of master and 4.4 branch. findById is used at line
3031 of master branch and line 3299 of 4.4 branch, so I think this
leads to an same problem.
The code snippets of master and 4.4 branch are below.
[master] server/src/com/cloud/api/ApiResponseHelper.java
2987 @Override
2988 public UsageRecordResponse createUsageResponse(Usage usageRecord) {
2989 UsageRecordResponse usageRecResponse = new
UsageRecordResponse();
2990
2991 Account account =
ApiDBUtils.findAccountById(usageRecord.getAccountId());
2992 if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
2993 //find the project
2994 Project project =
ApiDBUtils.findProjectByProjectAccountId(account.getId());
2995 usageRecResponse.setProjectId(project.getUuid());
2996 usageRecResponse.setProjectName(project.getName());
2997 } else {
2998 usageRecResponse.setAccountId(account.getUuid());
2999 usageRecResponse.setAccountName(account.getAccountName());
3000 }
3001
3002 Domain domain =
ApiDBUtils.findDomainById(usageRecord.getDomainId());
3003 if (domain != null) {
3004 usageRecResponse.setDomainId(domain.getUuid());
3005 }
3006
3007 if (usageRecord.getZoneId() != null) {
3008 DataCenter zone =
ApiDBUtils.findZoneById(usageRecord.getZoneId());
3009 if (zone != null) {
3010 usageRecResponse.setZoneId(zone.getUuid());
3011 }
3012 }
3013 usageRecResponse.setDescription(usageRecord.getDescription());
3014 usageRecResponse.setUsage(usageRecord.getUsageDisplay());
3015 usageRecResponse.setUsageType(usageRecord.getUsageType());
3016 if (usageRecord.getVmInstanceId() != null) {
3017 VMInstanceVO vm =
_entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
usageRecord.getVmInstanceId());
3018 if (vm != null) {
3019 usageRecResponse.setVirtualMachineId(vm.getUuid());
3020 }
3021 }
3022 usageRecResponse.setVmName(usageRecord.getVmName());
3023 if (usageRecord.getTemplateId() != null) {
3024 VMTemplateVO template =
ApiDBUtils.findTemplateById(usageRecord.getTemplateId());
3025 if (template != null) {
3026 usageRecResponse.setTemplateId(template.getUuid());
3027 }
3028 }
3029
3030 if (usageRecord.getUsageType() ==
UsageTypes.RUNNING_VM || usageRecord.getUsageType() ==
UsageTypes.ALLOCATED_VM) {
3031 ServiceOfferingVO svcOffering =
_entityMgr.findById(ServiceOfferingVO.class,
usageRecord.getOfferingId().toString());
3032 //Service Offering Id
3033 usageRecResponse.setOfferingId(svcOffering.getUuid());
3034 //VM Instance ID
3035 VMInstanceVO vm =
_entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
usageRecord.getUsageId().toString());
3036 if (vm != null) {
3037 usageRecResponse.setUsageId(vm.getUuid());
3038 }
3039 //Hypervisor Type
3040 usageRecResponse.setType(usageRecord.getType());
3041
[ACS 4.4] server/src/com/cloud/api/ApiResponseHelper.java
3255 @Override
3256 public UsageRecordResponse createUsageResponse(Usage usageRecord) {
3257 UsageRecordResponse usageRecResponse = new
UsageRecordResponse();
3258
3259 Account account =
ApiDBUtils.findAccountById(usageRecord.getAccountId());
3260 if (account.getType() == Account.ACCOUNT_TYPE_PROJECT) {
3261 //find the project
3262 Project project =
ApiDBUtils.findProjectByProjectAccountId(account.getId());
3263 usageRecResponse.setProjectId(project.getUuid());
3264 usageRecResponse.setProjectName(project.getName());
3265 } else {
3266 usageRecResponse.setAccountId(account.getUuid());
3267 usageRecResponse.setAccountName(account.getAccountName());
3268 }
3269
3270 Domain domain =
ApiDBUtils.findDomainById(usageRecord.getDomainId());
3271 if (domain != null) {
3272 usageRecResponse.setDomainId(domain.getUuid());
3273 }
3274
3275 if (usageRecord.getZoneId() != null) {
3276 DataCenter zone =
ApiDBUtils.findZoneById(usageRecord.getZoneId());
3277 if (zone != null) {
3278 usageRecResponse.setZoneId(zone.getUuid());
3279 }
3280 }
3281 usageRecResponse.setDescription(usageRecord.getDescription());
3282 usageRecResponse.setUsage(usageRecord.getUsageDisplay());
3283 usageRecResponse.setUsageType(usageRecord.getUsageType());
3284 if (usageRecord.getVmInstanceId() != null) {
3285 VMInstanceVO vm =
_entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
usageRecord.getVmInstanceId());
3286 if (vm != null) {
3287 usageRecResponse.setVirtualMachineId(vm.getUuid());
3288 }
3289 }
3290 usageRecResponse.setVmName(usageRecord.getVmName());
3291 if (usageRecord.getTemplateId() != null) {
3292 VMTemplateVO template =
ApiDBUtils.findTemplateById(usageRecord.getTemplateId());
3293 if (template != null) {
3294 usageRecResponse.setTemplateId(template.getUuid());
3295 }
3296 }
3297
3298 if (usageRecord.getUsageType() ==
UsageTypes.RUNNING_VM || usageRecord.getUsageType() ==
UsageTypes.ALLOCATED_VM) {
3299 ServiceOfferingVO svcOffering =
_entityMgr.findById(ServiceOfferingVO.class,
usageRecord.getOfferingId().toString());
3300 //Service Offering Id
3301 usageRecResponse.setOfferingId(svcOffering.getUuid());
3302 //VM Instance ID
3303 VMInstanceVO vm =
_entityMgr.findByIdIncludingRemoved(VMInstanceVO.class,
usageRecord.getUsageId().toString());
3304 if (vm != null) {
3305 usageRecResponse.setUsageId(vm.getUuid());
3306 }
3307 //Hypervisor Type
3308 usageRecResponse.setType(usageRecord.getType());
3309
In addition to this, I'm not sure whether findAccountById,
findProjectByProjectAccountId, findDomainById, findZoneById and
findTemplateById work correctly when resources are destroyed. Does
anyone know these methods are OK?
2014-05-29 18:25 GMT+09:00 sebgoa <ru...@gmail.com>:
>
> On May 29, 2014, at 11:20 AM, Rajani Karuturi <Ra...@citrix.com> wrote:
>
>> The fix is already in 4.4
>>
>> commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
>> Author: Sam Schmit <sa...@appcore.com>
>> AuthorDate: Fri May 2 13:07:34 2014 -0500
>> Commit: Daan Hoogland <da...@onecht.net>
>> 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
>>
>
> still need to check if this fix in 4.4 is complete. Hiroki's change was to 4.3 branch. I did not check 4.4.
>
>>
>> @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.
>>
>
> yes my bad, did not take time to look up the bug id.
>
>>
>> ~Rajani
>>
>>
>>
>> On 29-May-2014, at 2:25 pm, Daan Hoogland <da...@gmail.com> 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 <hi...@gmail.com> wrote:
>>>> sebgoa
>>>>
>>>>
>>>> 2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>>>>>
>>>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
>>
>
Best Regards
--
Hiroki Ohashi
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by sebgoa <ru...@gmail.com>.
On May 29, 2014, at 11:20 AM, Rajani Karuturi <Ra...@citrix.com> wrote:
> The fix is already in 4.4
>
> commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
> Author: Sam Schmit <sa...@appcore.com>
> AuthorDate: Fri May 2 13:07:34 2014 -0500
> Commit: Daan Hoogland <da...@onecht.net>
> 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
>
still need to check if this fix in 4.4 is complete. Hiroki's change was to 4.3 branch. I did not check 4.4.
>
> @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.
>
yes my bad, did not take time to look up the bug id.
>
> ~Rajani
>
>
>
> On 29-May-2014, at 2:25 pm, Daan Hoogland <da...@gmail.com> 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 <hi...@gmail.com> wrote:
>>> sebgoa
>>>
>>>
>>> 2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>>>>
>>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
>
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by Rajani Karuturi <Ra...@citrix.com>.
The fix is already in 4.4
commit 3a3457e7132d22f52aa38179d40a6eb9b0b29677
Author: Sam Schmit <sa...@appcore.com>
AuthorDate: Fri May 2 13:07:34 2014 -0500
Commit: Daan Hoogland <da...@onecht.net>
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 <da...@gmail.com> 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 <hi...@gmail.com> wrote:
>> sebgoa
>>
>>
>> 2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>>>
>>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by Daan Hoogland <da...@gmail.com>.
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 <hi...@gmail.com> wrote:
> sebgoa
>
>
> 2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>>
>> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by Hiroki Ohashi <hi...@gmail.com>.
sebgoa
2014-05-29 16:52 GMT+09:00 sebgoa <ru...@gmail.com>:
>
> On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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
Re: [ACS 4.3] listUsageRecords API bug related to CLOUDSTACK-6472
Posted by sebgoa <ru...@gmail.com>.
On May 29, 2014, at 9:26 AM, Hiroki Ohashi <hi...@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 <sa...@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 <ru...@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 <ru...@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 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
> 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