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

handle localhost session #141

Open
vnitinv opened this issue Apr 15, 2020 · 6 comments · Fixed by saltstack/salt#57281
Open

handle localhost session #141

vnitinv opened this issue Apr 15, 2020 · 6 comments · Fixed by saltstack/salt#57281
Assignees

Comments

@vnitinv
Copy link

vnitinv commented Apr 15, 2020

Description of Issue

[DEBUG   ] Subprocess Thread-4-Schedule-__proxy_keepalive added
[DEBUG   ] schedule.handle_func: adding this job to the jobcache with data {'id': 'fcr5a_native', 'fun': 'status.proxy_reconnect', 'fun_args': [], 'schedule': '__proxy_keepalive', 'jid': '20200414090540489456', 'pid': 74052}
[DEBUG   ] LazyLoaded status.proxy_reconnect
[ERROR   ] Unhandled exception running status.proxy_reconnect
Traceback (most recent call last):
  File "salt/utils/schedule.py", line 769, in handle_func
  File "/var/db/scripts/jet/salt/modules/status.py", line 1728, in proxy_reconnect
    is_alive = __proxy__[proxy_keepalive_fn](opts)
  File "/var/db/scripts/jet/salt/proxy/junos.py", line 154, in alive
    thisproxy['conn'].connected = ping()
  File "/var/db/scripts/jet/salt/proxy/junos.py", line 175, in ping
    if dev._conn._session._transport.is_active():
AttributeError: 'NoneType' object has no attribute 'is_active'
[DEBUG   ] schedule.handle_func: Removing /var/local/salt/cache/proxy/fcr5a_native/proc/20200414090540489456
[DEBUG   ] LazyLoaded mine.update
@vnitinv vnitinv self-assigned this Apr 15, 2020
@dmurphy18
Copy link

dmurphy18 commented Apr 28, 2020

@vnitinv I am working on the salt binary and from an email thread on the matter:

We figured out issue in supporting ioproc connectivity in salt binary and raised ticket in github. This issue is fixed and will send pull request soon.

Wondering if you could identify the PR or the fix, as fixing issues with the salt binary at the moment and would like to utilize the fix.
Thanks

@vnitinv
Copy link
Author

vnitinv commented Apr 29, 2020

@dmurphy18

For localhost, we dont prefer connection via ssh, but a local mechanism (name ioproc)
https://github.com/ncclient/ncclient/blob/master/ncclient/transport/third_party/junos/ioproc.py

This works all perfectly to make a connection even with the new local minion which we are building. There is a minor issue with keep-alive (ping function).

I was not sure if we should do the changes in an existing pull request to saltstack (as this change won't be used by any user) and also wanted to get that code checked in first before we make any new changes.

https://github.com/saltstack/salt/blob/master/salt/proxy/junos.py#L193

ping function need to change to

def ping():
    '''
    Ping?  Pong!
    '''
    dev = conn()
    # Check that the underlying netconf connection still exists.
    if dev._conn is None:
        return False
    
    # call rpc only if ncclient queue is empty. If not empty that means other
    # rpc call is going on.
    if hasattr(dev._conn, '_session'):
        if (
            dev._conn._session._transport is not None and
            dev._conn._session._transport.is_active()) or \
            (dev._conn._session._transport is None and
             isinstance(dev._conn._session, IOProc)):
            # there is no on going rpc call. buffer tell can be 1 as it stores
            # remaining char after "]]>]]>" which can be a new line char
            if dev._conn._session._buffer.tell() <= 1 and \
                    dev._conn._session._q.empty():
                return _rpc_file_list(dev)
            else:
                log.debug('skipped ping() call as proxy already getting data')
                return True
        else:
            # ssh connection is lost
            return False
    else:
        # other connection modes, like telnet
        return _rpc_file_list(dev)

@dmurphy18
Copy link

@vnitinv Thanks for that information. Actually I was about to make those changes to ping this morning myself, but I shall incorporate your code in the changes since I assume you have tested it etc. Converting the local native minion to using localhost, among a few other changes for Juniper switches.

@vnitinv
Copy link
Author

vnitinv commented Apr 29, 2020

@vnitinv Thanks for that information. Actually I was about to make those changes to ping this morning myself, but I shall incorporate your code in the changes since I assume you have tested it etc. Converting the local native minion to using localhost, among a few other changes for Juniper switches.

Getting salt on Junos box will be a great feature addition. Looking forward to it.

@vnitinv
Copy link
Author

vnitinv commented May 13, 2020

@dmurphy18 Did you fix this or want me to send a pull request for this? I don't see this code in salt repo as of today

@dmurphy18
Copy link

Go ahead and submit a PR, I made the change in the native minion and am still working on it (core dump on MX,not QFX) so it will be a while before it is ready for merge. Don't forget to include a test, and mark me as a Reviewer when you submit

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 a pull request may close this issue.

2 participants