napalm-automation-community / napalm-aos

NAPALM driver for Alcatel Lucent Enterprise AOS
Apache License 2.0
11 stars 13 forks source link

compare_config() returns bytes, not string? #10

Closed cs-1 closed 6 years ago

cs-1 commented 6 years ago

I run napalm-aos from ansible / ansible-napalm. It looks like there's a problem when doing an napalm_install_config. napalm tries to create a diff by calling compare_config() in aos.py. This fails with the following error message:

PLAY [Test napalm merge] ************************************************************************************************************************************************************

TASK [Gathering Facts] **************************************************************************************************************************************************************
ok: [XXXXX]

TASK [switch_merge_config : napalm install config] **********************************************************************************************************************************
fatal: [XXXXX]: FAILED! => {"changed": false, "msg": "cannot diff config: write() argument must be str, not bytes"}

PLAY RECAP **************************************************************************************************************************************************************************
XXXXX : ok=1    changed=0    unreachable=0    failed=1

It looks like compare_config() doesn't return a str type but a bytes type instead. Or do we have some other encoding issue?

napalmaos commented 6 years ago

Hi, Could you share yaml file and output?

ktbyers commented 6 years ago

Are you using python3?

There is a PR on the napalm-ansible repo regarding python3 issues.

Kirk

On Fri, Jun 22, 2018 at 12:17 PM napalmaos notifications@github.com wrote:

Hi, Could you share yaml file?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/napalm-automation-community/napalm-aos/issues/10#issuecomment-399395714, or mute the thread https://github.com/notifications/unsubscribe-auth/AA26hKPh0HkM0FUjQiAV4OSGQOTGfUoZks5t_MRNgaJpZM4UzQYW .

-- Kirk Byers ktbyers@twb-tech.com ktbyers@twb-tech.com Simplify through Automation

cs-1 commented 6 years ago

Yes, I'm indeed using python3.

This is the playbook that I'm using:

---
- name: Test napalm merge
  hosts: aos8_switches
  connection: local
  vars_prompt:
    - name: "passwd"
      prompt: "Password"
  roles:
    - switch_merge_config

Here's the role that's being used:

---
- name: napalm install config
  napalm_install_config:
    hostname={{ inventory_hostname }}
    username={{ user }}
    dev_os={{ os }}
    password={{ passwd }}
    config_file="/XXXX/ansible-playbooks/configs/{{ inventory_hostname }}-merge.txt"
    commit_changes=False
    replace_config=False
    diff_file="/XXXX/ansible-playbooks/diffs/{{ inventory_hostname }}.txt"
ktbyers commented 6 years ago

Yes, you will probably need to use Python2 and not Python3 until we fix the above issue and do more testing on it.

Obviously, you are welcome to work on this as well.

cs-1 commented 6 years ago

Thanks for the headsup, Kirk. Can you point me to the napalm-ansible issue that you mentioned? I'll have a look whether or not it's within my capabilities to fix it. Simply casting the output of the diff to str doesn't work because it'll include a "b" at the beginning to denote the original byte type.

ktbyers commented 6 years ago

https://github.com/napalm-automation/napalm-ansible/pull/134

cs-1 commented 6 years ago

I proposed a patch in https://github.com/napalm-automation/napalm-ansible/pull/134, I hope that it works.

vmuthukrishnan commented 6 years ago

Hi, In napalm-automation/napalm-ansible#134 you have proposed a patch to resolve this issue. Could you close this issue?

cs-1 commented 6 years ago

Yes, sure.