Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix active_provision quota check for infra VM request with invalid vm_template. #17158

Merged
merged 1 commit into from
Mar 20, 2018

Conversation

tinaafitz
Copy link
Member

Quota uses 'active' MiqRequests in its calculations for active provisions. Previously, quota used the MiqQueue entries to calculate active provisions. The MiqRequest approach is much more accurate, but is susceptible to old and/or invalid MiqRequests that appear to be 'active'.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1554989

Check for valid request.vm_template for VM infra provisioned storage calculations to avoid the following error caused by an invalid MIqRequest.

task_id([miq_provision_6000000002384]) MiqAeServiceModelBase.ar_method raised: : <undefined method provisioned_storage' for nil:NilClass [----] E, [2018-03-13T05:06:49.201944 #19065:d0efbc] ERROR -- : Q-task_id([miq_provision_6000000002384]) <AutomationEngine> /var/www/miq/vmdb/app/models/mixins/miq_provision_quota_mixin.rb:378:in storage'
[----] E, [2018-03-13T05:06:49.204481 #19065:6749c70] ERROR -- : Q-task_id([miq_provision_6000000002384]) The following error occurred during method evaluation:
[----] E, [2018-03-13T05:06:49.205527 #19065:6749c70] ERROR -- : Q-task_id([miq_provision_6000000002384]) NoMethodError: undefined method `provisioned_storage' for nil:NilClass
[----] E, [2018-03-13T05:06:49.206780 #19065:6749c70] ERROR -- : Q-task_id([miq_provision_6000000002384]) (druby://127.0.0.1:34153) /var/www/miq/vmdb/app/models/mixins/miq_provis

@tinaafitz
Copy link
Member Author

@miq-bot add_label bug
@miq-bot assign @gmcculloug
@mkanoor please review

end

def infra_storage(request)
if request.respond_to?(:vm_template) && request.vm_template.present?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use request.try(:vm_template).present?

@tinaafitz tinaafitz force-pushed the fix_quota_invalid_template branch from 5f558bd to 786acfa Compare March 15, 2018 12:45
end

def infra_storage(request)
if request.try(:vm_template).present?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into this more vm_template is just an alias for source app/models/miq_provision_request.rb#L2 which is the same problem we have seen on Service Requests when the Service Template has been removed.

It would be better if we could remove all requests that do not have a valid source anymore instead of dealing with it at this lower level.

end
end

def infra_storage(request)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, could you use verb names for methods that do stuff rather than nouns.
(sorry a nit but makes a difference for future developers)

@tinaafitz tinaafitz force-pushed the fix_quota_invalid_template branch from 786acfa to 36f688d Compare March 20, 2018 16:09
@tinaafitz tinaafitz force-pushed the fix_quota_invalid_template branch from 36f688d to 0fa6f8b Compare March 20, 2018 16:22
@miq-bot
Copy link
Member

miq-bot commented Mar 20, 2018

Checked commit tinaafitz@0fa6f8b with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@gmcculloug
Copy link
Member

@kbrock Thanks for you help on this one. 👍

@gmcculloug gmcculloug merged commit 9002fc7 into ManageIQ:master Mar 20, 2018
@gmcculloug gmcculloug added this to the Sprint 82 Ending Mar 26, 2018 milestone Mar 20, 2018
simaishi pushed a commit that referenced this pull request Mar 22, 2018
Fix active_provision quota check for infra VM request with invalid vm_template.
(cherry picked from commit 9002fc7)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1559550
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit f6e92211bed0c549db38dbda1fe9db2edaa9fd44
Author: Greg McCullough <[email protected]>
Date:   Tue Mar 20 16:06:23 2018 -0400

    Merge pull request #17158 from tinaafitz/fix_quota_invalid_template
    
    Fix active_provision quota check for infra VM request with invalid vm_template.
    (cherry picked from commit 9002fc741b6b5dda17885dccb135f370516213b8)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1559550

simaishi pushed a commit that referenced this pull request Apr 9, 2018
Fix active_provision quota check for infra VM request with invalid vm_template.
(cherry picked from commit 9002fc7)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1559551
@simaishi
Copy link
Contributor

simaishi commented Apr 9, 2018

Fine backport details:

$ git log -1
commit 9cac95b3f51cd6d9800d28da93d9c2f8fc011bb8
Author: Greg McCullough <[email protected]>
Date:   Tue Mar 20 16:06:23 2018 -0400

    Merge pull request #17158 from tinaafitz/fix_quota_invalid_template
    
    Fix active_provision quota check for infra VM request with invalid vm_template.
    (cherry picked from commit 9002fc741b6b5dda17885dccb135f370516213b8)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1559551

d-m-u pushed a commit to d-m-u/manageiq that referenced this pull request Jun 6, 2018
…mplate

Fix active_provision quota check for infra VM request with invalid vm_template.
(cherry picked from commit 9002fc7)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1559551

it "invalid vm_template does not raise error" do
create_requests
MiqRequest.first.update_attributes(:vm_template => nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first request will now have no source (aliased to the template), which we have a presence validation on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants