Bug 1368058
Summary: | The counter ae_state_retries is not incremented if $evm.root['ae_result'] = 'retry' is set in a state machine on_exit method | ||||||
---|---|---|---|---|---|---|---|
Product: | Red Hat CloudForms Management Engine | Reporter: | Peter McGowan <pmcgowan> | ||||
Component: | Automate | Assignee: | mkanoor | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Milan Falešník <mfalesni> | ||||
Severity: | high | Docs Contact: | |||||
Priority: | high | ||||||
Version: | 5.6.0 | CC: | cpelland, jhardy, mkanoor, obarenbo, simaishi, tfitzger | ||||
Target Milestone: | GA | Keywords: | TestOnly, ZStream | ||||
Target Release: | 5.8.0 | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Whiteboard: | automate | ||||||
Fixed In Version: | 5.8.0.0 | Doc Type: | If docs needed, set a value | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | |||||||
: | 1368176 1413769 (view as bug list) | Environment: | |||||
Last Closed: | 2017-06-12 16:45:41 UTC | Type: | Bug | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 1368176, 1413769 | ||||||
Attachments: |
|
Description
Peter McGowan
2016-08-18 08:59:09 UTC
Peter, The retry_logic was meant only for the main state and not for the on_entry, on_exit or on_error. The problem I guess is because we allow the scripts to directly set the root['ae_result'], which gets processed differently. I think for each of the methods we need to validate what they can set on_entry -> ae_result => ok | error | skip, ae_next_state => state_name on_exit -> ae_result => ok | error on_error -> ae_result -> ok | error | continue, ae_next_state => state_name main state -> ae_result -> ok| error | retry | restart, ae_next_state => state_name The problem with this enforcement is some of the older models have the actual state embedded in the on_entry, because we lacked the METHOD:: If we allow any method to set any of the variables it could become chaotic and indeterministic, so we have to be careful with this change. Thanks, Madhu I agree that we should change the older models such as ProvisionRequestApproval to use the METHOD:: style. I think that on_exit is the natural place to perform state post-processing, and a valid outcome of post processing logic might be that we need to retry the state. From a usability point of view it would be nice to have the following options: on_entry -> ae_result => ok | error | skip, ae_next_state => state_name on_exit -> ae_result => ok | error | retry on_error -> ae_result -> abort | continue, ae_next_state => state_name main state -> ae_result -> ok | error | retry | restart, ae_next_state => state_name Peter New commit detected on ManageIQ/manageiq/master: https://github.com/ManageIQ/manageiq/commit/098176af9e7cee3ca57fa5f8babe46ad4a26214f commit 098176af9e7cee3ca57fa5f8babe46ad4a26214f Author: Madhu Kanoor <mkanoor> AuthorDate: Tue Jan 3 13:46:12 2017 -0500 Commit: Madhu Kanoor <mkanoor> CommitDate: Tue Jan 3 13:46:12 2017 -0500 Increment the ae_state_retries when on_exit sets retry https://bugzilla.redhat.com/show_bug.cgi?id=1368058 .../engine/miq_ae_state_machine.rb | 8 +++++++- .../engine/miq_ae_state_machine_steps_spec.rb | 24 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) Created attachment 1244231 [details]
Reproducer domain
To reproduce:
1) Import the attached file, it will create a domain called OnExitRetry
2) Enable the domain
3) Go to Automate / Simulation
4) Simulate Request with instance OnExitRetry, execute methods
5) Click submit, open the tree on right and expand ae_state_retries
It should be 1 by now and subsequent clicks on Retry should raise the number if it works properly.
Verified in 5.8.0.8, the ae_state_retries increases. |