Skip to content

Commit

Permalink
[#20881] yugabyted: yugabyted dynamically updates the user specified …
Browse files Browse the repository at this point in the history
…configuration file.

Summary:
To address the above issue we have introduced the following changes in the behaviour of `--config` flag:
1. The `--config` flag will be specifically associated with the start command, removing its global applicability across all yugabyted commands.
2. Upon the execution of the start command with a specified configuration file (custom_config), yugabyted will no longer directly modify this user-provided file. Instead, it will:

  - Read and parse the contents of the custom_config file specified by the `--config` flag.

  - Create and update a yugabyted generated configuration file named yugabyted.conf within the base_dir/conf directory.

  - Populate yugabyted.conf with the settings from the custom_config file, supplemented by any necessary default settings.
Jira: DB-9867

Test Plan: Manual Testing

Reviewers: sgarg-yb, nikhil

Reviewed By: nikhil

Subscribers: yugabyted-dev, shikhar.sahay

Differential Revision: https://phorge.dev.yugabyte.com/D32360
  • Loading branch information
ShikharSahay committed Feb 29, 2024
1 parent 4fb530b commit 8be5a3c
Showing 1 changed file with 154 additions and 59 deletions.
213 changes: 154 additions & 59 deletions bin/yugabyted
Original file line number Diff line number Diff line change
Expand Up @@ -981,11 +981,6 @@ class ControlScript(object):
gen_certs_dir = self.configs.saved_data.get("gen_certs_dir")
certs_dir = self.configs.saved_data.get("certs_dir")
config_path = os.path.dirname(self.conf_file)
is_config_passed=False
for arg in sys.argv[1:]:
if (arg.startswith("--config")):
is_config_passed=True
break

if (self.conf_file == BREW_CONF_FILE):
Output.print_out("{} destroy is not supported for brew installations.".format(
Expand All @@ -1008,14 +1003,9 @@ class ControlScript(object):
shutil.rmtree(certs_dir)
Output.print_out("Deleted certs at {}.".format(certs_dir))

if is_config_passed:
if os.path.exists(self.conf_file):
os.remove(self.conf_file)
Output.print_out("Deleted conf file at {}.".format(self.conf_file))
else:
if os.path.isdir(config_path):
shutil.rmtree(config_path)
Output.print_out("Deleted conf at {}.".format(config_path))
if os.path.isdir(config_path):
shutil.rmtree(config_path)
Output.print_out("Deleted conf at {}.".format(config_path))

sys.exit(0)

Expand Down Expand Up @@ -2801,21 +2791,62 @@ class ControlScript(object):
ysql_hba_conf_csv_end_index]
ysql_hba_conf_csv_flag = ysql_hba_conf_csv_flag[ysql_hba_conf_csv_flag.find("{")+1:-1]

# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_hba_conf_csv_flag[0] == "'" and ysql_hba_conf_csv_flag[-1] == "'":
ysql_hba_conf_csv_flag = ysql_hba_conf_csv_flag[1:-1]
elif ysql_hba_conf_csv_flag[0] == '"' and ysql_hba_conf_csv_flag[-1] == '"':
ysql_hba_conf_csv_flag = ysql_hba_conf_csv_flag[1:-1]

# Check and handle if the ysql_hba_conf_csv_flag is
# empty after removing the curly braces
if ysql_hba_conf_csv_flag:
# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_hba_conf_csv_flag[0] == "'" and ysql_hba_conf_csv_flag[-1] == "'":
ysql_hba_conf_csv_flag = ysql_hba_conf_csv_flag[1:-1]
elif ysql_hba_conf_csv_flag[0] == '"' and ysql_hba_conf_csv_flag[-1] == '"':
ysql_hba_conf_csv_flag = ysql_hba_conf_csv_flag[1:-1]

yb_tserver_cmd.extend(["--ysql_hba_conf_csv={}".format(ysql_hba_conf_csv_flag)])
else:
# If the flag is empty, reset the configuration by not adding it
pass
# Remove the flag present in tserver_flags
if ysql_hba_conf_csv_start_index == 0:
tserver_flags = tserver_flags[ysql_hba_conf_csv_end_index+1:]
else:
tserver_flags = tserver_flags[:ysql_hba_conf_csv_start_index-1] + \
tserver_flags[ysql_hba_conf_csv_end_index:]

yb_tserver_cmd.extend(["--ysql_hba_conf_csv={}".format(ysql_hba_conf_csv_flag)])
# If allowed_preview_flags_csv is present
# Extract the value of allowed_preview_flags_csv
# Remove the flag present in the tserver_flags string
if tserver_flags.find("allowed_preview_flags_csv") != -1:
allowed_preview_flags_csv_start_index = tserver_flags.find('allowed_preview_flags_csv')
allowed_preview_flags_csv_end_index = tserver_flags.find('}',
allowed_preview_flags_csv_start_index) + 1
allowed_preview_flags_csv_flag = tserver_flags[allowed_preview_flags_csv_start_index:\
allowed_preview_flags_csv_end_index]
allowed_preview_flags_csv_flag = allowed_preview_flags_csv_flag[
allowed_preview_flags_csv_flag.find("{")+1:-1]

# Check and handle if the allowed_preview_flags_csv_flag is
# empty after removing the curly braces
if allowed_preview_flags_csv_flag:
# If the value was given through the config file
# remove the starting and ending quotes.
if allowed_preview_flags_csv_flag[0] == "'" and \
allowed_preview_flags_csv_flag[-1] == "'":
allowed_preview_flags_csv_flag = allowed_preview_flags_csv_flag[1:-1]
elif allowed_preview_flags_csv_flag[0] == '"' and \
allowed_preview_flags_csv_flag[-1] == '"':
allowed_preview_flags_csv_flag = allowed_preview_flags_csv_flag[1:-1]

yb_tserver_cmd.extend(["--allowed_preview_flags_csv={}".format(
allowed_preview_flags_csv_flag)])
else:
# If the flag is empty, reset the configuration by not adding it
pass
# Remove the flag present in tserver_flags
if allowed_preview_flags_csv_start_index == 0:
tserver_flags = tserver_flags[allowed_preview_flags_csv_end_index+1:]
else:
tserver_flags = tserver_flags[:allowed_preview_flags_csv_start_index-1] + \
tserver_flags[allowed_preview_flags_csv_end_index:]

# If ysql_ident_conf_csv is present
# Extract the value of ysql_ident_conf_csv
Expand All @@ -2829,22 +2860,27 @@ class ControlScript(object):
ysql_ident_conf_csv_flag = \
ysql_ident_conf_csv_flag[ysql_ident_conf_csv_flag.find("{")+1:-1]

# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_ident_conf_csv_flag[0] == "'" and ysql_ident_conf_csv_flag[-1] == "'":
ysql_ident_conf_csv_flag = ysql_ident_conf_csv_flag[1:-1]
elif ysql_ident_conf_csv_flag[0] == '"' and ysql_ident_conf_csv_flag[-1] == '"':
ysql_ident_conf_csv_flag = ysql_ident_conf_csv_flag[1:-1]

# Check and handle if the ysql_ident_conf_csv_flag is
# empty after removing the curly braces
if ysql_ident_conf_csv_flag:
# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_ident_conf_csv_flag[0] == "'" and ysql_ident_conf_csv_flag[-1] == "'":
ysql_ident_conf_csv_flag = ysql_ident_conf_csv_flag[1:-1]
elif ysql_ident_conf_csv_flag[0] == '"' and ysql_ident_conf_csv_flag[-1] == '"':
ysql_ident_conf_csv_flag = ysql_ident_conf_csv_flag[1:-1]

yb_tserver_cmd.extend(["--ysql_ident_conf_csv={}".format(ysql_ident_conf_csv_flag)])
else:
# If the flag is empty, reset the configuration by not adding it
pass
# Remove the flag present in tserver_flags
if ysql_ident_conf_csv_start_index == 0:
tserver_flags = tserver_flags[ysql_ident_conf_csv_end_index+1:]
else:
tserver_flags = tserver_flags[:ysql_ident_conf_csv_start_index-1] + \
tserver_flags[ysql_ident_conf_csv_end_index:]

yb_tserver_cmd.extend(["--ysql_ident_conf_csv={}".format(ysql_ident_conf_csv_flag)])

# If ysql_pg_conf_csv is present
# Extract the value of ysql_pg_conf_csv given through CLI
# Remove the flag present in the tserver_flags string
Expand All @@ -2855,22 +2891,27 @@ class ControlScript(object):
ysql_pg_conf_csv_end_index]
ysql_pg_conf_csv_flag = ysql_pg_conf_csv_flag[ysql_pg_conf_csv_flag.find("{")+1:-1]

# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_pg_conf_csv_flag[0] == "'" and ysql_pg_conf_csv_flag[-1] == "'":
ysql_pg_conf_csv_flag = ysql_pg_conf_csv_flag[1:-1]
elif ysql_pg_conf_csv_flag[0] == '"' and ysql_pg_conf_csv_flag[-1] == '"':
ysql_pg_conf_csv_flag = ysql_pg_conf_csv_flag[1:-1]

# Check and handle if the ysql_pg_conf_csv_flag is
# empty after removing the curly braces
if ysql_pg_conf_csv_flag:
# If the value was given through the config file
# remove the starting and ending quotes.
if ysql_pg_conf_csv_flag[0] == "'" and ysql_pg_conf_csv_flag[-1] == "'":
ysql_pg_conf_csv_flag = ysql_pg_conf_csv_flag[1:-1]
elif ysql_pg_conf_csv_flag[0] == '"' and ysql_pg_conf_csv_flag[-1] == '"':
ysql_pg_conf_csv_flag = ysql_pg_conf_csv_flag[1:-1]

yb_tserver_cmd.extend(["--ysql_pg_conf_csv={}".format(ysql_pg_conf_csv_flag)])
else:
# If the flag is empty, reset the configuration by not adding it
pass
# Remove the flag present in tserver_flags
if ysql_pg_conf_csv_start_index == 0:
tserver_flags = tserver_flags[ysql_pg_conf_csv_end_index+1:]
else:
tserver_flags = tserver_flags[:ysql_pg_conf_csv_start_index-1] + \
tserver_flags[ysql_pg_conf_csv_end_index:]

yb_tserver_cmd.extend(["--ysql_pg_conf_csv={}".format(ysql_pg_conf_csv_flag)])

if tserver_flags:
yb_tserver_cmd.extend(
["--{}".format(flag) for flag in tserver_flags.split(",")])
Expand Down Expand Up @@ -4899,8 +4940,32 @@ class ControlScript(object):
Output.log_error_and_exit("Other error occurred while checking for security of " +
"leader master: {}".format(err))

def handle_config_files(self, args, conf_dir, config_file, base_dir):
if not os.path.isdir(conf_dir):
if args.parser == "start":
os.makedirs(conf_dir)

if os.path.exists(config_file):
# Load already saved configs if yugabyted.conf exists
self.configs = Configs.parse_config_file(config_file, base_dir)
else:
# Initialise configs
self.configs = Configs(config_file, base_dir)
if args.parser == "start":
open(config_file, 'a').close()

if args.parser == "start" and args.config:
args.config = \
os.path.realpath(
os.path.abspath(
os.path.expanduser(args.config)))
user_configs = Configs.parse_user_config_file(args.config)
# Update saved configs with user configurations
self.configs.saved_data.update(user_configs)

return self.configs
# Parse the config file and input parent flags to validate and set them.
# Parent flags: --config, --data_dir, --log_dir
# Parent flags: --data_dir, --log_dir
def validate_and_set_parent_configs(self, args):

home_dir = os.path.expanduser("~")
Expand All @@ -4914,15 +4979,12 @@ class ControlScript(object):

has_errors = False

self.conf_file = args.config or base_dir_conf or \
self.conf_file = base_dir_conf or \
Configs.get_brew_config() or default_conf_path
self.conf_file = os.path.realpath(os.path.expanduser(self.conf_file))

conf_dir = os.path.dirname(self.conf_file)
if not os.path.isdir(conf_dir):
os.makedirs(conf_dir)

self.configs = Configs.parse_config_file(self.conf_file, base_dir)
self.configs = self.handle_config_files(args, conf_dir, self.conf_file, base_dir)

for path_args in ("data_dir", "log_dir"):
path = getattr(args, path_args, None)
Expand Down Expand Up @@ -4953,7 +5015,7 @@ class ControlScript(object):
if has_errors:
sys.exit(1)

parent_flags = ["config", "log_dir", "data_dir"]
parent_flags = ["log_dir", "data_dir"]
# Override configs and defaults with user specified variables
for k, v in get_kv(args.__dict__):
if (v is not None and k in parent_flags and k in self.configs.saved_data
Expand Down Expand Up @@ -5992,7 +6054,7 @@ class ControlScript(object):
if has_errors:
sys.exit(1)

parent_flags = ["config", "log_dir", "data_dir"]
parent_flags = ["log_dir", "data_dir"]
# Override configs and defaults with user specified variables
for k, v in get_kv(args.__dict__):
if (v is not None and k not in parent_flags and k in self.configs.saved_data
Expand All @@ -6007,9 +6069,6 @@ class ControlScript(object):
def run(self):
# Parent subparser for common args
common_parser = argparse.ArgumentParser(add_help=False)
common_parser.add_argument(
"--config", help="{} configuration file path".format(
SCRIPT_NAME), metavar="")
# TODO: Refactor data_dir to be a list for multi-node. How should the config file and
# data dir be set for local mulit-node setups? Note: daemon mode may be affected.
common_parser.add_argument(
Expand Down Expand Up @@ -6398,6 +6457,9 @@ class ControlScript(object):
cur_parser.add_argument(
"--fault_tolerance", metavar="",
help="Determines the type of deployment of the cluster. Default is None.")
cur_parser.add_argument(
"--config", help="{} user configuration file path".format(
SCRIPT_NAME), metavar="")

# Hidden commands for development/advanced users
cur_parser.add_argument(
Expand Down Expand Up @@ -6430,8 +6492,11 @@ class ControlScript(object):

Output.log("Running yugabyted command: '{}'".format(' '.join(sys.argv)))

Output.log("cmd = {} using config file: {} (args.config={})".format(
args.parser, self.conf_file, args.config))
Output.log("cmd = {} using config file: {}".format(args.parser, self.conf_file))

if args.parser == "start" and args.config:
Output.log("Updating yugabyted config file with"
" user-specified configs located at: {}".format(args.config))

# Initialize the script path of openssl_proxy.sh
OpenSSLProxy.init()
Expand Down Expand Up @@ -6594,8 +6659,9 @@ class Configs(object):

# Saves current configs to config file.
def save_configs(self):
with open(self.config_file, "w+") as f:
json.dump(self.saved_data, f, indent=4)
if os.path.exists(self.config_file):
with open(self.config_file, "w+") as f:
json.dump(self.saved_data, f, indent=4)

# Custom parser for reading config file.
@staticmethod
Expand All @@ -6609,13 +6675,42 @@ class Configs(object):
Output.log_error_and_exit(
"Failed to read config file {}: {}".format(config_file, str(e)))

configs.saved_data = Configs.expand_path_variables(configs.saved_data)

return configs

# Custom parser for reading user config file.
@staticmethod
def parse_user_config_file(user_conf_file):
user_configs = {}
if not os.path.exists(user_conf_file):
Output.log_error_and_exit("User config file {} does not exist.".format(user_conf_file))
try:
with open(user_conf_file, 'r') as f:
content = f.read()
# If the file is empty or contains only whitespace,
# user_configs remains an empty dictionary
if content.strip():
user_configs = json.loads(content)
except Exception as e:
Output.log_error_and_exit("Failed to read user config file {}: {}".format(
user_conf_file, str(e)))

user_configs = Configs.expand_path_variables(user_configs)

return user_configs

# Expand path variables in config files
@staticmethod
def expand_path_variables(configs):
paths = ["data_dir", "log_dir", "certs_dir", "gen_certs_dir", "ca_cert_file_path"]
for k, v in configs.saved_data.items():
if v is not None and k in paths:
if "$" in v:
configs.saved_data[k] = os.path.expanduser(os.path.expandvars(v))
for key, value in configs.items():
if value is not None and key in paths:
if "$" in value:
configs[key] = os.path.expanduser(os.path.expandvars(value))
else:
configs.saved_data[k] = os.path.expanduser(v)
configs[key] = os.path.expanduser(value)

return configs

@staticmethod
Expand Down

0 comments on commit 8be5a3c

Please sign in to comment.