Skip to content

Conversation

mikewiebe
Copy link
Contributor

Summary:
In later versions of Nexus, the vxlan vtep host reachability must be set properly before setting the ingress replication feature.

This update modified the ingress_replication setter to check the should intent for protocol and ensures that the vtep host reachability is set properly to support the protocol setting.

Testing:
Tests pass on n7k, n9k(latest, greensboro, freeport and camden), n9k-fretta)

@mikewiebe mikewiebe requested review from rahushen and saichint May 7, 2018 14:51
@saichint
Copy link

saichint commented May 7, 2018

👍

# Note: ingress-replication is not supported on all platforms.
# Use to_s.empty check to also handle nil check.
if ingress_replication.to_s.empty?
set_host_reachability(@set_args[:name], protocol)
Copy link
Contributor

@rahushen rahushen May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe move this before if/else as it is exercised in both code paths

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually.. I tested it and it fails if I move it out of the if/else block. The reason is that the two lines in the else block have to be executed first.

Copy link
Contributor

@rahushen rahushen May 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this work?

unless ingress_replication.to_s.empty?
  set_args_keys(state: 'no', protocol: ingress_replication)
  config_set('vxlan_vtep_vni', 'ingress_replication', @set_args)
end
set_host_reachability(@set_args[:name], protocol)
set_args_keys(state: '', protocol: protocol)
config_set('vxlan_vtep_vni', 'ingress_replication', @set_args)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup. That works. Thanks for the suggestion.

@rahushen
Copy link
Contributor

rahushen commented May 7, 2018

@mikewiebe .. Minor comment. Otherwise 👍

@rahushen rahushen merged commit 4543f46 into develop May 7, 2018
@mikewiebe mikewiebe deleted the rel200/fix_vxlan_vtep_vni branch February 1, 2019 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants