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.

Sign up for GitHub

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

Jump to bottom

Ansible Playbook Service add on_error method. #14583

Merged
merged 2 commits into from
Apr 4, 2017
Merged

Ansible Playbook Service add on_error method. #14583

merged 2 commits into from
Apr 4, 2017

Conversation

tinaafitz
Copy link
Member

@tinaafitz tinaafitz commented Mar 30, 2017 •
edited by gmcculloug
Loading

Enhance Generic Service state machine error handling. Cleanup after provisioning and retirement failures.

https://www.pivotaltracker.com/story/show/142796573

Related PR ManageIQ/manageiq-content#85

@tinaafitz
Copy link
Member Author

@miq-bot add_label enhancement, services

@gmcculloug gmcculloug requested a review from bzwei March 30, 2017 18:56
gmcculloug
gmcculloug reviewed Mar 30, 2017
View reviewed changes
app/models/service_ansible_playbook.rb
@@ -46,6 +46,12 @@ def postprocess(action)
delete_inventory(action) unless use_default_inventory?(hosts)
end

def on_error(action)
Copy link
Member

Choose a reason for hiding this comment

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

If this is only intended to be called from automate should we have a more descriptive method name?

Copy link
Contributor

Choose a reason for hiding this comment

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

All methods are intended to be called from automate. I am ok with this name.

@bzwei
Copy link
Contributor

bzwei commented Mar 30, 2017

@tinaafitz can you add spec for this method?

bzwei
bzwei reviewed Mar 30, 2017
View reviewed changes
spec/models/service_ansible_playbook_spec.rb Outdated
it 'handles provisioning error' do
expect(executed_service).to receive(:postprocess)
executed_service.on_error(action)
expect(service.retirement_state).to be_nil
Copy link
Contributor

Choose a reason for hiding this comment

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

expect(executed_service.retirement_state).to be_nil

bzwei
bzwei reviewed Mar 30, 2017
View reviewed changes
spec/models/service_ansible_playbook_spec.rb Outdated
it 'handles retirement error' do
executed_service.update_attributes(:retirement_state => 'Retiring')
expect(executed_service).to receive(:postprocess)
executed_service.on_error(retirement_action)
Copy link
Contributor

Choose a reason for hiding this comment

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

we can directly call executed_service.on_error(ResourceAction::RETIREMENT)

@miq-bot
Copy link
Member

miq-bot commented Mar 31, 2017

Checked commits tinaafitz/manageiq@689c058~...d47f71d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. 🍰

@gmcculloug gmcculloug requested a review from mkanoor March 31, 2017 13:14
@tinaafitz tinaafitz mentioned this pull request Mar 31, 2017
Merged
@gmcculloug
Copy link
Member

@mkanoor Please review

mkanoor
mkanoor approved these changes Apr 4, 2017
View reviewed changes
Copy link
Contributor

@mkanoor mkanoor left a comment

Choose a reason for hiding this comment

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

Looks good, we need to communicate to QE how we test this error scenario.

@gmcculloug gmcculloug merged commit d7f9fe0 into ManageIQ:master Apr 4, 2017
@gmcculloug gmcculloug added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 4, 2017
simaishi pushed a commit that referenced this pull request Apr 4, 2017
@gmcculloug @simaishi
Merge pull request #14583 from tinaafitz/service_on_error
d371a69 
Ansible Playbook Service add on_error method.
(cherry picked from commit d7f9fe0)
@simaishi
Copy link
Contributor

simaishi commented Apr 4, 2017

Fine backport details:

$ git log -1
commit d371a6968bedead08c8ce351e2c3813e158d2b34
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Tue Apr 4 10:51:23 2017 -0400

    Merge pull request #14583 from tinaafitz/service_on_error
    
    Ansible Playbook Service add on_error method.
    (cherry picked from commit d7f9fe01acf2075f0724a1bcdfe441a5066b8e86)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmcculloug gmcculloug gmcculloug left review comments

@bzwei bzwei bzwei approved these changes

@mkanoor mkanoor mkanoor approved these changes

Assignees

@gmcculloug gmcculloug

Labels
enhancement fine/backported lifecycle/services
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tinaafitz @bzwei @miq-bot @gmcculloug @simaishi @mkanoor