david22swan commented 5 years ago

PR to update the module to utilize a transport class rather than the current device one. Will be backwards compatible.

Code updated to include a transport class, lib/puppet/transport/cisco_nexus.rb.
Schema for transport class created, lib/puppet/transport/schema/cisco_nexus.rb.
Device file altered to be a wrapper for transport to maintain backwards compatibility, lib/puppet/util.network_device/cisco_nexus/device.rb.
Device values previously given, address and username, have been updated to be host and user.
Transport shim code added, lib/puppet_x/cisco/transport_shim.rb.
Tests for new transport class implemented, spec/unit/puppet/transport/cisco_nexus_spec.rb.
Tests for device class downgraded to a simple initialize check, spec/unit/puppet/util/network_device/cisco_nexus/device_spec.rb
Small update made to sync.yml for setting puppet_resource_api gem version.
PDK Update ran.
Function created by @willmeek to ensure backwards compatibility.

david22swan commented 5 years ago

Change has run through the adhoc pipelines and passed successfully.

https://jenkins-master-prod-1.delivery.puppetlabs.net/view/modules/view/networking/view/ad-hoc/view/ciscopuppet/job/forge-netdev_cisco-ciscopuppet_init-manual-parameters-cisco_adhoc/9/

willmeek commented 5 years ago

@david22swan Could you fixup the Rubocop issues.

codecov-io commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@1746f1d). Click here to learn what that means. The diff coverage is 14.34%.

@@            Coverage Diff            @@
##             develop    #550   +/-   ##
=========================================
  Coverage           ?   5.79%           
=========================================
  Files              ?     177           
  Lines              ?   20924           
  Branches           ?       0           
=========================================
  Hits               ?    1212           
  Misses             ?   19712           
  Partials           ?       0

Impacted Files	Coverage Δ
lib/puppet_x/cisco_nexus/transport_shim.rb	`0% <0%> (ø)`
lib/puppet/transport/schema/cisco_nexus.rb	`100% <100%> (ø)`
...b/puppet/util/network_device/cisco_nexus/device.rb	`54.16% <52.17%> (ø)`
lib/puppet/transport/cisco_nexus.rb	`95% <95%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1746f1d...4f7678b. Read the comment docs.

david22swan commented 5 years ago

Rubocop error's fixed. They were hiding behind the modules cloned down by the spec_prep

david22swan commented 5 years ago

@mikewiebe @chrisvanheuveln - Would you be able to review this pr please? Tests have passed clean on the 9k and while failures where encountered when running against the 7k, they were consistent with those seen on the released version.

mikewiebe commented 5 years ago

@david22swan Thanks for the PR. We will review this but it will likely be next week before we can get to it.

shermdog commented 5 years ago

I don't love changing the configuration key names from what we were using with puppet device. My preference would be some sort of backwards compatibility with the old keys, or at the very least well documented note on the change. (Same applies for cisco_ios)

willmeek commented 5 years ago

@shermdog @mikewiebe @chrisvanheuveln

Pushed a commit to https://github.com/cisco/cisco-network-puppet-module/pull/551 which contains a function that maps pre-transport fields to transport schema. Allows backwards compatibility.

david22swan commented 5 years ago

Work done by @willmeek to ensure backwards compatibility, https://github.com/cisco/cisco-network-puppet-module/pull/551, has been pulled into this pr.

mikewiebe commented 5 years ago

@david22swan I am running a full regression and will merge if things look ok.

cisco / cisco-network-puppet-module

(feat) - Implement Transport #550

Codecov Report