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

Configuration manager non explorer version #6782

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
6173e3a
First cut of Non-explorer version of Configuration Management
h-kataria Mar 15, 2020
006f335
Fixed value of `menu_section` in controllers
h-kataria Mar 16, 2020
09277ae
Added listnav partials
h-kataria Mar 16, 2020
c0aacb3
Added/fixed textual summary helpers
h-kataria Mar 16, 2020
ba12336
Fixed relationship links on textual summary screens
h-kataria Mar 16, 2020
e9f011f
load toolbars for Configuration Manager(s) and Configured System(s)
h-kataria Mar 16, 2020
f432d05
Getting toolbar buttons working.
h-kataria Mar 20, 2020
c67e3cb
Removed accidental add of debug statement
h-kataria Mar 23, 2020
126a2be
Further cleanup of routes and CS controller + toolbars working.
h-kataria Mar 23, 2020
dd14060
Removed Configuration Management explorer related files & code.
h-kataria Mar 24, 2020
c40dd5e
show gtl toolbar buttons on new screens
h-kataria Mar 24, 2020
a96ff9d
Addressing rubocop warnings
h-kataria Mar 24, 2020
88ae1db
Fixed code to load listnav and advanced search for new screens.
h-kataria Mar 24, 2020
a22109d
Fixed links on list view when viewing objects thru relationships.
h-kataria Mar 24, 2020
5e1a1aa
Added filters/Advanced Search Configuration Managers and Profiles lists
h-kataria Mar 25, 2020
2a09746
Listnav and Summary screen updates
h-kataria Mar 25, 2020
694bf6b
Fixed broken spec tests.
h-kataria Mar 25, 2020
b4ae380
Added Configured System tree builder file back
h-kataria Mar 25, 2020
4932109
Removed reference to configuration manager tree node
h-kataria Mar 25, 2020
689ccf7
Changes to load and make buttons work on sub-lists.
h-kataria Mar 30, 2020
19ab37e
Addressed rubocop warnings
h-kataria Mar 31, 2020
5241991
Renamed all occurrences of configuration_manager to ems_configuration
h-kataria Apr 2, 2020
34e0b77
Rename `configuration_manager` to `ems_configuration` in tests
h-kataria Apr 2, 2020
68d5bc7
Updated `Delete` button in EmsConfiguration(s) toolbars.
h-kataria Apr 7, 2020
af486f6
Updated identifiers in toolbars to use correct feature id.
h-kataria Apr 9, 2020
905f193
Fixed spec test
h-kataria Apr 9, 2020
3e7c4f7
Addressed PR feedback and some other minor issues fixed
h-kataria Apr 14, 2020
af02511
Added back missing method `construct_edit_for_audit`
h-kataria Apr 16, 2020
8790311
Fixed to show flash message when Configured System cannot be provisioned
h-kataria Apr 19, 2020
b9962af
Using a named_scope to exclude Ansible records from list view
h-kataria Apr 19, 2020
1003ca0
added a missing method, that is needed when creating a new provider
h-kataria Apr 21, 2020
87844e2
Added screen with "Add Provider" button if there are no Providers in db
h-kataria Apr 21, 2020
3f8593e
addressed PR feedback
h-kataria Apr 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 0 additions & 8 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -431,12 +431,6 @@ def report_data
@view.table = filter_parent_name_tenant(@view.table)
end

# Foreman has some unassigned rows which needs to be added after view is fetched
if options && options[:unassigned_profile_row] && options[:unassigned_configuration_profile]
options[:unassigned_profile_row][:id] ||= options[:unassigned_profile_row]['manager_id']
@view.table.data.push(options[:unassigned_profile_row])
@targets_hash[options[:unassigned_profile_row]['id']] = options[:unassigned_configuration_profile]
end
render :json => {
:settings => settings,
:data => view_to_hash(@view, true),
Expand Down Expand Up @@ -1990,8 +1984,6 @@ def controller_for_common_methods
"vm"
when 'automation_manager'
"automation_manager_provider"
when 'provider_foreman'
"configuration_manager_provider"
when "generic_object_definition" # tagging for nested list on the generic object class
"generic_object"
when "ansible_playbook"
Expand Down
5 changes: 1 addition & 4 deletions app/controllers/application_controller/advanced_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,7 @@ def adv_search_redraw_listnav_and_main
end

def adv_search_redraw_left_div
if x_active_tree.to_s == "configuration_manager_cs_filter_tree"
build_accordions_and_trees
load_or_clear_adv_search
elsif @edit[:in_explorer] || %w[storage_tree configuration_scripts_tree svcs_tree].include?(x_active_tree.to_s)
if @edit[:in_explorer] || %w[storage_tree configuration_scripts_tree svcs_tree].include?(x_active_tree.to_s)
tree_type = x_active_tree.to_s.sub(/_tree/, '').to_sym
builder = TreeBuilder.class_for_type(tree_type)
tree = builder.new(x_active_tree, @sb)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/application_controller/ci_processing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def process_elements(elements, klass, task, display_name = nil, order_field = ni
end

def explorer_controller?
%w[vm_cloud vm_infra vm_or_template infra_networking].include?(controller_name)
%w[vm_cloud vm_infra vm_or_template infra_networking automation_manager].include?(controller_name)
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't be configuration_manager?

Copy link
Contributor Author

@h-kataria h-kataria Mar 26, 2020

Choose a reason for hiding this comment

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

no, configuration_manger is no longer an explorer controller but shares some code with automation_manager controller and that is explorer style screen, so i need to add this here so i can redirect appropriately from shared methods.

Copy link
Member

Choose a reason for hiding this comment

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

OH, right

end

def process_element_destroy(element, klass, name)
Expand Down Expand Up @@ -306,7 +306,7 @@ def manager_button_operation(method, display_name)
def process_managers(managers, task)
controller_class = request.parameters[:controller]
provider_class = case controller_class
when 'provider_foreman' then ManageIQ::Providers::ConfigurationManager
when 'ems_configuration' then ManageIQ::Providers::ConfigurationManager
when 'automation_manager' then ManageIQ::Providers::AutomationManager
end

Expand Down
9 changes: 4 additions & 5 deletions app/controllers/application_controller/explorer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ def x_history
'start' => :s1, 'stop' => :s1, 'suspend' => :s1,
'reset' => :s1, 'terminate' => :s1, 'pause' => :s1,
'shelve' => :s1, 'shelve_offload' => :s1, 'chargeback' => :s1,
'foreman_pause' => :s1, 'foreman_resume' => :s1, 'manager_pause' => :s1,
'manager_resume' => :s1,
'manager_pause' => :s1, 'manager_resume' => :s1,

# group 2
'clone' => :s2, 'compare' => :s2, 'drift' => :s2,
Expand All @@ -55,6 +54,7 @@ def x_history
'tag' => :s2, 'timeline' => :s2, 'resize' => :s2,
'live_migrate' => :s2, 'attach' => :s2, 'detach' => :s2,
'evacuate' => :s2, 'service_dialog' => :s2, 'transform' => :s2,
'manager_configuration_script_service_dialog' => :s2,
'transform_mass' => :s2,
'associate_floating_ip' => :s2,
'disassociate_floating_ip' => :s2,
Expand All @@ -71,8 +71,7 @@ def x_history

def x_button
model, action = pressed2model_action(params[:pressed])

allowed_models = %w[common image instance vm miq_template provider automation storage configscript infra_networking automation_manager_provider configuration_manager_provider]
allowed_models = %w[common image instance vm miq_template provider automation storage infra_networking automation_manager_provider]
raise ActionController::RoutingError, 'invalid button action' unless
allowed_models.include?(model)

Expand Down Expand Up @@ -100,7 +99,7 @@ def x_button
send(method, Storage)
when 'infra_networking'
send(method, Switch)
when 'automation_manager_provider', 'configuration_manager_provider'
when 'automation_manager_provider'
send(method)
else
send(method, VmOrTemplate)
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/automation_manager_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ class AutomationManagerController < ApplicationController

include Mixins::GenericSessionMixin
include Mixins::ManagerControllerMixin
include Mixins::AutomationManagerControllerMixin
include Mixins::ExplorerPresenterMixin
include Mixins::EmsCommon::Core
include Mixins::EmsCommon::PauseResume
Expand Down Expand Up @@ -446,7 +447,7 @@ def valid_configuration_script_record?(configuration_script_record)
configuration_script_record.try(:id)
end

def configscript_service_dialog
def automation_manager_configuration_script_service_dialog
assert_privileges("automation_manager_configuration_script_service_dialog")
cs = ConfigurationScript.find_by(:id => params[:miq_grid_checks] || params[:id])
@edit = {:rec_id => cs.id}
Expand Down
52 changes: 52 additions & 0 deletions app/controllers/configuration_profile_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
class ConfigurationProfileController < ApplicationController
include Mixins::GenericListMixin
himdel marked this conversation as resolved.
Show resolved Hide resolved
include Mixins::GenericShowMixin
include Mixins::GenericSessionMixin
include Mixins::BreadcrumbsMixin

before_action :check_privileges
before_action :get_session_data
after_action :cleanup_action
after_action :set_session_data

def self.display_methods
%w[configured_systems]
end

def button
@edit = session[:edit] # Restore @edit for adv search box
params[:display] = @display if display_methods.include?(@display) # Were we displaying nested list

# Handle Toolbar Policy Tag Button
@refresh_div = "main_div" # Default div for button.rjs to refresh

if params[:pressed].starts_with?("configured_system_") # Handle buttons from sub-items screen
tag(ConfiguredSystem) if params[:pressed] == "configured_system_tag"
provision if params[:pressed] == "configured_system_provision"
end

if @refresh_div == "main_div" && @lastaction == "show_list"
replace_gtl_main_div
else
render_flash unless performed?
end
end

def show_list
opts = {:no_checkboxes => true}
process_show_list(opts)
end

private

def breadcrumbs_options
{
:breadcrumbs => [
{:title => _("Configuration Profile")},
{:title => _("Profiles"), :url => controller_url},
],
}
end

menu_section :conf
end
50 changes: 50 additions & 0 deletions app/controllers/configured_system_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
class ConfiguredSystemController < ApplicationController
include Mixins::GenericListMixin
include Mixins::GenericShowMixin
include Mixins::GenericSessionMixin
include Mixins::BreadcrumbsMixin
include Mixins::ManagerControllerMixin

before_action :check_privileges
before_action :get_session_data
after_action :cleanup_action
after_action :set_session_data

def self.table_name
@table_name ||= "configured_system"
end

def button
@edit = session[:edit] # Restore @edit for adv search box

# Handle Toolbar Policy Tag Button
@refresh_div = "main_div" # Default div for button.rjs to refresh
model = self.class.model
tag(model) if params[:pressed] == "configured_system_tag"
provision if params[:pressed] == "configured_system_provision"
render_flash unless performed?
end

def show_list
options = {:named_scope => :under_configuration_managers}
process_show_list(options)
end

private

def textual_group_list
[%i[properties relationships environment], %i[os tenancy tags]]
end
helper_method :textual_group_list

def breadcrumbs_options
{
:breadcrumbs => [
{:title => _("Configuration Management")},
{:title => _("Configured Systems"), :url => controller_url},
],
}
end

menu_section :conf
end
113 changes: 113 additions & 0 deletions app/controllers/ems_configuration_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
class EmsConfigurationController < ApplicationController
include Mixins::GenericListMixin
include Mixins::GenericShowMixin
include Mixins::GenericSessionMixin
include Mixins::BreadcrumbsMixin
include Mixins::GenericFormMixin
include Mixins::ManagerControllerMixin
include Mixins::FindRecord

before_action :check_privileges
before_action :get_session_data
after_action :cleanup_action
after_action :set_session_data

def self.model
ManageIQ::Providers::ConfigurationManager
end

def self.table_name
@table_name ||= "ems_configuration"
end

def self.display_methods
%w[configuration_profiles configured_systems]
end

def button
@edit = session[:edit] # Restore @edit for adv search box
params[:display] = @display if display_methods.include?(@display) # Were we displaying nested list

# Handle Toolbar Policy Tag Button
@refresh_div = "main_div" # Default div for button.rjs to refresh
model = self.class.model
tag(model) if params[:pressed] == "#{params[:controller]}_tag"
return if ["#{params[:controller]}_tag"].include?(params[:pressed]) && @flash_array.nil? # Tag screen showing

if params[:pressed].starts_with?("configured_system_") # Handle buttons from sub-items screen
tag(ConfiguredSystem) if params[:pressed] == "configured_system_tag"
provision if params[:pressed] == "configured_system_provision"
end

case params[:pressed]
when 'ems_configuration_edit_provider'
edit
when 'ems_configuration_add_provider'
new
when "ems_configuration_refresh_provider"
refresh
when "ems_configuration_delete_provider"
delete
end

if single_delete_test
single_delete_redirect
elsif (params[:pressed].ends_with?("_edit_provider") || params[:pressed] == "ems_configuration_add_provider") && @flash_array.nil?
if @flash_array
show_list
replace_gtl_main_div
else
javascript_redirect(:action => @refresh_partial, :id => @redirect_id)
end
elsif @refresh_div == "main_div" && @lastaction == "show_list"
replace_gtl_main_div
else
render_flash unless performed?
end
end

private

def self.model_to_name(_provmodel)
Dictionary.gettext('ems_configuration', :type => :ui_title, :translate => false)
end

def manager_prefix
"configuration_manager"
end

def privilege_prefix
"ems_configuration"
end

def refresh
assert_privileges("ems_configuration_refresh_provider")
manager_button_operation('refresh_ems', _('Refresh'))
end

def concrete_model
ManageIQ::Providers::ConfigurationManager
end

def provider_class
ManageIQ::Providers::Foreman::Provider
Copy link
Member

Choose a reason for hiding this comment

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

Why is Foreman still here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fryguy This is still here because currently UI does not have a type drop down in Add a new Provider form, i was hoping once we have new Data driven Provider form in place we will have a type drop down for user to select what type of Provider they want to add with their respective fields. Do you think i should i add a Type drop down on the Add form, if yes, what should be the options, then we can change this code to use value from the type field.

Copy link
Member

Choose a reason for hiding this comment

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

How do we do it for other providers?

Copy link
Member

Choose a reason for hiding this comment

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

Or is this related to the problem with the Provider model vs ExtManagementSystem model?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other provider have a type field on the form, based upon the type selected a Provider object is created. I am working on adding a Type field to the Add Configuration Manager form with similar logic that we have for other providers. Should get that in shortly.

Copy link
Contributor

@himdel himdel Apr 27, 2020

Choose a reason for hiding this comment

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

Is there a point to all this?

Consider that for now Foreman::Provider is precisely the class this controller needs.

None of this code should exists in the first place, aren't we just wasting time that could be spent on finishing the proper way of doing that via the API?

(I mean, introducing little tech debt is probably cheaper than designing technical debt to introduce :) .)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Fryguy I agree with @himdel that's the the reason i did not change Add new provider form in this PR. Even if we decide to make changes to existing form , i would prefer a separate follow up PR, this PR is getting way too big.

Copy link
Member

Choose a reason for hiding this comment

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

@h-kataria happy to help add permissions and supported_for_create to the provider, would rather that than hard coding Foreman

Copy link
Member

@Fryguy Fryguy Apr 28, 2020

Choose a reason for hiding this comment

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

If these are meant to be temporary, then I'm fine with that, especially as the goal is to move towards pluggable DDF for this feature.

aren't we just wasting time that could be spent on finishing the proper way of doing that via the API?

Isn't that what we are doing with this discussion? It's clear there's a general design issue with respect to provider/manager split, so discussing it is something we need to do anyway for the API. It helps to have that discussion in context with the code that is actually consuming it, hence me bringing in @agrare, as he has some opinions on how best to deal with the provider/manager split.

Also this thread started as me noticing Foreman in the code when the purpose of the PR is to remove Foreman from the code, so it stood out. It's not clear from the code that the purpose for the method is to drive the drop-down for adding a new provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

line of code in question does not drive the drop-down as currently there is no Type drop-down in the UI. Currently users can only add a Foreman Provider and this line is providing class for the new record, when the Add button is UI is pressed.

end

def breadcrumbs_options
{
:breadcrumbs => [
{:title => _("Configuration Management")},
{:title => _("Providers"), :url => controller_url},
],
}
end

def set_redirect_vars
@in_a_form = true
@redirect_controller = "ems_configuration"
@redirect_id = @provider_manager.id if @provider_manager.try(:id)
@refresh_partial = @provider_manager.try(:id) ? "edit" : "new"
end

menu_section :conf
end
Loading