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

Making the ANSI escape regexes configurable in the Shell class #17989

Open
wants to merge 1 commit into
base: devel
from

Conversation

@woody77
Copy link

commented Oct 12, 2016

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

Shell

ANSIBLE VERSION
ansible 2.3.0 (shell_ansi_escape_fix a616dcbe1a) last updated 2016/10/12 11:54:17 (GMT -700)
  lib/ansible/modules/core: (detached HEAD b4f6a25195) last updated 2016/10/11 11:18:35 (GMT -700)
  lib/ansible/modules/extras: (detached HEAD 734aa8f8e9) last updated 2016/10/11 11:18:40 (GMT -700)
  config file = 
  configured module search path = Default w/o overrides
SUMMARY

The module_utils Shell class uses a hardcoded set of regexes to filter out ansi color sequences and terminal escape codes. For using the Shell with some network appliances, this set of regexes needs to be overridden (or expanded).

This change allows a CliBase subclass to override the ansi escape regexes in the same manner that it uses to provide the prompt and errors with CLI_PROMPTS_RE and CLI_ERRORS_RE. This checks for a CLI_ANSI_RE, and if provided, uses that instead of the default ANSI_RE for sanitizing lines read from the remote shell.

Making the ANSI escape regexes configurable in the Shell class in the…
… same manner than the prompt and error regexes are configurable. This is for communicating with network appliances that return unusual codes in their shells
@woody77

This comment has been minimized.

Copy link
Author

commented Oct 13, 2016

The test failures seem to be a problem with the CI system:

2016-10-12 19:04:32 Pulling repository docker.io/ansible/ansible
2016-10-12 19:04:32 Tag opensuseleap not found in repository docker.io/ansible/ansible
2016-10-12 19:03:31 Pulling repository docker.io/ansible/ansible
2016-10-12 19:03:31 Tag centos7 not found in repository docker.io/ansible/ansible
@abadger

This comment has been minimized.

Copy link
Member

commented Jul 25, 2017

We took a look at this among a small group in today's meeting and weren't sure if this was the right API for doing ths. We thought that perhaps overriding the split method or passing a method into the constructor rather than a regex might make more sense. But we were limited in evaluating what would be appropriate by not knowing how this is intended to be used. We have here the ability to override the Shell() constructor but nothing that's calling the constructor with overridden values. So we're not seeing the complete picture and knowing what the API should look like to accommodate that.

Could you give us some more information about how this would be called?

needs_info

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2018

@woody77 This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

@woody77

This comment has been minimized.

Copy link
Author

commented Jan 31, 2018

The background on this was that I was using ansible with a network appliance which used a series of escape codes that were subtly different from those used by say a Cisco router. So I was calling the Shell() constructor with a different list of regexes (one was the same, but I had to add about 5-6 more to get Shell() to properly parse the output of the particular appliance I was working with).

The simplest method that I could find was to add the ability to provide a different set of regexes when instantiating the Shell class, so that it could be customized for communicating with different appliances.

IIRC, overriding split() looked like it was workable, but heavy-handed since the method performed all the right steps, it just needed a larger set of regexes to deal with some quirks in how the appliance returned prompts and did pagination.

OTOH, passing in a function that returned a list of appropriate regexes could also work. That, to me, feels like a stylistic choice, but retains the overall same functionality.

I don't have access to the full source that I was using at the time (this was for a project at a previous employer), so I'm going somewhat off of memory on this.

@ansibot ansibot removed the needs_info label Jan 31, 2018

@ansibot ansibot added feature and removed feature_pull_request labels Mar 2, 2018

@gundalow

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2018

bot_status

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Nov 30, 2018

Components

lib/ansible/module_utils/shell.py
support: community
maintainers:

Metadata

waiting_on: woody77
changes_requested_by: null
needs_info: False
needs_revision: True
needs_rebase: True
merge_commits: []
too many files or commits: False
mergeable_state: dirty
shippable_status: failure
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

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