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

Update cnos.py #16

Merged
merged 4 commits into from
Aug 11, 2020
Merged

Conversation

istaicu
Copy link
Contributor

@istaicu istaicu commented Apr 22, 2020

Added "no logging terminal" after log in.

SUMMARY

The problem was that, when trying to do a configuration on a Lenovo switch, in some cases, a timeout would occur with the following trace:

020-03-12 10:37:26,564 p=5491 u=root n=ansible | Traceback (most recent call last):
File "/usr/lib/python2.7/site-packages/ansible/utils/jsonrpc.py", line 45, in handle_request
result = rpc_method(*args, **kwargs)
File "/usr/lib/python2.7/site-packages/ansible/plugins/cliconf/init.py", line 42, in wrapped
return func(self, *args, **kwargs)
File "/usr/lib/python2.7/site-packages/ansible/plugins/cliconf/cnos.py", line 102, in edit_config
results.append(self.send_command(**line))
File "/usr/lib/python2.7/site-packages/ansible/plugins/cliconf/init.py", line 127, in send_command
resp = self._connection.send(**kwargs)
File "/usr/lib/python2.7/site-packages/ansible/plugins/connection/init.py", line 35, in wrapped
return func(self, *args, **kwargs)
File "/usr/lib/python2.7/site-packages/ansible/plugins/connection/network_cli.py", line 580, in send
response = self.receive(command, prompt, answer, newline, prompt_retry_check, check_all)
File "/usr/lib/python2.7/site-packages/ansible/plugins/connection/network_cli.py", line 529, in receive
data = self._ssh_shell.recv(256)
File "/usr/lib/python2.7/site-packages/paramiko/channel.py", line 665, in recv
out = self.in_buffer.read(nbytes, self.timeout)
File "/usr/lib/python2.7/site-packages/paramiko/buffered_pipe.py", line 156, in read
self._cv.wait(timeout)
File "/usr/lib64/python2.7/threading.py", line 362, in wait
_sleep(delay)
File "/usr/lib/python2.7/site-packages/ansible/plugins/connection/network_cli.py", line 596, in _handle_command_timeout
raise AnsibleConnectionFailure(msg)

After investigating, found out that the timeout occurred because some logs appear in the ssh terminal session in some cases, and the function that waits for the prompt, does not recognize the prompt in the response message, unless it is found at the end of the response. But in this case, the prompt is mixed up with the log message text. (This happens, for example, when trying to raise an interface (no shut command on an interface). When this command is issued, a log message immediately appears, showing the new state of the interface.)
And because of this timeout message, a chain reaction would occur and the switch could no longer be configured (timeout => switch stuck in the current submenu, and when trying to issue other cli commands, errors appear due to incorrect prompt).

The solution is to disable logging when ansible connects to the switch. This does not affect other connections in any way because the command takes effect only for the current session. Also, you would not be able to see those log messages anyway since there is no way to access that ansible ssh session, rendering them unnecessary.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/terminal/cnos.py

ADDITIONAL INFORMATION

Tested with the changes I propose and the timeout does not occur anymore. The switch configuration can be completely executed.

@ikhan2010 ikhan2010 requested a review from gundalow May 11, 2020 17:05
@gundalow
Copy link
Contributor

Can you please rebase your branch, that should fix the CI failure.

Added "no logging terminal"  after log in.
@istaicu istaicu force-pushed the lenovo_module_update branch from 6b3928c to 6c07333 Compare May 14, 2020 10:24
Fix backslash error.
@felixfontein
Copy link
Collaborator

Also, please add a changelog fragment.

@istaicu
Copy link
Contributor Author

istaicu commented May 14, 2020

Hi,

I've done a rebase and fixed a minor syntax error. Now the shippable tests pass. I've also added a changelog fragment.

Thank you,
Ioana

@felixfontein felixfontein changed the base branch from master to main July 3, 2020 11:55
@gundalow
Copy link
Contributor

gundalow commented Jul 9, 2020

@amuraleedhar @dkasberg as maintainers of the CNOS Ansible modules could you please take a look at this?
Thanks in advance

@amuraleedhar
Copy link

amuraleedhar commented Jul 9, 2020 via email

@istaicu
Copy link
Contributor Author

istaicu commented Aug 10, 2020

@gundalow
Hi,
Kind reminder about this pull request. Although Anil is no longer with us and Lenovo networking division is no longer an active project, it will be quite a few years until it will be completely shut down. Meanwhile, we still have to fix any issue we find. I am now in charge of the ansible part, and this is the change we need to make. Can you please integrate it into the main branch?

Thanks,
Ioana

@gundalow gundalow merged commit 4d4a5f6 into ansible-collections:main Aug 11, 2020
@felixfontein felixfontein added the needs_backport_to_stable_1 PR needs to be backported to the stable-1 branch label Aug 11, 2020
gundalow added a commit to gundalow/community.network that referenced this pull request Aug 11, 2020
@gundalow gundalow mentioned this pull request Aug 11, 2020
@gundalow
Copy link
Contributor

@amuraleedhar Anil, it's been a pleasure to work with you over the years. I wish you all the success at Cisco. FYI your GitHub provile still mentions Lenovo

@dkasberg Not sure if you'll see this, it's also been great working with you and I wish you luck in your future adventures.

@istaicu Thank you for keeping an eye on Lenovo networking. In #100 I've set you as a maintainer of these modules, so we know we can just hit merge on your PRs. Please do shout out if there is anything we can do to assist you, it's great to see that even though this isn't an active project you are still helping keep Ansible support going.

@istaicu
Copy link
Contributor Author

istaicu commented Aug 11, 2020

@gundalow Thank you very much for your support. I'll keep bugging you if there are things I don't know. Thanks.

@istaicu
Copy link
Contributor Author

istaicu commented Aug 11, 2020

@gundalow Can you please make Mihai Broc ( @mihaibroc ) a maintainer too? To have a backup.
Thanks

gundalow added a commit that referenced this pull request Aug 11, 2020
gundalow added a commit that referenced this pull request Aug 11, 2020
@gundalow
Copy link
Contributor

@gundalow Can you please make Mihai Broc ( @mihaibroc ) a maintainer too? To have a backup.
Thanks

Done. Welcome to the Ansible community @mihaibroc

felixfontein pushed a commit that referenced this pull request Aug 18, 2020
* Update cnos.py

Added "no logging terminal"  after log in.

* Update cnos.py

Fix backslash error.

* Create terminal_plugin_cnos_update.yml

* Update changelogs/fragments/terminal_plugin_cnos_update.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 4d4a5f6)
felixfontein pushed a commit that referenced this pull request Aug 18, 2020
Background #16 (comment)

(cherry picked from commit b726cd0)
felixfontein pushed a commit that referenced this pull request Aug 18, 2020
Requested in #16 (comment)

(cherry picked from commit b880c13)
@felixfontein felixfontein removed the needs_backport_to_stable_1 PR needs to be backported to the stable-1 branch label Aug 18, 2020
ernst-s pushed a commit to ernst-s/community.network that referenced this pull request Sep 8, 2020
* Update cnos.py

Added "no logging terminal"  after log in.

* Update cnos.py

Fix backslash error.

* Create terminal_plugin_cnos_update.yml

* Update changelogs/fragments/terminal_plugin_cnos_update.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit 4d4a5f6)
ernst-s pushed a commit to ernst-s/community.network that referenced this pull request Sep 8, 2020
ernst-s pushed a commit to ernst-s/community.network that referenced this pull request Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants