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

Add force reconnection feature to NAPALM Proxy Minions #48948

Merged
merged 3 commits into from Aug 6, 2018

Conversation

Projects
None yet
3 participants
@mirceaulinic
Copy link
Member

commented Aug 6, 2018

What does this PR do?

One of the major disadvantages of the Proxy Minions is their rigidity in terms of connectivity: the connection with the remote device is established with the remote device at the time of the Minion startup and that connection is going to be used forever.

If one would execute a command on the device using different parameters (due to various causes, such as unable to authenticate with the user specified in the Pillar and the authentication system - say TACACS+ is not available, or the user has a DNS in the Pillar and the resolver is currently down and would like to use temporarily the IP address instead, etc.), it requires a updating the Pillar data and Proxy Minion process restart. This is less than ideal.

Salt fortunately is as always flexible enough and we can add a magic argument, say force_reconnect which tells to establish a separate connection with the args specified in-line. Example:

Regular behaviour:

root@salt-master:/# salt napalm net.arp
napalm:
    ----------
    comment:
    out:
        |_
          ----------
          age:
              None
          interface:
              em1.0
          ip:
              128.0.0.16
          mac:
              02:00:00:00:00:10
        |_
          ----------
          age:
              1275.0
          interface:
              fxp0.0
          ip:
              172.31.0.1
          mac:
              02:8B:CA:89:F9:35
    result:
        True

With force_reconnect:

root@salt-master:/# salt napalm net.arp force_reconnect=True host=fake
napalm:
    The minion function caused an exception: Traceback (most recent call last):
      File "/usr/lib/python2.7/dist-packages/salt/minion.py", line 1635, in _thread_return
        return_data = minion_instance.executors[fname](opts, data, func, args, kwargs)
      File "/usr/lib/python2.7/dist-packages/salt/executors/direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "/usr/lib/python2.7/dist-packages/salt/utils/napalm.py", line 393, in func_wrapper
        wrapped_global_namespace['napalm_device'] = get_device(opts)
      File "/usr/lib/python2.7/dist-packages/salt/utils/napalm.py", line 340, in get_device
        network_device.get('DRIVER').open()
      File "/usr/local/lib/python2.7/dist-packages/napalm/junos/junos.py", line 107, in open
        self.device.open()
      File "/usr/local/lib/python2.7/dist-packages/jnpr/junos/device.py", line 1284, in open
        raise EzErrors.ConnectUnknownHostError(self)
    ConnectUnknownHostError: ConnectUnknownHostError(fake)

The above was used as an example proving that when force_reconnect=True it would indeed attempt to initiate a separate connection.

@rallytime
Copy link
Contributor

left a comment

This needs to be documented somewhere and added to the release notes.

@mirceaulinic mirceaulinic force-pushed the mirceaulinic:napalm-reconnect branch from d8267f4 to 18a9aab Aug 6, 2018

@mirceaulinic

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2018

Definitely @rallytime - I will add this to the NAPALM Proxy docs. For the release notes, I'm planning to finish some more things, and I'll add to the release notes everything I can add into Fluorine. Does this make sense?

@mirceaulinic

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2018

Pushed 7960783 to document this new feature.

@rallytime rallytime requested a review from gtmanfred Aug 6, 2018

@rallytime rallytime added the Fluorine label Aug 6, 2018

@rallytime rallytime merged commit 3290cf9 into saltstack:develop Aug 6, 2018

6 of 9 checks passed

jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has failed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has failed
Details
jenkins/pr/py2-centos-7 running py2-centos-7...
Details
WIP ready for review
Details
codeclimate All good!
Details
continuous-integration/jenkins/pr-merge This commit looks good
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has passed
Details
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.