napalm-automation / napalm-yang

Bindings for napalm based on YANG models
Apache License 2.0
54 stars 44 forks source link

How can you negate a config item? #134

Closed raddessi closed 6 years ago

raddessi commented 6 years ago

I may be dense but I can't seem to find any documentation about how to perform a no <something>. Specifically in this case I'm just trying to wipe the description on a port. I've tried setting it to False/None and an empty string, False/None just get converted to strings and an empty string fails entirely with the cli complaining about an incomplete command.

----------
          ID: interfaces_config
    Function: napalm_yang.managed
      Result: False
     Comment: Cannot execute "load_merge_candidate" on 192.168.255.2 as admin. Reason: Error [1002]: CLI command 9 of 10 'description' failed: invalid command [incomplete command (at token 1: None)]!

I am running with:

    - napalm-yang==0.1.0
    - pyang==1.7.4
    - pyangbind==0.8.0

I can think of a few other cases we would need to no <something> though, is this a supported feature?

raddessi commented 6 years ago

Similarly, I just ran in to the issue that passing interfaces.interface.Ethernet1.config.enabled = False will successfully disable an interface by setting "shutdown" on the EOS switch I'm testing on, but then flipping it back to True won't change anything (by passing a no shut) so it just continually fails the compliance report afterwards.

dbarrosop commented 6 years ago

All that is supported and should work, you are probably not merging properly the candidate into the running configuration. Would you mind sharing your code so I can verify my assumption?

dbarrosop commented 6 years ago

Ok, I did some testing and you found a bug in the mappings. I suspect your code wasn't correct anyway so, please, share it :P Will try to push a fix soon.

dbarrosop commented 6 years ago

So this was a combination of assumptions and a bug you uncovered. I have fixed it in #135. Would you care to test?

To check the current behavior, running the code:

from napalm_yang import base, models

running = base.Root()
running.add_model(models.openconfig_interfaces)
running.add_model(models.openconfig_interfaces)
running.interfaces.interface.add("et1")
running.interfaces.interface["et1"].config.enabled = False
running.interfaces.interface["et1"].config.description = "asdasd"

candidate = base.Root()
candidate.add_model(models.openconfig_interfaces)
candidate.interfaces.interface.add("et1")
candidate.interfaces.interface["et1"].config.mtu = 9000
candidate.interfaces.interface["et1"].config.enabled = True

print("running")
print(running.translate_config(profile=["eos"]))

print("translate")
print(candidate.translate_config(profile=["eos"]))

print("merge")
print(candidate.translate_config(profile=["eos"], merge=running))

Should print:

running
interface et1
    description asdasd
    shutdown
    exit

translate
interface et1
    mtu 9000
    exit

merge
interface et1
    mtu 9000
    default description
    no shutdown
    exit

The following should also work as you expect:

candidate = base.Root()
candidate.add_model(models.openconfig_interfaces)
candidate.interfaces.interface.add("et1")
candidate.interfaces.interface["et1"].config.description = ""
candidate.interfaces.interface["et1"].config.mtu = 9000
candidate.interfaces.interface["et1"].config.enabled = True
print("translate")
print(candidate.translate_config(profile=["eos"]))

Prints:

translate
interface et1
    mtu 9000
    default description
    exit
dbarrosop commented 6 years ago

Ok, just noticed there seems to be an issue with enabled still. I think it's a bug on pyangbind, will dig into it.

dbarrosop commented 6 years ago

For reference, this would fix that problem:

https://github.com/robshakir/pyangbind/pull/203

Let's see what they say.

raddessi commented 6 years ago

Hey you are fast :) I'm actually using napalm-salt to generate the config via the netyang module, I can share any data you would like but I haven't dug deep enough in to the netyang module to figure out where to dump the raw yang config yet.

Below is output from when I have manually logged on the switch and shut down a port on it, where the compliance report has failed due to it not bringing the port back up.

Here is the pillar I'm passing in to netyang.managed:

    openconfig_interfaces:
        ----------
        _kwargs:
            ----------
            filter:
                True
        interfaces:
            ----------
            interface:
                ----------
                Ethernet1:
                    ----------
                    config:
                        ----------
                        description:
                            no description
                        enabled:
                            True
                        mtu:
                            9000
                Ethernet2:
                    ----------
                    config:
                        ----------
                        description:
                            no description
                        enabled:
                            True
                        mtu:
                            9000

And here is the response from the state run:

----------
          ID: interfaces_config
    Function: napalm_yang.managed
      Result: True
     Comment: Already configured.

              Loaded config:

              interface Ethernet1
                  mtu 9000
                  description no description
                  exit
              interface Ethernet2
                  mtu 9000
                  description no description
                  exit
     Started: 12:28:36.546312
    Duration: 3858.276 ms
     Changes:   
              ----------
              compliance_report:
                  ----------
                  complies:
                      False
                  skipped:
                  to_dict:
                      ----------
                      complies:
                          False
                      extra:
                      missing:
                      present:
                          ----------
                          interfaces:
                              ----------
                              complies:
                                  False
                              diff:
                                  ----------
                                  complies:
                                      False
                                  extra:
                                  missing:
                                  present:
                                      ----------
                                      interface:
                                          ----------
                                          complies:
                                              False
                                          diff:
                                              ----------
                                              complies:
                                                  False
                                              extra:
                                              missing:
                                              present:
                                                  ----------
                                                  Ethernet1:
                                                      ----------
                                                      complies:
                                                          False
                                                      diff:
                                                          ----------
                                                          complies:
                                                              False
                                                          extra:
                                                          missing:
                                                          present:
                                                              ----------
                                                              config:
                                                                  ----------
                                                                  complies:
                                                                      False
                                                                  diff:
                                                                      ----------
                                                                      complies:
                                                                          False
                                                                      extra:
                                                                      missing:
                                                                      present:
                                                                          ----------
                                                                          description:
                                                                              ----------
                                                                              complies:
                                                                                  True
                                                                              nested:
                                                                                  False
                                                                          enabled:
                                                                              ----------
                                                                              actual_value:
                                                                                  False
                                                                              complies:
                                                                                  False
                                                                              expected_value:
                                                                                  True
                                                                              nested:
                                                                                  False
                                                                          mtu:
                                                                              ----------
                                                                              complies:
                                                                                  True
                                                                              nested:
                                                                                  False
                                                                  nested:
                                                                      True
                                                      nested:
                                                          True
                                                  Ethernet2:
                                                      ----------
                                                      complies:
                                                          True
                                                      nested:
                                                          True
                                          nested:
                                              True
                              nested:
                                  True
              diff:
              loaded_config:
                  interface Ethernet1
                      mtu 9000
                      description no description
                      exit
                  interface Ethernet2
                      mtu 9000
                      description no description
                      exit

Let me see if I can get it to print the raw yang it would generate from the pillar...

I can test https://github.com/napalm-automation/napalm-yang/pull/135 if you would like still.

raddessi commented 6 years ago

Oh, yeah, https://github.com/robshakir/pyangbind/pull/203 totally seems like the right way to go.

dbarrosop commented 6 years ago

It's fine, originally I made the assumption that "default" values would be ignored unless there was a "running" config indicating otherwise (note that in my example above "merge" was as you probably expected). That assumption is probably going to lead to confused users so let's go with robshakir/pyangbind#203 if they agree.

raddessi commented 6 years ago

Awesome, looks like they merged that in now. Do you need to do anything else here or should I test?

dbarrosop commented 6 years ago

Yeah, I need to regenerate the bindings with new pyangbind. Give me a couple of days to do so and then I will give you a branch so you can test.

dbarrosop commented 6 years ago

did you have the chance to test #135?

dbarrosop commented 6 years ago

Closing, let me know if the mentioned PR doesn't solve the problem.