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

[enh] Iteration on sanity / anomaly checks for app operations #785

Merged
merged 4 commits into from Sep 17, 2019

Conversation

@alexAubin
Copy link
Member

commented Aug 23, 2019

The problem

Currently we now do some checks that services requested by an app are up and running (and other anomaly checks) before some operations like install and upgrade ...Which is cool, but we can do better :

  • checking before and after the operations
  • check nginx and fail2ban all the time even if not explicitly required by the app, because those are related to regular issues
  • combine this with the dpkg check (and in the future we can add other basic sanity checks in that vein)

Solution

Rework a few things to implement this

PR Status

Tested and working using the PR about app unit tests ... Though maybe still need some work because not all tests are implemented and there are still weird issues in the test

How to test

Eeeeh using the app unit test, or manually run stuff like app install with nginx down. Or try to install an app that break nginx or fail2ban.

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
@Psycojoker

This comment has been minimized.

Copy link
Member

commented Aug 25, 2019

LGTM after a quick review. Maybe I'll have energy to do a better review later.

Also: super great to have this PR ❤️

@alexAubin alexAubin added this to the 3.8.x milestone Sep 1, 2019

@alexAubin alexAubin modified the milestones: 3.8.x, 3.7.x Sep 14, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2019

Hmso since there's some delay for 3.7, proposing to add this for 3.7 as well (also because I'd like to merge #779 which is also helpful for #769 ...)

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2019

So proposing to merge soonish

@alexAubin alexAubin force-pushed the moar-sanity-check-for-app-operations branch from 8932ade to 703e549 Sep 14, 2019

alexAubin added 3 commits Aug 23, 2019
Here we keep need to keep going and only display an error, otherwise …
…the rest of the file ain't properly cleaned up
Properly handle the sanity checks right after upgrades (in combinatio…
…n with managing the regular error code...). This is similar to what's done for app_install

@alexAubin alexAubin force-pushed the moar-sanity-check-for-app-operations branch from 703e549 to c530325 Sep 15, 2019

@alexAubin alexAubin referenced this pull request Sep 15, 2019
0 of 4 tasks complete

@alexAubin alexAubin changed the base branch from stretch-unstable to stop_on_failed_app_upgrade Sep 15, 2019

@alexAubin

This comment has been minimized.

Copy link
Member Author

commented Sep 15, 2019

c.f. #779 (comment) ... was painful but finally fixed a few things thanks to the tests. Rebased on top of #769 for various reason. Temporarily changed the base branch in case you wanna read the diff.

upgrade_retcode = -1
except Exception:
import traceback
logger.exception(m18n.n('unexpected_error', error=u"\n" + traceback.format_exc()))

This comment has been minimized.

Copy link
@Psycojoker

Psycojoker Sep 15, 2019

Member

If you do a , exc_info=1 you should normally get the traceback in the logger (so you don't need to use traceback directly)

This comment has been minimized.

Copy link
@alexAubin

alexAubin Sep 15, 2019

Author Member

I suppose, but I just copied that part from app_install, and turns out you're the author 😅
9e1cd73

This comment has been minimized.

Copy link
@Psycojoker

Psycojoker Sep 17, 2019

Member

Oupsi 😅 (I probably wasn't aware of that at that time)

@alexAubin alexAubin changed the base branch from stop_on_failed_app_upgrade to stretch-unstable Sep 17, 2019

@alexAubin alexAubin merged commit f125a8a into stretch-unstable Sep 17, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the moar-sanity-check-for-app-operations branch Sep 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.