[SNMP:Contact] Fix for config replace failure with SNMP Contact issue

config/main.py

@@ -3822,41 +3822,27 @@ def replace_community(db, current_community, new_community):
 def contact(db):
     pass
 
-
-def is_valid_email(email):
-    return bool(re.search(r"^[\w\.\+\-]+\@[\w]+\.[a-z]{2,3}$", email))
-
-
 @contact.command('add')
-@click.argument('contact', metavar='<contact_name>', required=True)
-@click.argument('contact_email', metavar='<contact_email>', required=True)
+@click.argument('contact', metavar='<contact>', required=True, nargs=-1)
 @clicommon.pass_db
-def add_contact(db, contact, contact_email):
-    """ Add snmp contact name and email """
-    snmp = db.cfgdb.get_table("SNMP")
+def add_contact(db, contact):
+    """ Add snmp contact"""
+    config_db = ValidatedConfigDBConnector(db.cfgdb)
+    if isinstance(contact, tuple):
+        contact = " ".join(contact)
+    snmp = config_db.get_table("SNMP")
     try:
         if snmp['CONTACT']:
-            click.echo("Contact already exists.  Use sudo config snmp contact modify instead")
+            click.echo("Contact already exists, Use sudo config snmp contact modify instead")
             sys.exit(1)
-        else:
-            db.cfgdb.set_entry('SNMP', 'CONTACT', {contact: contact_email}) # TODO: ERROR IN YANG MODEL. Contact name is not defined as key
-            click.echo("Contact name {} and contact email {} have been added to "
-                       "configuration".format(contact, contact_email))
-            try:
-                click.echo("Restarting SNMP service...")
-                clicommon.run_command(['systemctl', 'reset-failed', 'snmp.service'], display_cmd=False)
-                clicommon.run_command(['systemctl', 'restart', 'snmp.service'], display_cmd=False)
-            except SystemExit as e:
-                click.echo("Restart service snmp failed with error {}".format(e))
-                raise click.Abort()
     except KeyError:
         if "CONTACT" not in snmp.keys():
-            if not is_valid_email(contact_email):
-                click.echo("Contact email {} is not valid".format(contact_email))
-                sys.exit(2)
-            db.cfgdb.set_entry('SNMP', 'CONTACT', {contact: contact_email})
-            click.echo("Contact name {} and contact email {} have been added to "
-                       "configuration".format(contact, contact_email))
+            try:
+                config_db.set_entry('SNMP', 'CONTACT', {'Contact': contact})
+                click.echo("SNMP Contact {} has been added to configuration".format(contact))
+            except ValueError as e:
+                ctx = click.get_current_context()
+                ctx.fail("Failed to set SNMP contact. Error: {}".format(e))
             try:
                 click.echo("Restarting SNMP service...")
                 clicommon.run_command(['systemctl', 'reset-failed', 'snmp.service'], display_cmd=False)
@@ -3867,67 +3853,59 @@ def add_contact(db, contact, contact_email):
 
 
 @contact.command('del')
-@click.argument('contact', metavar='<contact_name>', required=True)
+@click.argument('contact', metavar='<contact>', required=True, nargs=-1)
 @clicommon.pass_db
-def del_contact(db, contact):
-    """ Delete snmp contact name and email """
+def delete_contact(db, contact):
+    """ Delete snmp contact"""
+    config_db = ValidatedConfigDBConnector(db.cfgdb)
+    if isinstance(contact, tuple):
+        contact = " ".join(contact)
     snmp = db.cfgdb.get_table("SNMP")
     try:
-        if not contact in (list(snmp['CONTACT'].keys()))[0]:
-            click.echo("SNMP contact {} is not configured".format(contact))
-            sys.exit(1)
-        else:
-            db.cfgdb.set_entry('SNMP', 'CONTACT', None)
-            click.echo("SNMP contact {} removed from configuration".format(contact))
+        if contact == snmp['CONTACT']['Contact']:
+            try:
+                config_db.set_entry('SNMP', 'CONTACT', None)
+                click.echo("SNMP Contact {} removed from configuration".format(contact))
+            except (ValueError, JsonPatchConflict) as e:
+                ctx = click.get_current_context()
+                ctx.fail("Failed to remove SNMP contact from configuration. Error: {}".format(e))
             try:
                 click.echo("Restarting SNMP service...")
                 clicommon.run_command(['systemctl', 'reset-failed', 'snmp.service'], display_cmd=False)
                 clicommon.run_command(['systemctl', 'restart', 'snmp.service'], display_cmd=False)
             except SystemExit as e:
                 click.echo("Restart service snmp failed with error {}".format(e))
                 raise click.Abort()
+        else:
+            click.echo("SNMP Contact {} does not exist.".format(contact))
+            sys.exit(1)
     except KeyError:
         if "CONTACT" not in snmp.keys():
-            click.echo("Contact name {} is not configured".format(contact))
+            click.echo("SNMP Contact {} is not configured".format(contact))
             sys.exit(2)
 
 
 @contact.command('modify')
-@click.argument('contact', metavar='<contact>', required=True)
-@click.argument('contact_email', metavar='<contact email>', required=True)
+@click.argument('contact', metavar='<contact>', required=True, nargs=-1)
 @clicommon.pass_db
-def modify_contact(db, contact, contact_email):
+def modify_contact(db, contact):
     """ Modify snmp contact"""
-    snmp = db.cfgdb.get_table("SNMP")
+    config_db = ValidatedConfigDBConnector(db.cfgdb)
+    if isinstance(contact, tuple):
+        contact = " ".join(contact)
+    snmp = config_db.get_table("SNMP")
     try:
-        current_snmp_contact_name = (list(snmp['CONTACT'].keys()))[0]
-        if current_snmp_contact_name == contact:
-            current_snmp_contact_email = snmp['CONTACT'][contact]
-        else:
-            current_snmp_contact_email = ''
-        if contact == current_snmp_contact_name and contact_email == current_snmp_contact_email:
-            click.echo("SNMP contact {} {} already exists".format(contact, contact_email))
+        snmp_contact = snmp['CONTACT']['Contact']
+        if contact in snmp_contact:
+            click.echo("SNMP contact {} already exists".format(contact))
             sys.exit(1)
-        elif contact == current_snmp_contact_name and contact_email != current_snmp_contact_email:
-            if not is_valid_email(contact_email):
-                click.echo("Contact email {} is not valid".format(contact_email))
-                sys.exit(2)
-            db.cfgdb.mod_entry('SNMP', 'CONTACT', {contact: contact_email})
-            click.echo("SNMP contact {} email updated to {}".format(contact, contact_email))
-            try:
-                click.echo("Restarting SNMP service...")
-                clicommon.run_command(['systemctl', 'reset-failed', 'snmp.service'], display_cmd=False)
-                clicommon.run_command(['systemctl', 'restart', 'snmp.service'], display_cmd=False)
-            except SystemExit as e:
-                click.echo("Restart service snmp failed with error {}".format(e))
-                raise click.Abort()
         else:
-            if not is_valid_email(contact_email):
-                click.echo("Contact email {} is not valid".format(contact_email))
-                sys.exit(2)
-            db.cfgdb.set_entry('SNMP', 'CONTACT', None)
-            db.cfgdb.set_entry('SNMP', 'CONTACT', {contact: contact_email})
-            click.echo("SNMP contact {} and contact email {} updated".format(contact, contact_email))
+            try:
+                config_db.mod_entry('SNMP', 'CONTACT', {'Contact': contact})
+                click.echo("SNMP contact {} modified in configuration".format(contact))
+            except ValueError as e:
+                ctx = click.get_current_context()
+                ctx.fail("Failed to modify SNMP contact. Error: {}".format(e))
             try:
                 click.echo("Restarting SNMP service...")
                 clicommon.run_command(['systemctl', 'reset-failed', 'snmp.service'], display_cmd=False)
@@ -3936,9 +3914,8 @@ def modify_contact(db, contact, contact_email):
                 click.echo("Restart service snmp failed with error {}".format(e))
                 raise click.Abort()
     except KeyError:
-        if "CONTACT" not in snmp.keys():
-            click.echo("Contact name {} is not configured".format(contact))
-            sys.exit(3)
+        click.echo("Cannot modify SNMP Contact.  You must use 'config snmp contact add' command.'")
+        sys.exit(2)
 
 
 @snmp.group(cls=clicommon.AbbreviationGroup)

doc/Command-Reference.md

@@ -9821,9 +9821,9 @@ This command displays the global SNMP configuration that includes the location,
   Emerald City
 
 
-  SNMP_CONTACT    SNMP_CONTACT_EMAIL
-  --------------  --------------------
-  joe             [email protected]
+  CONTACT
+  --------------
+  joe [email protected]
 
 
   Community String    Community Type
@@ -9876,9 +9876,9 @@ This command displays the SNMP contact setting.
 - Example:
   ```
   admin@sonic:~$ show runningconfiguration snmp contact
-  Contact    Contact Email
-  ---------  ---------------
-  joe        [email protected]
+  Contact
+  ---------
+  joe [email protected]
   ```
 
 - Usage:
@@ -9889,7 +9889,7 @@ This command displays the SNMP contact setting.
 - Example:
   ```
   admin@sonic:~$ show runningconfiguration snmp contact --json
-  {'joe': '[email protected]'}
+  {'Contact': 'joe [email protected]'}
   ```
 
 **show runningconfiguration snmp community**
@@ -9989,13 +9989,13 @@ This command is used to add, delete, or modify the SNMP contact.
 
 - Usage:
   ```
-  config snmp contact add <contact> <contact_email>
+  config snmp contact add <contact>
   ```
 
 - Example:
   ```
   admin@sonic:~$ sudo config snmp contact add joe [email protected]
-  Contact name joe and contact email [email protected] have been added to configuration
+  Contact joe [email protected] has been added to configuration
   Restarting SNMP service...
   ```
 
@@ -10006,20 +10006,20 @@ This command is used to add, delete, or modify the SNMP contact.
 
 - Example:
   ```
-  admin@sonic:~$ sudo config snmp contact del joe
-  SNMP contact joe removed from configuration
+  admin@sonic:~$ sudo config snmp contact del joe [email protected] 
+  SNMP contact joe [email protected] removed from configuration
   Restarting SNMP service...
   ```
 
 - Usage:
   ```
-  config snmp contact modify <contact> <contact_email>
+  config snmp contact modify <contact>
   ```
 
 - Example:
   ```
   admin@sonic:~$ sudo config snmp contact modify test [email protected]
-  SNMP contact test and contact email [email protected] updated
+  SNMP contact test [email protected] updated
   Restarting SNMP service...
   ```
 

scripts/db_migrator.py

@@ -856,6 +856,20 @@ def migrate_aaa(self):
             if keys:
                 self.configDB.delete(self.configDB.CONFIG_DB, authorization_key)
 
+    def migrate_config_db_snmp_contact(self):
+        """
+        Migrate SNMP": { "CONTACT": { "joe": "[email protected]"}} to
+                SNMP": { "CONTACT": { "Contact": "joe [email protected]"}}
+        As per the changes to fix DB schema based on sonic-snmp.yang
+        """
+        snmp_contact = self.configDB.get_entry('SNMP', 'CONTACT')
+        if 'Contact' not in snmp_contact:
+            contact_name = list(snmp_contact.keys())[0]
+            contact_email = snmp_contact[contact_name]
+            self.configDB.set_entry('SNMP', 'CONTACT', None)
+            snmp_contact = {'Contact': contact_name + ' ' + contact_email}
+            self.configDB.set_entry('SNMP', 'CONTACT', snmp_contact)
+
     def version_unknown(self):
         """
         version_unknown tracks all SONiC versions that doesn't have a version
@@ -1231,6 +1245,10 @@ def version_202405_01(self):
         Version 202405_01.
         """
         log.log_info('Handling version_202405_01')
+
+        if self.configDB.keys(self.configDB.CONFIG_DB, "SNMP|CONTACT"):
+            self.migrate_config_db_snmp_contact()
+
         self.set_version('version_202411_01')
         return 'version_202411_01'
 

show/main.py

@@ -1589,7 +1589,7 @@ def community(db, json_output):
 def contact(db, json_output):
     """show SNMP running configuration contact"""
     snmp = db.cfgdb.get_table('SNMP')
-    snmp_header = ["Contact", "Contact Email"]
+    snmp_header = ["Contact"]
     snmp_body = []
     if json_output:
         try:
@@ -1601,9 +1601,8 @@ def contact(db, json_output):
     else:
         try:
             if snmp['CONTACT']:
-                snmp_contact = list(snmp['CONTACT'].keys())
-                snmp_contact_email = [snmp['CONTACT'][snmp_contact[0]]]
-                snmp_body.append([snmp_contact[0], snmp_contact_email[0]])
+                snmp_contact = [snmp['CONTACT']['Contact']]
+                snmp_body.append(snmp_contact)
         except KeyError:
             snmp['CONTACT'] = ''
         click.echo(tabulate(snmp_body, snmp_header))
@@ -1670,7 +1669,7 @@ def show_run_snmp(db, ctx):
     snmp_users = db.cfgdb.get_table('SNMP_USER')
     snmp_location_header = ["Location"]
     snmp_location_body = []
-    snmp_contact_header = ["SNMP_CONTACT", "SNMP_CONTACT_EMAIL"]
+    snmp_contact_header = ["Contact"]
     snmp_contact_body = []
     snmp_comm_header = ["Community String", "Community Type"]
     snmp_comm_body = []
@@ -1687,9 +1686,8 @@ def show_run_snmp(db, ctx):
     click.echo("\n")
     try:
         if snmp_contact_location_table['CONTACT']:
-            snmp_contact = list(snmp_contact_location_table['CONTACT'].keys())
-            snmp_contact_email = [snmp_contact_location_table['CONTACT'][snmp_contact[0]]]
-            snmp_contact_body.append([snmp_contact[0], snmp_contact_email[0]])
+            snmp_contact = [snmp_contact_location_table['CONTACT']['Contact']]
+            snmp_contact_body.append(snmp_contact)
     except KeyError:
         snmp_contact_location_table['CONTACT'] = ''
     click.echo(tabulate(snmp_contact_body, snmp_contact_header))