Allow pausing/resuming foreman and ansible tower providers #5173

skateman · 2019-01-21T13:41:50Z

First I split up the EmsCommon a little and reorganized it into submodules, so the pause/resume can included into the Foreman/Ansible code separately. After this I was able to add the actions into X_BUTTON_ALLOWED_ACTIONS and then define the pause/resume methods in the controllers.

If a provider is disabled, it shows for every its child a warning:

Depends on: ManageIQ/manageiq#18375
Parent issue: ManageIQ/manageiq#17489

@miq-bot add_label hammer/no, providers/ansible_tower, providers/foreman, pending core
@miq-bot add_reviewer @martinpovolny
@miq-bot add_reviewer @himdel
@miq-bot add_reviewer @mzazrivec

miq-bot · 2019-01-21T13:42:13Z

@skateman Cannot apply the following labels because they are not recognized: providers/ansible_tower, providers/foreman

skateman · 2019-01-21T13:42:19Z

app/controllers/provider_foreman_controller.rb

@@ -189,6 +189,18 @@ def provider_active_tree?

  private

+  def provider_foreman_pause
+    @record = find_record(ExtManagementSystem, params[:id])
+    @record.pause! # TODO: reload toolbar button enablement after the pause


@martinpovolny I need your help about this, if it is even possible and if not, how else should I do this?

You need to queue it. There should be plenty of examples using initiate_wait_for_task ;).

EDIT: looks like I was wrong in this case, sorry :)

martinpovolny · 2019-01-22T08:00:10Z

app/controllers/automation_manager_controller.rb

@@ -8,6 +8,7 @@ class AutomationManagerController < ApplicationController
  include Mixins::GenericSessionMixin
  include Mixins::ManagerControllerMixin
  include Mixins::ExplorerPresenterMixin
+  include EmsCommon


This one frightens me. EmsCommon is a hydra. I am more for splitting it and cleaning up rather then adding it to more places.

martinpovolny · 2019-01-22T08:01:04Z

app/controllers/ems_common.rb

+    if defined?(textual_group_list)
+      helper_method :textual_group_list
+      private :textual_group_list
+    end


This is it. You are adding exception to a stuff that is included into far too many places :-(

My red light is on.

martinpovolny · 2019-01-22T08:01:34Z

app/controllers/provider_foreman_controller.rb

@@ -8,6 +8,7 @@ class ProviderForemanController < ApplicationController
  include Mixins::GenericSessionMixin
  include Mixins::ManagerControllerMixin
  include Mixins::ExplorerPresenterMixin
+  include EmsCommon


martinpovolny · 2019-01-22T08:02:15Z

app/helpers/provider_foreman_helper.rb

+  #              end
+
+  #   provider.try(:enabled?) == false
+  # end


forgot to remove this?

skateman · 2019-01-22T08:42:37Z

Okay @martinpovolny I'll move around and split up the EmsCommon, extract the pause/resume into a submodule and load only that wen it is possible.

@miq-bot add_label wip

skateman · 2019-01-22T12:24:33Z

@slemrmartin can you please test this?

skateman · 2019-01-22T12:43:20Z

@martinpovolny I hope you can reset your red lights, this should be okay now.

miq-bot · 2019-01-31T14:01:59Z

Checked commits skateman/manageiq-ui-classic@c48d637~...6b14c5a with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
28 files checked, 42 offenses detected

app/controllers/mixins/ems_common.rb

app/controllers/mixins/ems_common/angular.rb

❗ - Line 86, Col 7 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

app/controllers/mixins/ems_common/core.rb

❗ - Line 22, Col 11 - Style/MultilineIfModifier - Favor a normal if-statement over a modifier clause in a multiline statement.
❗ - Line 28, Col 9 - Style/MultipleComparison - Avoid comparing a variable with multiple items in a conditional, use Array#include? instead.
❗ - Line 48, Col 61 - Layout/ExtraSpacing - Unnecessary spacing detected.
❗ - Line 49, Col 13 - Style/RescueStandardError - Avoid rescuing without specifying an error class.

app/controllers/mixins/ems_common/pause_resume.rb

❗ - Line 34, Col 34 - Rails/DynamicFindBy - Use find_by instead of dynamic find_by_id.

app/helpers/application_helper/toolbar/automation_manager_provider_center.rb

❗ - Line 34, Col 35 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
❗ - Line 43, Col 35 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

app/helpers/application_helper/toolbar/automation_manager_providers_center.rb

❗ - Line 51, Col 32 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
❗ - Line 62, Col 32 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

app/helpers/application_helper/toolbar/configuration_manager_provider_center.rb

❗ - Line 17, Col 60 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
❗ - Line 32, Col 35 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
❗ - Line 41, Col 35 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

app/helpers/application_helper/toolbar/configuration_manager_providers_center.rb

❗ - Line 48, Col 32 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
❗ - Line 59, Col 32 - Layout/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.

slemrmartin · 2019-02-04T09:50:23Z

@skateman
Now cloud and infra managers can be updated (through edit dialog) when paused, with no side-effects, that's correct.
Ansible tower is not working in the same way, it's not possible to update, validation error is still there.
EDIT: I missed your comment, I'll check it where is zone added to update

slemrmartin · 2019-02-04T12:35:08Z

@skateman zone's update of EMS by provider fixed in ManageIQ/manageiq-providers-ansible_tower#155

skateman · 2019-02-18T06:19:13Z

@slemrmartin after your provider fix, does it work as it should?

slemrmartin · 2019-02-19T08:40:44Z

@skateman yes, now waiting for review

skateman · 2019-02-20T03:52:56Z

@miq-bot rm_label pending core
@slemrmartin please retest if it works as you wanted it

martinpovolny · 2019-02-20T09:26:09Z

app/controllers/mixins/ems_common/core.rb

+
+        return if emss.empty?
+
+        if task == "destroy"


10 ... 28 should be a new method

martinpovolny · 2019-02-20T09:27:14Z

app/controllers/mixins/ems_common/core.rb

+             :product => Vmdb::Appliance.PRODUCT_NAME,
+             :model   => ui_lookup(:table => table_name),
+             :models  => ui_lookup(:tables => table_name)}) if @flash_array.nil?
+        elsif task == "pause_ems" || task == "resume_ems"


28 ... 43 should be a new method

Yes, it should be 😉 but definitely not in this PR, if you look at the commits, this is just reorganizing some pieces of code, not inventing anything new.

martinpovolny · 2019-02-20T09:27:36Z

app/controllers/mixins/ems_common/core.rb

+            ems.pause! if action == "pause"
+            ems.resume! if action == "resume"
+          end
+        else


43 .. 62 should be a new method

Same as above

martinpovolny · 2019-02-20T09:35:11Z

spec/helpers/application_helper_spec.rb

@@ -1352,4 +1352,47 @@ def controller
      expect(calculate_toolbars).to include("center_tb" => "container_projects_center")
    end
  end
+
+  describe '#provider_paused?' do


So there's test coverage for the new method.

Do you think that is sufficient for such a big PR?

As the size of the PR comes from cut&paste, I don't think this is such a big PR...

skateman · 2019-02-25T07:25:51Z

@martinpovolny your questions have been answered, can you please merge this?

…mixin) this was moved to mixins/ems_common/angular.rb in ManageIQ#5173 the file is dead now, removing

miq-bot added hammer/no pending core labels Jan 21, 2019

miq-bot requested review from martinpovolny, himdel and mzazrivec January 21, 2019 13:42

skateman commented Jan 21, 2019

View reviewed changes

miq-bot added the wip label Jan 21, 2019

skateman changed the title ~~[WIP] Add buttons for pausing/resuming foreman and ansible tower providers~~ [WIP] Allow pausing/resuming foreman and ansible tower providers Jan 21, 2019

skateman force-pushed the pause-resume-foreman-ansible branch 5 times, most recently from f73e9ad to 82509f2 Compare January 21, 2019 18:19

skateman changed the title ~~[WIP] Allow pausing/resuming foreman and ansible tower providers~~ Allow pausing/resuming foreman and ansible tower providers Jan 21, 2019

miq-bot removed the wip label Jan 21, 2019

martinpovolny reviewed Jan 22, 2019

View reviewed changes

skateman force-pushed the pause-resume-foreman-ansible branch from 82509f2 to a8434f1 Compare January 22, 2019 08:41

miq-bot changed the title ~~Allow pausing/resuming foreman and ansible tower providers~~ [WIP] Allow pausing/resuming foreman and ansible tower providers Jan 22, 2019

miq-bot added the wip label Jan 22, 2019

skateman force-pushed the pause-resume-foreman-ansible branch from adc76b0 to 2b12a65 Compare January 22, 2019 09:27

skateman changed the title ~~[WIP] Allow pausing/resuming foreman and ansible tower providers~~ Allow pausing/resuming foreman and ansible tower providers Jan 22, 2019

miq-bot removed the wip label Jan 22, 2019

skateman force-pushed the pause-resume-foreman-ansible branch from bee6095 to f5d5c3a Compare January 22, 2019 11:49

skateman added 4 commits January 31, 2019 14:58

Add partial and its presenter display mechanism for provider pause

0f5c1c6

Display provider disabled warning for ansible tower and foreman

67ec7a5

Do not send the zone with the form when editing a paused provider

41e139f

Do not display the zone when editing a paused foreman/ansible prov

6b14c5a

skateman force-pushed the pause-resume-foreman-ansible branch from 0f86afd to 6b14c5a Compare January 31, 2019 13:59

miq-bot removed the unmergeable label Jan 31, 2019

This was referenced Feb 4, 2019

Changed provider zone when EMS paused/resumed ManageIQ/manageiq-providers-ansible_tower#155

Merged

Suspend provider ManageIQ/manageiq#17489

Closed

miq-bot removed the pending core label Feb 20, 2019

martinpovolny reviewed Feb 20, 2019

View reviewed changes

martinpovolny merged commit 8f187f3 into ManageIQ:master Feb 26, 2019

martinpovolny self-assigned this Feb 26, 2019

martinpovolny added this to the Sprint 106 Ending Mar 4, 2019 milestone Feb 26, 2019

skateman deleted the pause-resume-foreman-ansible branch February 26, 2019 09:13

martinpovolny mentioned this pull request Feb 26, 2019

Split method process_emss. #5276

Merged

martinpovolny added automation/ansible automation/ansible tower and removed automation/ansible labels Feb 26, 2019

himdel mentioned this pull request Jun 5, 2019

Remove unused EmsCommonAngular mixin (replaced by EmsCommon::Angular) #5674

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow pausing/resuming foreman and ansible tower providers #5173

Allow pausing/resuming foreman and ansible tower providers #5173

skateman commented Jan 21, 2019 •

edited

Loading

miq-bot commented Jan 21, 2019

skateman Jan 21, 2019

himdel Jan 21, 2019 •

edited

Loading

martinpovolny Jan 22, 2019

martinpovolny Jan 22, 2019

martinpovolny Jan 22, 2019

martinpovolny Jan 22, 2019

skateman Jan 22, 2019

skateman commented Jan 22, 2019

skateman commented Jan 22, 2019

skateman commented Jan 22, 2019

miq-bot commented Jan 31, 2019

slemrmartin commented Feb 4, 2019 •

edited

Loading

slemrmartin commented Feb 4, 2019

skateman commented Feb 18, 2019

slemrmartin commented Feb 19, 2019

skateman commented Feb 20, 2019 •

edited

Loading

martinpovolny Feb 20, 2019 •

edited

Loading

martinpovolny Feb 20, 2019

skateman Feb 20, 2019

martinpovolny Feb 20, 2019

skateman Feb 20, 2019

martinpovolny Feb 20, 2019

skateman Feb 20, 2019

skateman commented Feb 25, 2019

Allow pausing/resuming foreman and ansible tower providers #5173

Allow pausing/resuming foreman and ansible tower providers #5173

Conversation

skateman commented Jan 21, 2019 • edited Loading

miq-bot commented Jan 21, 2019

Choose a reason for hiding this comment

himdel Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skateman commented Jan 22, 2019

skateman commented Jan 22, 2019

skateman commented Jan 22, 2019

miq-bot commented Jan 31, 2019

slemrmartin commented Feb 4, 2019 • edited Loading

slemrmartin commented Feb 4, 2019

skateman commented Feb 18, 2019

slemrmartin commented Feb 19, 2019

skateman commented Feb 20, 2019 • edited Loading

martinpovolny Feb 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skateman commented Feb 25, 2019

skateman commented Jan 21, 2019 •

edited

Loading

himdel Jan 21, 2019 •

edited

Loading

slemrmartin commented Feb 4, 2019 •

edited

Loading

skateman commented Feb 20, 2019 •

edited

Loading

martinpovolny Feb 20, 2019 •

edited

Loading