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

Add component install-position, install-component and deprecate location #1095

Merged
merged 31 commits into from
May 29, 2024

Conversation

dplore
Copy link
Member

@dplore dplore commented Apr 13, 2024

Fixes #1076

Change Scope

  • Deprecate the location leaf as it's current usage varies across implementations.
  • Introduce the install-position, install-component which more precisely define where a component is installed into a target index or install-position of a target install-component.

This change is backwards compatible.

The operational use case forinstall-position is: as a hardware replacement technician I want to receive an indication of the location of a replaceable hardware component within some network device physical package (chassis, shelf, linecard, etc). An example subset of the replaceable components in scope could include a chassis, fan, fabric, powersupply , linecard module or transceiver module. The value of the position indication should exactly match some physical label or display on the hardware or if that is not feasible, it should exactly match documentation identifying the available positions on the hardware component(s).

Tree view

*** /Users/dloher/old-tree.txt  Thu May  2 15:29:21 2024
--- new-tree.txt        Thu May  2 15:29:08 2024
***************
--- 8670,8678 ----
module: openconfig-platform
  +--rw components
     +--rw component* [name]
        +--rw name                           -> ../config/name
        +--rw config
        |  +--rw name?   string
        +--ro state          |  +--ro name?                           string
          |  +--ro type?                           union
          |  +--ro id?                             string
!         |  x--ro location?                       string
>         |  +--ro install-position?               string
>         |  +--ro install-component?              string
          |  +--ro description?                    string
          |  +--ro mfg-name?                       string
          |  +--ro mfg-date?                       oc-yang:date

[... snip ...]

        +--rw oc-linecard:linecard
           +--rw oc-linecard:config
           |  +--rw oc-linecard:power-admin-state?   oc-platform-types:component-power-type
           +--ro oc-linecard:state
           |  +--ro oc-linecard:power-admin-state?   oc-platform-types:component-power-type
!          |  x--ro oc-linecard:slot-id?             string

Platform Implementations

  • IOS XR
openconfig-platform-types:PORT
   /components/component[name="Port0/1/0/12"]/state/parent = "0/1/CPU0-NPU1"
   /components/component[name="Port0/1/0/12"]/state/location = "0/1/CPU0"

   New values:
   /components/component[name="Port0/1/0/12"]/state/install-position = "12"
   /components/component[name="Port0/1/0/12"]/state/install-component = "0/0/CPU0"

(IOSXR on the 8800 series names linecards including a CPU identifier)

  • JunOS
openconfig-platform-types:PORT components:
/components/component[name="FPC0:PIC0:PORT1"]/state/location = "FPC0:PIC0:PORT1"

New value:
/components/component[name="FPC0:PIC0:PORT1"]/state/install-position = "1"
/components/component[name="FPC0:PIC0:PORT1"]/state/install-component = "FPC0:PIC0"
  • EOS
openconfig-platform-types:PORT components:
/components/component[name="Ethernet4/1-Port"]/state/location = "4/1"

New value:
/components/component[name="Ethernet4/1-Port"]/state/install-position = "1"
/components/component[name="FPC0:PIC0:PORT1"]/state/install-component = "Linecard4"

@OpenConfigBot
Copy link

OpenConfigBot commented Apr 13, 2024

No major YANG version changes in commit f01c1fe

@LimeHat
Copy link

LimeHat commented Apr 13, 2024

This is an interesting example.
Does an ASIC (NPU1) represent a helpful parent-position value for this use case?

@dplore
Copy link
Member Author

dplore commented Apr 13, 2024

This is an interesting example. Does an ASIC (NPU1) represent a helpful parent-position value for this use case?

No, the port to NPU does not map to the parent-position use case.

It is perhaps and example of why location may not be needed. The port to NPU component relationship is good to represent using the parent leaf. location is redundant.

@LimeHat
Copy link

LimeHat commented Apr 13, 2024

It is perhaps and example of why location may not be needed. The port to NPU component relationship is good to represent using the parent leaf. location is redundant.

Perhaps, but it also shows that the parent-position is not applicable in some cases. In fact, the location leaf in this scenario would be more useful, since the location doesn't have to be directly related to the parent.

Just want to make sure you're ok with this (some components may not have a parent-position, or have a parent-position that isn't useful)

@dplore dplore marked this pull request as ready for review April 16, 2024 00:38
@dplore dplore requested a review from a team as a code owner April 16, 2024 00:38
@dplore
Copy link
Member Author

dplore commented Apr 16, 2024

It is perhaps and example of why location may not be needed. The port to NPU component relationship is good to represent using the parent leaf. location is redundant.

Perhaps, but it also shows that the parent-position is not applicable in some cases. In fact, the location leaf in this scenario would be more useful, since the location doesn't have to be directly related to the parent.

Just want to make sure you're ok with this (some components may not have a parent-position, or have a parent-position that isn't useful)

I expanded on the description for parent-position.

@rgwilton
Copy link

@dplore, for clarity, in your example for IOS XR, should the parent-position not be '12' rather than 'NPU1'?

@dplore
Copy link
Member Author

dplore commented Apr 24, 2024

@dplore, for clarity, in your example for IOS XR, should the parent-position not be '12' rather than 'NPU1'?

Yes, thanks for this. I updated the example in the PR description to reflect this.

In that particular example, the parent of the PORT is an NPU. The operational use case we are solving for requires a mapping from the PORT to the position in the LINECARD (or CHASSIS in a fixed form factor device). Looking at this physically, both relationships exist; the port is wired to an NPU and NPU also occupies a position in the LINECARD/CHASSIS. So the hierarchy is PORT -> INTEGRATED_CIRCUIT -> LINECARD -> CHASSIS or PORT -> INTEGRATED_CIRCUIT -> CHASSIS or . We see this often. There could be additional additional ancestors in the tree between PORT and LINECARD/CHASSIS.

@SydneyCaulfeild and I discussed and had some ideas on how to resolve this programmatically. I'll let her comment.

@LimeHat
Copy link

LimeHat commented Apr 24, 2024

Based on the updated example for IOS XR, it seems that parent-position should be ancestor-position instead?

According to the conversation above, my understanding was that the parent-position would not be applicable to a port when the parent is not a linecard module.

@SydneyCaulfeild
Copy link
Contributor

I will still need this new leaf to be applicable even when the parent is not a linecard module. For example, the ports on an Arista have ancestors of PORT -> INTEGRATED_CIRCUIT -> LINECARD, and I need the slot of the port in the linecard. As another example, I need the relative location of a power supply in a power tray on Cisco 8808 devices. Here, the power tray is the grandparent of the power supply (using component names, it goes from 0/PT0-PM1 (power supply) -> 0/PT0-Power Module Slot 1" (no type) -> 0/PT0 (power tray)). There are other components within a parent of CHASSIS that I need the relative location of, such as a fabric card on some Arista devices.

I agree this should likely be renamed to ancestor-position to avoid confusion, because the position is not necessarily within the direct parent.

To make this leaf more clear, I think we should update the description to say that this leaf is "the relative location of the component within the first ancestor that is x". I believe x could be something like "is an FRU" (perhaps using the state/removable path) or "has part and serial numbers". In all examples I have seen, the intermediate parent that we want to "ignore" (ex INTEGRATED_CIRCUIT or NPU) does not have a part or serial number. They are also not removable.

Does anyone have any suggestions on which path would be best to describe the ancestor that we want the relative location from?

@SydneyCaulfeild
Copy link
Contributor

SydneyCaulfeild commented Apr 25, 2024

I would also like to point out that some components already apply this logic of "use the location of a removable ancestor" in their current state/location path, so I don't think that this constraint is necessarily new.

For example, in a Cisco 8808, a component of type POWER_SUPPLY is the grandchild of a power tray. Example names: 0/PT0-PM1 power supply -> 0/PT0-Power Module Slot 1 intermediate component -> 0/PT0 power tray. The state/location of the power supply right now is 0/PT0 which is the name of the grandparent, not the parent. (I find it confusing that it holds the name and not the location, but either way this shows that it skips the immediate parent).

@dplore
Copy link
Member Author

dplore commented May 2, 2024

This was re-reviewed in the April 30 2024 OC operators meeting without objection.

@oscargdd wanted to review as his organization was running into similar challenges using the location leaf. @oscargdd please review and if you approve.

@dplore
Copy link
Member Author

dplore commented May 2, 2024

Setting this to last call for May 14th to give Oscar time to review.

@dplore dplore changed the title Add component parent-position and deprecate location Add component install-position, install-component and deprecate location May 2, 2024
@dplore
Copy link
Member Author

dplore commented May 2, 2024

Refreshed the PR description. @SydneyCaulfeild please review if it meets your expectations.

@SydneyCaulfeild
Copy link
Contributor

@dplore the PR meets my expectations. Thanks for adding those examples!

@dplore dplore requested a review from s19nal May 28, 2024 16:46
Copy link

@s19nal s19nal left a comment

Choose a reason for hiding this comment

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

looks like all the open comments have been addressed Darren - happy to approve this one.

@dplore dplore merged commit 9eeeb20 into master May 29, 2024
14 checks passed

leaf install-position {
type leafref {
path "../name";
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this now incorrect according to the description?

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you for catching this. Yes, it should've remained as a string. I will update it now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Semantics and format of Component Location
10 participants