[MultiDB] Add multidb support to sonic-ztp #16

rajendra-dendukuri · 2020-06-06T18:24:05Z

No description provided.

rajendra-dendukuri · 2020-06-06T18:26:55Z

retest buster please

wendani · 2020-06-10T03:27:09Z

src/usr/lib/ztp/ztp-profile.sh

@@ -264,7 +256,7 @@ if [ "$CMD" = "remove" ] ; then
            fi
        else
            # Remove ZTP configuration from config-db
-            redis-cli -n 4 DEL "ZTP|mode"
+            sonic-db-cli CONFIG_DB DEL "ZTP|mode"


May need to direct to /dev/null to avoid sonic-ztp[xxx]: 1 in syslog

Thanks. I fixed it.

Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>

lguohan · 2020-06-25T11:27:27Z

retest this please

`sonic_installer` has been renamed `sonic-installer`. Update the file name everywhere it is used.

…cal/bin/ (sonic-net#19) Update paths to reflect new sonic-utilities install location. As of PR sonic-net/sonic-utilities#1122, sonic-utilities is built and installed as a Python Wheel package. As such, the installation directory of the scripts/entrypoints has changed from `/usr/bin/` to `/usr/local/bin`. This patch updates all references to the old location to either reference the new location, or remove the absolute path entirely, where applicable.

qiluo-msft · 2020-09-21T21:43:19Z

src/usr/lib/ztp/plugins/configdb-json

@@ -47,6 +50,11 @@ class ConfigDBJson:
            raise TypeError('Snmp provided with invalid argument type.')
        self.__input_file = input_file

+        self.configDB = ConfigDBConnector()
+        if self.configDB is None:


self.configDB [](start = 11, length = 13)

Could you clarify when it is None? #Closed

Fixed in latest commit

The new code seems worse. Why construct ConfigDBConnector twice?

In reply to: 492363061 [](ancestors = 492363061)

My bad. I did not resolve the merge conflict properly and ended up having two copies of the code. I fixed it now.

qiluo-msft · 2020-09-21T21:49:41Z

src/usr/lib/ztp/plugins/configdb-json

                updateActivity('configdb-json: Removing ZTP configuation from Config DB')
                logger.info('configdb-json: Configuration change detected. Removing ZTP configuation from Config DB.')
-                runCommand('redis-cli -n 4 DEL "ZTP|mode"', capture_stdout=False)
+                self.configDB.delete_table("ZTP")


delete_table [](start = 30, length = 12)

Original code is deleting a entry, and new code is deleting a table. Is it expected? #Closed

Fixed in latest commit

qiluo-msft · 2020-09-21T21:53:05Z

tests/test_configdb-json.py

+        cmd = "/bin/sed -i -e 's/\"mac\": \".*\"/\"mac\": \"invalid2\"/' " + str(fh_after)
+        rc  = runCommand(cmd)
+        cmd = "/bin/sed -i -e 's/\"hwsku\": \".*\"/\"hwsku\": \"invalid3\"/' " + str(fh_after)
+        rc  = runCommand(cmd)


Could you just parse config-before.json, modify in memory and write to config-after.json? #Closed

Fixed in latest commit

qiluo-msft · 2020-09-21T21:54:02Z

tests/test_configdb-json.py

+        cmd = "/bin/sed -i -e 's/\"mac\": \".*\"/\"mac\": \"invalid2\"/' " + str(fh_after)
+        rc  = runCommand(cmd)
+        cmd = "/bin/sed -i -e 's/\"hwsku\": \".*\"/\"hwsku\": \"" + hwsku + "_dup"  +"\"/' " + str(fh_after)
+        rc  = runCommand(cmd)


The same #Closed

Fixed in latest commit

qiluo-msft · 2020-09-21T22:01:32Z

src/usr/lib/ztp/ztp-engine.py

-    def __link_scan(self):
+    ## Redis DB connectors
+    configDB = None
+    applDB   = None


You mixed using Class Variables and Instance Variables. Please check some tutorial, such as https://careerkarma.com/blog/python-class-variables/ #Closed

Fixed in latest commit

qiluo-msft

As comments

Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>

…dukuri/sonic-ztp into multidb_ztp_cleanup

qiluo-msft · 2020-09-23T17:37:22Z

@dzhangalibaba Could you also help review this PR?

qiluo-msft

LGTM. Please also resolve other reviewers' comments.

dzhangalibaba · 2020-09-23T18:33:24Z

src/usr/lib/ztp/ztp-engine.py

+        # Connect to AppDB
+        try:
+            if self.applDB is None:
+                self.applDB = SonicV2Connector(host='127.0.0.1')


remove "host='127.0.0.1' " , it is not needed , all info should come from database_config.json and the host here didn't take effect.

Implemented the recommended change.

[MultiDB] Add multidb support to sonic-ztp

3643b55

wendani reviewed Jun 10, 2020

View reviewed changes

Hide stray output message while removing ZTP config

76d4dfe

Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>

rajendra-dendukuri mentioned this pull request Jun 10, 2020

Initial release #12

Merged

Filter out non-printable characters read from syseeprom (sonic-net#17)

5af8c95

Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>

jleveque and others added 4 commits August 7, 2020 00:02

Update all references to new 'sonic-installer' file name

732c87f

Update all references to new 'sonic-installer' file name (sonic-net#18)

dd025bc

`sonic_installer` has been renamed `sonic-installer`. Update the file name everywhere it is used.

Update paths to reflect new utilities install location, /usr/local/bin/

a87b7e9

lguohan requested a review from qiluo-msft September 20, 2020 17:47

qiluo-msft reviewed Sep 21, 2020

View reviewed changes

qiluo-msft requested review from jleveque and qiluo-msft September 21, 2020 21:44

qiluo-msft reviewed Sep 21, 2020

View reviewed changes

qiluo-msft requested changes Sep 21, 2020

View reviewed changes

rajendra-dendukuri added 4 commits September 22, 2020 21:24

[MultiDB] Add multidb support to sonic-ztp

c8e84af

Hide stray output message while removing ZTP config

9dba1f4

Signed-off-by: Rajendra Dendukuri <rajendra.dendukuri@broadcom.com>

Merge branch 'multidb_ztp_cleanup' of https://github.com/rajendra-den…

b99c394

…dukuri/sonic-ztp into multidb_ztp_cleanup

Fix additional review comments

01d5710

Remove duplicate configdb connector creation

aa45468

qiluo-msft approved these changes Sep 23, 2020

View reviewed changes

dzhangalibaba suggested changes Sep 23, 2020

View reviewed changes

Remove references to host address while creating a DB connector

84cd125

dzhangalibaba approved these changes Sep 23, 2020

View reviewed changes

qiluo-msft merged commit 6f0305b into sonic-net:master Sep 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MultiDB] Add multidb support to sonic-ztp #16

[MultiDB] Add multidb support to sonic-ztp #16

rajendra-dendukuri commented Jun 6, 2020

rajendra-dendukuri commented Jun 6, 2020

wendani Jun 10, 2020

rajendra-dendukuri Jun 10, 2020

lguohan commented Jun 25, 2020

qiluo-msft Sep 21, 2020 •

edited

Loading

rajendra-dendukuri Sep 23, 2020

qiluo-msft Sep 23, 2020

rajendra-dendukuri Sep 23, 2020

qiluo-msft Sep 21, 2020 •

edited

Loading

rajendra-dendukuri Sep 23, 2020

qiluo-msft Sep 21, 2020 •

edited

Loading

rajendra-dendukuri Sep 23, 2020

qiluo-msft Sep 21, 2020 •

edited

Loading

rajendra-dendukuri Sep 23, 2020

qiluo-msft Sep 21, 2020 •

edited

Loading

rajendra-dendukuri Sep 23, 2020

qiluo-msft left a comment

qiluo-msft commented Sep 23, 2020

qiluo-msft left a comment

dzhangalibaba Sep 23, 2020

rajendra-dendukuri Sep 23, 2020

[MultiDB] Add multidb support to sonic-ztp #16

[MultiDB] Add multidb support to sonic-ztp #16

Conversation

rajendra-dendukuri commented Jun 6, 2020

rajendra-dendukuri commented Jun 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Jun 25, 2020

qiluo-msft Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 21, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

qiluo-msft commented Sep 23, 2020

qiluo-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Sep 21, 2020 •

edited

Loading

qiluo-msft Sep 21, 2020 •

edited

Loading

qiluo-msft Sep 21, 2020 •

edited

Loading

qiluo-msft Sep 21, 2020 •

edited

Loading

qiluo-msft Sep 21, 2020 •

edited

Loading