Fixed quota calculations for multiple vms in requested method.

[billfitzgerald0120]

Values for cloud providers were incorrect when multiple vms were selected.
Included Azure to cloud providers which was missing.

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

@miq-bot add_label bug, fine/yes, euwe/yes

[billfitzgerald0120]

@tinaafitz
Please Review

[tinaafitz]

@billfitzgerald0120 Looks good.

[tinaafitz]

@gmcculloug Looks good, please review.

[gmcculloug]

For consistency 3072 should be 3.kilobytes.

[gmcculloug]

Two comments that apply to the 3 cloud calculation methods update in this PR.

1. The methods cloud_number_of_cpus, cloud_vm_memory and cloud_storage all start with the condition return unless args_hash[:flavor]. These methods are only called from one method cloud_value.

I would move this check into the caller and the methods can assume that they are being called only when a flavor is present in the hash.

2. Constantly dereferencing args_hash[:flavor] is not efficient and makes the logic harder to read. Change the methods to set the variable flavor at the top of the methods.

As an example, with these two changes the cloud_vm_memory method would become:

def cloud_vm_memory(args_hash)
  flavor = args_hash[:flavor]

  $evm.log(:info, "Retrieving flavor #{flavor.name} memory => #{flavor.memory}")
  default_option((flavor.memory * args_hash[:number_of_vms]), args_hash[:options_array])
end

Note: Since the other two args_hash values are only referenced once I did not assign them to variables.

[billfitzgerald0120]

@gmcculloug @tinaafitz Made all changes except for cloud_storage which doesn't use flavor for Google.

[gmcculloug]

@billfitzgerald0120 @tinaafitz Is there a valid use-case where we are evaluating quota without the flavor selected? Understanding that Google does not need it for storage does it make sense doing the storage calculation for any of the cloud providers if we know memory and cpu values will be nil?

Also want to note that cloud_storage was checking for a valid flavor instance prior to this change regardless of the provider type.

-def cloud_storage(flavor, dialog_array, resource)
-  return unless flavor
-
-  storage = if provision_type(resource) == 'google'
-              get_option_value(resource, :boot_disk_size).gigabytes
+def cloud_storage(args_hash)
+  flavor = args_hash[:flavor]
+  storage = if provision_type(args_hash[:resource]) == 'google'

Just trying to rationalize the need for the added complexity.

[gmcculloug]

@billfitzgerald0120 I would prefer this be an early return like the unless args_hash[:cloud] check above and avoid wrapping the case statement in a condition block.

[miq-bot]

Checked commit billfitzgerald0120@88a75a6 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]

@gmcculloug @tinaafitz Made changes as requested.

[gmcculloug]

@tinaafitz Can you give this one more quick review after the last round of changes?

[tinaafitz]

@billfitzgerald0120 @gmcculloug Looks good.

[simaishi]

Euwe backport (to manageiq repo) details:

$ git log -1
commit be1febba336d189aef2437b5e003553e476e3332
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Wed Jun 21 15:07:27 2017 -0400

    Merge pull request #128 from billfitzgerald0120/requested_number_of_vms
    
    Fixed quota calculations for multiple vms in requested method.
    (cherry picked from commit 85dd4414d6d6c4a8bc3b726bac5625974c7caad3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1465088

[simaishi]

Fine backport details:

$ git log -1
commit 30bba6f72010eb612e03e7cc254bd6e602bc69e6
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Wed Jun 21 15:07:27 2017 -0400

    Merge pull request #128 from billfitzgerald0120/requested_number_of_vms
    
    Fixed quota calculations for multiple vms in requested method.
    (cherry picked from commit 85dd4414d6d6c4a8bc3b726bac5625974c7caad3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1465087