Allow the suspension of cloud, infra and physical infra providers #4269

skateman · 2018-07-10T11:12:47Z

This is a followup on #4012 as @romanblanco is not available. Except of adding the necessary toolbar buttons, there are a few screens which display zones, these have been adjusted to not display the __maintenance__ zone.

Parent issue: ManageIQ/manageiq#17489
Depends on: ManageIQ/manageiq#17452
RFE BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1455145

@miq-bot add_label pending core
@miq-bot add_reviewer @slemrmartin

miq-bot · 2018-07-10T11:13:11Z

@skateman 'slemrmartin' is an invalid reviewer, ignoring...

miq-bot · 2018-10-30T14:11:42Z

This pull request is not mergeable. Please rebase and repush.

slemrmartin · 2018-12-19T08:48:28Z

@skateman Changes in core after this implementation:

ExtManagementSystem.backup_zone becomes ExtManagementSystem.zone_before_pause
Zone::MAINTENANCE_ZONE_NAME becomes MiqRegion.my_region.maintenance_zone.name

EDIT: use Zone.maintenance_zone.name instead, it's cached version of ^^

skateman · 2018-12-19T11:06:01Z

@slemrmartin what about Zone.visible?

slemrmartin · 2018-12-19T11:53:28Z

Zone.visible is available, but it's recommended not to make equal zone's visibility and Zone.maintenance_zone. Maintenance zone is only one of possible invisible zones.

skateman · 2018-12-19T16:11:19Z

@slemrmartin can you please test this if it does what you want? 😉

skateman · 2018-12-21T19:13:22Z

@miq-bot rm_label pending core

Fryguy · 2018-12-21T19:19:14Z

app/helpers/ems_cloud_helper/textual_summary.rb

@@ -146,7 +146,8 @@ def textual_security_groups
  end

  def textual_zone
-    {:label => _("Managed by Zone"), :icon => "pficon pficon-zone", :value => @record.zone.name}
+    zone = @record.zone.visible? ? @record.zone : @record.backup_zone


This method is not called backup_zone anymore...I think it's zone_before_pause

I feel like this section is wrong. This is showing the EMS as being in a zone that it's not really in, which can be confusing to end users as well as us trying to diagnose customer issues. IMO, we should just keep the original code and show that it's in the Maintenance Zone or wherever. Or, clarify it better with something like the following...

zone = @record.zone_before_pause? ? "#{@record.zone.name} (originally in #{@record.zone_before_pause.name})" : @record.zone.name

Same in the copy-paste areas below.

app/presenters/tree_builder_ops.rb

Fryguy · 2018-12-21T19:21:47Z

Most of this looks good to me, however I don't agree with some of the decisions on when to honor Zone#visible. See ManageIQ/manageiq#17489 (comment) for my entire opinion on the presentation of these.

slemrmartin · 2019-01-02T13:44:54Z

There is some problem I encountered only once - default zone had visible set to nil. But I cannot reproduce it, don't merge it now please

slemrmartin · 2019-01-02T14:45:18Z

app/helpers/ems_container_helper/textual_summary.rb

@@ -71,7 +71,12 @@ def textual_port
  end

  def textual_zone
-    {:label => _("Managed by Zone"), :icon => "pficon pficon-zone", :value => @record.zone.name}
+    zone = if @record.zone.visible?


if @record.zone.visible? elsif @record.zone == Zone.maintenance_zone end

is better, the same for other helpers below

Hm, what other cases can happen? Because this seems to me as fixing possible backend issues on the frontend 😞

Your code doesn't cause error, but invisible zone won't have zone_before_pause relation set every time.

Current condition could confuse other developers that if zone.visible is false, then zone_before_pause is always there.

martinpovolny · 2019-01-08T15:50:44Z

Ok, then just:

Please, move the duplicated textual summary code to a mixin.

martinpovolny · 2019-01-08T15:52:11Z

Oh. Ok, you did that.

So how well is this one tested? Did you test this in the UI, @slemrmartin ?

skateman · 2019-01-08T15:52:12Z

@martinpovolny already done

slemrmartin · 2019-01-09T09:01:57Z

So how well is this one tested? Did you test this in the UI, @slemrmartin ?

@martinpovolny not the last version, I'll do it asap

skateman · 2019-01-11T13:39:30Z

@slemrmartin any update on this?

slemrmartin · 2019-01-11T14:18:36Z

@slemrmartin any update on this?

Sorry I worked on another PR, I'll look after weekend

slemrmartin · 2019-01-15T09:58:41Z

Generally it's working (tested on Amazon)

UI quadicon changed
workers are disabled
EMSes are moved to maintenance zone
alert is shown in provider/instance detail and cannot be dismissed.
new provider and server cannot be moved to Maintenance zone (not visible in select)

@skateman great job, only small changes needed:

when EMS paused, toolbar button "Refresh relationships and Power states" (in detail page) should be disabled.
On Configuration page, Zone.description is shown (nice name)
On provider detail page, Zone.name is shown (ugly name - __maintenance__cdbe9712-9e10-4796-8a04-fd0460312bce (originally in default)). @martinpovolny what about change it to Zone.description here too?
instance/vm toolbar doesn't change when provider is paused (although alert in instance detail is present). EDIT: alert in instances list is present, alert in instance detail is not present)

More comments in issue

(cherry picked from commit c214b72) (cherry picked from commit 04eab55) (cherry picked from commit 410cc8d)

(cherry picked from commit 2961d88)

(cherry picked from commit 9a63cd1)

(cherry picked from commit 81bc35e)

slemrmartin

👍
Additional functionalities will be done in next PR (described in related Issue)

miq-bot · 2019-01-15T16:04:28Z

Checked commits skateman/manageiq-ui-classic@7665ca6~...34d26d9 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
34 files checked, 13 offenses detected

**

💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:

warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

app/helpers/application_helper/toolbar/ems_cloud_center.rb

❗ - Line 42, 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 51, 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/ems_clouds_center.rb

❗ - Line 53, 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 64, 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/ems_infra_center.rb

❗ - 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.
❗ - Line 50, 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/ems_infras_center.rb

❗ - Line 53, 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 64, 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/ems_physical_infra_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/ems_physical_infras_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.

skateman · 2019-01-15T16:25:08Z

@martinpovolny coverage should be okay now 😉

miq-bot added the pending core label Jul 10, 2018

This was referenced Jul 10, 2018

Suspend provider ManageIQ/manageiq#17489

Closed

Suspend providers ManageIQ/manageiq#17452

Merged

skateman changed the title ~~Allow the suspension of cloud, infra and physical infra providers~~ [WIP] Allow the suspension of cloud, infra and physical infra providers Jul 11, 2018

miq-bot added the wip label Jul 11, 2018

skateman force-pushed the suspend-provider-zones branch from d71fb2a to fbd5e21 Compare July 11, 2018 15:37

skateman changed the title ~~[WIP] Allow the suspension of cloud, infra and physical infra providers~~ Allow the suspension of cloud, infra and physical infra providers Jul 13, 2018

miq-bot removed the wip label Jul 13, 2018

romanblanco mentioned this pull request Jul 16, 2018

[WIP] Adding toolbar buttons to suspend a provider #4012

Closed

slemrmartin mentioned this pull request Oct 1, 2018

[WIP] Make provider's enable/disable methods protected ManageIQ/manageiq#18037

Closed

3 tasks

miq-bot added the unmergeable label Oct 30, 2018

skateman force-pushed the suspend-provider-zones branch 2 times, most recently from d10f34a to 6665066 Compare December 19, 2018 13:50

miq-bot removed the unmergeable label Dec 19, 2018

miq-bot removed the pending core label Dec 21, 2018

Fryguy reviewed Dec 21, 2018

View reviewed changes

app/presenters/tree_builder_ops.rb Show resolved Hide resolved

skateman force-pushed the suspend-provider-zones branch 2 times, most recently from cce5ae7 to 4978042 Compare January 2, 2019 12:11

slemrmartin reviewed Jan 2, 2019

View reviewed changes

skateman force-pushed the suspend-provider-zones branch from 4978042 to d056dfa Compare January 2, 2019 15:24

romanblanco and others added 7 commits January 15, 2019 13:20

Added Pause/Resume buttons for cloud, infra, physical providers

7665ca6

(cherry picked from commit c214b72) (cherry picked from commit 04eab55) (cherry picked from commit 410cc8d)

Updated operations to pause/resume a provider

6ebbf2c

(cherry picked from commit 2961d88)

Added warning about providers state

b098f08

(cherry picked from commit 9a63cd1)

Do not display the hidden zone for suspended providers in the UI

8a3150d

(cherry picked from commit 81bc35e)

Display the original zone on summary screens of suspended providers

5d1a13f

Hide the zone in the provider edit form if the provider is suspended

3c77864

Performance optimizations for Zone.visible queries for tasks and OPS

08c8342

skateman force-pushed the suspend-provider-zones branch from b02fad8 to baaae77 Compare January 15, 2019 12:29

slemrmartin approved these changes Jan 15, 2019

View reviewed changes

skateman force-pushed the suspend-provider-zones branch from baaae77 to b761e77 Compare January 15, 2019 14:20

skateman added 2 commits January 15, 2019 17:01

Move the textual_zone method into a separate mixin

c0f5ffa

Disable the provider refresh button for suspended providers

34d26d9

skateman force-pushed the suspend-provider-zones branch from b761e77 to 34d26d9 Compare January 15, 2019 16:01

martinpovolny merged commit e44cf9a into ManageIQ:master Jan 17, 2019

martinpovolny added this to the Sprint 103 Ending Jan 21, 2019 milestone Jan 17, 2019

martinpovolny added hammer/no enhancement compute/cloud compute/infrastructure labels Jan 17, 2019

skateman deleted the suspend-provider-zones branch January 17, 2019 13:26

carbonin mentioned this pull request Feb 12, 2019

Fix database connections page #5235

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow the suspension of cloud, infra and physical infra providers #4269

Allow the suspension of cloud, infra and physical infra providers #4269

skateman commented Jul 10, 2018 •

edited

Loading

miq-bot commented Jul 10, 2018

miq-bot commented Oct 30, 2018

slemrmartin commented Dec 19, 2018 •

edited

Loading

skateman commented Dec 19, 2018

slemrmartin commented Dec 19, 2018

skateman commented Dec 19, 2018

skateman commented Dec 21, 2018

Fryguy Dec 21, 2018 •

edited

Loading

Fryguy Dec 21, 2018

Fryguy commented Dec 21, 2018

slemrmartin commented Jan 2, 2019

slemrmartin Jan 2, 2019

skateman Jan 2, 2019

slemrmartin Jan 2, 2019 •

edited

Loading

martinpovolny commented Jan 8, 2019

martinpovolny commented Jan 8, 2019

skateman commented Jan 8, 2019

slemrmartin commented Jan 9, 2019 •

edited

Loading

skateman commented Jan 11, 2019

slemrmartin commented Jan 11, 2019

slemrmartin commented Jan 15, 2019 •

edited

Loading

slemrmartin left a comment

miq-bot commented Jan 15, 2019

skateman commented Jan 15, 2019

Allow the suspension of cloud, infra and physical infra providers #4269

Allow the suspension of cloud, infra and physical infra providers #4269

Conversation

skateman commented Jul 10, 2018 • edited Loading

miq-bot commented Jul 10, 2018

miq-bot commented Oct 30, 2018

slemrmartin commented Dec 19, 2018 • edited Loading

skateman commented Dec 19, 2018

slemrmartin commented Dec 19, 2018

skateman commented Dec 19, 2018

skateman commented Dec 21, 2018

Fryguy Dec 21, 2018 • edited Loading

Choose a reason for hiding this comment

Fryguy Dec 21, 2018

Choose a reason for hiding this comment

Fryguy commented Dec 21, 2018

slemrmartin commented Jan 2, 2019

slemrmartin Jan 2, 2019

Choose a reason for hiding this comment

skateman Jan 2, 2019

Choose a reason for hiding this comment

slemrmartin Jan 2, 2019 • edited Loading

Choose a reason for hiding this comment

martinpovolny commented Jan 8, 2019

martinpovolny commented Jan 8, 2019

skateman commented Jan 8, 2019

slemrmartin commented Jan 9, 2019 • edited Loading

skateman commented Jan 11, 2019

slemrmartin commented Jan 11, 2019

slemrmartin commented Jan 15, 2019 • edited Loading

slemrmartin left a comment

Choose a reason for hiding this comment

miq-bot commented Jan 15, 2019

skateman commented Jan 15, 2019

skateman commented Jul 10, 2018 •

edited

Loading

slemrmartin commented Dec 19, 2018 •

edited

Loading

Fryguy Dec 21, 2018 •

edited

Loading

slemrmartin Jan 2, 2019 •

edited

Loading

slemrmartin commented Jan 9, 2019 •

edited

Loading

slemrmartin commented Jan 15, 2019 •

edited

Loading