-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
Simplify acceptance, allow hack for Debian 8 to return 1 or 100 #413
Conversation
wyardley
commented
Oct 24, 2017
- simplify acceptance tests
- allow the hack to install openhpid on Debian 8 to continue even if it exits 1 or 100 (100 seems to be what the error is making it exit).
# For Debian 8 "jessie", we need | ||
# - pacemaker and crmsh delivered in jessie-backports only | ||
# - openhpid post-install may fail (https://bugs.debian.org/785287) | ||
if fact('lsbdistcodename') == 'jessie' | ||
if fact('os.family') == 'Debian' && fact('os.release.major') == '8' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked your proposal to use the codename fact to simplify the conditional in #411, it makes the code simpler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I had suggested it mostly if we were doing it for both 7 / 8. I could switch it back, but I think maybe this is clearer (and also forget whether the lsb* facts are as reliably there?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this is good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the os.release.major approach.
on host, 'echo deb http://ftp.debian.org/debian jessie-backports main >> /etc/apt/sources.list' | ||
on host, 'apt-get update && apt-get install -y openhpid' | ||
on host, 'apt-get update && apt-get install -y openhpid', acceptable_exit_codes: [0, 1, 100] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, apt-get rc is 100 when a sub-command fails.
# For Debian 8 "jessie", we need | ||
# - pacemaker and crmsh delivered in jessie-backports only | ||
# - openhpid post-install may fail (https://bugs.debian.org/785287) | ||
if fact('lsbdistcodename') == 'jessie' | ||
if fact('os.family') == 'Debian' && fact('os.release.major') == '8' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this is good
# For Debian 8 "jessie", we need | ||
# - pacemaker and crmsh delivered in jessie-backports only | ||
# - openhpid post-install may fail (https://bugs.debian.org/785287) | ||
if fact('lsbdistcodename') == 'jessie' | ||
if fact('os.family') == 'Debian' && fact('os.release.major') == '8' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the os.release.major approach.
now tests are failing for centos 7 too |
The acceptance test for centos 7 will fail until we finalize and merge #410 . |
Then thank you both :) |