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 Service Provisioning cloud_tenant issue #223

Merged
merged 1 commit into from
Feb 7, 2018

Conversation

tinaafitz
Copy link
Member

Call new cloud_tenant helper method to avoid provision error.

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

Provision error:
[----] I, [2018-01-31T13:04:42.560968 #4325:f61134] INFO -- : Q-task_id([miq_provision_1000000000024]) Starting Phase <provision_error>
[----] E, [2018-01-31T13:04:42.629955 #4325:f61134] ERROR -- : Q-task_id([miq_provision_1000000000024]) MIQ(ManageIQ::Providers::Openstack::CloudManager::Provision#provision_error) [[NoMethodError]: undefined method []' for nil:NilClass] encountered during phase [start_clone_task] [----] E, [2018-01-31T13:04:42.630183 #4325:f61134] ERROR -- : Q-task_id([miq_provision_1000000000024]) /var/www/miq/vmdb/app/models/mixins/provider_object_mixin.rb:29:in connection_source'
/var/www/miq/vmdb/app/models/mixins/provider_object_mixin.rb:5:in with_provider_connection' /var/www/miq/vmdb/app/models/manageiq/providers/openstack/cloud_manager/provision/cloning.rb:45:in start_clone'
/var/www/miq/vmdb/app/models/manageiq/providers/cloud_manager/provision/state_machine.rb:37:in start_clone_task' /var/www/miq/vmdb/app/models/miq_request_task/state_machine.rb:21:in signal'
/var/www/miq/vmdb/app/models/miq_provision/state_machine.rb:18:in prepare_provision' /var/www/miq/vmdb/app/models/miq_request_task/state_machine.rb:21:in signal'
/var/www/miq/vmdb/app/models/manageiq/providers/cloud_manager/provision/state_machine.rb:28:in poll_volumes_complete' /var/www/miq/vmdb/app/models/miq_request_task/state_machine.rb:21:in signal'
/var/www/miq/vmdb/app/models/manageiq/providers/cloud_manager/provision/state_machine.rb:15:in prepare_volumes' /var/www/miq/vmdb/app/models/miq_request_task/state_machine.rb:21:in signal'

Service Dialog populates cloud_tenant as a string, code is expecting it to be an array.
----] I, [2018-01-31T13:04:42.543243 #4325:f61134] INFO -- :
Q-task_id([miq_provision_1000000000024])
MIQ(ManageIQ::Providers::Openstack::CloudManager::Provision#log_clone_options)
Prov Options: :cloud_tenant = "1000000000001"

Although these code changes resolves the issue, I think further investigation is necessary because the hash key :tenant_name isn't used in the connection_source method, the method works just because it's being passed a hash, when previously it wasn't.

@tinaafitz
Copy link
Member Author

@miq-bot assign @agrare

@miq-bot
Copy link
Member

miq-bot commented Feb 5, 2018

Checked commit tinaafitz@1137017 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. ⭐

@agrare
Copy link
Member

agrare commented Feb 5, 2018

@aufi do you know if cloud_tenant is a required option here? If it is nil will the connection fail or just use the default tenant?

@aufi
Copy link
Member

aufi commented Feb 6, 2018

It is not required, it can be nil. I think the default tenant is added somewhere "under" provisioning code, this can fail on undefined method :name for Nil.

@agrare
Copy link
Member

agrare commented Feb 6, 2018

Thanks @aufi, this LGTM 👍

@tinaafitz tinaafitz changed the title [WIP] Fix Service Provisioning cloud_tenant issue Fix Service Provisioning cloud_tenant issue Feb 6, 2018
@miq-bot miq-bot removed the wip label Feb 6, 2018
@agrare agrare merged commit df2b8df into ManageIQ:master Feb 7, 2018
@agrare agrare added the bug label Feb 7, 2018
@agrare agrare added this to the Sprint 79 Ending Feb 12, 2018 milestone Feb 7, 2018
@aufi
Copy link
Member

aufi commented Feb 9, 2018

@tinaafitz @agrare Sorry for my not clear comment, but cloud_tenant option is not required in UI, so nil can be passed to provision code, which causes failure (undefined method name for Nil - http://paste.openstack.org/show/666914/). Could we get a followup PR which will fix it?

@agrare
Copy link
Member

agrare commented Feb 9, 2018

Ah my bad @aufi I read that as "its not a problem if the tennent is nil"

@agrare
Copy link
Member

agrare commented Feb 9, 2018

@tinaafitz Can we make that a required field on the dialog?

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

Successfully merging this pull request may close these issues.

4 participants