-
Notifications
You must be signed in to change notification settings - Fork 130
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
Minor fixes to nsenter connection plugin #249
Minor fixes to nsenter connection plugin #249
Conversation
- Ensure the nsoption_pid option is retrieved in _connect instead of __init__ to prevent a crasher due to initialization order - Replace the use of --all-namespaces with specific namespaces to support compatibility with Busybox nsenter (for example, Alpine)
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.
Thanks for your contribution! Can you please add a changelog fragment?
@jgoldschrafe I'm planning to do the bugfix release I announced last week by latest this weekend, so if you want to get this in, please try to work on it until then :) |
Updated, thank you! |
Co-authored-by: Felix Fontein <felix@fontein.de>
Backport to stable-1: 💚 backport PR created✅ Backport PR branch: Backported as #251 🤖 @patchback |
* Minor fixes to nsenter connection plugin - Ensure the nsoption_pid option is retrieved in _connect instead of __init__ to prevent a crasher due to initialization order - Replace the use of --all-namespaces with specific namespaces to support compatibility with Busybox nsenter (for example, Alpine) * minor tidy * Fix PEP8 violation * Changelog fragment * Update changelogs/fragments/249-nsenter-fixes.yml Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Jeff Goldschrafe <jeff.goldschrafe@flatiron.com> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit d224752)
@jgoldschrafe thanks for fixing this! |
* Minor fixes to nsenter connection plugin - Ensure the nsoption_pid option is retrieved in _connect instead of __init__ to prevent a crasher due to initialization order - Replace the use of --all-namespaces with specific namespaces to support compatibility with Busybox nsenter (for example, Alpine) * minor tidy * Fix PEP8 violation * Changelog fragment * Update changelogs/fragments/249-nsenter-fixes.yml Co-authored-by: Felix Fontein <felix@fontein.de> Co-authored-by: Jeff Goldschrafe <jeff.goldschrafe@flatiron.com> Co-authored-by: Felix Fontein <felix@fontein.de> (cherry picked from commit d224752) Co-authored-by: Jeff Goldschrafe <jgoldschrafe@users.noreply.github.com>
Related to: - https://docs.ansible.com/ansible/latest/collections/community/docker/nsenter_connection.html - ansible-collections/community.docker#249 The Docker community collection that's installed in Alpine 3.15 is old and doesn't contain the aforementioned nsenter plugin patch, so `--connection=community.docker.nsenter` doesn't work there.
SUMMARY
Two fixes to the nsenter connection plugin:
nsenter_pid
option is retrieved in_connect
instead of__init__
to prevent a crasher due to bad initialization order--all-namespaces
with specific namespaces to support compatibility with Busybox nsenter (used on for example, Alpine containers)ISSUE TYPE
COMPONENT NAME
nsenter connection plugin
ADDITIONAL INFORMATION
N/A