Update openconfig-gnsi-acctz.yang

[adityasingh-anet]

• Delete idle-timeouts and counters-last-cleared leaves
• Split source-counters and client-counters to be under separate augments

Change Scope

• https://github.com/openconfig/gnsi/blob/main/acctz/gnsi-acctz.yang has received updates that are not in openconfig/public. In particular, the idle-timeouts and counters-last-cleared leaves have been deleted. This PR will bring those changes into openconfig/public

• source-counters should not be under the grpc-server list since they cannot be associated to a specific grpc-server. Instead create another augment so source-counters are on a different path than client-counters

• Address typo in the source-records list description

• This change is not backwards compatible

pyang output:

module: openconfig-gnsi-acctz

  augment /oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server:
    +--ro acctz
       +--ro state
          +--ro counters
             +--ro history-istruncated?   oc-yang:counter64
             +--ro record-requests?       oc-yang:counter64
             +--ro record-responses?      oc-yang:counter64
  augment /oc-sys:system/oc-sys:aaa/oc-sys:accounting:
    +--ro acctz
       +--ro source-records
          +--ro source-record* [service type]
             +--ro service    -> ../state/service
             +--ro type       -> ../state/type
             +--ro state
                +--ro service?    service-request
                +--ro type?       service-type
                +--ro counters
                   +--ro records?   oc-yang:counter64

Platform Implementations

None

[dplore]

@mihirpitale-googler to review

[dplore]

/gcbrun

[OpenConfigBot]

No major YANG version changes in commit ec04408

[dplore]

@morrowc for review

[dplore]

@haussli for review.

[dplore]

Related to change made in acctz at openconfig/gnsi#175

[dplore]

@adityasingh-anet to fix the linter errors, we need a "state" container added on top of the counters container. For example:

"/system/grpc-servers/grpc-server/acctz/state/counters/history-istruncated"

[adityasingh-anet]

To address the linter errors I have added a "state" container in the client counters path.
Additionally, the augment path being used for the source-counters has been changed from system/aaa/state to state/aaa/accounting since only one state component is allowed in the path and the source-records list elements are already in a state container.

My original review comment has been updated with the new pyang output

[dplore]

/gcbrun

[dplore]

Set last call to Oct 22,2024

[dplore]

/gcbrun

[haussli]

Comments on line 213 & 219 seem incorrect; seems unrelated to certz.

[LimeHat]

1. The non-breaking label seems incorrect. it is breaking, and more in one way

• removal of existing leafs
• relocation of other leafs

2. i'd suggest to deprecate the leaves instead of removing them

[dplore]

It is a breaking change as written but in a v0.x model where we expect breaking changes until the model is "mature". I will label this as breaking.

Per yang definition, by deprecating leaves we are asserting that a NOS should support both the old, deprecated nodes and any new nodes meant to replace them simultaneously. The deprecation is merely an informational notice that the node will be removed in the future.

OC's specification for yang model versioning has an implication for v0.x models. OC generally adopts version 1.x and above once there are working implementations of this model. In practice, we have many v0.x models which are implemented yet are considered immature.

Which NOS supports the v0.2.0 model today? Do you want to see this incremented to v1.x or stay at v0.x? In full disclosure, we are not yet consuming the leaves which are undergoing a breaking change here (at least not in any code). I believe our intended consumption of this model will be to use the very latest revision and not with the older leaves. @morrowc for any additional comments on our usage.

There is no right or wrong answer here. It is just some consensus on whether we should move from v0.x to v1.x. If there is no consensus, I'll just leave this at v0.3.0 for now.

[dplore]

Comments on line 213 & 219 seem incorrect; seems unrelated to certz.

Thanks for catching this @haussli . @adityasingh-anet I added a suggested edit to the description for you to consider.

[LimeHat]

Which NOS supports the v0.2.0 model today?

I don't know, but do we a confirmation that there are no existing implementations? :-)

We don't have an implementation, so i'm not too concerned about this particular change.
But I think a consistent approach in general would be beneficial to the community.

A similar precedent happened in #1020 , where a leaf was removed, and we had a working implementation. This caused some issues down the line when we needed to upgrade to a new OC release.

[adityasingh-anet]

Comments on line 213 & 219 seem incorrect; seems unrelated to certz.

I believe the comment was originally added to indicate that acctz is augmenting the same list that certz was already using. I agree that that description might be confusing so I've changed the comments to be more specific to acctz and not reference a different model.

[dplore]

/gcbrun

[dplore]

/gcbrun

[dplore]

/gcbrun

[dplore]

/gcbrun

[dplore]

Please restructure the /system/aaa/acctz tree to be compliant with OC style for lists.

Please make the /system/aaa/acctz paths follow the OC style guide for lists. I'd suggest the following path structure:

/system/aaa/acctz/source-records/source-record/state/service
/system/aaa/acctz/source-records/source-record/state/type
/system/aaa/acctz/source-records/source-record/state/counters/records

[dplore]

Marking my review as request changes

[adityasingh-anet]

I have restructured /system/aaa/accounting/acctz:

Here is the new pyang (also updated in original message):

module: openconfig-gnsi-acctz

  augment /oc-sys:system/oc-sys-grpc:grpc-servers/oc-sys-grpc:grpc-server:
    +--ro acctz
       +--ro state
          +--ro counters
             +--ro history-istruncated?   oc-yang:counter64
             +--ro record-requests?       oc-yang:counter64
             +--ro record-responses?      oc-yang:counter64
  augment /oc-sys:system/oc-sys:aaa/oc-sys:accounting:
    +--ro acctz
       +--ro source-records
          +--ro source-record* [service type]
             +--ro service    -> ../state/service
             +--ro type       -> ../state/type
             +--ro state
                +--ro service?    service-request
                +--ro type?       service-type
                +--ro counters
                   +--ro records?   oc-yang:counter64

[dplore]

/gcbrun