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

EOS EAPI autocomplete params update #28062

Open
wants to merge 4 commits into
base: devel
from

Conversation

@vinpk
Copy link

commented Aug 11, 2017

On EOS EAPI params, we need to set and pass "autoComplete: true" to be able to use command auto complete during execution which is the standard for CLI transport in Ansible.

SUMMARY

This is to make sure identical behavior for both CLI and EAPI transport methods for Arista switches.
Need to add additional parameter "autoComplete: true" passed to EAPI .

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

module_utils/eos.py

ANSIBLE VERSION
ansible 2.4.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/vinp/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/vinp/.local/lib/python2.7/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 2.7.13 (default, Jan 19 2017, 14:48:08) [GCC 6.3.0 20170118]

ADDITIONAL INFORMATION

This is to make sure identical behavior for both CLI and EAPI transport methods for Arista switches.
Need to add additional parameter "autoComplete: true" passed to EAPI .

Before this change: 
---------------------
fatal: [lab-arista-swi-4.us.com]: FAILED! => {
    "changed": false, 
    "code": 1002, 
    "failed": true, 
    "invocation": {
        "module_args": {
            "auth_pass": null, 
            "authorize": false, 
            "commands": [
                {
                    "answer": null, 
                    "command": "show ver", 
                    "output": "text", 
                    "prompt": null
                }
            ], 
            "host": "lab-arista-swi-4.us.com", 
            "interval": 1, 
            "match": "all", 
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
            "port": 443, 
            "provider": {
                "auth_pass": null, 
                "authorize": false, 
                "connection": "local", 
                "host": "lab-arista-swi-4.us.com", 
                "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
                "port": 443, 
                "ssh_keyfile": null, 
                "timeout": 90, 
                "transport": "eapi", 
                "use_ssl": true, 
                "username": "vinpk", 
                "validate_certs": false
            }, 
            "retries": 10, 
            "ssh_keyfile": null, 
            "timeout": 90, 
            "transport": "cli", 
            "url_password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
            "url_username": "vinpk", 
            "use_ssl": true, 
            "username": "vinpk", 
            "validate_certs": false, 
            "wait_for": null
        }
    }, 
    "msg": "CLI command 2 of 2 'show ver' failed: invalid command"
}
	

After change: 
---------------
ok: [lab-arista-swi-4.us.com] => {
    "changed": false, 
    "failed": false, 
    "invocation": {
        "module_args": {
            "auth_pass": null, 
            "authorize": false, 
            "commands": [
                {
                    "answer": null, 
                    "command": "show ver", 
                    "output": "text", 
                    "prompt": null
                }
            ], 
            "host": "lab-arista-swi-4.us.com", 
            "interval": 1, 
            "match": "all", 
            "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
            "port": 443, 
            "provider": {
                "auth_pass": null, 
                "authorize": false, 
                "connection": "local", 
                "host": "lab-arista-swi-4.us.com", 
                "password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
                "port": 443, 
                "ssh_keyfile": null, 
                "timeout": 90, 
                "transport": "eapi", 
                "use_ssl": true, 
                "username": "vinpk", 
                "validate_certs": false
            }, 
            "retries": 10, 
            "ssh_keyfile": null, 
            "timeout": 90, 
            "transport": "cli", 
            "url_password": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER", 
            "url_username": "vinpk", 
            "use_ssl": true, 
            "username": "vinpk", 
            "validate_certs": false, 
            "wait_for": null
        }
    }, 
    "stdout": [
        "Arista DCS-7050TX-64-R\nHardware version:    01.10\nSerial number:       XXXXXXX\nSystem MAC address:  xx.xx.xx\n\nSoftware image version: 4.18.1.1F\nArchitecture:           i386\nInternal build version: 4.18.1.1F-5127975.41811F\nInternal build ID:      xxxxxx-id\n\nUptime:                 5 days, 14 hours and 16 minutes\nTotal memory:           3838080 kB\nFree memory:            2086868 kB"
    ], 
    "stdout_lines": [
        [
            "Arista DCS-7050TX-64-R", 
            "Hardware version:    01.10", 
            "Serial number:       XXXXXXX", 
            "System MAC address:  xx.xx.xx", 
            "", 
            "Software image version: 4.18.1.1F", 
            "Architecture:           i386", 
            "Internal build version: 4.18.1.1F-5127975.41811F", 
            "Internal build ID:      xxxxxx-id", 
            "", 
            "Uptime:                 5 days, 14 hours and 16 minutes", 
            "Total memory:           3838080 kB", 
            "Free memory:            2086868 kB"
        ]
    ]
}

EOS EAPI params update
On EOS EAPI params, we need to set and pass "autoComplete: true" to be able to use command auto complete during execution.
@privateip

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

to play devils advocate here, why not just use the full command?

This change will not work as written as there are versions of EOS running eAPI that do not support that argument. There needs to be logic to check for that condition

@s-hertel s-hertel removed the needs_triage label Aug 11, 2017

@vinpk

This comment has been minimized.

Copy link
Author

commented Aug 14, 2017

Okay, version 4.15 doesn't support autoComplete params which I didn't test earlier.

I see no other way to identify this argument support without running a random command, which would only incur additional delay. As this pull's intent is only for consistency and not a bug, we can avoid and use full commands as you suggested.

Cheers

@vinpk vinpk closed this Aug 14, 2017

@vinpk vinpk reopened this Aug 15, 2017

@vinpk

This comment has been minimized.

Copy link
Author

commented Aug 15, 2017

Added the logic to add autoComplete if accepted by the device.
Still as previously commented, fractional delay applicable. Please review and decide appropriately.

Thanks

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Aug 15, 2017

The test ansible-test sanity --test pep8 failed with the following error:

lib/ansible/module_utils/eos.py:305:32: W291 trailing whitespace

click here for bot help

@ansibot ansibot removed the needs_revision label Aug 15, 2017

@vinpk vinpk changed the title EOS EAPI params update ready_for_review: EOS EAPI params update Aug 15, 2017

@vinpk

This comment has been minimized.

Copy link
Author

commented Aug 16, 2017

ready_for_review

@ansibot

This comment has been minimized.

@ansibot ansibot added the stale_ci label Aug 24, 2017

@privateip

This comment has been minimized.

Copy link
Member

commented Aug 25, 2017

@vinpk you still didnt address my question, about why we need this in the API. Is there a valid use case where the shortened command is the only option? This seems just more trouble and testing without much gain in enhancement

@vinpk

This comment has been minimized.

Copy link
Author

commented Aug 26, 2017

@privateip As referenced, it's not a bug; just to maintain consistency similar to other device vendors between cli and api transport methods. Yes, it's not mandatory fix. We can close this if you don't feel it's necessary.
Cheers

@vinpk vinpk changed the title ready_for_review: EOS EAPI params update EOS EAPI autocomplete params update Aug 28, 2017

@privateip

This comment has been minimized.

Copy link
Member

commented Aug 30, 2017

@vinpk im going to tag this for 2.5 as we are now in core freeze and this will give us time to consider it. With the upcoming implementation of cliconf as part of 2.5, I believe we will have a much better implementation to support this PR that will not rely on sending arbitrary commands to determine if the target platform supports auto-complete.

@privateip privateip added this to the 2.5.0 milestone Aug 30, 2017

@ansibot

This comment has been minimized.

Copy link
Contributor

commented Oct 17, 2017

@ansibot ansibot removed the new_contributor label Nov 3, 2017

@privateip privateip self-requested a review Nov 15, 2017

@privateip privateip self-assigned this Nov 15, 2017

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

@gundalow gundalow removed this from the 2.5.0 milestone Mar 5, 2019

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