From d8ae2a0019ad3f6abad400b8a1671f2bb7c52ff1 Mon Sep 17 00:00:00 2001 From: yozhao101 <56170650+yozhao101@users.noreply.github.com> Date: Thu, 22 Oct 2020 20:01:07 -0700 Subject: [PATCH] [hostcfgd] Enable/disable the container service only when the feature state was changed. (#5689) **- Why I did it** If we ran the CLI commands `sudo config feature autorestart snmp disabled/enabled` or `sudo config feature autorestart swss disabled/enabled`, then SNMP container will be stopped and started. This behavior was not expected since we updated the `auto_restart` field not update `state` field in `FEATURE` table. The reason behind this issue is that either `state` field or `auto_restart` field was updated, the function `update_feature_state(...)` will be invoked which then starts snmp.timer service. The snmp.timer service will first stop snmp.service and later start snmp.service. In order to solve this issue, the function `update_feature_state(...)` will be only invoked if `state` field in `FEATURE` table was updated. **- How I did it** When the demon `hostcfgd` was activated, all the values of `state` field in `FEATURE` table of each container will be cached. Each time the function `feature_state_handler(...)` is invoked, it will determine whether the `state` field of a container was changed or not. If it was changed, function `update_feature_state(...)` will be invoked and the cached value will also be updated. Otherwise, nothing will be done. **- How to verify it** We can run the CLI commands `sudo config feature autorestart snmp disabled/enabled` or `sudo config feature autorestart swss disabled/enabled` to check whether SNMP container is stopped and started. We also can run the CLI commands `sudo config feature state snmp disabled/enabled` or `sudo config feature state swss disabled/enabled` to check whether the container is stopped and restarted. Signed-off-by: Yong Zhao --- files/image_config/hostcfgd/hostcfgd | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/files/image_config/hostcfgd/hostcfgd b/files/image_config/hostcfgd/hostcfgd index 0d4010f5a6..61a142583a 100755 --- a/files/image_config/hostcfgd/hostcfgd +++ b/files/image_config/hostcfgd/hostcfgd @@ -238,6 +238,8 @@ class HostConfigDaemon: self.iptables = Iptables() self.iptables.load(lpbk_table) self.is_multi_npu = device_info.is_multi_npu() + # Cache the values of 'state' field in 'FEATURE' table of each container + self.cached_feature_states = {} def update_feature_state(self, feature_name, state, feature_table): has_timer = ast.literal_eval(feature_table[feature_name].get('has_timer', 'False')) @@ -310,6 +312,9 @@ class HostConfigDaemon: syslog.syslog(syslog.LOG_WARNING, "Eanble state of feature '{}' is None".format(feature_name)) continue + # Store the initial value of 'state' field in 'FEATURE' table of a specific container + self.cached_feature_states[feature_name] = state + self.update_feature_state(feature_name, state, feature_table) def aaa_handler(self, key, data): @@ -352,7 +357,10 @@ class HostConfigDaemon: syslog.syslog(syslog.LOG_WARNING, "Enable state of feature '{}' is None".format(feature_name)) return - self.update_feature_state(feature_name, state, feature_table) + # Enable/disable the container service if the feature state was changed from its previous state. + if self.cached_feature_states[feature_name] != state: + self.cached_feature_states[feature_name] = state + self.update_feature_state(feature_name, state, feature_table) def start(self): # Update all feature states once upon starting