Bug #23859
closedTernary operation with vm_exists? in orchestration compute
Description
In app/models/concerns/orchestration/compute.rb file there is this method:
def queue_compute
return unless compute? && errors.empty?
# Create a new VM if it doesn't already exist or update an existing vm
vm_exists? ? queue_compute_create : queue_compute_update
end
When I read the ternary operator it seems to do the exact contrary of what it is explained in the comment.
And when I test it, indeed it does not create the VM in compute provider.
I think it should be:
def queue_compute
return unless compute? && errors.empty?
# Create a new VM if it doesn't already exist or update an existing vm
!vm_exists? ? queue_compute_create : queue_compute_update
end
Then it works and it finally creates the new VM in compute provider.
I am surprised that nobody else found this before.
Maybe I misunderstood something?
Updated by Anonymous over 6 years ago
- Related to Feature #18064: Ability to import and provision existing VMs added
Updated by Anonymous over 6 years ago
Could you open a pull request whith this, please?
Personally I'd find this easier to follow the logic:
vm_exists? ? queue_compute_update : queue_compute_create
Updated by Timo Goebel over 6 years ago
We already have a fix for this as part of https://github.com/theforeman/foreman/pull/5569.
Updated by Anonymous over 6 years ago
I'm unsure about the actual impact of this bug.
If it's relevant, it would good to have it as a separate commit to cherry-pick it down to the supported releases.
Updated by The Foreman Bot over 6 years ago
- Status changed from New to Ready For Testing
- Pull request https://github.com/theforeman/foreman/pull/5675 added
Updated by Tristan Robert over 6 years ago
The PR refactors vm_exists? but my original issue still remains. I will make another issue (feature enhancement exactly) and a new PR.
Updated by Anonymous over 6 years ago
- Translation missing: en.field_release set to 353
Updated by Tristan Robert over 6 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset 2a6bce4de57782966e6f5aaaaf415756fe981c0f.
Updated by Ewoud Kohl van Wijngaarden over 6 years ago
- Category set to Compute resources
- Triaged set to No