napalm-automation-community / napalm-fortios

Apache License 2.0
30 stars 27 forks source link

merge does a replace, and rollback raises an error #46

Open jmcgill298 opened 7 years ago

jmcgill298 commented 7 years ago

Description of Issue/Question

load_merge_candidate() replaces the "blocks" entire config with contents in the file rollback() fails due to 'fnsysctl ls -l data2/config' not being a valid command

Did you follow the steps from https://github.com/napalm-automation/napalm#faq

Setup

Ubuntu, Python 2.7.12, napalm-base==0.23.2, napalm-fortios==0.4.0, pyfg==0.49, paramiko==2.1.2, netmiko==1.3.0, FortiGate-VM64-KVM v5.2.5,build701 (GA)

napalm-fortios version

(Paste verbatim output from pip freeze | grep napalm-fortios between quotes below)

napalm-fortios==0.4.0

FortiOS version

(Paste verbatim output from get sys status between quotes below)

FortiGate-VM64-KVM # get sys status
Version: FortiGate-VM64-KVM v5.2.5,build0701,151203 (GA)
Virus-DB: 16.00560(2012-10-19 08:31)
Extended DB: 1.00000(2012-10-17 15:46)
IPS-DB: 5.00555(2014-10-07 01:21)
IPS-ETDB: 0.00000(2001-01-01 00:00)
Botnet DB: 1.00000(2012-05-28 22:51)
License Status: Valid
VM Resources: 1 CPU/1 allowed, 970 MB RAM/1024 MB allowed
BIOS version: 04000002
Log hard disk: Need format
Hostname: FortiGate-VM64-KVM
Operation Mode: NAT
Current virtual domain: root
Max number of virtual domains: 1
Virtual domains status: 1 in NAT mode, 0 in TP mode
Virtual domain configuration: disable
FIPS-CC mode: disable
Current HA mode: standalone
Branch point: 701
Release Version Information: GA
FortiOS x86-64: Yes
System time: Tue Apr  4 08:05:31 2017

Steps to Reproduce the Issue

Error Traceback

(Paste the complete traceback of the exception between quotes below)

load_merge_candidate() does not have any tracebacks

rollback():
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/jmcgill/ans23/local/lib/python2.7/site-packages/napalm_fortios/fortios.py", line 144, in rollback
    'fnsysctl cat data2/config/{rollback_file}'.format(rollback_file))
KeyError: u'rollback_file'

>>> o = fw._execute_command_with_vdom('fnsysctl ls -l data2/config', vdom=None)
>>> o
[u' Unknown action 0', u'']

FortiGate-VM64-KVM # fnsysctl ls -l data2/config
Unknown action 0
jmcgill298 commented 7 years ago

Example of load_merge_candidate: Before:

config firewall policy
        edit 5
          set service "HTTPS"
          set schedule "always"
          set srcaddr "users"
          set dstintf "any"
          set srcintf "any"
          set action accept
          set dstaddr "web_server"
        next
    end

New Config File:

config firewall policy
    edit 6
        set srcintf "any"
        set dstintf "any"
        set srcaddr "ansible"
        set dstaddr "target"
        set action accept
        set schedule "always"
        set service "SSH"

Using Napalm:

fw.load_merge_candidate(filename=new_conf)
>>> print(fw.compare_config())
    config firewall policy
        delete 5
        edit 6
          set service "SSH"
          set schedule "always"
          set srcaddr "ansible"
          set dstintf "any"
          set srcintf "any"
          set action accept
          set dstaddr "target"
        next
    end

Knowingly commit bad merge results:

FortiGate-VM64-KVM # show firewall policy
config firewall policy
    edit 6
        set uuid a8c5f4f6-1944-51e7-98b2-41983beb9039
        set srcintf "any"
        set dstintf "any"
        set srcaddr "ansible"
        set dstaddr "target"
        set action accept
        set schedule "always"
        set service "SSH"
    next
end
ebeahan commented 7 years ago

Thanks for submitting the issue. I will investigate. Does appear (based on limited info from searching as I wasn't able to find anything official from Fortinet) that the fnsysctl command was pulled from FortiOS in a recent release(s). Will also investigate possible alternatives for rolling back.

jmcgill298 commented 7 years ago

@ebeahan any update on the merge issue?

ebeahan commented 7 years ago

@jmcgill298 sorry not yet. I've done some initial testing and was able to recreate the merge issue you described, but I haven't been able to pin down where/what is causing the issue. I need to dig into the napalm-fortios driver and possibly pyFG to figure out what's going on to cause that behavior. I'll try to look into it more by the beginning of next week.

ebeahan commented 7 years ago

@dbarrosop Would you be able to take a look? Based on the logic I'm seeing in the pyFG FortiConfig class when compare_config is being called, it looks like any blocks present in the running_config but not the candidate_config get set in a "delete" statement?

https://github.com/spotify/pyfg/blob/master/pyFG/forticonfig.py#L160-L162

dbarrosop commented 7 years ago

@ebeahan sorry for the late response. Yeah, you are correct.

Regarding the issue, has someone contacted/talked with fortinet?

ebeahan commented 7 years ago

@dbarrosop Thanks for clarifying. I don't have any vendor contacts with Fortinet anymore, so I haven't been able to confirm what version the fnsysctl command got pulled starting out.

awlx commented 7 years ago

I am running version v5.4.1 and I still have fnsysctlon all devices.

rwat commented 6 years ago

confirmed that fnsysctl is present on v5.6.2