Skip to content

[library/action_sync.py]: This module will make sure that actions such as add-topo, deploy-mg and remove-topo are in sync with current topo.#2295

Open
praveen-li wants to merge 2 commits intosonic-net:masterfrom
praveen-li:action_sync_msft
Open

[library/action_sync.py]: This module will make sure that actions such as add-topo, deploy-mg and remove-topo are in sync with current topo.#2295
praveen-li wants to merge 2 commits intosonic-net:masterfrom
praveen-li:action_sync_msft

Conversation

@praveen-li
Copy link
Member

Description of PR

Summary:
Fixes # (issue)

[library/action_sync.py]: This module will make sure that actions such as add-topo, deploy-mg and remove-topo are in sync with current topo.

Changes:
-- New ansible module to make sure add-topo, deploy-mg and remove-topo are in sync.
-- related changes in other playbooks.

How I did:
-- Use shelve to store a DB in a file.
-- DB will have info about current topo, dut and server.
-- This will make sure, a topo is removed before adding new one.
-- Also deploy-mg, remove-topo will be allowed only for currently loaded topo.

Note: I did it for add-topo, deploy-mg and remove-topo , but can be extended for renumber etc.

Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

How I did:
-- Use shelve to store a DB in a file.
-- DB will have info about current topo, dut and server.
-- This will make sure, a topo is removed before adding new one.
-- Also deploy-mg, remove-topo will be allowed only for currently loaded topo.

What is the motivation for this PR?

Today from same sonic-mgmt docker, 2 users may end up running 2 separate add-topo commands which may messed up the Setup. Ideally deploy-mg, remove-topo should be allowed only for currently loaded topo. And add-topo must be allowed only after removing current-topo.
With this PR I use a shelve DB, which makes sure actions are in sync. Also it stored last executed commands.

How did you do it?

How I did:
-- Use shelve to store a DB in a file.
-- DB will have info about current topo, dut and server.
-- This will make sure, a topo is removed before adding new one.
-- Also deploy-mg, remove-topo will be allowed only for currently loaded topo.

How did you verify/test it?

Testing is recorded in a video, But below I copied

Steps:
1.) $ ./testbed-cli.sh add-topo vms-celdx10-t1-16 ~/.password -e ptf_imagetag=202006 -vvv

2.) $ ./testbed-cli.sh add-topo vms-celdx10-t1-8 ~/.password -e ptf_imagetag=202006 -vvv

If fails due to prev topo:

                    "2020-09-29 18:26:18.426128: -SUCC- add-topo topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2",
                    "2020-09-29 18:30:12.101652: -SUCC- run-tests topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2 test=bgp_facts"
,
                    "2020-09-29 18:30:41.133856: -SUCC- run-tests topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2 test=bgp_fact",
                    "2020-09-29 22:04:59.211082: -FAIL- add-topo topo=vms-celdx10-t1-8 dut=falco-open19-flex server=server_2"
                ],
                "success": [
                    "2020-09-29 17:09:04.402483: run-tests topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2 test=bgp_facts",
                    "2020-09-29 17:11:13.223194: remove-topo topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2",
                    "2020-09-29 18:26:18.425939: add-topo topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2",
                    "2020-09-29 18:30:12.101459: run-tests topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2 test=bgp_facts",
                    "2020-09-29 18:30:41.133659: run-tests topo=vms-celdx10-t1-16 dut=falco-test-dut04 server=server_2 test=bgp_fact"
                ]
            },
            "current_topo": {
                "deploy-mg": false,
                "dut": "falco-test-dut04",
                "server": "server_2",
                "topo_name": "vms-celdx10-t1-16"
            }

./testbed-cli.sh deploy-mg vms-celdx10-t0-8 lab ~/.password
FAIL

                    "2020-09-29 22:04:59.211082: -FAIL- deploy-mg topo=vms-celdx10-t1-8 dut=falco-open19-flex server=server_2"
                ],
                "success": [ .... ]
            },
            "current_topo": {
                "deploy-mg": false,
                "dut": "falco-test-dut04",
                "server": "server_2",
                "topo_name": "vms-celdx10-t1-16"
            }

3.) ./testbed-cli.sh deploy-mg vms-celdx10-t0-16 lab ~/.password

PASS

            "current_topo": {
                "deploy-mg": true,
                "dut": "falco-test-dut04",
                "server": "server_2",
                "topo_name": "vms-celdx10-t1-16"
            }

4.) ./testbed-cli.sh run-tests vms-celdx10-t0-16 lab bgp_facts
PASS
5.) ./testbed-cli.sh remove-topo vms-celdx10-t1-8 ~/.password -vvv

           "2020-09-29 22:09:47.984047: -FAIL- remove-topo topo=vms-celdx10-t1-8 dut=falco-open19-flex server=server_2"
            ],
            "success": [.....]
        },
        "current_topo": {
            "deploy-mg": false,
            "dut": "falco-test-dut04",
            "server": "server_2",
            "topo_name": "vms-celdx10-t1-16"
        }
6.) ./testbed-cli.sh remove-topo vms-celdx10-t1-16 ~/.password -vvv
PASS

#### Any platform specific information?

#### Supported testbed topology if it's a new test case?

### Documentation 
<!--
(If it's a new feature, new test case)
Did you update documentation/Wiki relevant to your implementation?
Link to the wiki page?
-->

Praveen Chaudhary added 2 commits September 29, 2020 17:31
…h as add-topo, deploy-mg and remove-topo are in sync with current topo.

Changes:
-- New ansible module to make sure add-topo, deploy-mg and remove-topo are in sync.
-- related changes in other playbooks.

How I did:
-- Use shelve to store a DB in a file.
-- DB will have info about current topo, dut and server.
-- This will make sure, a topo is removed before adding new one.
-- Also deploy-mg, remove-topo will be allowed only for currently loaded topo.

Signed-off-by: Praveen Chaudhary<pchaudhary@linkedin.com>
RB=
G=lnos-reviewers
R=pchaudhary,pmao,rmolina,samaity,sfardeen,zxu
A=
Signed-off-by: Praveen Chaudhary <pchaudhary@linkedin.com>
@wangxin wangxin self-requested a review October 20, 2020 00:02
Copy link
Collaborator

@wangxin wangxin left a comment

Choose a reason for hiding this comment

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

It is a good idea to have a method to track what has been executed on each testbed. But it seems that the current approach does not meet the needs of a lab with multiple different test setups.

self.logSuccess()
else:
self.logFailure()
raise Exception("{} failed".format(self.action))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Assume we have multiple testbeds defined in testbed.csv: tb01, tb02.
If add-topo is executed for tb01, then run add-topo for tb02 would fail, right? Because db['current_topo'] is not none and db['current_topo']['topo_name'] would be tb01. The else branch will be hit.
If my understanding is correct, this approach won't work for a lab with multiple different test setups.

@praveen-li praveen-li requested a review from a team as a code owner March 2, 2022 17:37
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
…submodule head (sonic-net#11705)

Kernel:
* 86c4b66 2022-07-28 | [Mellanox] Add new kernel patches from HW-MGMT package V.7.0020.3005 (sonic-net#287) (HEAD -> 202205) [Kebo Liu]
* 3a8416a 2022-07-05 | [patch] mlxsw: i2c: Prevent transaction execution for special chip (sonic-net#279) [Stepan Blyshchak]

swss:
* 3f69944 2022-08-10 | Set internal class state to reflect the actual state (sonic-net#2410) (HEAD -> 202205, tag: foo) [Prince Sunny]
* 87e98eb 2022-08-09 | [portsorch] Expose supported FEC modes to STABE_DB and check whether FEC mode is supported before setting it (sonic-net#2400) [Stephen Sun]
* e71ab99 2022-07-29 | portsorch: initial support for link-training (sonic-net#2359) [Dante (Kuo-Jung) Su]
* ed5e5be 2022-07-08 | Port configuration incremental update support (sonic-net#2305) [Junchao-Mellanox]

utilities:
* 0df3ba8 2022-08-12 | Revert "Improve the way to check port type of RJ45 port (sonic-net#2249)" (HEAD -> 202205) [Ying Xie]
* 9b21903 2022-08-12 | Fix test failure in dump table test in 202205 (sonic-net#2307) (HEAD -> 202205, github/202205) [Stephen Sun]
* 750d1db 2022-08-11 | Convert IPv6 addresses to lowercase in apply-patch (sonic-net#2299) (HEAD -> 202205) [dbarashinvd]
* 555947e 2022-08-09 | [config][muxcable] add support to enable/disable ycable telemetry (sonic-net#2297) [vdahiya12]
* 978f416 2022-08-09 | Fix GCU bug when backend service modifying config (sonic-net#2295) [jingwenxie]
* 8fed381 2022-08-02 | [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB (sonic-net#2223) (github/202205) [Stephen Sun]
* a1a09e4 2022-07-29 | Improve the way to check port type of RJ45 port (sonic-net#2249) [Stephen Sun]
* 9bdbfb8 2022-05-19 | sonic-utils: initial support for link-training (sonic-net#2071) [Dante (Kuo-Jung) Su]
* c088ec4 2022-08-10 | Support to enable fips for the command sonic_installer (sonic-net#2154) (sonic-net#2303) [xumia]

platform-daemon:
* 767cfb6 2022-08-09 | [ycabled] add capability to enable/disable telemetry (sonic-net#279) (HEAD -> 202205) [vdahiya12]

linkmgrd:
* cf1ba2b 2022-08-12 | wait for handler to be completed (sonic-net#114) (HEAD -> 202205, github/202205) [Jing Zhang]
* e99026c 2022-08-11 | [lgtm]: add uuid-dev to lgtm prepare (sonic-net#112) (HEAD -> 202205) [Jing Zhang]
* bd1b7f0 2022-08-11 | Adjust `DbInterfaceRaceConditionCheck` to Wait Longer for Handlers to be executed (sonic-net#111) (HEAD -> 202205, github/202205) [Jing Zhang]
* e9dc6b2 2022-08-11 | Backoff mux probing for server down scenario (sonic-net#106) [Jing Zhang]
* 0d61171 2022-08-09 | Fix race condition caused by strand `wrap` method (sonic-net#104) [Jing Zhang]
* e9ede7d 2022-07-02 | Enforce switch after config mux to active (sonic-net#95) [Longxiang Lyu]
* 15dbc30 2022-06-30 | Add unittest to verify mux toggle active (sonic-net#94) [Longxiang Lyu]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants