New vyos driver - Githubissues

ppieprzycki commented 8 years ago

Hello All,

I'd like to submit new driver to this repo. This driver was created from already existing PoC https://github.com/dos1506/VyOSDriver

However I've rewritten couple things like:

Connection management now is handled by netmiko istead directly with paramiko.
There are two methods of loading configuration:
- load_replace_candidate - requires config file in bracket style form like you can find on a vyos in /config/config.boot. It allows fully load configuration and it's atomic. In case of syntax error new config won't be installed. Example in following lines

Example config: system { login { banner { pre-login "Test banner" } }

Example response while I was trying load config with syntax error TASK: [Push Configuration] **** failed: [vyos2] => {"failed": true} msg: cannot load config: Failed replace config: configure [edit] vagrant@vyos2# load /var/tmp/candidate_running.conf Loading configuration from '/var/tmp/candidate_running.conf'... Invalid config file (syntax error): error at line 1, text [{] Failed to parse specified config file No configuration changes to commit

load_merge_candidate - requires conifig in set-style format like "set system login banner pre-login aaaa"

handled "compare", "discard", "commit" and "rollback" functions
we can use ssh keys by specify file in optional_args optional_args: {'port': 2200, 'ssh_keyfile': '/path/to/ssh_priv_key_file'}
Fixes in getters

ktbyers commented 8 years ago

@ppieprzycki Okay, I will see if I can review the Netmiko parts of this.

dbarrosop commented 8 years ago

@ppieprzycki would be great if you could implement the new testing framework rather than the old one:

http://napalm.readthedocs.io/en/develop/development/testing_framework.html

It's a bit "special" but it's very powerful. Feel free to throw me any questions you might have.

dbarrosop commented 8 years ago

Could you also add a files like the following:

https://github.com/napalm-automation/napalm-eos/blob/develop/setup.cfg https://github.com/napalm-automation/napalm-eos/blob/develop/requirements-dev.txt

Sorry, but looks like we have changed some "standards" and we haven't updated the skeleton.

ktbyers commented 8 years ago

@ppieprzycki In the class init you should add support for all Netmiko arguments. This will let you support all the standard Netmiko arguments and make the NAPALM drivers that use Netmiko consistent.

Here is the pattern we have been standardizing on:

        # Netmiko possible arguments
        netmiko_argument_map = {
            'port': None,
            'secret': '',
            'verbose': False,
            'global_delay_factor': 1,
            'use_keys': False,
            'key_file': None,
            'ssh_strict': False,
            'system_host_keys': False,
            'alt_host_keys': False,
            'alt_key_file': '',
            'ssh_config_file': None,
        }    

        fields = netmiko_version.split('.')
        fields = [int(x) for x in fields]
        maj_ver, min_ver, bug_fix = fields
        if maj_ver >= 2:
            netmiko_argument_map['allow_agent'] = False
        elif maj_ver == 1 and min_ver >= 1:
            netmiko_argument_map['allow_agent'] = False

        # Build dict of any optional Netmiko args
        self.netmiko_optional_args = {} 
        for k, v in netmiko_argument_map.items():
            try: 
                self.netmiko_optional_args[k] = optional_args[k]
            except KeyError:
                pass 
        self.global_delay_factor = optional_args.get('global_delay_factor', 1)
        self.port = optional_args.get('port', 22)

You will also need to add this import:

from netmiko import __version__ as netmiko_version

Then in open(), Netmiko is called as follows (below is 'cisco_ios', but you get idea):

        self.device = ConnectHandler(device_type='cisco_ios',
                                     host=self.hostname,
                                     username=self.username,
                                     password=self.password,
                                     **self.netmiko_optional_args)

See this for reference:

https://github.com/napalm-automation/napalm-ios/blob/develop/napalm_ios/ios.py

ktbyers commented 8 years ago

Note, Netmiko does have a commit() method for vyos.

ppieprzycki commented 8 years ago

I've added support for Netmiko arguments like in ios driver. Works with password and ssh key. Would be great if usage of the name optional_args 'key_file' would be in future consistent in other drivers.

Still working on testing framework for this driver. Up to few days should be ready

ppieprzycki commented 7 years ago

@dbarrosop Probably I need some clarification regarding tests. I'm having problem with situations when data is changing. For example counters. I'm getting in this tests errors How to handle those cases ?

Also I'm testing on a vm in certain topology (eg bgp session). Sould I design configuration of this vm that should fit to some topology ?

Please find output of test following

======================================================= test session starts ======================================================= platform linux2 -- Python 2.7.6, pytest-3.0.3, py-1.4.31, pluggy-0.4.0 -- /usr/bin/python cachedir: .cache rootdir: /home/piotr/github/github2/napalm-vyos-test, inifile: setup.cfg plugins: cov-2.4.0 collected 24 items

test/unit/test_getters.py::TestGetter::test_get_arp_table[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_get_interfaces[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_get_probes_config[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_lldp_neighbors_detail[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_ntp_peers[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_get_lldp_neighbors[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_ntp_stats[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_ping[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py Resulting JSON object was: {"success": {"packet_loss": 0, "rtt_stddev": 0.014, "rtt_min": 0.089, "results": [{"rtt": 0.096, "ip_address": "8.8.8.8"}], "rtt_avg": 0.096, "rtt_max": 0.114, "probes_sent": 5}} FAILED test/unit/test_getters.py::TestGetter::test_get_optics[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_interfaces_counters[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py Resulting JSON object was: {"eth1": {"tx_multicast_packets": -1, "tx_discards": 0, "tx_octets": 129227034, "tx_errors": 0, "rx_octets": 129286254, "tx_unicast_packets": 1127505, "rx_errors": 0, "tx_broadcast_packets": -1, "rx_multicast_packets": 0, "rx_broadcast_packets": -1, "rx_discards": 0, "rx_unicast_packets": 1127335}, "eth0": {"tx_multicast_packets": -1, "tx_discards": 0, "tx_octets": 342794459, "tx_errors": 0, "rx_octets": 136814996, "tx_unicast_packets": 1234437, "rx_errors": 0, "tx_broadcast_packets": -1, "rx_multicast_packets": 0, "rx_broadcast_packets": -1, "rx_discards": 0, "rx_unicast_packets": 1239261}} FAILED test/unit/test_getters.py::TestGetter::test_get_bgp_neighbors[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py Resulting JSON object was: {"global": {"router_id": "10.2.2.2", "peers": {"10.0.12.1": {"is_enabled": true, "uptime": 907200, "remote_as": 65001, "description": "", "remote_id": "10.1.1.1", "local_as": 65002, "is_up": true, "address_family": {"ipv4": {"sent_prefixes": -1, "accepted_prefixes": 4, "received_prefixes": 4}}}}}} FAILED test/unit/test_getters.py::TestGetter::test_get_mac_address_table[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_route_to[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_facts[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py Resulting JSON object was: {"uptime": 1044845, "vendor": "VyOS", "hostname": "vyos2", "fqdn": "", "os_version": "1.1.7", "serial_number": "0", "model": "VirtualBox", "interface_list": ["eth1", "eth0", "lo"]} FAILED test/unit/test_getters.py::TestGetter::test_get_bgp_neighbors_detail[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_bgp_config[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_interfaces_ip[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_get_users[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED test/unit/test_getters.py::TestGetter::test_traceroute[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_probes_results[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_config_filtered[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_environment[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py Resulting JSON object was: {"fans": {"invalid": {"status": false}}, "cpu": {"0": {"%usage": 1.0}}, "temperature": {"invalid": {"is_alert": false, "temperature": 0.0, "is_critical": false}}, "power": {"invalid": {"status": true, "output": 0.0, "capacity": 0.0}}, "memory": {"available_ram": 250112, "used_ram": 227820}} FAILED test/unit/test_getters.py::TestGetter::test_get_config[no_test_case_found] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py SKIPPED test/unit/test_getters.py::TestGetter::test_get_snmp_information[normal] <- ../../../../../usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py PASSED

---------- coverage: platform linux2, python 2.7.6-final-0 -----------

Name Stmts Miss Cover

napalm_vyos/init.py 8 8 0% napalm_vyos/utils/init.py 0 0 100%

napalm_vyos/vyos.py 346 346 0%

TOTAL 354 354 0%

Example errors:

           print("Resulting JSON object was: {}".format(json.dumps(result)))
            raise AssertionError("Expected result varies on some keys {}".format(

                                                                      json.dumps(diff)))
E AssertionError: Expected result varies on some keys {"success": {"rtt_min": {"expected": 0.086, "result": 0.089}, "rtt_avg": {"expected": 0.175, "result": 0.096}, "results": {"expected": [{"rtt": 0.175, "ip_address": "8.8.8.8"}], "result": [{"rtt": 0.096, "ip_address": "8.8.8.8"}]}, "rtt_max": {"expected": 0.417, "result": 0.114}, "rtt_stddev": {"expected": 0.123, "result": 0.014}}}

/usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py:91: AssertionError

            print("Resulting JSON object was: {}".format(json.dumps(result)))
            raise AssertionError("Expected result varies on some keys {}".format(

                                                                      json.dumps(diff)))
E AssertionError: Expected result varies on some keys {"global": {"peers": {"10.0.12.1": {"uptime": {"expected": 864000, "result": 907200}}}}}

/usr/local/lib/python2.7/dist-packages/napalm_base/test/getters.py:91: AssertionError

dbarrosop commented 7 years ago

@ppieprzycki Sorry for the late response:

Probably I need some clarification regarding tests. I'm having problem with situations when data is changing. For example counters. I'm getting in this tests errors. How to handle those cases ?

Add an elipsis ("..."). For example:

https://github.com/napalm-automation/napalm-eos/blob/develop/test/unit/mocked_data/test_get_facts/normal/expected_result.json#L14

ppieprzycki commented 7 years ago

@dbarrosop Thanks for hint. Now I have only passed or skipped in results.

Please let me know if I should prepare initial config according to environment described in file https://github.com/napalm-automation/napalm-base/blob/develop/vagrant/Vagrantfile especially ip address

Also there is relared pull request in ansible module repo for adding this device type https://github.com/napalm-automation/napalm-ansible/pull/36

Regards

test/unit/test_getters.py::TestGetter::test_get_ntp_servers[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_arp_table[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_interfaces[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_probes_config[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_lldp_neighbors_detail[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_ntp_peers[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_lldp_neighbors[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_ntp_stats[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_network_instances[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_optics[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_interfaces_counters[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_bgp_neighbors[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_mac_address_table[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_route_to[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_facts[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_bgp_neighbors_detail[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_bgp_config[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_interfaces_ip[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_users[normal] PASSED
test/unit/test_getters.py::TestGetter::test_is_alive[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_traceroute[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_probes_results[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_config_filtered[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_ping[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_environment[normal] PASSED
test/unit/test_getters.py::TestGetter::test_get_config[no_test_case_found] SKIPPED
test/unit/test_getters.py::TestGetter::test_get_snmp_information[normal] PASSED

dbarrosop commented 7 years ago

This is great! Give us some time to parse and test all your work.

Is there any possibility we can get a vagrant box/VM?

ppieprzycki commented 7 years ago

@dbarrosop yes it's possible to do some checks with vm for vyos.

I've just fixes expected outputs for tests to be able meet ci environment topology

Probably what we need will be additional image in Vagrant file and provisioning script https://github.com/ppieprzycki/napalm-base/commit/bd3a597c0b78f8ce26998691620fc5a8312c42fa

We need additional config of the base vm. Like config for quagga I can't see how other images use this base vm

vagrant@precise64:~$ sudo cat /etc/quagga/bgpd.conf ! ! Zebra configuration saved from vty ! 2016/11/29 15:07:09 ! ! router bgp 65001 bgp router-id 10.0.2.100 neighbor 10.0.1.222 remote-as 65002 neighbor 10.0.12.2 remote-as 65002 ! line vty !

dbarrosop commented 7 years ago

Probably what we need will be additional image in Vagrant file and provisioning script ppieprzycki/napalm-base@bd3a597

Yes, please, send a PR to napalm-base only with the update to the vagrantfile.

I can't see how other images use this base vm

It's used manually by this:

https://github.com/napalm-automation/napalm-eos/blob/develop/test/unit/TestEOSDriver.py#L35

Mostly used for the first release and whenever there is a change to one of the methods that manipulate configuration. That part is hard to automate and given that they changed VERY rarely it's not worth automating.

dbarrosop commented 7 years ago

Any update on the vagrant box and the testing framework? Also, make sure all tests pass, including pylama .

Thanks!

ppieprzycki commented 7 years ago

@dbarrosop I'll fix code during this weekend. Code need edit code to be able to pass pylama tests.

ppieprzycki commented 7 years ago

@dbarrosop I've just pushed fixed code that pass pylama tests. Also Checked again locally with py.test

Created also pull request in napalm-base repo that will add vyos image into Vagrantfile https://github.com/napalm-automation/napalm-base/pull/165/files

py.test test/unit/test_getters.py ======================================================================== test session starts ======================================================================== platform linux2 -- Python 2.7.6, pytest-3.0.3, py-1.4.31, pluggy-0.4.0 rootdir: /home/piotr/github/github2/napalm-vyos, inifile: plugins: cov-2.4.0, pylama-7.3.1 collected 27 items

test/unit/test_getters.py s..ss.s.ss..ss.ss..ssss..s.

============================================================== 12 passed, 15 skipped in 16.27 seconds ===============================================================

napalm-automation-community / napalm-vyos

New vyos driver #4

Name Stmts Miss Cover

napalm_vyos/vyos.py 346 346 0%