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.

Sign up for GitHub

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

Jump to bottom

Update gNMI-1.4 with FAN_TRAY type. #3046

Merged
merged 43 commits into from
Jul 16, 2024
Merged

Update gNMI-1.4 with FAN_TRAY type. #3046

merged 43 commits into from
Jul 16, 2024

Conversation

SydneyCaulfeild
Copy link
Contributor

No description provided.

@SydneyCaulfeild SydneyCaulfeild requested review from a team as code owners May 31, 2024 19:15
@SydneyCaulfeild SydneyCaulfeild marked this pull request as draft May 31, 2024 19:15
@coveralls
Copy link

coveralls commented May 31, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 9962499111

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 55.5%
Totals Coverage Status
Change from base Build 9960910543: 0.0%
Covered Lines: 1983
Relevant Lines: 3573
💛 - Coveralls

@SydneyCaulfeild SydneyCaulfeild changed the title add fan_tray type to readme Update gNMI-1.4 with state/model-name. Jun 4, 2024
@SydneyCaulfeild SydneyCaulfeild changed the title Update gNMI-1.4 with state/model-name. Update gNMI-1.4 with FAN_TRAY type. Jun 4, 2024
@SydneyCaulfeild
Copy link
Contributor Author

SydneyCaulfeild commented Jun 5, 2024 •
edited
Loading

Here are some examples of vendor hardware models that have fan trays along with how many fan trays they are expected to have. These fan tray components should have components/component/state/type of "FAN_TRAY".

  • Cisco 8808
    • This has 4 fan tray components (names "0/FT0", "0/FT1", "0/FT2", "0/FT3").
    • Each fan tray has 4 fan subcomponents.
  • Juniper PTX10008
    • This has 2 fan tray components (names "Fan Tray 0", "Fan Tray 1").
    • These do not have child fan subcomponents.
  • Nokia 7250-ixr10e
    • This has 4 fan tray components (names "Fan1", "Fan2", "Fan3", "Fan4").
    • These do not have child fan subcomponents.

Other devices such as Arista DCS-7808 do not have fan tray components.

@LimeHat
Copy link
Contributor

LimeHat commented Jun 6, 2024

Since not all chassis are expected to have FAN components, should the FAN_TRAY component validation just replace FAN validation? Or there should be a switch/case statement that enables/disables FAN validation per vendor.

@dplore dplore mentioned this pull request Jun 8, 2024
Closed
@SydneyCaulfeild
Copy link
Contributor Author

Since not all chassis are expected to have FAN components, should the FAN_TRAY component validation just replace FAN validation? Or there should be a switch/case statement that enables/disables FAN validation per vendor.

I added a deviation to customize which devices support the FAN_TRAY type. From my understanding though, a device could have either 1) just FAN_TRAY, 2) FAN_TRAY with child FANs, or 3) just FANs that are not in a tray). So I think we'll want to continue to validate both components because the FAN_TRAY type is not replacing the FAN type - they are different components.

@SydneyCaulfeild SydneyCaulfeild marked this pull request as ready for review June 10, 2024 22:17
@LimeHat
Copy link
Contributor

LimeHat commented Jun 10, 2024 •
edited
Loading

So I think we'll want to continue to validate both components because the FAN_TRAY type is not replacing the FAN type - they are different components.

But the FAN is not a mandatory component in this case (like you indicated in # 1). It can be present, but it doesn't have to; which is not accounted for in the current code.

@OpenConfigBot
Copy link

OpenConfigBot commented Jun 24, 2024 •
edited
Loading

Pull Request Functional Test Report for #3046 / 9c81c8e

Virtual Devices

Device Test Test Documentation Job Raw Log
Arista cEOS status
 gNMI-1.4: Telemetry: Inventory
 2af28ad7 Log
Cisco 8000E status
 gNMI-1.4: Telemetry: Inventory
 a49026a7 Log
Cisco XRd status
 gNMI-1.4: Telemetry: Inventory
 e5f77335 Log
Juniper ncPTX status
 gNMI-1.4: Telemetry: Inventory
 05f75790 Log
Nokia SR Linux status
 gNMI-1.4: Telemetry: Inventory
 d226d27b Log
Openconfig Lemming status
 gNMI-1.4: Telemetry: Inventory
 801fc1b2 Log

Hardware Devices

Device Test Test Documentation Raw Log
Arista 7808 status
 gNMI-1.4: Telemetry: Inventory
Cisco 8808 status
 gNMI-1.4: Telemetry: Inventory
Juniper PTX10008 status
 gNMI-1.4: Telemetry: Inventory
Nokia 7250 IXR-10e status
 gNMI-1.4: Telemetry: Inventory

Help

dplore
dplore previously approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@dplore dplore left a comment

Choose a reason for hiding this comment

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

LGTM

@SydneyCaulfeild SydneyCaulfeild merged commit 0db4711 into openconfig:main Jul 16, 2024
13 checks passed
@SydneyCaulfeild SydneyCaulfeild deleted the update-gnmi-1.4-with-fan_tray branch July 16, 2024 21:31
bkreddy143 pushed a commit to nokia/featureprofiles that referenced this pull request Jul 17, 2024
@SydneyCaulfeild @bkreddy143
Update gNMI-1.4 with FAN_TRAY type. (openconfig#3046)
91dafec 
* add fan_tray type to readme

* Add FAN_TRAY type to telemetry_inventory_test.go.

* add NumFanTrays arg

* skip fan tray telemetry check when 0 are expected

* add FAN_TRAY as platform_type to README.

* validate that fan tray parent is the chassis.

* validate fan component's parent.

* Add deviation for fan tray type.

* Use fan tray deviation in telemetry_inventory_test.

* fix call to FanTrayTypeUnsupported.

* Remove switch_chip_id_unsupported from Nokia.

* Add arg_num_fans.

* Skip fan validation when args_num_fans is 0.

* Update args.go

* Fix metadata.pb.go.

* Fix metadata.pb.go.

* Fix deviations.go typo.

* Only check that FAN has ancestor chassis, not the immediate parent type.

* Fix metadata.pg.go.

* FIx metadata.pb.go.

* Fix metadata.pb.go.

* Remove fan_tray deviation from metadata.proto.

* Remove fan_tray deviation from deviations.go.

* Remove fan_tray deviation from metadata.textproto.

* Update metadata.proto

* Remove fan_tray deviation from telemetry_inventory_test.go.

* Fix metadata.pb.go.

* Fix brackets in telemetry_inventory_test.go.
frasieroh pushed a commit to aristanetworks/openconfig-featureprofiles that referenced this pull request Jul 17, 2024
@SydneyCaulfeild @frasieroh
Update gNMI-1.4 with FAN_TRAY type. (openconfig#3046)
f4b2651 
* add fan_tray type to readme

* Add FAN_TRAY type to telemetry_inventory_test.go.

* add NumFanTrays arg

* skip fan tray telemetry check when 0 are expected

* add FAN_TRAY as platform_type to README.

* validate that fan tray parent is the chassis.

* validate fan component's parent.

* Add deviation for fan tray type.

* Use fan tray deviation in telemetry_inventory_test.

* fix call to FanTrayTypeUnsupported.

* Remove switch_chip_id_unsupported from Nokia.

* Add arg_num_fans.

* Skip fan validation when args_num_fans is 0.

* Update args.go

* Fix metadata.pb.go.

* Fix metadata.pb.go.

* Fix deviations.go typo.

* Only check that FAN has ancestor chassis, not the immediate parent type.

* Fix metadata.pg.go.

* FIx metadata.pb.go.

* Fix metadata.pb.go.

* Remove fan_tray deviation from metadata.proto.

* Remove fan_tray deviation from deviations.go.

* Remove fan_tray deviation from metadata.textproto.

* Update metadata.proto

* Remove fan_tray deviation from telemetry_inventory_test.go.

* Fix metadata.pb.go.

* Fix brackets in telemetry_inventory_test.go.
@LimeHat LimeHat mentioned this pull request Aug 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dplore dplore dplore approved these changes

@LimeHat LimeHat Awaiting requested review from LimeHat

Assignees

@dplore dplore

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@SydneyCaulfeild @coveralls @LimeHat @OpenConfigBot @dplore