Merge pull request #136 from mssonicbld/sonicbld/202205-merge

[code sync] Merge code from sonic-net/sonic-buildimage:202205 to 202205

dockers/docker-snmp/snmpd.conf.j2

@@ -13,47 +13,12 @@
 #  AGENT BEHAVIOUR
 #
 
-# Listen for connections on all ip addresses, including eth0, ipv4 lo for multi-asic platform
-# Listen on managment and loopback0 ips for single asic platform
+#  Listen for connections on all ip addresses, including eth0, ipv4 lo
 #
-{% macro protocol(ip_addr) %}
-{%- if ip_addr|ipv6 -%}
-{{ 'udp6' }}
-{%- else -%}
-{{ 'udp' }}
-{%- endif -%}
-{% endmacro %}
-
 {% if SNMP_AGENT_ADDRESS_CONFIG %}
 {% for (agentip, port, vrf) in SNMP_AGENT_ADDRESS_CONFIG %}
-agentAddress {{ protocol(agentip) }}:[{{ agentip }}]{% if port %}:{{ port }}{% endif %}{% if vrf %}%{{ vrf }}{% endif %}{{ "" }}
-{% endfor %}
-{% elif NAMESPACE_COUNT is not defined or NAMESPACE_COUNT|int <= 1 %}
-{% if MGMT_INTERFACE is defined %}
-{% for intf, ip in MGMT_INTERFACE %}
-{% set agentip = ip.split('/')[0]|lower %}
-{% set zoneid = '' %}
-# Use interface as zoneid for link local ipv6
-{% if agentip.startswith('fe80') %}
-{% set zoneid = '%' + intf %}
-{% endif %}
-agentAddress {{ protocol(agentip) }}:[{{ agentip }}{{ zoneid }}]:161
-{% endfor %}
-{% endif %}
-{% if LOOPBACK_INTERFACE is defined %}
-{% for lo in LOOPBACK_INTERFACE %}
-{% if lo | length == 2 %}
-{% set intf = lo[0] %}
-{% set agentip = lo[1].split('/')[0]|lower %}
-{% set zoneid = '' %}
-# Use interface as zoneid for link local ipv6
-{% if agentip.startswith('fe80') %}
-{% set zoneid = '%' + intf %}
-{% endif %}
-agentAddress {{ protocol(agentip) }}:[{{ agentip }}{{ zoneid }}]:161
-{% endif %}
+agentAddress {{ agentip }}{% if port %}:{{ port }}{% endif %}{% if vrf %}%{{ vrf }}{% endif %}{{ "" }}
 {% endfor %}
-{% endif %}
 {% else %}
 agentAddress udp:161
 agentAddress udp6:161

dockers/docker-snmp/start.sh

@@ -16,14 +16,11 @@ mkdir -p /etc/ssw /etc/snmp
 # Parse snmp.yml and insert the data in Config DB
 /usr/bin/snmp_yml_to_configdb.py
 
-ADD_PARAM=$(printf '%s {"NAMESPACE_COUNT":"%s"}' "-a" "$NAMESPACE_COUNT")
-
 SONIC_CFGGEN_ARGS=" \
     -d \
     -y /etc/sonic/sonic_version.yml \
     -t /usr/share/sonic/templates/sysDescription.j2,/etc/ssw/sysDescription \
     -t /usr/share/sonic/templates/snmpd.conf.j2,/etc/snmp/snmpd.conf \
-    $ADD_PARAM \
 "
 
 sonic-cfggen $SONIC_CFGGEN_ARGS

src/sonic-host-services/scripts/hostcfgd

@@ -352,7 +352,7 @@ class FeatureHandler(object):
         # sync has_per_asic_scope to CONFIG_DB in namespaces in multi-asic platform
         for ns, db in self.ns_cfg_db.items():
             db.mod_entry('FEATURE', feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)})
-    
+
     def update_systemd_config(self, feature_config):
         """Updates `Restart=` field in feature's systemd configuration file
         according to the value of `auto_restart` field in `FEATURE` table of `CONFIG_DB`.
@@ -427,7 +427,7 @@ class FeatureHandler(object):
             unit_file_state = self.get_systemd_unit_state("{}.{}".format(feature_name, feature_suffixes[-1]))
             if unit_file_state == "enabled" or not unit_file_state:
                 continue
-            cmds = []    
+            cmds = []
             for suffix in feature_suffixes:
                 cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix))
 
@@ -474,11 +474,28 @@ class FeatureHandler(object):
         self.set_feature_state(feature, self.FEATURE_STATE_DISABLED)
 
     def resync_feature_state(self, feature):
-        self._config_db.mod_entry('FEATURE', feature.name, {'state': feature.state})
+        current_entry = self._config_db.get_entry('FEATURE', feature.name)
+        current_feature_state = current_entry.get('state') if current_entry else None
 
-        # resync the feature state to CONFIG_DB in namespaces in multi-asic platform
-        for ns, db in self.ns_cfg_db.items():
-            db.mod_entry('FEATURE', feature.name, {'state': feature.state})
+        if feature.state == current_feature_state:
+            return
+
+        # feature.state might be rendered from a template, so that it should resync CONFIG DB
+        # FEATURE table to override the template value to valid state value
+        # ('always_enabled', 'always_disabled', 'disabled', 'enabled'). However, we should only
+        # resync feature state in two cases:
+        #     1. the rendered feature state is always_enabled or always_disabled, it means that the
+        #        feature state is immutable and potential state change during HostConfigDaemon.load
+        #        in redis should be skipped;
+        #     2. the current feature state in DB is a template which should be replaced by rendered feature
+        #        state
+        # For other cases, we should not resync feature.state to CONFIG DB to avoid overriding user configuration.
+        if self._feature_state_is_immutable(feature.state) or self._feature_state_is_template(current_feature_state):
+            self._config_db.mod_entry('FEATURE', feature.name, {'state': feature.state})
+
+            # resync the feature state to CONFIG_DB in namespaces in multi-asic platform
+            for ns, db in self.ns_cfg_db.items():
+                db.mod_entry('FEATURE', feature.name, {'state': feature.state})
 
     def set_feature_state(self, feature, state):
         self._feature_state_table.set(feature.name, [('state', state)])
@@ -487,6 +504,12 @@ class FeatureHandler(object):
         for ns, tbl in self.ns_feature_state_tbl.items():
             tbl.set(feature.name, [('state', state)])
 
+    def _feature_state_is_template(self, feature_state):
+        return feature_state not in ('always_enabled', 'always_disabled', 'disabled', 'enabled')
+
+    def _feature_state_is_immutable(self, feature_state):
+        return feature_state in ('always_enabled', 'always_disabled')
+
 class Iptables(object):
     def __init__(self):
         '''

src/sonic-host-services/tests/hostcfgd/hostcfgd_test.py

@@ -241,6 +241,67 @@ def test_feature_config_parsing_defaults(self):
         assert swss_feature.has_global_scope
         assert not swss_feature.has_per_asic_scope
 
+    @mock.patch('hostcfgd.FeatureHandler.update_systemd_config', mock.MagicMock())
+    @mock.patch('hostcfgd.FeatureHandler.update_feature_state', mock.MagicMock())
+    @mock.patch('hostcfgd.FeatureHandler.sync_feature_asic_scope', mock.MagicMock())
+    def test_feature_resync(self):
+        mock_db = mock.MagicMock()
+        mock_db.get_entry = mock.MagicMock()
+        mock_db.mod_entry = mock.MagicMock()
+        mock_feature_state_table = mock.MagicMock()
+
+        feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
+        feature_table = {
+            'sflow': {
+                'state': 'enabled',
+                'auto_restart': 'enabled',
+                'delayed': 'True',
+                'has_global_scope': 'False',
+                'has_per_asic_scope': 'True',
+            }
+        }
+        mock_db.get_entry.return_value = None
+        feature_handler.sync_state_field(feature_table)
+        mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'enabled'})
+        mock_db.mod_entry.reset_mock()
+
+        feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
+        mock_db.get_entry.return_value = {
+            'state': 'disabled',
+        }
+        feature_handler.sync_state_field(feature_table)
+        mock_db.mod_entry.assert_not_called()
+
+        feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
+        feature_table = {
+            'sflow': {
+                'state': 'always_enabled',
+                'auto_restart': 'enabled',
+                'delayed': 'True',
+                'has_global_scope': 'False',
+                'has_per_asic_scope': 'True',
+            }
+        }
+        feature_handler.sync_state_field(feature_table)
+        mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'always_enabled'})
+        mock_db.mod_entry.reset_mock()
+
+        feature_handler = hostcfgd.FeatureHandler(mock_db, mock_feature_state_table, {})
+        mock_db.get_entry.return_value = {
+            'state': 'some template',
+        }
+        feature_table = {
+            'sflow': {
+                'state': 'enabled',
+                'auto_restart': 'enabled',
+                'delayed': 'True',
+                'has_global_scope': 'False',
+                'has_per_asic_scope': 'True',
+            }
+        }
+        feature_handler.sync_state_field(feature_table)
+        mock_db.mod_entry.assert_called_with('FEATURE', 'sflow', {'state': 'enabled'})
+
 
 class TesNtpCfgd(TestCase):
     """