Bug 1614864 - cancel_requested flag stored in options gets overridden
Summary: cancel_requested flag stored in options gets overridden
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Automate
Version: 5.9.4
Hardware: Unspecified
OS: Unspecified
high
urgent
Target Milestone: GA
: 5.10.0
Assignee: Bill Wei
QA Contact: Kedar Kulkarni
URL:
Whiteboard:
: 1630013 (view as bug list)
Depends On:
Blocks: 1610533 1616076
TreeView+ depends on / blocked
 
Reported: 2018-08-10 14:51 UTC by Fabien Dupont
Modified: 2019-02-12 16:51 UTC (History)
12 users (show)

Fixed In Version: 5.10.0.23
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1616076 (view as bug list)
Environment:
Last Closed: 2019-02-12 16:51:42 UTC
Category: ---
Cloudforms Team: CFME Core
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Fabien Dupont 2018-08-10 14:51:41 UTC
Description of problem:
During migration cancellation tests, it has been observed that the change to the task options hash could be overridden, hence not taking into account the cancellation request.

The UI calls the API to add a key/value pair in the options hash to request cancellation. Because the options hash is cached, the automation code works on a copy of the hash that may not contains that key/value pair and when calling set_option to update the options hash, it will override with its own copy.

Version-Release number of selected component (if applicable): 5.9.4.3

How reproducible: Random

Steps to Reproduce:
1. Start a migration
2. Request cancellation from the UI
3. Check the task object

Actual results: Sometimes, the cancel_requested key is present, sometimes it's not.

Expected results: Changes to an option should not be subject to race condition.

Additional info:

Comment 3 Bill Wei 2018-08-13 21:30:39 UTC
The solution is to move the flag out of options into its own column. Reassign to Hui who provides the development work.

Comment 6 Greg McCullough 2018-08-14 15:17:22 UTC
For 5.9.z release we will be going with the fix from https://github.com/ManageIQ/manageiq-automation_engine/pull/211 to resolve this issue.

The PRs mentioned in comment 4 and 5 will be moved to upstream fixes only.

Moving the issue to Bill and will adjust PR labels.

Comment 7 CFME Bot 2018-08-14 21:23:22 UTC
New commit detected on ManageIQ/manageiq-automation_engine/master:

https://github.com/ManageIQ/manageiq-automation_engine/commit/20ae4d68aa3f38348767f417f88c1df3bebd9406
commit 20ae4d68aa3f38348767f417f88c1df3bebd9406
Author:     Bill Wei <bilwei>
AuthorDate: Fri Aug 10 16:53:22 2018 -0400
Commit:     Bill Wei <bilwei>
CommitDate: Fri Aug 10 16:53:22 2018 -0400

    lock model object when  #set_option

    As a temporary solution. It has a performance penalty. Ideally only one thread should update the options field.
    fixes https://bugzilla.redhat.com/show_bug.cgi?id=1614864

 lib/miq_automation_engine/service_models/mixins/miq_ae_service_miq_request_mixin.rb | 6 +-
 spec/service_models/miq_ae_service_miq_request_spec.rb | 16 +-
 2 files changed, 10 insertions(+), 12 deletions(-)

Comment 9 Kedar Kulkarni 2018-10-08 20:57:58 UTC
Simple Browser Refresh operation after cancel migration, wipes out the fact that we requested cancel migration.

Comment 13 Kedar Kulkarni 2018-10-09 14:40:35 UTC
https://github.com/ManageIQ/manageiq-v2v/issues/699

Comment 14 CFME Bot 2018-10-18 13:33:13 UTC
New commit detected on ManageIQ/manageiq-v2v/hammer:

https://github.com/ManageIQ/manageiq-v2v/commit/b0f6f4a446e148078fa1f1f8227dd804beaf0b88
commit b0f6f4a446e148078fa1f1f8227dd804beaf0b88
Author:     Michael Ro <mikerodev>
AuthorDate: Thu Oct 18 08:19:05 2018 -0400
Commit:     Michael Ro <mikerodev>
CommitDate: Thu Oct 18 08:19:05 2018 -0400

    Merge pull request #708 from AparnaKarve/fix_cancel_issues

    [699] Fix cancellation issues based on new backend changes

    (cherry picked from commit c45b98b81cbf38a5cc2d0f43b0fd2dc13ce0e4e7)

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

 app/javascript/react/screens/App/Plan/components/PlanRequestDetailList/PlanRequestDetailList.js | 31 +-
 app/javascript/react/screens/App/Plan/helpers.js | 2 +-
 2 files changed, 23 insertions(+), 10 deletions(-)

Comment 18 Fabien Dupont 2018-11-06 14:50:50 UTC
Last PR merged. Moving to POST.
Should be good in next 5.10 build.

Comment 19 Brett Thurber 2018-11-06 15:03:22 UTC
*** Bug 1630013 has been marked as a duplicate of this bug. ***

Comment 20 CFME Bot 2018-11-06 15:26:22 UTC
New commit detected on ManageIQ/manageiq-content/hammer:

https://github.com/ManageIQ/manageiq-content/commit/dc010c59c769cd2e9bd0fa082b1be570d892333d
commit dc010c59c769cd2e9bd0fa082b1be570d892333d
Author:     Greg McCullough <gmccullo>
AuthorDate: Tue Nov  6 09:39:33 2018 -0500
Commit:     Greg McCullough <gmccullo>
CommitDate: Tue Nov  6 09:39:33 2018 -0500

    Merge pull request #457 from fdupont-redhat/v2v_use_cancelation_status

    Use task cancelation_status to trigger cancelation

    (cherry picked from commit 2160a0e410fa196b6a0061a62aa36e2705586c8e)

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

 content/automate/ManageIQ/System/CommonMethods/MiqAe.class/__methods__/weightedupdatestatus.rb | 2 +-
 content/automate/ManageIQ/Transformation/Common.class/__methods__/assesstransformation.rb | 2 +-
 spec/content/automate/ManageIQ/Transformation/Common.class/__methods__/assesstransformation_spec.rb | 9 +-
 3 files changed, 4 insertions(+), 9 deletions(-)

Comment 21 Kedar Kulkarni 2018-11-08 17:53:51 UTC
On 5.10.0.23 I was able to cancel in progress migrations.


Note You need to log in before you can comment on or make changes to this bug.