Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(suzieq/poller/worker/services/interfaces.py): change xcvr unsupported adminState for NX-OS #931

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

beufanet
Copy link
Contributor

Related Issue

Fixes #930

Description

XCVR not inserted should be managed as admin down interface.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

New Behavior

When dealing with Cisco NX-OS, output for shutdown interface when SFP is not inserted is the following :

cisco# show interface brief
<...>
--------------------------------------------------------------------------------
Ethernet      VLAN    Type Mode   Status  Reason                   Speed     Port
Interface                                                                    Ch #
--------------------------------------------------------------------------------
Eth1/1        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/2        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/3        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/4        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/5        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/6        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/7        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/8        --      eth  routed down    XCVR not inserted          auto(D) --
Eth1/9        1       eth  trunk  up      none                        25G(D) 9
Eth1/10       --      eth  routed up      none                        25G(D) --
Eth1/11       1       eth  trunk  up      none                        25G(D) --
Eth1/12       --      eth  routed up      none                        25G(D) --

Even if the interface is in shutdown adminState.

...

Contrast to Current Behavior

In the following line, this is considered as if the link is not connected, even if the interface is set to shutdown state.

                if entry['reason'] in ["link not connected",
                                       "xcvr not inserted"]:
                    entry['state'] = 'notConnected'

Creating a specific case for the entry reason "xcvr not inserted" to change adminState to down:

               if entry['reason'] == "xcvr not inserted":
                    entry['state'] = 'notConnected'
                    entry['adminState'] = 'down'

Discussion: Benefits and Drawbacks

It will help to show interfaces really down, as an interface with no XCVR is by default shutdown.

Changes to the Documentation

None

Proposed Release Note Entry

Consider NX-OS interface with Reason 'XCVR not inserted' as adminState=down
...

Comments

None

Double Check

  • [X ] I have read the comments and followed the CONTRIBUTING.md.
  • I have explained my PR according to the information in the comments or in a linked issue.
  • My PR source branch is created from the develop branch.
  • My PR targets the develop branch.
  • All my commits have --signoff applied

…rted adminState for NX-OS

Signed-off-by: Fabien VINCENT <[email protected]>
…rted adminState for NX-OS

Signed-off-by: Fabien VINCENT <[email protected]>
@beufanet beufanet force-pushed the nx-os-xcvr-interface branch from 37b6a54 to 5b2c1a5 Compare March 26, 2024 08:31
@ddutt
Copy link
Member

ddutt commented Mar 26, 2024

Thank you @beufanet we'll get this incorporated into the next release.

@ddutt ddutt merged commit 4943848 into netenglabs:develop Apr 10, 2024
13 checks passed
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.

[Feature]: Cisco NX-OS: consider xcvr interfaces down instead of not connected
3 participants