cisco / cisco-network-puppet-module

Apache License 2.0
55 stars 68 forks source link

ensure => absent on Ethernet interfaces fails #424

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hey all, I'm running through some basic manifests for testing & validation of the module against our config. I have a manifest for each corresponding feature that applies some basic dummy configuration then a removal class that cleans it up.

In this manifest, I have two Ethernet interfaces to become part of a port-channel, which then is configured as a vPC Peer Link. However, when I run through the removal class to un-configure the Ethernet interfaces (using cisco_interface { 'Ethernet1/10': ensure => absent, } ), a CLI error is produced. See below:

vpc.pp

...<truncated>
  cisco_interface { "port-channel${peerlink_po_id}" :
    ensure                  => present,
    description             => 'vPC Peerlink',
    switchport_mode         => 'trunk',
    vpc_peer_link           => true,
    stp_port_type           => 'network',
    storm_control_broadcast => '90.00',
    storm_control_multicast => '90.00',
    shutdown                => false,
    require                 => Cisco_vpc_domain["${domain}"],
  }
  $peerlink_interfaces.each |String $interface| {
    cisco_interface { $interface :
      ensure                  => present,
      description             => "vPC Peerlink Po${peerlink_po_id}",
      switchport_mode         => 'trunk',
      stp_port_type           => 'network',
      storm_control_broadcast => '90.00',
      storm_control_multicast => '90.00',
      shutdown                => false,
    }
    cisco_interface_channel_group { "${interface}-Po${peerlink_po_id}" :
      ensure        => present,
      interface     => $interface,
      channel_group => $peerlink_po_id,
      require       => Cisco_interface["port-channel${peerlink_po_id}"],
    }
  }
...

vpc_removal.pp

...<truncated>
  $peerlink_interfaces.each |String $interface| {
    cisco_interface_channel_group { "${interface}-Po${peerlink_po_id}" :
      ensure => absent,
    }
    cisco_interface { $interface :
      ensure => absent,
    }
  }

Here's the --debug run while trying to run the vpc_removal class:

Notice: /Stage[main]/Profile::Cisco::Testcase::Vpc_remove/Cisco_interface[Ethernet1/10]/ensure: removed
Debug: Set state using data format 'cli'
Debug:   to value(s):
    no interface ethernet1/10
Debug: Input (cli_conf): 'no interface ethernet1/10'
Debug: Sending HTTP request to NX-API at localhost:
{"accept-encoding"=>["gzip;q=1.0,deflate;q=0.6,identity;q=0.3"], "accept"=>["*/*"], "user-agent"=>["Ruby"], "cookie"=>["nxapi_auth=admin:local"], "content-type"=>["application/json"]}
{"ins_api":{"version":"1.0","type":"cli_conf","chunk":"0","sid":"1","input":"no interface ethernet1/10","output_format":"json"}}
Debug: HTTP Response: OK
{
        "ins_api":      {
                "sid":  "eoc",
                "type": "cli_conf",
                "version":      "1.0",
                "outputs":      {
                        "output":       {
                                "code": "400",
                                "msg":  "CLI execution error",
                                "clierror":     "Invalid range\n"
                        }
                }
        }
}
Error: /Stage[main]/Profile::Cisco::Testcase::Vpc_remove/Cisco_interface[Ethernet1/10]: Could not evaluate: [interface ethernet1/10] The command 'no interface ethernet1/10' was rejected with error:
Invalid range

Debug: /Stage[main]/Profile::Cisco::Testcase::Vpc_remove/Cisco_interface_channel_group[Ethernet1/11-Po100]: Nothing to manage: no ensure and the resource doesn't exist
Notice: /Stage[main]/Profile::Cisco::Testcase::Vpc_remove/Cisco_interface[Ethernet1/11]/ensure: removed
Debug: Set state using data format 'cli'
Debug:   to value(s):
    no interface ethernet1/11
Debug: Input (cli_conf): 'no interface ethernet1/11'
Debug: Sending HTTP request to NX-API at localhost:
{"accept-encoding"=>["gzip;q=1.0,deflate;q=0.6,identity;q=0.3"], "accept"=>["*/*"], "user-agent"=>["Ruby"], "cookie"=>["nxapi_auth=admin:local"], "content-type"=>["application/json"]}
{"ins_api":{"version":"1.0","type":"cli_conf","chunk":"0","sid":"1","input":"no interface ethernet1/11","output_format":"json"}}
Debug: HTTP Response: OK
{
        "ins_api":      {
                "sid":  "eoc",
                "type": "cli_conf",
                "version":      "1.0",
                "outputs":      {
                        "output":       {
                                "code": "400",
                                "msg":  "CLI execution error",
                                "clierror":     "Invalid range\n"
                        }
                }
        }
}
Error: /Stage[main]/Profile::Cisco::Testcase::Vpc_remove/Cisco_interface[Ethernet1/11]: Could not evaluate: [interface ethernet1/11] The command 'no interface ethernet1/11' was rejected with error:
Invalid range

Manually applying these commands via CLI results in the same error. I believe a fix for this would be to use default interface EthernetX/X, which would remove all configuration on a physical interface and restore it to default state, which is essentially what I'm trying to accomplish with the ensure => absent attribute.

ghost commented 7 years ago

Looks related to the destroy for interface.rb in node_utils:

https://github.com/cisco/cisco-network-node-utils/blob/develop/lib/cisco_node_utils/cmd_ref/interface.yaml#L49L51

I'm not sure the best solution for this, as most logical interfaces have to be removed via the no interface Vlan|loopback|port-channel command but the same command fails for physical interfaces.

mikewiebe commented 7 years ago

@tomcooperca This is an interesting case. As you pointed out physical interfaces are not really ensurable because if they are present they will always exist (can't be destroyed) regardless of their individual attributes. Let me discuss how we might restore a physical interface to it's default state with my development team and get back to you.

For now you could use the cisco_command_config provider to issue the default interface EthernetX/X command.

krisamundson commented 7 years ago

What if "ensure => absent" translated to "default the interface and shutdown"? The administrative state would have to merge with the "shutdown" parameter -- if shutdown was false then it would default the interface and 'no shut'. If shutdown was true or not defined it would default the interface and shutdown.

chrisvanheuveln commented 7 years ago

@krisamundson The "system default switchport" config settings interfere with shutdown so that probably wouldn't fly.

When I originally wrote the interface provider I suggested early on that we support "ensure => absent" for physical interfaces == default interface. It's easy enough to implement in the node utils destroy method but as Mike points out it's difficult to reliably detect idempotency here. I suppose you could just use the output from a "show run interface" command (no "all"), though that would have to take into account the "system default" configs and likely a few other things.

I suspect it would be a bit buggy but it's worth reconsidering this idea.

ghost commented 7 years ago

PS: for my use case, the command_config works fine as it's just to remove test config and is only run once anyways. FWIW, I'm also using the shutdown attribute to admin-down ports that are unused and mark them as "vacant" in the description. I'm almost inclined to think that the ensure remain as-is, as it already removes logical interfaces from the configuration. Looks to be a behaviour inherit to NX-OS where the physical interface is forever "present" in the running config even if it is defaulted or not in use.

ghost commented 7 years ago

What about ensure => purged for this type?

chrisvanheuveln commented 7 years ago

Sure, 'purged' is probably more meaningful than absent in this case. There would still be an issue with idempotency though.

mikewiebe commented 7 years ago

@tomcooperca Good to know that cisco_command_config will work for your use case. Agree with Chris/Tom that purged is probably a better choice.

ghost commented 7 years ago

Would a regex check on show run interface Ethernet1/1 like this be sufficient? If an interface is set back to default, it should only contain the interface declaration and nothing else. Thinking something like this:

Has default interface Ethernet1/2 been executed (yes in this case)? Regex: ^interface Ethernet1\/2\n^$ Check the interface configuration:

show running-config interface Ethernet 1/2

!Command: show running-config interface Ethernet1/2
!Time: Thu Feb 16 14:25:23 2017

version 7.0(3)I4(2)

interface Ethernet1/2

Matches successfully.

Has default interface Ethernet1/1 occurred? Regex: ^interface Ethernet1\/1\n^$

show running-config interface Ethernet 1/1

!Command: show running-config interface Ethernet1/1
!Time: Thu Feb 16 14:25:39 2017

version 7.0(3)I4(2)

interface Ethernet1/1
  ip address 10.0.0.1/31

No match found (there's configuration present below the interface Ethernet1/1 stanza.

Could this be sufficient and/or testable? Or would this interfere with existing logical interfaces for the cisco_interface type?

mikewiebe commented 7 years ago

Generally speaking, if no attributes are visible under the interface using the show running command the interface attributes are in the default state. I need to double check behavior with the various system level default commands that @chrisvanheuveln was referring to earlier to be sure we can't get into a state where an attribute is visible yet still default. I also need to check across all of the nxos platforms we support to make sure the behavior is consistent.

ghost commented 7 years ago

Got it, makes perfect sense - keep me posted!

mikewiebe commented 7 years ago

@tomcooperca We tried multiple solutions for this but decided it would be best to add a property that would allow you to purge the config on ethernet interfaces.

Let us know if this works for you and we can close this out.