From 93c03a9a8ff58761629c53b6371156736e540769 Mon Sep 17 00:00:00 2001 From: dnessett Date: Thu, 27 Feb 2025 11:31:06 -0700 Subject: [PATCH 1/6] Fix bugs in minionswarm. Updated help with significant enhancements. Added username to salt-master command. Added ability to turn off open_mode. --- tests/minionswarm.py | 187 +++++++++++++++++++++++++++++++------- tests/support/runtests.py | 2 +- 2 files changed, 153 insertions(+), 36 deletions(-) diff --git a/tests/minionswarm.py b/tests/minionswarm.py index dc2ed7179cd0..cdf34b81f69d 100644 --- a/tests/minionswarm.py +++ b/tests/minionswarm.py @@ -17,11 +17,11 @@ import tempfile import time import uuid +import getpass -import salt import salt.utils.files import salt.utils.yaml -import tests.support.runtests +import support.runtests OSES = [ "Arch", @@ -34,25 +34,39 @@ "Solaris", ] VERS = [ - "2014.1.6", - "2014.7.4", - "2015.5.5", - "2015.8.0", + "3006.1", + "3006.5", + "3006.9", + "3007.1", ] - def parse(): """ Parse the cli options """ - parser = optparse.OptionParser() + guidance = ( + "\n\n To execute salt commands against minionswarm you must include the configuration\n" + " file using the -c option. For commands such as salt, salt-key, salt-cp,\n" + " and salt-run use -c /master. For example, when using the\n" + " default for --temp-dir, the configuration directory would be\n" + " /tmp/sroot/master. If the master runs on a different machine you\n" + " must execute the command on that machine using the master config file\n" + " there. For the salt-call command, which is a minion side\n" + " command, use -c /-. For example to\n" + " execute salt-call on the first minion using the default values for\n" + " temp-dir and name use -c /tmp/sroot/minion-0. The commands salt-api,\n" + " salt-cloud, salt-extend, salt-master, salt-minion, salt-proxy,\n" + " salt-ssh, salt-syndic and spm are not supoported." + ) + usage = "usage: python %prog [options]" + guidance + parser = optparse.OptionParser(usage) parser.add_option( "-m", "--minions", dest="minions", default=5, type="int", - help="The number of minions to make", + help="The number of minions to make (default = 5)", ) parser.add_option( "-M", @@ -64,18 +78,18 @@ def parse(): parser.add_option( "--master", dest="master", - default="salt", - help="The location of the salt master that this swarm will serve", + default="localhost", + help="The location of the salt master that this swarm will serve. (default = localhost)", ) parser.add_option( "--name", "-n", dest="name", - default="ms", + default="minion", help=( "Give the minions an alternative id prefix, this is used " "when minions from many systems are being aggregated onto " - "a single master" + "a single master. (default = minion)" ), ) parser.add_option( @@ -111,7 +125,7 @@ def parse(): "--keep-modules", dest="keep", default="", - help="A comma delimited list of modules to enable", + help="A comma delimited list of modules to enable. (default = None)", ) parser.add_option( "-f", @@ -124,26 +138,30 @@ def parse(): parser.add_option( "--temp-dir", dest="temp_dir", - default=None, - help="Place temporary files/directories here", + default="/tmp/sroot", + help="Place temporary files/directories here, (default = /tmp/sroot)", ) parser.add_option( "--no-clean", action="store_true", default=False, - help="Don't cleanup temporary files/directories", + help="Don't cleanup temporary files/directories. " + "If specified, you must manually recursively delete " + "the swarm root (see --temp-dir) before running " + "minionswarm again, e.g., using the default swarm " + "root, rm -fr /tmp/srooot" ) parser.add_option( "--root-dir", dest="root_dir", default=None, - help="Override the minion root_dir config", + help="Override the minion root_dir config. (default = None)", ) parser.add_option( "--transport", dest="transport", default="zeromq", - help="Declare which transport to use, default is zeromq", + help="Declare which transport to use, (default = zeromq)", ) parser.add_option( "--start-delay", @@ -158,7 +176,15 @@ def parse(): default="", help="Pass in a configuration directory containing base configuration.", ) - parser.add_option("-u", "--user", default=tests.support.runtests.this_user()) + parser.add_option( + "--open-mode", + dest="open_mode", + default=True, + action="store_true", + help="Turn off authentication at the Master. Default is True to align " + "this version of minionswarm with previous version." + ) + parser.add_option("-u", "--user", default=support.runtests.this_user()) options, _args = parser.parse_args() @@ -286,8 +312,9 @@ def start_minions(self): Iterate over the config files and start up the minions """ self.prep_configs() + username = getpass.getuser() for path in self.confs: - cmd = "salt-minion -c {} --pid-file {}".format(path, f"{path}.pid") + cmd = "salt-minion -c {} --user={} --pid-file {}".format(path, username, f"{path}.pid") if self.opts["foreground"]: cmd += " -l debug &" else: @@ -314,7 +341,6 @@ def mkconf(self, idx): { "id": minion_id, "user": self.opts["user"], - "cachedir": os.path.join(dpath, "cache"), "master": self.opts["master"], "log_file": os.path.join(dpath, "minion.log"), "grains": {}, @@ -329,12 +355,12 @@ def mkconf(self, idx): minion_pub = os.path.join(self.pki, "minion.pub") shutil.copy(minion_pem, minion_pkidir) shutil.copy(minion_pub, minion_pkidir) - data["pki_dir"] = minion_pkidir + data.update({"pki_dir": minion_pkidir}) elif self.opts["transport"] == "tcp": - data["transport"] = "tcp" + data.update({"transport": "tcp"}) if self.opts["root_dir"]: - data["root_dir"] = self.opts["root_dir"] + data.update({"root_dir": self.opts["root_dir"]}) path = os.path.join(dpath, "minion") @@ -343,21 +369,63 @@ def mkconf(self, idx): modpath = os.path.join(os.path.dirname(salt.__file__), "modules") fn_prefixes = (fn_.partition(".")[0] for fn_ in os.listdir(modpath)) ignore = [fn_prefix for fn_prefix in fn_prefixes if fn_prefix not in keep] - data["disable_modules"] = ignore + data.update({"disable_modules": ignore}) if self.opts["rand_os"]: data["grains"]["os"] = random.choice(OSES) if self.opts["rand_ver"]: data["grains"]["saltversion"] = random.choice(VERS) if self.opts["rand_machine_id"]: - data["grains"]["machine_id"] = hashlib.md5(minion_id).hexdigest() + try: + minion_id_encode = minion_id.encode(encoding="utf-8", errors="strict") + data["grains"]["machine_id"] = hashlib.md5(minion_id_encode).hexdigest() + except UnicodeEncodeError: + print("\n'minion id contains illegal character. Shutting down.") + sys.exit([1]) + data["grains"]["machine_id"] = hashlib.md5(minion_id_encode).hexdigest() if self.opts["rand_uuid"]: data["grains"]["uuid"] = str(uuid.uuid4()) + + data = self._update_minion_conf(data) with salt.utils.files.fopen(path, "w+") as fp_: salt.utils.yaml.safe_dump(data, fp_) self.confs.add(dpath) + def _update_minion_conf(self, data): # pylint: disable=W0221 + """ + Modify the minion config to contain cachedir and sock_dir definitions. Unless cachedir and sock_dir + are modified as indicated, alt-master will think they are /var/cache/salt and /var/run/salt/master, + respectively, which generally will not exist. Also, specify the extmods directory path and the + pki/minion directory path. + """ + + cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/minion") + sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") + extension_modules_dir_path = os.path.join(self.swarm_root, "var/cache/salt/minion/extmods") + pki_dir_path = os.path.join(self.swarm_root, "etc/salt/pki/minion") + + try: + os.makedirs(cachdir_path) + except FileExistsError: + # directory already exists + pass + try: + os.makedirs(sock_dir_path) + except FileExistsError: + # directory already exists + pass + + data.update( + { + "cachedir": cachdir_path, + "sock_dir": sock_dir_path, + "extension_modules" : extension_modules_dir_path, + "pki_dir": pki_dir_path, + } + ) + return data + def prep_configs(self): """ Prepare the confs set @@ -390,9 +458,11 @@ def start(self): def start_master(self): """ - Do the master start - """ - cmd = "salt-master -c {} --pid-file {}".format(self.conf, f"{self.conf}.pid") + Do the master start.. Run the master as the user under which minionswarm runs. + """ + + username = getpass.getuser() + cmd = "salt-master '--config-dir={}' --user={} --pid-file {}".format(self.conf, username, f"{self.conf}.pid") if self.opts["foreground"]: cmd += " -l debug &" else: @@ -401,27 +471,74 @@ def start_master(self): def mkconf(self): # pylint: disable=W0221 """ - Make a master config and write it' + Make the config file with standard values """ + data = {} if self.opts["config_dir"]: spath = os.path.join(self.opts["config_dir"], "master") with salt.utils.files.fopen(spath) as conf: data = salt.utils.yaml.safe_load(conf) + head, tail = os.path.split(self.conf) + if self.opts["transport"] == "tcp": + data.update({"transport": "tcp"}) data.update( { "log_file": os.path.join(self.conf, "master.log"), - "open_mode": True, # TODO Pre-seed keys + "pki_dir": os.path.join(head, "pki"), } ) - + data.update({"open_mode": self.opts["open_mode"]}) + + # TODO Pre-seed keys + os.makedirs(self.conf) path = os.path.join(self.conf, "master") - + + data = self._update_master_conf(data) + with salt.utils.files.fopen(path, "w+") as fp_: salt.utils.yaml.safe_dump(data, fp_) - + + def _update_master_conf(self, data): # pylint: disable=W0221 + """ + Modify the master config to contain cachedir and sock_dir definitions. Unless cachedir and sock_dir + are modified as indicated, alt-master will think they are /var/cache/salt and /var/run/salt/master, + respectively, which generally will not exist. + """ + + key_logfile_path = os.path.join(self.swarm_root, "var/log/salt/key") + cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/master") + sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") + sqlite_queue_dir_path = os.path.join(self.swarm_root, "var/cache/salt/master") + + try: + os.makedirs(cachdir_path) + except FileExistsError: + # directory already exists + pass + try: + os.makedirs(sock_dir_path) + except FileExistsError: + # directory already exists + pass + + data.update( + { + "key_logfile": key_logfile_path, + "cachedir": cachdir_path, + "sock_dir": sock_dir_path, + "sqlite_queue_dir" : sqlite_queue_dir_path, + } + ) + + return data + def shutdown(self): + """ + Shutdown master + """ + print("Killing master") subprocess.call('pkill -KILL -f "python.*salt-master"', shell=True) print("Master killed") diff --git a/tests/support/runtests.py b/tests/support/runtests.py index 046c20bd01ff..c816b447fb12 100644 --- a/tests/support/runtests.py +++ b/tests/support/runtests.py @@ -50,7 +50,7 @@ import salt.utils.path import salt.utils.platform -import tests.support.paths as paths +import support.paths as paths try: import pwd From d63f46490a09867b7b6bd34ce8546cbbb9f328fb Mon Sep 17 00:00:00 2001 From: dnessett Date: Fri, 28 Feb 2025 13:19:32 -0700 Subject: [PATCH 2/6] cleaned up some problems with the parser.add_options and added a clarificationn to the config-dir help text. --- tests/minionswarm.py | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/minionswarm.py b/tests/minionswarm.py index cdf34b81f69d..8549a2e97f19 100644 --- a/tests/minionswarm.py +++ b/tests/minionswarm.py @@ -56,7 +56,7 @@ def parse(): " execute salt-call on the first minion using the default values for\n" " temp-dir and name use -c /tmp/sroot/minion-0. The commands salt-api,\n" " salt-cloud, salt-extend, salt-master, salt-minion, salt-proxy,\n" - " salt-ssh, salt-syndic and spm are not supoported." + " salt-ssh, salt-syndic and spm are not supported." ) usage = "usage: python %prog [options]" + guidance parser = optparse.OptionParser(usage) @@ -79,18 +79,18 @@ def parse(): "--master", dest="master", default="localhost", - help="The location of the salt master that this swarm will serve. (default = localhost)", + help="The location of the salt master that this swarm will serve. (default = localhost) " + "The standard port used by daemon masters is 4506. Masters can be specified using " + ":. For example, 192.168.1.2:4506", ) parser.add_option( "--name", "-n", dest="name", default="minion", - help=( - "Give the minions an alternative id prefix, this is used " + help="Give the minions an alternative id prefix, this is used " "when minions from many systems are being aggregated onto " - "a single master. (default = minion)" - ), + "a single master. (default = minion)", ) parser.add_option( "--rand-os", @@ -149,7 +149,7 @@ def parse(): "If specified, you must manually recursively delete " "the swarm root (see --temp-dir) before running " "minionswarm again, e.g., using the default swarm " - "root, rm -fr /tmp/srooot" + "root, rm -fr /tmp/srooot", ) parser.add_option( "--root-dir", @@ -161,7 +161,10 @@ def parse(): "--transport", dest="transport", default="zeromq", - help="Declare which transport to use, (default = zeromq)", + help="Declare which transport to use, (default = zeromq). Currently, " + "tcp/TLS and ws/TLS are not supported, since they require the " + "establishment of a certificate infrastructure and use of PKI " + "keys manaaged by that infrastructure." ) parser.add_option( "--start-delay", @@ -174,13 +177,16 @@ def parse(): "-c", "--config-dir", default="", - help="Pass in a configuration directory containing base configuration.", + help="Pass in a configuration directory containing base configuration. " + "If a configuration directory is specified, at a minimum, it must " + "have a master and minion configuration file and these files " + "must not be empty. For example, each could have a user: " + "entry." ) parser.add_option( "--open-mode", dest="open_mode", default=True, - action="store_true", help="Turn off authentication at the Master. Default is True to align " "this version of minionswarm with previous version." ) From 8579bbfc7e3ec6e1e5f1bf24ea82bcf47619c90f Mon Sep 17 00:00:00 2001 From: dnessett Date: Sat, 1 Mar 2025 12:24:11 -0700 Subject: [PATCH 3/6] added changelog file --- changelog/67214.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/67214.fixed.md diff --git a/changelog/67214.fixed.md b/changelog/67214.fixed.md new file mode 100644 index 000000000000..617ac7e9aab8 --- /dev/null +++ b/changelog/67214.fixed.md @@ -0,0 +1 @@ +Fixed bugs in minionswarm and added ability turn off open-mode. From 214cf9354da4bc091d58a29ff85d54056969e234 Mon Sep 17 00:00:00 2001 From: dnessett Date: Sat, 1 Mar 2025 13:24:48 -0700 Subject: [PATCH 4/6] Eliminated some trailing whitespace errors revealed by nox -e lint-tests --- tests/minionswarm.py | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/minionswarm.py b/tests/minionswarm.py index 8549a2e97f19..7857ccb85afe 100644 --- a/tests/minionswarm.py +++ b/tests/minionswarm.py @@ -391,7 +391,7 @@ def mkconf(self, idx): data["grains"]["machine_id"] = hashlib.md5(minion_id_encode).hexdigest() if self.opts["rand_uuid"]: data["grains"]["uuid"] = str(uuid.uuid4()) - + data = self._update_minion_conf(data) with salt.utils.files.fopen(path, "w+") as fp_: @@ -405,12 +405,12 @@ def _update_minion_conf(self, data): # pylint: disable=W0221 respectively, which generally will not exist. Also, specify the extmods directory path and the pki/minion directory path. """ - + cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/minion") sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") extension_modules_dir_path = os.path.join(self.swarm_root, "var/cache/salt/minion/extmods") pki_dir_path = os.path.join(self.swarm_root, "etc/salt/pki/minion") - + try: os.makedirs(cachdir_path) except FileExistsError: @@ -421,7 +421,7 @@ def _update_minion_conf(self, data): # pylint: disable=W0221 except FileExistsError: # directory already exists pass - + data.update( { "cachedir": cachdir_path, @@ -466,7 +466,7 @@ def start_master(self): """ Do the master start.. Run the master as the user under which minionswarm runs. """ - + username = getpass.getuser() cmd = "salt-master '--config-dir={}' --user={} --pid-file {}".format(self.conf, username, f"{self.conf}.pid") if self.opts["foreground"]: @@ -479,7 +479,7 @@ def mkconf(self): # pylint: disable=W0221 """ Make the config file with standard values """ - + data = {} if self.opts["config_dir"]: spath = os.path.join(self.opts["config_dir"], "master") @@ -495,29 +495,29 @@ def mkconf(self): # pylint: disable=W0221 } ) data.update({"open_mode": self.opts["open_mode"]}) - + # TODO Pre-seed keys - + os.makedirs(self.conf) path = os.path.join(self.conf, "master") - + data = self._update_master_conf(data) - + with salt.utils.files.fopen(path, "w+") as fp_: salt.utils.yaml.safe_dump(data, fp_) - + def _update_master_conf(self, data): # pylint: disable=W0221 """ Modify the master config to contain cachedir and sock_dir definitions. Unless cachedir and sock_dir are modified as indicated, alt-master will think they are /var/cache/salt and /var/run/salt/master, respectively, which generally will not exist. """ - + key_logfile_path = os.path.join(self.swarm_root, "var/log/salt/key") cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/master") sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") sqlite_queue_dir_path = os.path.join(self.swarm_root, "var/cache/salt/master") - + try: os.makedirs(cachdir_path) except FileExistsError: @@ -527,8 +527,8 @@ def _update_master_conf(self, data): # pylint: disable=W0221 os.makedirs(sock_dir_path) except FileExistsError: # directory already exists - pass - + pass + data.update( { "key_logfile": key_logfile_path, @@ -537,14 +537,14 @@ def _update_master_conf(self, data): # pylint: disable=W0221 "sqlite_queue_dir" : sqlite_queue_dir_path, } ) - + return data - + def shutdown(self): """ Shutdown master """ - + print("Killing master") subprocess.call('pkill -KILL -f "python.*salt-master"', shell=True) print("Master killed") From 4e58ed6e5c72a0102b230dec58ed5c5c49e4349a Mon Sep 17 00:00:00 2001 From: Danny Nessett Date: Wed, 21 May 2025 19:10:22 -0600 Subject: [PATCH 5/6] Pre-commit black and Pre-commit isort fixed problems with tests/minionswarm.py and tests/support/runtests.py --- tests/minionswarm.py | 80 +++++++++++++++++++++------------------ tests/support/runtests.py | 3 +- 2 files changed, 46 insertions(+), 37 deletions(-) diff --git a/tests/minionswarm.py b/tests/minionswarm.py index 7857ccb85afe..448eedc7b966 100644 --- a/tests/minionswarm.py +++ b/tests/minionswarm.py @@ -6,6 +6,7 @@ """ # pylint: disable=resource-leakage +import getpass import hashlib import optparse # pylint: disable=deprecated-module import os @@ -17,11 +18,11 @@ import tempfile import time import uuid -import getpass + +import support.runtests import salt.utils.files import salt.utils.yaml -import support.runtests OSES = [ "Arch", @@ -40,23 +41,24 @@ "3007.1", ] + def parse(): """ Parse the cli options """ guidance = ( - "\n\n To execute salt commands against minionswarm you must include the configuration\n" - " file using the -c option. For commands such as salt, salt-key, salt-cp,\n" - " and salt-run use -c /master. For example, when using the\n" - " default for --temp-dir, the configuration directory would be\n" - " /tmp/sroot/master. If the master runs on a different machine you\n" - " must execute the command on that machine using the master config file\n" - " there. For the salt-call command, which is a minion side\n" - " command, use -c /-. For example to\n" - " execute salt-call on the first minion using the default values for\n" - " temp-dir and name use -c /tmp/sroot/minion-0. The commands salt-api,\n" - " salt-cloud, salt-extend, salt-master, salt-minion, salt-proxy,\n" - " salt-ssh, salt-syndic and spm are not supported." + "\n\n To execute salt commands against minionswarm you must include the configuration\n" + " file using the -c option. For commands such as salt, salt-key, salt-cp,\n" + " and salt-run use -c /master. For example, when using the\n" + " default for --temp-dir, the configuration directory would be\n" + " /tmp/sroot/master. If the master runs on a different machine you\n" + " must execute the command on that machine using the master config file\n" + " there. For the salt-call command, which is a minion side\n" + " command, use -c /-. For example to\n" + " execute salt-call on the first minion using the default values for\n" + " temp-dir and name use -c /tmp/sroot/minion-0. The commands salt-api,\n" + " salt-cloud, salt-extend, salt-master, salt-minion, salt-proxy,\n" + " salt-ssh, salt-syndic and spm are not supported." ) usage = "usage: python %prog [options]" + guidance parser = optparse.OptionParser(usage) @@ -80,8 +82,8 @@ def parse(): dest="master", default="localhost", help="The location of the salt master that this swarm will serve. (default = localhost) " - "The standard port used by daemon masters is 4506. Masters can be specified using " - ":. For example, 192.168.1.2:4506", + "The standard port used by daemon masters is 4506. Masters can be specified using " + ":. For example, 192.168.1.2:4506", ) parser.add_option( "--name", @@ -89,8 +91,8 @@ def parse(): dest="name", default="minion", help="Give the minions an alternative id prefix, this is used " - "when minions from many systems are being aggregated onto " - "a single master. (default = minion)", + "when minions from many systems are being aggregated onto " + "a single master. (default = minion)", ) parser.add_option( "--rand-os", @@ -146,10 +148,10 @@ def parse(): action="store_true", default=False, help="Don't cleanup temporary files/directories. " - "If specified, you must manually recursively delete " - "the swarm root (see --temp-dir) before running " - "minionswarm again, e.g., using the default swarm " - "root, rm -fr /tmp/srooot", + "If specified, you must manually recursively delete " + "the swarm root (see --temp-dir) before running " + "minionswarm again, e.g., using the default swarm " + "root, rm -fr /tmp/srooot", ) parser.add_option( "--root-dir", @@ -162,9 +164,9 @@ def parse(): dest="transport", default="zeromq", help="Declare which transport to use, (default = zeromq). Currently, " - "tcp/TLS and ws/TLS are not supported, since they require the " - "establishment of a certificate infrastructure and use of PKI " - "keys manaaged by that infrastructure." + "tcp/TLS and ws/TLS are not supported, since they require the " + "establishment of a certificate infrastructure and use of PKI " + "keys manaaged by that infrastructure.", ) parser.add_option( "--start-delay", @@ -178,17 +180,17 @@ def parse(): "--config-dir", default="", help="Pass in a configuration directory containing base configuration. " - "If a configuration directory is specified, at a minimum, it must " - "have a master and minion configuration file and these files " - "must not be empty. For example, each could have a user: " - "entry." + "If a configuration directory is specified, at a minimum, it must " + "have a master and minion configuration file and these files " + "must not be empty. For example, each could have a user: " + "entry.", ) parser.add_option( "--open-mode", dest="open_mode", default=True, help="Turn off authentication at the Master. Default is True to align " - "this version of minionswarm with previous version." + "this version of minionswarm with previous version.", ) parser.add_option("-u", "--user", default=support.runtests.this_user()) @@ -320,7 +322,9 @@ def start_minions(self): self.prep_configs() username = getpass.getuser() for path in self.confs: - cmd = "salt-minion -c {} --user={} --pid-file {}".format(path, username, f"{path}.pid") + cmd = "salt-minion -c {} --user={} --pid-file {}".format( + path, username, f"{path}.pid" + ) if self.opts["foreground"]: cmd += " -l debug &" else: @@ -408,7 +412,9 @@ def _update_minion_conf(self, data): # pylint: disable=W0221 cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/minion") sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") - extension_modules_dir_path = os.path.join(self.swarm_root, "var/cache/salt/minion/extmods") + extension_modules_dir_path = os.path.join( + self.swarm_root, "var/cache/salt/minion/extmods" + ) pki_dir_path = os.path.join(self.swarm_root, "etc/salt/pki/minion") try: @@ -426,7 +432,7 @@ def _update_minion_conf(self, data): # pylint: disable=W0221 { "cachedir": cachdir_path, "sock_dir": sock_dir_path, - "extension_modules" : extension_modules_dir_path, + "extension_modules": extension_modules_dir_path, "pki_dir": pki_dir_path, } ) @@ -465,10 +471,12 @@ def start(self): def start_master(self): """ Do the master start.. Run the master as the user under which minionswarm runs. - """ + """ username = getpass.getuser() - cmd = "salt-master '--config-dir={}' --user={} --pid-file {}".format(self.conf, username, f"{self.conf}.pid") + cmd = "salt-master '--config-dir={}' --user={} --pid-file {}".format( + self.conf, username, f"{self.conf}.pid" + ) if self.opts["foreground"]: cmd += " -l debug &" else: @@ -534,7 +542,7 @@ def _update_master_conf(self, data): # pylint: disable=W0221 "key_logfile": key_logfile_path, "cachedir": cachdir_path, "sock_dir": sock_dir_path, - "sqlite_queue_dir" : sqlite_queue_dir_path, + "sqlite_queue_dir": sqlite_queue_dir_path, } ) diff --git a/tests/support/runtests.py b/tests/support/runtests.py index c816b447fb12..8d739e3ef856 100644 --- a/tests/support/runtests.py +++ b/tests/support/runtests.py @@ -48,9 +48,10 @@ import os import shutil +import support.paths as paths + import salt.utils.path import salt.utils.platform -import support.paths as paths try: import pwd From 2a0b777565940229de6af6da132d47e61c25cb1e Mon Sep 17 00:00:00 2001 From: "Daniel A. Wozniak" Date: Mon, 8 Jun 2026 22:14:14 -0700 Subject: [PATCH 6/6] Fix tests/minionswarm.py import + correctness bugs Address review feedback on #67214: * tests/support/runtests.py: restore canonical package-qualified import. ``import support.paths as paths`` only resolved when the CWD was ``tests/`` and broke ``from tests.support.runtests import RUNTIME_VARS`` -- used by 50+ conftests/tests. Use ``from tests.support import paths`` instead. This was the root cause of the 393 failing Test Salt / Test Package jobs. * tests/minionswarm.py: use ``from tests.support import runtests`` and add a sys.path bootstrap so the script can still be executed directly via ``python tests/minionswarm.py`` (the original ``cd tests/`` reproduction from #67214 also still works). * tests/minionswarm.py: collapse duplicated ``machine_id`` assignment, fix invalid ``sys.exit([1])`` -> ``sys.exit(1)``. * tests/minionswarm.py: master ``sock_dir`` pointed at ``var/run/salt/minion`` instead of ``var/run/salt/master``. * tests/minionswarm.py: ``--open-mode`` was declared with ``default=True`` and no action, so ``--open-mode=False`` parsed to the string ``"False"`` (truthy). Switched to ``--no-open-mode`` (action="store_false") so the flag does what the help text claims. * tests/minionswarm.py: typo fixes (``/tmp/srooot`` -> ``/tmp/sroot``, ``manaaged`` -> ``managed``, ``alt-master`` -> ``salt-master``). --- tests/minionswarm.py | 41 +++++++++++++++++++++++++-------------- tests/support/runtests.py | 3 +-- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/minionswarm.py b/tests/minionswarm.py index 448eedc7b966..c3f6f02793ba 100644 --- a/tests/minionswarm.py +++ b/tests/minionswarm.py @@ -19,10 +19,20 @@ import time import uuid -import support.runtests - -import salt.utils.files -import salt.utils.yaml +# When this module is executed directly (``python tests/minionswarm.py``) +# the parent of ``tests/`` is not on ``sys.path``, which makes the +# ``from tests.support import runtests`` import below fail. Pre-pend the +# repository root so the script remains runnable without ``PYTHONPATH`` +# manipulation. This is a no-op when the module is imported normally +# (e.g. by pytest) because the repository root is already on ``sys.path``. +_REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)) +if _REPO_ROOT not in sys.path: + sys.path.insert(0, _REPO_ROOT) + +import salt # noqa: E402 +import salt.utils.files # noqa: E402 +import salt.utils.yaml # noqa: E402 +from tests.support import runtests # noqa: E402 OSES = [ "Arch", @@ -151,7 +161,7 @@ def parse(): "If specified, you must manually recursively delete " "the swarm root (see --temp-dir) before running " "minionswarm again, e.g., using the default swarm " - "root, rm -fr /tmp/srooot", + "root, rm -fr /tmp/sroot", ) parser.add_option( "--root-dir", @@ -166,7 +176,7 @@ def parse(): help="Declare which transport to use, (default = zeromq). Currently, " "tcp/TLS and ws/TLS are not supported, since they require the " "establishment of a certificate infrastructure and use of PKI " - "keys manaaged by that infrastructure.", + "keys managed by that infrastructure.", ) parser.add_option( "--start-delay", @@ -186,13 +196,15 @@ def parse(): "entry.", ) parser.add_option( - "--open-mode", + "--no-open-mode", + action="store_false", dest="open_mode", default=True, - help="Turn off authentication at the Master. Default is True to align " - "this version of minionswarm with previous version.", + help="Disable open_mode on the Master (require authenticated minion keys). " + "Default leaves open_mode enabled to align this version of minionswarm with " + "the previous version.", ) - parser.add_option("-u", "--user", default=support.runtests.this_user()) + parser.add_option("-u", "--user", default=runtests.this_user()) options, _args = parser.parse_args() @@ -388,10 +400,9 @@ def mkconf(self, idx): if self.opts["rand_machine_id"]: try: minion_id_encode = minion_id.encode(encoding="utf-8", errors="strict") - data["grains"]["machine_id"] = hashlib.md5(minion_id_encode).hexdigest() except UnicodeEncodeError: print("\n'minion id contains illegal character. Shutting down.") - sys.exit([1]) + sys.exit(1) data["grains"]["machine_id"] = hashlib.md5(minion_id_encode).hexdigest() if self.opts["rand_uuid"]: data["grains"]["uuid"] = str(uuid.uuid4()) @@ -405,7 +416,7 @@ def mkconf(self, idx): def _update_minion_conf(self, data): # pylint: disable=W0221 """ Modify the minion config to contain cachedir and sock_dir definitions. Unless cachedir and sock_dir - are modified as indicated, alt-master will think they are /var/cache/salt and /var/run/salt/master, + are modified as indicated, salt-master will think they are /var/cache/salt and /var/run/salt/master, respectively, which generally will not exist. Also, specify the extmods directory path and the pki/minion directory path. """ @@ -517,13 +528,13 @@ def mkconf(self): # pylint: disable=W0221 def _update_master_conf(self, data): # pylint: disable=W0221 """ Modify the master config to contain cachedir and sock_dir definitions. Unless cachedir and sock_dir - are modified as indicated, alt-master will think they are /var/cache/salt and /var/run/salt/master, + are modified as indicated, salt-master will think they are /var/cache/salt and /var/run/salt/master, respectively, which generally will not exist. """ key_logfile_path = os.path.join(self.swarm_root, "var/log/salt/key") cachdir_path = os.path.join(self.swarm_root, "var/cache/salt/master") - sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/minion") + sock_dir_path = os.path.join(self.swarm_root, "var/run/salt/master") sqlite_queue_dir_path = os.path.join(self.swarm_root, "var/cache/salt/master") try: diff --git a/tests/support/runtests.py b/tests/support/runtests.py index 8d739e3ef856..281945f5228c 100644 --- a/tests/support/runtests.py +++ b/tests/support/runtests.py @@ -48,10 +48,9 @@ import os import shutil -import support.paths as paths - import salt.utils.path import salt.utils.platform +from tests.support import paths try: import pwd