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.

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

Handle setup playbook failure better #15313

Merged

Conversation

carbonin
Copy link
Member

@carbonin carbonin commented Jun 6, 2017

This PR allows us to re-run the setup playbook if we fail the first time around.

When #15225 made us run the setup playbook only once, it had a side-effect of not allowing us to run the playbook again if and when we failed the first time.

This PR handles that situation by removing the secret key from the database when the playbook run fails. This in turn causes EmbeddedAnsible.configured? to return false the next time we attempt to run the playbook.

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

@carbonin carbonin requested a review from jrafanie June 6, 2017 13:34
@carbonin carbonin changed the title Handle setup playbook failure better [WIP] Handle setup playbook failure better Jun 6, 2017
@carbonin carbonin added the wip label Jun 6, 2017
Previously if we had a value in the database, but the file
didn't exist on the filesystem .configured? would raise an error
when it should really just return false and we will write out
the value from the database to the filesystem.

https://bugzilla.redhat.com/show_bug.cgi?id=1439783
https://bugzilla.redhat.com/show_bug.cgi?id=1458886
This will force `.configured?` to false the next time `.start` is run
allowing us to retry the configuration.

Before this change, users would have to blank the SECRET_KEY file
on the filesystem to force a retry.

https://bugzilla.redhat.com/show_bug.cgi?id=1439783
https://bugzilla.redhat.com/show_bug.cgi?id=1458886
@carbonin carbonin force-pushed the handle_setup_playbook_failure_better branch from 248c8c7 to f80e6dd Compare June 6, 2017 14:09
@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2017

Checked commits carbonin/manageiq@42eb2f8~...f80e6dd with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

@carbonin carbonin changed the title [WIP] Handle setup playbook failure better Handle setup playbook failure better Jun 6, 2017
@carbonin carbonin removed the wip label Jun 6, 2017
@jrafanie
Copy link
Member

jrafanie commented Jun 6, 2017

:shipit: LGTM

@jrafanie jrafanie merged commit 388a40c into ManageIQ:master Jun 6, 2017
@jrafanie jrafanie added this to the Sprint 63 Ending Jun 19, 2017 milestone Jun 6, 2017
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 76ab38ed580157f44fde670ff882ec8b1f6177ea
Author: Joe Rafaniello <jrafanie@users.noreply.github.com>
Date:   Tue Jun 6 11:55:41 2017 -0400

    Merge pull request #15313 from carbonin/handle_setup_playbook_failure_better
    
    Handle setup playbook failure better
    (cherry picked from commit 388a40cfbafa61f670d3e4f670e9d5f7d8d1cf50)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1460803
    https://bugzilla.redhat.com/show_bug.cgi?id=1460805

@carbonin carbonin deleted the handle_setup_playbook_failure_better branch October 13, 2017 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants