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

isis policy metric type leafs fixed for action and condition #989

Merged
merged 6 commits into from
Dec 26, 2023

Conversation

awebsters
Copy link
Contributor

@awebsters awebsters commented Nov 2, 2023

Change Scope

2 changes:
set-metric-type leaf under is-is policy condition.
This field was added in #863 but is named incorrectly. As seen in the implementation examples in that PR, the leaves shouldn't be prefixed with the word 'set' because its meant to be matching on metric-type. We made the change so the leaf's name is match-route-type.

set-metric-type leaf under is-is policy action

Was depreciated in #841 in favor of 'set-metric-style-type' leaf. However, this change depreciated a leaf that was supposed to have type metric-type for a new leaf of type metric-style (metric style is still valid in the actions branch). We now do not support a metric-type leaf under the action. We create a new one called set-route-type.

We called these new leaves *-route-type so the leaves under condition and action could match in the naming convention without breaking the backward compatibility of the actions branch.

  • Change is not backward compatible. We have changed the name of a leaf under the conditions branch. We propose this as okay because the leaf is very new and the current name is unusable.

Platform Implementations

The new naming convention of set/match-route-type aligns with a proposed standard RFC 9067

Resulting Tree

module: openconfig-routing-policy
  +--rw routing-policy
     +--rw defined-sets
     |  +--rw prefix-sets
     |  |  +--rw prefix-set* [name]
     |  |     +--rw name        -> ../config/name
     |  |     +--rw config
     |  |     |  +--rw name?   string
     |  |     |  +--rw mode?   enumeration
     |  |     +--ro state
     |  |     |  +--ro name?   string
     |  |     |  +--ro mode?   enumeration
     |  |     +--rw prefixes
     |  |        +--rw prefix* [ip-prefix masklength-range]
     |  |           +--rw ip-prefix           -> ../config/ip-prefix
     |  |           +--rw masklength-range    -> ../config/masklength-range
     |  |           +--rw config
     |  |           |  +--rw ip-prefix           oc-inet:ip-prefix
     |  |           |  +--rw masklength-range?   string
     |  |           +--ro state
     |  |              +--ro ip-prefix           oc-inet:ip-prefix
     |  |              +--ro masklength-range?   string
     |  +--rw neighbor-sets
     |  |  +--rw neighbor-set* [name]
     |  |     +--rw name      -> ../config/name
     |  |     +--rw config
     |  |     |  +--rw name?      string
     |  |     |  +--rw address*   oc-inet:ip-address
     |  |     +--ro state
     |  |        +--ro name?      string
     |  |        +--ro address*   oc-inet:ip-address
     |  +--rw tag-sets
     |     +--rw tag-set* [name]
     |        +--rw name      -> ../config/name
     |        +--rw config
     |        |  +--rw name?        string
     |        |  +--rw tag-value*   oc-pol-types:tag-type
     |        +--ro state
     |           +--ro name?        string
     |           +--ro tag-value*   oc-pol-types:tag-type
     +--rw policy-definitions
        +--rw policy-definition* [name]
           +--rw name          -> ../config/name
           +--rw config
           |  +--rw name?   string
           +--ro state
           |  +--ro name?   string
           +--rw statements
              +--rw statement* [name]
                 +--rw name          -> ../config/name
                 +--rw config
                 |  +--rw name?   string
                 +--ro state
                 |  +--ro name?   string
                 +--rw conditions
                 |  +--rw config
                 |  |  +--rw call-policy?           -> ../../../../../../../policy-definitions/policy-definition/name
                 |  |  +--rw install-protocol-eq?   identityref
                 |  +--ro state
                 |  |  +--ro call-policy?           -> ../../../../../../../policy-definitions/policy-definition/name
                 |  |  +--ro install-protocol-eq?   identityref
                 |  +--rw match-interface
                 |  |  +--rw config
                 |  |  +--ro state
                 |  +--rw match-prefix-set
                 |  |  +--rw config
                 |  |  |  +--rw prefix-set?          -> ../../../../../../../../defined-sets/prefix-sets/prefix-set/config/name
                 |  |  |  +--rw match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  |  +--ro state
                 |  |     +--ro prefix-set?          -> ../../../../../../../../defined-sets/prefix-sets/prefix-set/config/name
                 |  |     +--ro match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  +--rw match-neighbor-set
                 |  |  +--rw config
                 |  |  |  +--rw neighbor-set?        -> ../../../../../../../../defined-sets/neighbor-sets/neighbor-set/name
                 |  |  |  +--rw match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  |  +--ro state
                 |  |     +--ro neighbor-set?        -> ../../../../../../../../defined-sets/neighbor-sets/neighbor-set/name
                 |  |     +--ro match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  +--rw match-tag-set
                 |  |  +--rw config
                 |  |  |  +--rw tag-set?             -> ../../../../../../../../defined-sets/tag-sets/tag-set/name
                 |  |  |  +--rw match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  |  +--ro state
                 |  |     +--ro tag-set?             -> ../../../../../../../../defined-sets/tag-sets/tag-set/name
                 |  |     +--ro match-set-options?   oc-pol-types:match-set-options-restricted-type
                 |  +--rw oc-isis-pol:isis-conditions
                 |     +--rw oc-isis-pol:config
                 |     |  +--rw oc-isis-pol:level-eq?            isis-types:level-number
                 |     |  +--rw oc-isis-pol:match-metric-type?   isis-types:metric-type
                 |     +--ro oc-isis-pol:state
                 |        +--ro oc-isis-pol:level-eq?            isis-types:level-number
                 |        +--ro oc-isis-pol:match-metric-type?   isis-types:metric-type
                 +--rw actions
                    +--rw config
                    |  +--rw policy-result?   policy-result-type
                    +--ro state
                    |  +--ro policy-result?   policy-result-type
                    +--rw set-tag
                    |  +--rw config
                    |  |  +--rw mode?   enumeration
                    |  +--ro state
                    |  |  +--ro mode?   enumeration
                    |  +--rw inline
                    |  |  +--rw config
                    |  |  |  +--rw tag*   oc-pol-types:tag-type
                    |  |  +--ro state
                    |  |     +--ro tag*   oc-pol-types:tag-type
                    |  +--rw reference
                    |     +--rw config
                    |     |  +--rw tag-set?   -> ../../../../../../../../../defined-sets/tag-sets/tag-set/config/name
                    |     +--ro state
                    |        +--ro tag-set?   -> ../../../../../../../../../defined-sets/tag-sets/tag-set/config/name
                    +--rw oc-isis-pol:isis-actions
                       +--rw oc-isis-pol:config
                       |  +--rw oc-isis-pol:set-level?               isis-types:level-number
                       |  +--rw oc-isis-pol:set-metric-type?         isis-types:metric-type
                       |  +--rw oc-isis-pol:set-metric-style-type?   isis-types:metric-style
                       |  +--rw oc-isis-pol:set-metric?              isis-types:wide-metric
                       +--ro oc-isis-pol:state
                          +--ro oc-isis-pol:set-level?               isis-types:level-number
                          +--ro oc-isis-pol:set-metric-type?         isis-types:metric-type
                          +--ro oc-isis-pol:set-metric-style-type?   isis-types:metric-style
                          +--ro oc-isis-pol:set-metric?              isis-types:wide-metric

@awebsters awebsters requested a review from a team as a code owner November 2, 2023 18:41
Copy link

google-cla bot commented Nov 2, 2023

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@OpenConfigBot
Copy link

OpenConfigBot commented Nov 2, 2023

Major YANG version changes in commit b8106ce:

@awebsters awebsters force-pushed the isis_policy_metric_type branch from 5f785b6 to e6cd8b2 Compare November 3, 2023 20:41
release/models/isis/openconfig-isis-policy.yang Outdated Show resolved Hide resolved
release/models/isis/openconfig-isis-policy.yang Outdated Show resolved Hide resolved
@awebsters
Copy link
Contributor Author

@dplore why is the breaking changes check failing? I tried to update the major version so the release is 1.0

@dplore
Copy link
Member

dplore commented Nov 20, 2023

@dplore why is the breaking changes check failing? I tried to update the major version so the release is 1.0

@wenovus The intent here is to leave this module at a 0.x.x revision, so I think the check should permit this breaking change ?

@wenovus
Copy link
Contributor

wenovus commented Nov 20, 2023

@dplore why is the breaking changes check failing? I tried to update the major version so the release is 1.0

@wenovus The intent here is to leave this module at a 0.x.x revision, so I think the check should permit this breaking change ?

Sorry about this, the test cases for the script were incorrect. fix at openconfig/models-ci#94

@wenovus
Copy link
Contributor

wenovus commented Nov 20, 2023

After #1003 is merged into the master branch, a sync from master should fix it.

@awebsters awebsters requested a review from dplore November 21, 2023 16:12
@dplore
Copy link
Member

dplore commented Nov 22, 2023

Thanks for the updates @awebsters .

  • Can you provide a pyang rendering of the tree for reviewer convenience and add it to the PR description?

@awebsters
Copy link
Contributor Author

Thanks for the updates @awebsters .

  • Can you provide a pyang rendering of the tree for reviewer convenience and add it to the PR description?

Added rendering of openconfig-routing-policy with isis augmentations

@awebsters
Copy link
Contributor Author

@dplore any update on this

@awebsters
Copy link
Contributor Author

@dplore have you had a chance to view this again?

@dplore dplore self-assigned this Dec 19, 2023
@dplore
Copy link
Member

dplore commented Dec 19, 2023

I plan to re-review by end of month and target to include this change in OC release 3.0.0

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.

This change LGTM. Last call for comments. This will merge on Dec 26th.

@dplore dplore merged commit 3ae632d into openconfig:master Dec 26, 2023
5 checks passed
@awebsters awebsters deleted the isis_policy_metric_type branch July 17, 2024 17:51
romeyod pushed a commit to romeyod/aftNextHop that referenced this pull request Sep 19, 2024
…fig#989)

* isis policy metric type leafs fixed for action and condition

---------

Co-authored-by: Wen Bo Li <[email protected]>
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.

4 participants