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

prov_dialog customize - don't fail when no :sysprep_enabled #6953

Merged
merged 1 commit into from
Apr 28, 2020
Merged

prov_dialog customize - don't fail when no :sysprep_enabled #6953

merged 1 commit into from
Apr 28, 2020

Conversation

himdel
Copy link
Contributor

@himdel himdel commented Apr 6, 2020

This code changed (#5774) from conditions like...

- if (@edit && @edit[:new] && @edit[:new][:sysprep_enabled] && @edit[:new][:sysprep_enabled][0] == "fields") || (@options && @options[:sysprep_enabled] && @options[:sysprep_enabled][0] == "fields")

to one assignment...

- edit_or_options = (@edit && @edit[:new] && @edit[:new][:sysprep_enabled]) || (@options && @options[:sysprep_enabled])

...and conditions like...

- if edit_or_options[0] == "fields"

Which then crashes when trying to clone a RHV template, because edit_or_options is nil...

Error caught: [ActionView::Template::Error] undefined method `[]' for nil:NilClass
/opt/rh/cfme-gemset/bundler/gems/cfme-ui-classic-d44320707dfe/app/views/shared/views/_prov_dialog.html.haml:195:in `__opt_rh_cfme_gemset_bundler_gems_cfme_ui_classic_d________dfe_app_views_shared_views__prov_dialog_html_haml___2839729305696764418_47250275323280'

The else branch doesn't look inside edit_or_options, so initializing to an empty array works as before.

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

This code changed from conditions like...

    - if (@edit && @edit[:new] && @edit[:new][:sysprep_enabled] && @edit[:new][:sysprep_enabled][0] == "fields") || (@options && @options[:sysprep_enabled] && @options[:sysprep_enabled][0] == "fields")

to one assignment...

    - edit_or_options = (@edit && @edit[:new] && @edit[:new][:sysprep_enabled]) || (@options && @options[:sysprep_enabled])

...and conditions like...

    - if edit_or_options[0] == "fields"

Which then crashes when trying to clone a RHV template, because `edit_or_options` is nil...

    Error caught: [ActionView::Template::Error] undefined method `[]' for nil:NilClass
    /opt/rh/cfme-gemset/bundler/gems/cfme-ui-classic-d44320707dfe/app/views/shared/views/_prov_dialog.html.haml:195:in `__opt_rh_cfme_gemset_bundler_gems_cfme_ui_classic_d________dfe_app_views_shared_views__prov_dialog_html_haml___2839729305696764418_47250275323280'

The else branch doesn't look inside edit_or_options, so initializing to an empty array works as before.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1797706
@miq-bot
Copy link
Member

miq-bot commented Apr 6, 2020

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/89152bd27dc232b72160ba35e5d0873b91192d9e with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint
1 file checked, 1 offense detected

app/views/shared/views/_prov_dialog.html.haml

  • ⚠️ - Line 187 - Avoid using instance variables in partials views

@skateman
Copy link
Member

skateman commented Apr 7, 2020

I'm trying to test this and getting this page when cloning:
Screenshot from 2020-04-07 11-39-00

Am I doing something wrong or is it a bug?

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

Well, any errrors?

@skateman
Copy link
Member

skateman commented Apr 7, 2020

No errors in any console, but if I press the submit button, I'm getting this:

Error caught: [NoMethodError] undefined method `get_dialog_order' for nil:NilClass
/home/skateman/Repositories/ManageIQ/manageiq-ui-classic/app/controllers/application_controller/miq_request_methods.rb:581:in `validate_fields'
/home/skateman/Repositories/ManageIQ/manageiq-ui-classic/app/controllers/application_controller/miq_request_methods.rb:655:in `prov_req_submit'
/home/skateman/Repositories/ManageIQ/manageiq-ui-classic/app/controllers/application_controller/miq_request_methods.rb:217:in `prov_edit'

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

Oh well, sounds like cloning is broken too then, for some case.
Create an issue with full steps please?

But if I go to Compute > Infrastructure > VMs,
pick a template under the RHV provider,
Lifecycle > Clone

it works for me..

clone

@skateman
Copy link
Member

skateman commented Apr 7, 2020

Oh, you're going from the Templates accordion? Try it from the VM & Templates...in the meantime I'm going to test it from the Templates

Nah...same issue, tried with multiple RHV templates from Dan's DB 😕

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

Ah, yeah, the BZ was about cloning templates, so I cloned a template.
But cloning is a separate issue, the bug is what happens next, when viewing the request.

And after the clone, this is what I see in the request details..

request

Customization tab works :)

@skateman
Copy link
Member

skateman commented Apr 7, 2020

Okay, any way to get there with the broken cloning?

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

Well, if you already have a clone request in the db, maybe you just need to update created_at to see it?

Or I can send you my db :)

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

I'm not sure whis will help but... (MiqRequest.last.attributes)

{"id"=>308, "description"=>"Clone from [amaya-insights-el7.0] to [xxx]", "approval_state"=>"pending_approval", "type"=>"MiqProvisionRequest", "created_on"=>Tue, 07 Apr 2020 12:13:59 UTC +00:00, "updated_on"=>Tue, 07 Apr 2020 12:14:00 UTC +00:00, "fulfilled_on"=>nil, "requester_id"=>1, "requester_name"=>"Administrator", "request_type"=>"clone_to_vm", "request_state"=>"pending", "message"=>"VM Provisioning - Request Created", "status"=>"Ok", "options"=>{:src_vm_id=>[2686, "amaya-insights-el7.0"], :request_type=>:clone_to_vm, :miq_request_dialog_name=>"miq_provision_redhat_dialogs_clone_to_vm", :owner_email=>"[email protected]", :current_tab_key=>:hardware, :owner_phone=>nil, :owner_country=>nil, :owner_phone_mobile=>nil, :owner_title=>nil, :owner_first_name=>nil, :owner_manager=>nil, :owner_address=>nil, :owner_company=>nil, :owner_last_name=>nil, :owner_manager_mail=>nil, :owner_city=>nil, :owner_department=>nil, :owner_load_ldap=>nil, :owner_manager_phone=>nil, :owner_state=>nil, :owner_office=>nil, :owner_zip=>nil, :request_notes=>nil, :vm_tags=>[], :customization_template_script=>nil, :sysprep_computer_name=>nil, :sysprep_organization=>nil, :sysprep_domain=>nil, :sysprep_admin_password=>nil, :sysprep_locale_ui=>nil, :sysprep_locale_user=>nil, :sysprep_locale_input=>nil, :sysprep_locale_system=>nil, :sysprep_machine_object_ou=>nil, :sysprep_product_key=>nil, :sysprep_timezone=>[nil, nil], :sysprep_domain_admin=>nil, :sysprep_domain_password=>nil, :dns_servers=>nil, :dns_suffixes=>nil, :root_password=>nil, :addr_mode=>["static", "Static"], :gateway=>nil, :hostname=>nil, :ip_addr=>nil, :subnet_mask=>nil, :placement_cluster_name=>[4, "Default"], :placement_dc_name=>[28, "Default"], :cluster_filter=>[nil, nil], :placement_auto=>[false, 0], :placement_host_name=>[nil, nil], :number_of_vms=>[1, "1"], :vm_description=>nil, :vm_prefix=>nil, :provision_type=>["iso", "ISO"], :linked_clone=>[false, 0], :vm_name=>"xxx", :iso_image_id=>["IsoImage::6", "CentOS-7-x86_64-DVD-1708.iso"], :pxe_server_id=>[nil, nil], :schedule_type=>["immediately", "Immediately on Approval"], :vm_auto_start=>[true, 1], :schedule_time=>Wed, 08 Apr 2020 00:00:00 UTC +00:00, :retirement=>[0, "Indefinite"], :retirement_warn=>[604800, "1 Week"], :stateless=>[false, 0], :vlan=>["<Template>", "<Use template nics>"], :mac_address=>nil, :disk_format=>["raw", "raw"], :disk_sparsity=>["default", "Same as in template"], :number_of_sockets=>[1, "1"], :cores_per_socket=>[1, "1"], :vm_memory=>["1024", "1024"], :memory_reserve=>1024, :network_adapters=>[1, "1"], :start_date=>"4/8/2020", :start_hour=>"00", :start_min=>"00", :placement_ds_name=>[nil, nil], :src_vm_nics=>[], :src_vm_lans=>[], :customize_enabled=>["enabled"], :src_ems_id=>[22, "RHV"], :requester_group=>"EvmGroup-super_administrator", :owner_group=>"EvmGroup-super_administrator", :delivered_on=>nil}, "userid"=>"admin", "source_id"=>2686, "source_type"=>"VmOrTemplate", "destination_id"=>nil, "destination_type"=>nil, "tenant_id"=>1, "service_order_id"=>nil, "process"=>true, "cancelation_status"=>nil, "initiated_by"=>"user"}

@himdel
Copy link
Contributor Author

himdel commented Apr 7, 2020

Or, you can find any similar request and clear sysprep_enabled. That should cause the same failure.

@h-kataria h-kataria requested a review from skateman April 13, 2020 16:03
@h-kataria h-kataria self-assigned this Apr 13, 2020
@skateman
Copy link
Member

@himdel following the steps from the BZ, I wasn't able to reproduce this issue on the latest master. Can you please verify and/or help me with reproducing?

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

@skateman Can you be more specific? Which part did you fail to reproduce? And have you tried with the provided request?

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

@skateman But most likely, you need to

diff --git a/app/helpers/provision_customize_helper.rb b/app/helpers/provision_customize_helper.rb
index 40ef149208..d095a65d53 100644
--- a/app/helpers/provision_customize_helper.rb
+++ b/app/helpers/provision_customize_helper.rb
@@ -8,6 +8,7 @@ module ProvisionCustomizeHelper
   end
 
   def select_check?(workflow)
+    return true
     (workflow.kind_of?(ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow) ||
       workflow.kind_of?(ManageIQ::Providers::Redhat::InfraManager::ProvisionWorkflow)) &&
       !workflow.supports_pxe? && !workflow.supports_iso?

to make sure the section is rendered.

(It may render empty when when the right keys are not in options, not sure if that happens in real life too.)

@skateman
Copy link
Member

@himdel I have a new database and there I created this request:

#<MiqProvisionRequest id: 296, description: "Clone from [cbucur-rhel74-minimal-template] to [fo...", approval_state: "pending_approval", type: "MiqProvisionRequest", created_on: "2020-04-14 12:10:44", updated_on: "2020-04-14 12:10:46", fulfilled_on: nil, requester_id: 1, requester_name: "Administrator", request_type: "clone_to_vm", request_state: "pending", message: "VM Provisioning - Request Created", status: "Ok", options: {:src_vm_id=>[3231, "cbucur-rhel74-minimal-template"], :request_type=>:clone_to_vm, :miq_request_dialog_name=>"miq_provision_vmware_dialogs_clone_to_vm", :owner_email=>"[email protected]", :current_tab_key=>:network, :owner_phone=>nil, :owner_country=>nil, :owner_phone_mobile=>nil, :owner_title=>nil, :owner_first_name=>nil, :owner_manager=>nil, :owner_address=>nil, :owner_company=>nil, :owner_last_name=>nil, :owner_manager_mail=>nil, :owner_city=>nil, :owner_department=>nil, :owner_load_ldap=>nil, :owner_manager_phone=>nil, :owner_state=>nil, :owner_office=>nil, :owner_zip=>nil, :request_notes=>nil, :vm_tags=>[], :dns_servers=>nil, :sysprep_organization=>nil, :sysprep_password=>nil, :sysprep_server_license_mode=>["perServer", "Per server"], :ldap_ous=>[nil, nil], :sysprep_timezone=>[nil, nil], :dns_suffixes=>nil, :sysprep_product_id=>nil, :sysprep_identification=>["domain", "Domain"], :sysprep_per_server_max_connections=>"5", :sysprep_computer_name=>nil, :sysprep_workgroup_name=>"WORKGROUP", :sysprep_spec_override=>[false, 0], :addr_mode=>["dhcp", "DHCP"], :linux_host_name=>nil, :sysprep_domain_admin=>nil, :sysprep_change_sid=>[true, 1], :sysprep_domain_name=>[nil, nil], :sysprep_upload_file=>nil, :gateway=>nil, :ip_addr=>nil, :linux_domain_name=>nil, :sysprep_domain_password=>nil, :sysprep_auto_logon=>[true, 1], :sysprep_enabled=>["disabled", "<None>"], :sysprep_delete_accounts=>[false, 0], :sysprep_upload_text=>nil, :wins_servers=>nil, :subnet_mask=>nil, :sysprep_full_name=>nil, :sysprep_auto_logon_count=>[1, "1"], :placement_storage_profile=>[nil, nil], :placement_cluster_name=>[nil, nil], :cluster_filter=>[nil, nil], :host_filter=>[nil, nil], :ds_filter=>[nil, nil], :placement_host_name=>[nil, nil], :placement_ds_name=>[nil, nil], :rp_filter=>[nil, nil], :placement_auto=>[true, 1], :placement_folder_name=>[nil, nil], :placement_rp_name=>[nil, nil], :placement_dc_name=>[nil, nil], :number_of_vms=>[1, "1"], :vm_description=>nil, :vm_prefix=>nil, :vm_name=>"foobar", :host_name=>nil, :provision_type=>["vmware", "VMware"], :linked_clone=>[nil, nil], :snapshot=>[nil, nil], :vm_filter=>[nil, nil], :schedule_type=>["immediately", "Immediately on Approval"], :vm_auto_start=>[true, 1], :schedule_time=>Wed, 15 Apr 2020 00:00:00 UTC +00:00, :retirement=>[0, "Indefinite"], :retirement_warn=>[604800, "1 Week"], :vlan=>["dmesser-management", "dmesser-management"], :mac_address=>nil, :disk_format=>["unchanged", "Default"], :cpu_limit=>-1, :memory_limit=>-1, :number_of_sockets=>[2, "2"], :cores_per_socket=>[1, "1"], :cpu_reserve=>0, :vm_memory=>["4096", "4096"], :memory_reserve=>0, :network_adapters=>[1, "1"], :start_date=>"4/15/2020", :start_hour=>"00", :start_min=>"00", :src_vm_nics=>["Network adapter 1"], :src_vm_lans=>["DPortGroup-Public-2"], :customize_enabled=>["enabled"], :src_ems_id=>[1, "vCenter"], :requester_group=>"EvmGroup-super_administrator", :owner_group=>"EvmGroup-super_administrator", :delivered_on=>nil}, userid: "admin", source_id: 3231, source_type: "VmOrTemplate", destination_id: nil, destination_type: nil, tenant_id: 1, service_order_id: nil, process: true, cancelation_status: nil, initiated_by: "user">

I tried this, but also manually removed the sysprep_enabled from it and still nothing. I can go to the configuration tab both on the summary screen and the edit screen as well.
Screenshot from 2020-04-14 14-14-12

Screenshot from 2020-04-14 14-13-45

No change after your suggested true either 😕

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

Well, then you need this diff:

diff --git a/app/helpers/provision_customize_helper.rb b/app/helpers/provision_customize_helper.rb
index 40ef149208..d095a65d53 100644
--- a/app/helpers/provision_customize_helper.rb
+++ b/app/helpers/provision_customize_helper.rb
@@ -8,6 +8,7 @@ module ProvisionCustomizeHelper
   end
 
   def select_check?(workflow)
+    return true
     (workflow.kind_of?(ManageIQ::Providers::Vmware::InfraManager::ProvisionWorkflow) ||
       workflow.kind_of?(ManageIQ::Providers::Redhat::InfraManager::ProvisionWorkflow)) &&
       !workflow.supports_pxe? && !workflow.supports_iso?
diff --git a/app/views/shared/views/_prov_dialog.html.haml b/app/views/shared/views/_prov_dialog.html.haml
index c9c5a70e3d..a07cced57f 100644
--- a/app/views/shared/views/_prov_dialog.html.haml
+++ b/app/views/shared/views/_prov_dialog.html.haml
@@ -183,8 +183,11 @@
         :prefix                     => "/miq_request/",
         :keys                       => keys})
   - when :customize
+    = "EDIT: #{@edit.try(:[], :new).try(:to_json)}"
+    = "OPT: #{@options.try(:slice, :sysprep_enabled).try(:to_json)}"
     - if select_check?(wf)
-      - edit_or_options = (@edit && @edit[:new] && @edit[:new][:sysprep_enabled]) || (@options && @options[:sysprep_enabled])
+      - edit_or_options = (@edit && @edit[:new] && @edit[:new][:sysprep_enabled]) || (@options && @options[:sysprep_enabled]) || []
+      = "EO: #{edit_or_options.try(:to_json)}"
       - keys = [:sysprep_enabled]
       = render(:partial => "/miq_request/prov_dialog_fieldset",
         :locals         => {:workflow => wf,

.. and tell me what's going on :)

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

(and then remove the || [] fix for testing :D)

@skateman
Copy link
Member

skateman commented Apr 14, 2020

When displaying:

EDIT: OPT: {"sysprep_enabled":["disabled","\u003cNone\u003e"]} EO: ["disabled","\u003cNone\u003e"]

When editing:

EDIT: {"src_vm_id":[3231,"cbucur-rhel74-minimal-template"],"request_type":"clone_to_vm","miq_request_dialog_name":"miq_provision_vmware_dialogs_clone_to_vm","owner_email":"[email protected]","current_tab_key":"customize","owner_phone":null,"owner_country":null,"owner_phone_mobile":null,"owner_title":null,"owner_first_name":null,"owner_manager":null,"owner_address":null,"owner_company":null,"owner_last_name":null,"owner_manager_mail":null,"owner_city":null,"owner_department":null,"owner_load_ldap":null,"owner_manager_phone":null,"owner_state":null,"owner_office":null,"owner_zip":null,"request_notes":null,"vm_tags":[],"dns_servers":null,"sysprep_organization":null,"sysprep_password":null,"sysprep_server_license_mode":["perServer","Per server"],"ldap_ous":[null,null],"sysprep_timezone":[null,null],"dns_suffixes":null,"sysprep_product_id":null,"sysprep_identification":["domain","Domain"],"sysprep_per_server_max_connections":"5","sysprep_computer_name":null,"sysprep_workgroup_name":"WORKGROUP","sysprep_spec_override":[false,0],"addr_mode":["dhcp","DHCP"],"linux_host_name":null,"sysprep_domain_admin":null,"sysprep_change_sid":[true,1],"sysprep_domain_name":[null,null],"sysprep_upload_file":null,"gateway":null,"ip_addr":null,"linux_domain_name":null,"sysprep_domain_password":null,"sysprep_auto_logon":[true,1],"sysprep_delete_accounts":[false,0],"sysprep_upload_text":null,"wins_servers":null,"subnet_mask":null,"sysprep_full_name":null,"sysprep_auto_logon_count":[1,"1"],"placement_storage_profile":[null,null],"placement_cluster_name":[null,null],"cluster_filter":[null,null],"host_filter":[null,null],"ds_filter":[null,null],"placement_host_name":[null,null],"placement_ds_name":[null,null],"rp_filter":[null,null],"placement_auto":[true,1],"placement_folder_name":[null,null],"placement_rp_name":[null,null],"placement_dc_name":[null,null],"number_of_vms":[1,"1"],"vm_description":null,"vm_prefix":null,"vm_name":"foobar","host_name":null,"provision_type":["vmware","VMware"],"linked_clone":[null,null],"snapshot":[null,null],"vm_filter":[null,null],"schedule_type":["immediately","Immediately on Approval"],"vm_auto_start":[true,1],"schedule_time":"2020-04-15T00:00:00.000Z","retirement":[0,"Indefinite"],"retirement_warn":[604800,"1 Week"],"vlan":["dmesser-management","dmesser-management"],"mac_address":null,"disk_format":["unchanged","Default"],"cpu_limit":-1,"memory_limit":-1,"number_of_sockets":[2,"2"],"cores_per_socket":[1,"1"],"cpu_reserve":0,"vm_memory":["4096","4096"],"memory_reserve":0,"network_adapters":[1,"1"],"start_date":"4/15/2020","start_hour":"0","start_min":"0","src_vm_nics":["Network adapter 1"],"src_vm_lans":["DPortGroup-Public-2"],"customize_enabled":["enabled"],"src_ems_id":[1,"vCenter"],"requester_group":"EvmGroup-super_administrator","owner_group":"EvmGroup-super_administrator","delivered_on":null,"sysprep_enabled":["disabled","\u003cNone\u003e"]} OPT: EO: ["disabled","\u003cNone\u003e"]

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

Well, there's your answer then - there's :sysprep_enabled in @edit[:new] and options, that's not the kind of request this fails for.

@skateman
Copy link
Member

Well, but the DB record doesn't have it 😕

@himdel
Copy link
Contributor Author

himdel commented Apr 14, 2020

¯\_(ツ)_/¯ I can't help you with the provider stuff, I suggest just mocking the data.

@skateman
Copy link
Member

I imported your DB dump and can't reproduce the issue with the only request you have available under Services -> requests

@mzazrivec mzazrivec merged commit 158ca6f into ManageIQ:master Apr 28, 2020
simaishi pushed a commit that referenced this pull request May 7, 2020
prov_dialog customize - don't fail when no :sysprep_enabled

(cherry picked from commit 158ca6f)

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

simaishi commented May 7, 2020

Ivanchuk backport details:

$ git log -1
commit dc0ddaebe0f9d1db2cafe0d59262bb5157655c2c
Author: Milan Zázrivec <[email protected]>
Date:   Tue Apr 28 08:21:47 2020 +0200

    Merge pull request #6953 from himdel/prov-dialog-fix

    prov_dialog customize - don't fail when no :sysprep_enabled

    (cherry picked from commit 158ca6fcdfdeaa34464d5c14275264d51c304bd6)

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

simaishi pushed a commit that referenced this pull request May 7, 2020
prov_dialog customize - don't fail when no :sysprep_enabled

(cherry picked from commit 158ca6f)

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

simaishi commented May 7, 2020

Jansa backport details:

$ git log -1
commit 0badcf639213f2488eff3dac55bcdf1f8d13be38
Author: Milan Zázrivec <[email protected]>
Date:   Tue Apr 28 08:21:47 2020 +0200

    Merge pull request #6953 from himdel/prov-dialog-fix

    prov_dialog customize - don't fail when no :sysprep_enabled

    (cherry picked from commit 158ca6fcdfdeaa34464d5c14275264d51c304bd6)

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

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