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

[GCU] Handling type1 lists #2171

Merged
merged 5 commits into from
May 25, 2022
Merged

[GCU] Handling type1 lists #2171

merged 5 commits into from
May 25, 2022

Conversation

ghooo
Copy link
Contributor

@ghooo ghooo commented May 18, 2022

What I did

Fixes #2034

Which lists where handled before in ConfigDb to YANG conversion?

  • Dictionary of key to Object e.g.
"TACPLUS_SERVER": {
    "1.1.1.1": {            <= Key: Object (i.e. {...})
        "priority": "1", 
        "tcp_port": "49"
    }, 
    "1.1.1.2": {
        "priority": "1", 
        "tcp_port": "49"
    },
    "1.1.1.3": {
        "priority": "1", 
        "tcp_port": "49"
    } 
}
  • List of values
"VLAN": {
    "Vlan1000": {
        "vlanid": "1000",
        "dhcp_servers": [    <= Values
            "192.0.0.1",
            "192.0.0.2",
            "192.0.0.3",
            "192.0.0.4"
        ]
    }
}
  • But there is no handling of Dictionary of Key to Value
"DOT1P_TO_TC_MAP": {
    "Dot1p_to_tc_map1": {    <= Key: Value
            "1": "1",
            "2": "2",
            "3": "1",
            "4": "2"
        }
    },

Refer to sonic-net/sonic-buildimage#7375 for more info about how Type 1 list ConfigDb is getting converted to SonicYang.

After checking how type1 is handled during ConfigDb to SonicYang conversion. Check the unit-tests here for converting ConfigDb Path to SonicYang xpath.

Also added CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests.

How I did it

When handling a list, check if it is of type1. If that's the case call type1 list handling.

How to verify it

unit-test

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

# Example where the leaf specified is not the key:
# xpath: /sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']/tc
# path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
next_token = xpath_tokens[token_index+1]
Copy link
Contributor

@qiluo-msft qiluo-msft May 18, 2022

Choose a reason for hiding this comment

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

token_index

Protect for index out of range. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is protected in line 777

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let me add a comment to line 777 for clarification, and to indicate that it is the end of the xpath_tokens

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

Choose a reason for hiding this comment

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

How about token_index+1 > len(xpath_tokens)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@lgtm-com

This comment was marked as outdated.

@qiluo-msft
Copy link
Contributor

Please resolve conflict.

@ghooo
Copy link
Contributor Author

ghooo commented May 20, 2022

Please resolve conflict.

Resolved

@ghooo ghooo merged commit f5af780 into sonic-net:master May 25, 2022
dprital added a commit to dprital/sonic-buildimage that referenced this pull request May 25, 2022
Update sonic-utilities submodule pointer to include the following:
* [GCU] Handling type1 lists ([sonic-net#2171](sonic-net/sonic-utilities#2171))
* [yang] extend ConfigMgmt constructor to pass YANG options ([sonic-net#2118](sonic-net/sonic-utilities#2118))
* [dump] implement ACL modules ([sonic-net#2153](sonic-net/sonic-utilities#2153))
* show commands for SYSTEM READY ([sonic-net#1851](sonic-net/sonic-utilities#1851))
* [GCU] Handling non-compliant leaf-list with string values ([sonic-net#2174](sonic-net/sonic-utilities#2174))
* Add sonic-delayed.target to Application Extension .timer file generator ([sonic-net#2176](sonic-net/sonic-utilities#2176))
* [portconfig] Allow to configure interface mtu for physical ports ([#l](https://github.com/Azure/sonic-utilities/pull/l))
* Broadcast Unknown-multicast and Unknown-unicast Storm-control  ([sonic-net#928](sonic-net/sonic-utilities#928))
* sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071))
* [portchannel] Added ACL/PBH binding checks to the port before getting added to portchannel ([sonic-net#2151](sonic-net/sonic-utilities#2151))
* Modify override testcase to cover PORT admin_status ([sonic-net#2165](sonic-net/sonic-utilities#2165))
* [GCU] Validate peer_group_range ip_range are correct ([sonic-net#2145](sonic-net/sonic-utilities#2145))
* [auto-ts] add memory check ([sonic-net#2116](sonic-net/sonic-utilities#2116))
* support new interface types CR8/SR8/KR8/LR8 which are brougnt by SAI V.1.10.2 ([sonic-net#2167](sonic-net/sonic-utilities#2167))
* [scripts/fast-reboot] Add option to include ssd-upgrader-part boot option with SONiC partition ([sonic-net#2150](sonic-net/sonic-utilities#2150))
* [config reload] Fix invalid rstrip. ([sonic-net#2157](sonic-net/sonic-utilities#2157))
* Accept 0 for queue and dscp ([sonic-net#2162](sonic-net/sonic-utilities#2162))

Signed-off-by: dprital <drorp@nvidia.com>
yxieca pushed a commit that referenced this pull request Jun 17, 2022
#### What I did
Fixes #2034

Which lists where handled before in ConfigDb to YANG conversion?
- Dictionary of key to Object e.g.
```
"TACPLUS_SERVER": {
    "1.1.1.1": {            <= Key: Object (i.e. {...})
        "priority": "1", 
        "tcp_port": "49"
    }, 
    "1.1.1.2": {
        "priority": "1", 
        "tcp_port": "49"
    },
    "1.1.1.3": {
        "priority": "1", 
        "tcp_port": "49"
    } 
}
```
- List of values
```
"VLAN": {
    "Vlan1000": {
        "vlanid": "1000",
        "dhcp_servers": [    <= Values
            "192.0.0.1",
            "192.0.0.2",
            "192.0.0.3",
            "192.0.0.4"
        ]
    }
}
```
- But there is no handling of Dictionary of Key to Value
```
"DOT1P_TO_TC_MAP": {
    "Dot1p_to_tc_map1": {    <= Key: Value
            "1": "1",
            "2": "2",
            "3": "1",
            "4": "2"
        }
    },
```

Refer to sonic-net/sonic-buildimage#7375 for more info about how Type 1 list ConfigDb is getting converted to SonicYang.

After checking how type1 is handled during ConfigDb to SonicYang conversion. Check the unit-tests here for converting ConfigDb Path to SonicYang xpath.


Also added CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests.

#### How I did it

When handling a list, check if it is of type1. If that's the case call type1 list handling.

#### How to verify it
unit-test

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
@yxieca
Copy link
Contributor

yxieca commented Jun 22, 2022

@ghooo cherry-picking this changed to 202205 branch causes PR test to fail, can you look into this issue?

yxieca pushed a commit that referenced this pull request Jun 23, 2022
#### What I did
Fixes #2034

Which lists where handled before in ConfigDb to YANG conversion?
- Dictionary of key to Object e.g.
```
"TACPLUS_SERVER": {
    "1.1.1.1": {            <= Key: Object (i.e. {...})
        "priority": "1", 
        "tcp_port": "49"
    }, 
    "1.1.1.2": {
        "priority": "1", 
        "tcp_port": "49"
    },
    "1.1.1.3": {
        "priority": "1", 
        "tcp_port": "49"
    } 
}
```
- List of values
```
"VLAN": {
    "Vlan1000": {
        "vlanid": "1000",
        "dhcp_servers": [    <= Values
            "192.0.0.1",
            "192.0.0.2",
            "192.0.0.3",
            "192.0.0.4"
        ]
    }
}
```
- But there is no handling of Dictionary of Key to Value
```
"DOT1P_TO_TC_MAP": {
    "Dot1p_to_tc_map1": {    <= Key: Value
            "1": "1",
            "2": "2",
            "3": "1",
            "4": "2"
        }
    },
```

Refer to sonic-net/sonic-buildimage#7375 for more info about how Type 1 list ConfigDb is getting converted to SonicYang.

After checking how type1 is handled during ConfigDb to SonicYang conversion. Check the unit-tests here for converting ConfigDb Path to SonicYang xpath.


Also added CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests.

#### How I did it

When handling a list, check if it is of type1. If that's the case call type1 list handling.

#### How to verify it
unit-test

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants