Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhanced Feature Table state enable/disable for multi-asic platforms. #5358

Merged
merged 7 commits into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions files/build_templates/init_cfg.json.j2
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
"{{feature}}": {
"state": "{{state}}",
"has_timer" : {{has_timer | lower()}},
"has_global_scope": {% if feature + '.service' in installer_services.split(' ') %}true{% else %}false{% endif %},
"has_per_asic_scope": {% if feature + '@.service' in installer_services.split(' ') %}true{% else %}false{% endif %},
"auto_restart": "{{autorestart}}",
"high_mem_alert": "disabled"
}{% if not loop.last %},{% endif -%}
Expand Down
103 changes: 62 additions & 41 deletions files/image_config/hostcfgd/hostcfgd
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import copy
import jinja2
import ipaddr as ipaddress
from swsssdk import ConfigDBConnector
from sonic_py_common import device_info

# FILE
PAM_AUTH_CONF = "/etc/pam.d/common-auth-sonic"
Expand Down Expand Up @@ -42,45 +43,6 @@ def obfuscate(data):
return data


def update_feature_state(feature_name, state, has_timer):
feature_suffixes = ["service"] + (["timer"] if ast.literal_eval(has_timer) else [])
if state == "enabled":
start_cmds = []
for suffix in feature_suffixes:
start_cmds.append("sudo systemctl unmask {}.{}".format(feature_name, suffix))
# If feature has timer associated with it, start/enable corresponding systemd .timer unit
# otherwise, start/enable corresponding systemd .service unit
start_cmds.append("sudo systemctl enable {}.{}".format(feature_name, feature_suffixes[-1]))
start_cmds.append("sudo systemctl start {}.{}".format(feature_name, feature_suffixes[-1]))
for cmd in start_cmds:
syslog.syslog(syslog.LOG_INFO, "Running cmd: '{}'".format(cmd))
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "'{}' failed. RC: {}, output: {}"
.format(err.cmd, err.returncode, err.output))
continue
syslog.syslog(syslog.LOG_INFO, "Feature '{}.{}' is enabled and started"
.format(feature_name, feature_suffixes[-1]))
elif state == "disabled":
stop_cmds = []
for suffix in reversed(feature_suffixes):
stop_cmds.append("sudo systemctl stop {}.{}".format(feature_name, suffix))
stop_cmds.append("sudo systemctl disable {}.{}".format(feature_name, suffix))
stop_cmds.append("sudo systemctl mask {}.{}".format(feature_name, suffix))
for cmd in stop_cmds:
syslog.syslog(syslog.LOG_INFO, "Running cmd: '{}'".format(cmd))
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "'{}' failed. RC: {}, output: {}"
.format(err.cmd, err.returncode, err.output))
continue
syslog.syslog(syslog.LOG_INFO, "Feature '{}' is stopped and disabled".format(feature_name))
else:
syslog.syslog(syslog.LOG_ERR, "Unexpected state value '{}' for feature '{}'"
.format(state, feature_name))


class Iptables(object):
def __init__(self):
Expand Down Expand Up @@ -280,6 +242,65 @@ class HostConfigDaemon:
self.iptables = Iptables()
self.iptables.load(lpbk_table)

def update_feature_state(self, feature_name, state, feature_table):
has_timer = ast.literal_eval(feature_table[feature_name]['has_timer'])
has_global_scope = ast.literal_eval(feature_table[feature_name]['has_global_scope'])
has_per_asic_scope = ast.literal_eval(feature_table[feature_name]['has_per_asic_scope'])

# Create feature name suffix depending feature is running in host or namespace or in both
feature_name_suffix_list = (([feature_name] if has_global_scope or not device_info.is_multi_npu() else []) +
([(feature_name + '@' + str(asic_inst)) for asic_inst in range(device_info.get_num_npus())
if has_per_asic_scope and device_info.is_multi_npu()]))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make is_multi_npu() a class member?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


if not feature_name_suffix_list:
syslog.syslog(syslog.LOG_ERR, "Feature '{}' service not available"
.format(feature_name))

feature_suffixes = ["service"] + (["timer"] if has_timer else [])

if state == "enabled":
start_cmds = []
for feature_name_suffix in feature_name_suffix_list:
for suffix in feature_suffixes:
start_cmds.append("sudo systemctl unmask {}.{}".format(feature_name_suffix, suffix))
Comment on lines +265 to +266
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great work. I know it merged already, but wanted to follow up the story here if we have plan to support enabling/disabling services for particular asics in system?

More specifically, says there are 2 asics in system, we want to disable lldp on asic 1, but not asic 2.

I imagine that to do that, we need

(1) Create entry like FEATURE|lldp|1 with enabled = false and FEATURE|lldp|2 with enabled = true. hostcfgd listens and reacts to the change. It will turn off lldp@1.service but keep lldp@2.service on.

Or (2) Create hostcfgd@1 for asic 1 and hostcfgd@2 for asic 2. hostcfgd@1 listens database@1 CONFIG_DB, and reacts to FEATURE|lldp. Same for hostcfgd@2 which listens database@2 CONFIG_DB. But this one may not work if we want to enable/disable database. If database doesn't exits, hostcfgd@x will have nothing to listen and react.

# If feature has timer associated with it, start/enable corresponding systemd .timer unit
# otherwise, start/enable corresponding systemd .service unit
start_cmds.append("sudo systemctl enable {}.{}".format(feature_name_suffix, feature_suffixes[-1]))
start_cmds.append("sudo systemctl start {}.{}".format(feature_name_suffix, feature_suffixes[-1]))
for cmd in start_cmds:
syslog.syslog(syslog.LOG_INFO, "Running cmd: '{}'".format(cmd))
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "'{}' failed. RC: {}, output: {}"
.format(err.cmd, err.returncode, err.output))
syslog.syslog(syslog.LOG_ERR, "Feature '{}.{}' failed to be enabled and started"
.format(feature_name, feature_suffixes[-1]))
return
syslog.syslog(syslog.LOG_INFO, "Feature '{}.{}' is enabled and started"
.format(feature_name, feature_suffixes[-1]))
elif state == "disabled":
stop_cmds = []
for feature_name_suffix in feature_name_suffix_list:
for suffix in reversed(feature_suffixes):
stop_cmds.append("sudo systemctl stop {}.{}".format(feature_name_suffix, suffix))
stop_cmds.append("sudo systemctl disable {}.{}".format(feature_name_suffix, suffix))
stop_cmds.append("sudo systemctl mask {}.{}".format(feature_name_suffix, suffix))
for cmd in stop_cmds:
syslog.syslog(syslog.LOG_INFO, "Running cmd: '{}'".format(cmd))
try:
subprocess.check_call(cmd, shell=True)
except subprocess.CalledProcessError as err:
syslog.syslog(syslog.LOG_ERR, "'{}' failed. RC: {}, output: {}"
.format(err.cmd, err.returncode, err.output))
syslog.syslog(syslog.LOG_ERR, "Feature '{}' failed to be stopped and disabled".format(feature_name))
return
syslog.syslog(syslog.LOG_INFO, "Feature '{}' is stopped and disabled".format(feature_name))
else:
syslog.syslog(syslog.LOG_ERR, "Unexpected state value '{}' for feature '{}'"
.format(state, feature_name))


def update_all_feature_states(self):
feature_table = self.config_db.get_table('FEATURE')
for feature_name in feature_table.keys():
Expand All @@ -292,7 +313,7 @@ class HostConfigDaemon:
syslog.syslog(syslog.LOG_WARNING, "Eanble state of feature '{}' is None".format(feature_name))
continue

update_feature_state(feature_name, state, feature_table[feature_name]['has_timer'])
self.update_feature_state(feature_name, state, feature_table)

def aaa_handler(self, key, data):
self.aaacfg.aaa_update(key, data)
Expand Down Expand Up @@ -334,7 +355,7 @@ class HostConfigDaemon:
syslog.syslog(syslog.LOG_WARNING, "Enable state of feature '{}' is None".format(feature_name))
return

update_feature_state(feature_name, state, feature_table[feature_name]['has_timer'])
self.update_feature_state(feature_name, state, feature_table)

def start(self):
# Update all feature states once upon starting
Expand Down