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

Spec test for excluding orphaned vms from quota. #132

Merged
merged 2 commits into from
Jun 27, 2017

Conversation

billfitzgerald0120
Copy link
Contributor

@billfitzgerald0120 billfitzgerald0120 commented Jun 22, 2017

Modified spec test count to reflect change made by branic to exclude VMs with no EMS in the Quota used calculation.

I have included branic's change in this PR.

#129

@billfitzgerald0120
Copy link
Contributor Author

@miq-bot add_label enhancement

@billfitzgerald0120
Copy link
Contributor Author

@tinaafitz Please Review

@miq-bot
Copy link
Member

miq-bot commented Jun 22, 2017

Checked commits billfitzgerald0120/manageiq-content@4543f41~...7c090fd with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@billfitzgerald0120
Copy link
Contributor Author

@miq-bot assign @gmcculloug

Copy link
Member

@tinaafitz tinaafitz left a comment

Choose a reason for hiding this comment

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

@billfitzgerald0120 Looks good.
@gmcculloug Please review.

@branic
Copy link
Contributor

branic commented Jun 26, 2017

@billfitzgerald0120 @tinaafitz @gmcculloug I'm wondering if retired VMs should also be excluded from the count. The quota returned for the other objects do no include retired VMs.

@mkanoor mkanoor self-requested a review June 27, 2017 15:46
@mkanoor mkanoor merged commit ee6a58c into ManageIQ:master Jun 27, 2017
@tinaafitz
Copy link
Member

@branic Normally, retired VMs have no ems because they've been removed from the provider. They remain in our VMDB for reporting purposes. I know that customers have changed the retirement workflow to mark the VM as retired, but not remove it from the provider until some time in the future. Is your concern the customer modified retirement workflow?

@branic
Copy link
Contributor

branic commented Jul 6, 2017

@tinaafitz The retired VMs being included in the count came up from a customer I was working with. They have a two stage retirement. In the first stage the VM is just powered off and marked as retired. In the second stage the VM is removed from the provider some number of days after the initial retirement. In the time between the two stages VMs that were retired from the first stage were being counted in the quota for Maximum VMs.

As their retirement process was customized and is not what happens OOTB I don't think that the base MIQ code needs to be changed to support this.

@billfitzgerald0120
Copy link
Contributor Author

@miq-bot add_label fine/yes.
@simaishi
This is needed for #196

Thank you

@miq-bot
Copy link
Member

miq-bot commented Nov 21, 2017

@billfitzgerald0120 Cannot apply the following label because they are not recognized: fine/yes.

@tinaafitz
Copy link
Member

@simaishi I created a 5.8 BZ for this issue: https://bugzilla.redhat.com/show_bug.cgi?id=1515979

simaishi pushed a commit that referenced this pull request Nov 28, 2017
Spec test for excluding orphaned vms from quota.
(cherry picked from commit ee6a58c)

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

Fine backport details:

$ git log -1
commit 32c9bbdc25365805cdda607b0c31e28fd4b01b23
Author: Madhu Kanoor <[email protected]>
Date:   Tue Jun 27 11:47:16 2017 -0400

    Merge pull request #132 from billfitzgerald0120/quota_noems
    
    Spec test for excluding orphaned vms from quota.
    (cherry picked from commit ee6a58caa606825b391e0b870a161bc0d2f7e4b7)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1518374

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.

7 participants