Skip to content

Commit

Permalink
[hostcfgd] Enable/disable the container service only when the feature…
Browse files Browse the repository at this point in the history
… state was changed. (sonic-net#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 <[email protected]>
  • Loading branch information
yozhao101 authored and abdosi committed Oct 23, 2020
1 parent ea28f2d commit d8ae2a0
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion files/image_config/hostcfgd/hostcfgd
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit d8ae2a0

Please sign in to comment.