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

don't ignore SSH connection plugin options (move SSH connection plugin options away from PlayContext) #49470

Open
wants to merge 7 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@pilou-
Contributor

pilou- commented Dec 4, 2018

SUMMARY
  • Take in account SSH connection plugin options mentioned in 1a70681. For example Ansible 2.7, ANSIBLE_SSH_COMMON_ARGS environment variable is documented but ignored: output of ANSIBLE_SSH_COMMON_ARGS="-o \"Port=2222\"" ansible all -i test, -m ping -vvvv |grep "SSH: EXEC" command doesn't mention Port=2222.
  • remove SSH connection plugin options from main configuration
  • doc: mention config file and environment variables in variable precedence section

Integration tests provided.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

config

ADDITIONAL INFORMATION

Note that ANSIBLE_SSH_ARGS (and other SSH related environment variables) will be removed from docs/docsite/rst/reference_appendices/config.rst.

Could be added to https://github.com/ansible/ansible/projects/8.

Thanks to @adriensaladin for pointing out this issue.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 4, 2018

@pilou- pilou- force-pushed the pilou-:dont_ignore_plugin_options branch 3 times, most recently from fc664e2 to ad3641e Dec 4, 2018

@samdoran samdoran requested a review from bcoca Dec 4, 2018

@samdoran samdoran removed the needs_triage label Dec 4, 2018

@sivel sivel added the P3 label Dec 4, 2018

@pilou- pilou- force-pushed the pilou-:dont_ignore_plugin_options branch from ad3641e to 15b4ec4 Dec 5, 2018

pilou- added some commits Dec 2, 2018

Move SSH plugin options
- Keep ssh_executable in base.yml since this value is used in order to
  determine which connection plugin to instantiate:
  https://github.com/ansible/ansible/blob/34c57b4c4291f7d4fb51df8a0459b37428e344c4/lib/ansible/playbook/play_context.py#L602
- retries: use default value from base.yml (SSH connection plugin
  default value is ignored)
Remove SSH connection plugin options from PlayContext
don't break backward compatibility: default values for plugin options
were ignored, then remove them
SSH connection plugin: use get_option
ssh_executable is still a PlayContext attribute

@pilou- pilou- force-pushed the pilou-:dont_ignore_plugin_options branch from 15b4ec4 to d37bd1f Dec 5, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 5, 2018

@pilou-

This comment has been minimized.

Contributor

pilou- commented Dec 5, 2018

  • Shippable was not able to execute integration tests on mac OS, T=osx/10.11/3 (the same for T=osx/10.11/2 T=osx/10.11/1)
    Initializing new osx/10.11 instance ************************************.
    Trying endpoint: https://parallels-1.testing.ansible.com (threshold 1)
    Service Unavailable: threshold: running instance count 1 meets or exceeds threshold 1
    Trying endpoint: https://parallels-2.testing.ansible.com (threshold 1)
    Service Unavailable: threshold: running instance count 1 meets or exceeds threshold 1
    Trying endpoint: https://parallels-3.testing.ansible.com (threshold 1)
    Skipping invalid osx/10.11 instance ************************************.
    
  • T=linux/fedora24/1 failure due to network errors

@pilou- pilou- changed the title from WIP: don't ignore SSH connection plugin options (move SSH connection plugin options away from PlayContext) to don't ignore SSH connection plugin options (move SSH connection plugin options away from PlayContext) Dec 5, 2018

@ansible ansible deleted a comment from ansibot Dec 5, 2018

@ansible ansible deleted a comment from ansibot Dec 5, 2018

@ansible ansible deleted a comment from ansibot Dec 5, 2018

@mscherer

This comment has been minimized.

Contributor

mscherer commented Dec 5, 2018

FYI, I did delete duplicate comments from the bot

@ansibot ansibot added needs_revision and removed WIP labels Dec 5, 2018

@bcoca

So i've been doing something similar, but avoiding the requirement of play_context

#38861 <= among other things now puts 'options' in the inheritance path for play objects, so we can now pull the info from the tasks

#49397 <= closer to this ticket, its moving the resolution into the connection plugins themselves

env: [{name: ANSIBLE_SSH_CONTROL_PATH_DIR}]
ini:
- {key: control_path_dir, section: ssh_connection}
yaml: {key: ssh_connection.control_path_dir}
ANSIBLE_SSH_EXECUTABLE:

This comment has been minimized.

@bcoca

bcoca Dec 5, 2018

Member

why leave this one here?

This comment has been minimized.

@pilou-

pilou- Dec 6, 2018

Contributor

C.ANSIBLE_SSH_EXECUTABLE is used before connection plugin instantiation, in order to choose which connection plugin to use.

This comment has been minimized.

@bcoca

bcoca Dec 11, 2018

Member

that we need to fix

@@ -234,22 +227,6 @@ def set_play(self, play):
if play.force_handlers is not None:
self.force_handlers = play.force_handlers
def set_options_from_plugin(self, plugin):

This comment has been minimized.

@bcoca

bcoca Dec 5, 2018

Member

this is here for backwards compatibility, plugins should now have the 'cannonical' data but plenty of systems still query play_context and it should keep a copy of them (for now) .. until we can deprecate/remove it in the future

This comment has been minimized.

@pilou-

pilou- Dec 6, 2018

Contributor

Documentation states there is no backward compatibility for the Python API isn't it ?

This comment has been minimized.

@bcoca

bcoca Dec 11, 2018

Member

no, but there are still things using play_context in core, so until they can ALL be migrated away, we need to keep the data in sync

@pilou- pilou- force-pushed the pilou-:dont_ignore_plugin_options branch from d37bd1f to 736c1a0 Dec 6, 2018

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