From 99dfdd89fb268116d42c213e8eaa23273f97f4b0 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:11:33 -0500 Subject: [PATCH 01/15] Refactor: python: Remove the silent arg to RemoteExec. It's always False. Ref T680 --- python/pacemaker/_cts/remote.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 6c5ab6f561f..e69bc3a8afe 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -101,18 +101,16 @@ class RemoteExec: It runs a command on another machine using ssh and scp. """ - def __init__(self, command, cp_command, silent=False): + def __init__(self, command, cp_command): """ Create a new RemoteExec instance. Arguments: command -- The ssh command string to use for remote execution cp_command -- The scp command string to use for copying files - silent -- Should we log command status? """ self.command = command self.cp_command = cp_command - self._silent = silent self._our_node = os.uname()[1].lower() def _fixcmd(self, cmd): @@ -133,13 +131,11 @@ def _cmd(self, args): def _log(self, args): """Log a message.""" - if not self._silent: - logging.log(args) + logging.log(args) def _debug(self, args): """Log a message at the debug level.""" - if not self._silent: - logging.debug(args) + logging.debug(args) def call_async(self, node, command, delegate=None): """ @@ -180,7 +176,7 @@ def __call__(self, node, command, synchronous=True, verbose=2): proc = Popen(self._cmd([node, command]), stdout=PIPE, stderr=PIPE, close_fds=True, shell=True) - if not synchronous and proc.pid > 0 and not self._silent: + if not synchronous and proc.pid > 0: aproc = AsyncCmd(node, command, proc=proc) aproc.start() return (rc, result) @@ -275,6 +271,5 @@ def getInstance(self): """ if not RemoteFactory.instance: RemoteFactory.instance = RemoteExec(RemoteFactory.command, - RemoteFactory.cp_command, - False) + RemoteFactory.cp_command) return RemoteFactory.instance From 61146da7ab7114e68edffbe69c02402a9634e8b4 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:12:50 -0500 Subject: [PATCH 02/15] Refactor: python: Remove _log and _debug from RemoteExec. There's no need for these functions now that they're just pass throughs. Ref T680 --- python/pacemaker/_cts/remote.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index e69bc3a8afe..ddd05d7e29d 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -129,14 +129,6 @@ def _cmd(self, args): return ret - def _log(self, args): - """Log a message.""" - logging.log(args) - - def _debug(self, args): - """Log a message at the debug level.""" - logging.debug(args) - def call_async(self, node, command, delegate=None): """ Run the given command on the given remote system and do not wait for it to complete. @@ -185,12 +177,12 @@ def __call__(self, node, command, synchronous=True, verbose=2): result = proc.stdout.readlines() proc.stdout.close() else: - self._log("No stdout stream") + logging.log("No stdout stream") rc = proc.wait() if verbose > 0: - self._debug(f"cmd: target={node}, rc={rc}: {command}") + logging.debug(f"cmd: target={node}, rc={rc}: {command}") result = convert2string(result) @@ -199,11 +191,11 @@ def __call__(self, node, command, synchronous=True, verbose=2): proc.stderr.close() for err in errors: - self._debug(f"cmd: stderr: {err}") + logging.debug(f"cmd: stderr: {err}") if verbose == 2: for line in result: - self._debug(f"cmd: stdout: {line}") + logging.debug(f"cmd: stdout: {line}") return (rc, result) @@ -222,7 +214,7 @@ def copy(self, source, target, silent=False): rc = os.system(cmd) if not silent: - self._debug(f"cmd: rc={rc}: {cmd}") + logging.debug(f"cmd: rc={rc}: {cmd}") return rc From ae21bbe17c90dce16df423c100aea0c843921b6f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:15:05 -0500 Subject: [PATCH 03/15] Refactor: python: Remove the silent argument from RemoteExec.copy. It's only ever True in one place, and I don't think I care about that. We can just always log. Ref T680 --- python/pacemaker/_cts/audits.py | 4 ++-- python/pacemaker/_cts/remote.py | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/python/pacemaker/_cts/audits.py b/python/pacemaker/_cts/audits.py index 95165d48d06..14101a2fef2 100644 --- a/python/pacemaker/_cts/audits.py +++ b/python/pacemaker/_cts/audits.py @@ -1,7 +1,7 @@ """Auditing classes for Pacemaker's Cluster Test Suite (CTS).""" __all__ = ["AuditConstraint", "AuditResource", "ClusterAudit", "audit_list"] -__copyright__ = "Copyright 2000-2025 the Pacemaker project contributors" +__copyright__ = "Copyright 2000-2026 the Pacemaker project contributors" __license__ = "GNU General Public License version 2 or later (GPLv2+) WITHOUT ANY WARRANTY" import re @@ -859,7 +859,7 @@ def _store_remote_cib(self, node, target): for line in lines: self._cm.rsh("localhost", f"echo \'{line[:-1]}\' >> {filename}", verbose=0) - if self._cm.rsh.copy(filename, f"root@{target}:{filename}", silent=True) != 0: + if self._cm.rsh.copy(filename, f"root@{target}:{filename}") != 0: self._cm.log("Could not store configuration") return None diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index ddd05d7e29d..df0c5c392b4 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -199,7 +199,7 @@ def __call__(self, node, command, synchronous=True, verbose=2): return (rc, result) - def copy(self, source, target, silent=False): + def copy(self, source, target): """ Perform a copy of the source file to the remote target. @@ -213,9 +213,7 @@ def copy(self, source, target, silent=False): cmd = f"{self.cp_command} '{source}' '{target}'" rc = os.system(cmd) - if not silent: - logging.debug(f"cmd: rc={rc}: {cmd}") - + logging.debug(f"cmd: rc={rc}: {cmd}") return rc def exists_on_all(self, filename, hosts): From 653d9dd09b867ef081683e03721ccb9b3feecb2f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:20:45 -0500 Subject: [PATCH 04/15] Refactor: python: Change how the subprocess module is imported. It defines some fairly common function names like "run" that I want to use, so it makes sense to import the whole module and force us to do "subprocess.popen" instead. --- python/pacemaker/_cts/remote.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index df0c5c392b4..b8f0f16c4ff 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -7,7 +7,7 @@ import re import os -from subprocess import Popen, PIPE +import subprocess from threading import Thread from pacemaker._cts import logging @@ -66,7 +66,9 @@ def run(self): if not self._proc: # pylint: disable=consider-using-with - self._proc = Popen(self._command, stdout=PIPE, stderr=PIPE, close_fds=True, shell=True) + self._proc = subprocess.Popen(self._command, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, close_fds=True, + shell=True) logging.debug(f"cmd: async: target={self._node}, pid={self._proc.pid}: {self._command}") self._proc.wait() @@ -165,8 +167,8 @@ def __call__(self, node, command, synchronous=True, verbose=2): rc = 0 result = None # pylint: disable=consider-using-with - proc = Popen(self._cmd([node, command]), - stdout=PIPE, stderr=PIPE, close_fds=True, shell=True) + proc = subprocess.Popen(self._cmd([node, command]), stdout=subprocess.PIPE, + stderr=subprocess.PIPE, close_fds=True, shell=True) if not synchronous and proc.pid > 0: aproc = AsyncCmd(node, command, proc=proc) From 60ff7d453952e8c8b73725f68e7d4b0c4820f227 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:25:59 -0500 Subject: [PATCH 05/15] Refactor: python: Get rid of the cp_command argument to RemoteExec. This is only ever used in one place. It can just be an argument array instead. And while I'm at it, do what the TODO suggested and use subprocess instead. Ref T680 --- python/pacemaker/_cts/remote.py | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index b8f0f16c4ff..9700b5be38f 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -103,16 +103,14 @@ class RemoteExec: It runs a command on another machine using ssh and scp. """ - def __init__(self, command, cp_command): + def __init__(self, command): """ Create a new RemoteExec instance. Arguments: command -- The ssh command string to use for remote execution - cp_command -- The scp command string to use for copying files """ self.command = command - self.cp_command = cp_command self._our_node = os.uname()[1].lower() def _fixcmd(self, cmd): @@ -205,18 +203,13 @@ def copy(self, source, target): """ Perform a copy of the source file to the remote target. - This function uses the cp_command provided when the RemoteExec object - was created. - - Returns the return code of the cp_command. + Returns the return code of the copy process. """ - # @TODO Use subprocess module with argument array instead - # (self.cp_command should be an array too) - cmd = f"{self.cp_command} '{source}' '{target}'" - rc = os.system(cmd) - - logging.debug(f"cmd: rc={rc}: {cmd}") - return rc + # -B: batch mode, -q: no stats (quiet) + p = subprocess.run(["scp", "-B", "-q", f"'{source}'", f"'{target}'"], + check=False) + logging.debug(f"cmd: rc={p.returncode}: {p.args}") + return p.returncode def exists_on_all(self, filename, hosts): """Return True if specified file exists on all specified hosts.""" @@ -249,9 +242,6 @@ class RemoteFactory: "-o ConnectTimeout=10 -o TCPKeepAlive=yes " "-o ServerAliveCountMax=3 ") - # -B: batch mode, -q: no stats (quiet) - cp_command = "scp -B -q" - instance = None # pylint: disable=invalid-name @@ -262,6 +252,5 @@ def getInstance(self): If no instance exists, create one and then return that. """ if not RemoteFactory.instance: - RemoteFactory.instance = RemoteExec(RemoteFactory.command, - RemoteFactory.cp_command) + RemoteFactory.instance = RemoteExec(RemoteFactory.command) return RemoteFactory.instance From 4523ceaa31ee1d66506c3588079571106e63c47e Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:35:34 -0500 Subject: [PATCH 06/15] Refactor: python: Get rid of the convert2string function. If we pass universal_newlines=True when we create a Popen object, the resulting streams will be text instead of bytes (when we support python >= 3.7, this is also called text=True). Additionally, it's unclear to me that this was actually useful in the first place. We were logging the byte strings before converting them to text. Ref T680 --- python/pacemaker/_cts/remote.py | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 9700b5be38f..370d1c4c8c5 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -13,29 +13,6 @@ from pacemaker._cts import logging -def convert2string(lines): - """ - Convert byte strings to UTF-8 strings. - - Lists of byte strings are converted to a list of UTF-8 strings. All other - text formats are passed through. - """ - if isinstance(lines, bytes): - return lines.decode("utf-8") - - if isinstance(lines, list): - lst = [] - for line in lines: - if isinstance(line, bytes): - line = line.decode("utf-8") - - lst.append(line) - - return lst - - return lines - - class AsyncCmd(Thread): """A class for doing the hard work of running a command on another machine.""" @@ -68,7 +45,7 @@ def run(self): # pylint: disable=consider-using-with self._proc = subprocess.Popen(self._command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, close_fds=True, - shell=True) + shell=True, universal_newlines=True) logging.debug(f"cmd: async: target={self._node}, pid={self._proc.pid}: {self._command}") self._proc.wait() @@ -85,12 +62,9 @@ def run(self): for line in err: logging.debug(f"cmd: stderr[{self._proc.pid}]: {line}") - err = convert2string(err) - if self._proc.stdout: out = self._proc.stdout.readlines() self._proc.stdout.close() - out = convert2string(out) if self._delegate: self._delegate.async_complete(self._proc.pid, self._proc.returncode, out, err) @@ -166,7 +140,8 @@ def __call__(self, node, command, synchronous=True, verbose=2): result = None # pylint: disable=consider-using-with proc = subprocess.Popen(self._cmd([node, command]), stdout=subprocess.PIPE, - stderr=subprocess.PIPE, close_fds=True, shell=True) + stderr=subprocess.PIPE, close_fds=True, shell=True, + universal_newlines=True) if not synchronous and proc.pid > 0: aproc = AsyncCmd(node, command, proc=proc) @@ -184,8 +159,6 @@ def __call__(self, node, command, synchronous=True, verbose=2): if verbose > 0: logging.debug(f"cmd: target={node}, rc={rc}: {command}") - result = convert2string(result) - if proc.stderr: errors = proc.stderr.readlines() proc.stderr.close() From 43cef6cfcbf39ec9a1e45152430b81c795d2c69f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:44:55 -0500 Subject: [PATCH 07/15] Refactor: python: Don't pass a command string to RemoteExec. This is also used in one place, so just define it as a private variable in the class itself. Note that it would really make a lot of sense to also have this be a list of arguments, but in order to do that properly we also need to change everywhere that runs a command to pass a list as well. That's a project for another day. Ref T680 --- python/pacemaker/_cts/remote.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 370d1c4c8c5..991d1033fc7 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -77,14 +77,19 @@ class RemoteExec: It runs a command on another machine using ssh and scp. """ - def __init__(self, command): - """ - Create a new RemoteExec instance. - - Arguments: - command -- The ssh command string to use for remote execution - """ - self.command = command + def __init__(self): + """Create a new RemoteExec instance.""" + + # @TODO This should be an argument list that gets used with subprocess, + # but making that change will require changing everywhere that __call__ + # or call_async pass a command string. + # + # -n: no stdin, -x: no X11, + # -o ServerAliveInterval=5: disconnect after 3*5s if the server + # stops responding + self._command = "ssh -l root -n -x -o ServerAliveInterval=5 " \ + "-o ConnectTimeout=10 -o TCPKeepAlive=yes " \ + "-o ServerAliveCountMax=3" self._our_node = os.uname()[1].lower() def _fixcmd(self, cmd): @@ -99,7 +104,7 @@ def _cmd(self, args): if sysname is None or sysname.lower() in [self._our_node, "localhost"]: ret = command else: - ret = f"{self.command} {sysname} '{self._fixcmd(command)}'" + ret = f"{self._command} {sysname} '{self._fixcmd(command)}'" return ret @@ -208,13 +213,6 @@ class RemoteFactory: # Class variables - # -n: no stdin, -x: no X11, - # -o ServerAliveInterval=5: disconnect after 3*5s if the server - # stops responding - command = ("ssh -l root -n -x -o ServerAliveInterval=5 " - "-o ConnectTimeout=10 -o TCPKeepAlive=yes " - "-o ServerAliveCountMax=3 ") - instance = None # pylint: disable=invalid-name @@ -225,5 +223,5 @@ def getInstance(self): If no instance exists, create one and then return that. """ if not RemoteFactory.instance: - RemoteFactory.instance = RemoteExec(RemoteFactory.command) + RemoteFactory.instance = RemoteExec() return RemoteFactory.instance From 8c4bf00cd84eed8e0a4ed311bbe7ae8506f4b2f8 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 15:58:00 -0500 Subject: [PATCH 08/15] Refactor: python: Remove the synchronous= arg to RemoteExec.__call__. * It completely duplicates what the async_call method does. * There's one caller in LogAudit that was using it totally incorrectly. verbose=0 doesn't matter because that argument was never getting passed to AsyncCmd, so it would log however it wanted regardless. And, rc would always be 0 because we don't wait around for the process to give us a valid return code so the error message would never be logged. Ref T680 --- python/pacemaker/_cts/audits.py | 5 +---- python/pacemaker/_cts/clustermanager.py | 8 ++++---- python/pacemaker/_cts/remote.py | 8 +------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/python/pacemaker/_cts/audits.py b/python/pacemaker/_cts/audits.py index 14101a2fef2..7669b94becb 100644 --- a/python/pacemaker/_cts/audits.py +++ b/python/pacemaker/_cts/audits.py @@ -136,10 +136,7 @@ def _test_logging(self): for node in self._cm.env["nodes"]: cmd = f"logger -p {self._cm.env['syslog_facility']}.info {prefix} {node} {suffix}" - - (rc, _) = self._cm.rsh(node, cmd, synchronous=False, verbose=0) - if rc != 0: - self._cm.log(f"ERROR: Cannot execute remote command [{cmd}] on {node}") + self._cm.rsh.call_async(node, cmd) for k, w in watch.items(): if watch_pref is None: diff --git a/python/pacemaker/_cts/clustermanager.py b/python/pacemaker/_cts/clustermanager.py index eb890d33929..bcb19b9ae4d 100644 --- a/python/pacemaker/_cts/clustermanager.py +++ b/python/pacemaker/_cts/clustermanager.py @@ -323,7 +323,7 @@ def start_cm_async(self, node, verbose=False): log_fn(f"Starting {self.name} on node {node}") self._install_config(node) - self.rsh(node, self.templates["StartCmd"], synchronous=False) + self.rsh.call_async(node, self.templates["StartCmd"]) self.expected_status[node] = "up" def stop_cm(self, node, verbose=False, force=False): @@ -348,7 +348,7 @@ def stop_cm_async(self, node): """Stop the cluster manager on a given node without blocking.""" self.debug(f"Stopping {self.name} on node {node}") - self.rsh(node, self.templates["StopCmd"], synchronous=False) + self.rsh.call_async(node, self.templates["StopCmd"]) self.expected_status[node] = "down" def startall(self, nodelist=None, verbose=False, quick=False): @@ -457,8 +457,8 @@ def unisolate_node(self, target, nodes=None): # Limit the amount of time we have asynchronous connectivity for # Restore both sides as simultaneously as possible - self.rsh(target, self.templates["FixCommCmd"] % node, synchronous=False) - self.rsh(node, self.templates["FixCommCmd"] % target, synchronous=False) + self.rsh.call_async(target, self.templates["FixCommCmd"] % node) + self.rsh.call_async(node, self.templates["FixCommCmd"] % target) self.debug(f"Communication restored between {target} and {node}") def prepare(self): diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 991d1033fc7..b4dafe0d53a 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -124,7 +124,7 @@ def call_async(self, node, command, delegate=None): aproc.start() return aproc - def __call__(self, node, command, synchronous=True, verbose=2): + def __call__(self, node, command, verbose=2): """ Run the given command on the given remote system. @@ -134,7 +134,6 @@ def __call__(self, node, command, synchronous=True, verbose=2): Arguments: node -- The remote machine to run on command -- The command to run, as a string - synchronous -- Should we wait for the command to complete? verbose -- If 0, do not log anything. If 1, log the command and its return code but not its output. If 2, additionally log command output. @@ -148,11 +147,6 @@ def __call__(self, node, command, synchronous=True, verbose=2): stderr=subprocess.PIPE, close_fds=True, shell=True, universal_newlines=True) - if not synchronous and proc.pid > 0: - aproc = AsyncCmd(node, command, proc=proc) - aproc.start() - return (rc, result) - if proc.stdout: result = proc.stdout.readlines() proc.stdout.close() From 8d3597410a6097a2ef5c0df4a1edb0b821ba460c Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 16:05:57 -0500 Subject: [PATCH 09/15] Refactor: python: Get rid of the RemoteFactory class. This doesn't really do anything interesting now. It just returns the singleton instance of RemoteExec, but I don't think there's much savings to be had in doing this instead of just creating a new instance each time. It's not a very complicated class with lots of variables that take time to set up. This also allows us to get rid of all of the pylint not-callable pragmas since that level of indirection has been removed. Ref T680 --- python/pacemaker/_cts/CTS.py | 8 +++---- python/pacemaker/_cts/clustermanager.py | 13 ++---------- python/pacemaker/_cts/environment.py | 13 ++---------- python/pacemaker/_cts/remote.py | 21 +------------------ python/pacemaker/_cts/tests/cibsecret.py | 2 -- python/pacemaker/_cts/tests/componentfail.py | 2 -- python/pacemaker/_cts/tests/ctstest.py | 4 ++-- .../pacemaker/_cts/tests/maintenancemode.py | 8 ------- .../_cts/tests/nearquorumpointtest.py | 2 -- python/pacemaker/_cts/tests/reattach.py | 8 ------- python/pacemaker/_cts/tests/remotedriver.py | 8 ------- .../pacemaker/_cts/tests/resourcerecover.py | 8 ------- python/pacemaker/_cts/tests/resynccib.py | 10 +-------- python/pacemaker/_cts/tests/simulstoplite.py | 2 -- python/pacemaker/_cts/tests/splitbraintest.py | 2 -- python/pacemaker/_cts/tests/stonithdtest.py | 2 -- python/pacemaker/_cts/tests/stoptest.py | 2 -- python/pacemaker/_cts/watcher.py | 6 ++---- 18 files changed, 13 insertions(+), 108 deletions(-) diff --git a/python/pacemaker/_cts/CTS.py b/python/pacemaker/_cts/CTS.py index 607b2e98e2a..01c602d3d13 100644 --- a/python/pacemaker/_cts/CTS.py +++ b/python/pacemaker/_cts/CTS.py @@ -12,7 +12,7 @@ from pacemaker._cts.environment import EnvFactory from pacemaker._cts.input import should_continue from pacemaker._cts import logging -from pacemaker._cts.remote import RemoteFactory +from pacemaker._cts.remote import RemoteExec class CtsLab: @@ -133,14 +133,12 @@ def __init__(self, env): def _node_booted(self, node): """Return True if the given node is booted (responds to pings).""" - # pylint: disable=not-callable - (rc, _) = RemoteFactory().getInstance()("localhost", f"ping -nq -c1 -w1 {node}", verbose=0) + (rc, _) = RemoteExec()("localhost", f"ping -nq -c1 -w1 {node}", verbose=0) return rc == 0 def _sshd_up(self, node): """Return true if sshd responds on the given node.""" - # pylint: disable=not-callable - (rc, _) = RemoteFactory().getInstance()(node, "true", verbose=0) + (rc, _) = RemoteExec()(node, "true", verbose=0) return rc == 0 def wait_for_node(self, node, timeout=300): diff --git a/python/pacemaker/_cts/clustermanager.py b/python/pacemaker/_cts/clustermanager.py index bcb19b9ae4d..10d565d1374 100644 --- a/python/pacemaker/_cts/clustermanager.py +++ b/python/pacemaker/_cts/clustermanager.py @@ -21,18 +21,9 @@ from pacemaker._cts.environment import EnvFactory from pacemaker._cts import logging from pacemaker._cts.patterns import PatternSelector -from pacemaker._cts.remote import RemoteFactory +from pacemaker._cts.remote import RemoteExec from pacemaker._cts.watcher import LogWatcher -# pylint doesn't understand that self._rsh is callable (it stores the -# singleton instance of RemoteExec, as returned by the getInstance method -# of RemoteFactory). -# @TODO See if type annotations fix this. - -# I think we could also fix this by getting rid of the getInstance methods, -# but that's a project for another day. For now, just disable the warning. -# pylint: disable=not-callable - # ClusterManager has a lot of methods. # pylint: disable=too-many-public-methods @@ -64,7 +55,7 @@ def __init__(self): self.ns = NodeStatus(self.env) self.our_node = os.uname()[1].lower() self.partitions_expected = 1 - self.rsh = RemoteFactory().getInstance() + self.rsh = RemoteExec() self.templates = PatternSelector(self.name) self._cib_factory = ConfigFactory(self) diff --git a/python/pacemaker/_cts/environment.py b/python/pacemaker/_cts/environment.py index 629bd71cadd..525a8fb5f15 100644 --- a/python/pacemaker/_cts/environment.py +++ b/python/pacemaker/_cts/environment.py @@ -15,7 +15,7 @@ from pacemaker.buildoptions import BuildOptions from pacemaker._cts import logging -from pacemaker._cts.remote import RemoteFactory +from pacemaker._cts.remote import RemoteExec from pacemaker._cts.watcher import LogKind @@ -26,15 +26,6 @@ class Environment: This consists largely of processing and storing command line parameters. """ - # pylint doesn't understand that self._rsh is callable (it stores the - # singleton instance of RemoteExec, as returned by the getInstance method - # of RemoteFactory). - # @TODO See if type annotations fix this. - - # I think we could also fix this by getting rid of the getInstance methods, - # but that's a project for another day. For now, just disable the warning. - # pylint: disable=not-callable - def __init__(self, args): """ Create a new Environment instance. @@ -67,7 +58,7 @@ def __init__(self, args): self.random_gen = random.Random() - self._rsh = RemoteFactory().getInstance() + self._rsh = RemoteExec() self._parse_args(args) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index b4dafe0d53a..8ffd7888d1a 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -1,6 +1,6 @@ """Remote command runner for Pacemaker's Cluster Test Suite (CTS).""" -__all__ = ["RemoteExec", "RemoteFactory"] +__all__ = ["RemoteExec"] __copyright__ = "Copyright 2014-2026 the Pacemaker project contributors" __license__ = "GNU General Public License version 2 or later (GPLv2+) WITHOUT ANY WARRANTY" @@ -200,22 +200,3 @@ def exists_on_none(self, filename, hosts): return False return True - - -class RemoteFactory: - """A class for constructing a singleton instance of a RemoteExec object.""" - - # Class variables - - instance = None - - # pylint: disable=invalid-name - def getInstance(self): - """ - Return the previously created instance of RemoteExec. - - If no instance exists, create one and then return that. - """ - if not RemoteFactory.instance: - RemoteFactory.instance = RemoteExec() - return RemoteFactory.instance diff --git a/python/pacemaker/_cts/tests/cibsecret.py b/python/pacemaker/_cts/tests/cibsecret.py index f8ceec93aa7..adb0de05267 100644 --- a/python/pacemaker/_cts/tests/cibsecret.py +++ b/python/pacemaker/_cts/tests/cibsecret.py @@ -16,8 +16,6 @@ # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # This comes from include/config.h as private API, assuming pacemaker is built diff --git a/python/pacemaker/_cts/tests/componentfail.py b/python/pacemaker/_cts/tests/componentfail.py index 5a81c602333..4f839bbe9a9 100644 --- a/python/pacemaker/_cts/tests/componentfail.py +++ b/python/pacemaker/_cts/tests/componentfail.py @@ -16,8 +16,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/tests/ctstest.py b/python/pacemaker/_cts/tests/ctstest.py index 331bfaebbac..5d5cf7a55bd 100644 --- a/python/pacemaker/_cts/tests/ctstest.py +++ b/python/pacemaker/_cts/tests/ctstest.py @@ -8,7 +8,7 @@ from pacemaker._cts import logging from pacemaker._cts.environment import EnvFactory -from pacemaker._cts.remote import RemoteFactory +from pacemaker._cts.remote import RemoteExec from pacemaker._cts.timer import Timer from pacemaker._cts.watcher import LogWatcher @@ -49,7 +49,7 @@ def __init__(self, cm): self._cm = cm self._env = EnvFactory().getInstance() - self._rsh = RemoteFactory().getInstance() + self._rsh = RemoteExec() self._timers = {} self.benchmark = True # which tests to benchmark diff --git a/python/pacemaker/_cts/tests/maintenancemode.py b/python/pacemaker/_cts/tests/maintenancemode.py index efc0fa5493f..3b1a6a5b37c 100644 --- a/python/pacemaker/_cts/tests/maintenancemode.py +++ b/python/pacemaker/_cts/tests/maintenancemode.py @@ -13,14 +13,6 @@ from pacemaker._cts.tests.starttest import StartTest from pacemaker._cts.timer import Timer -# Disable various pylint warnings that occur in so many places throughout this -# file it's easiest to just take care of them globally. This does introduce the -# possibility that we'll miss some other cause of the same warning, but we'll -# just have to be careful. - -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable - class MaintenanceMode(CTSTest): """Toggle nodes in and ount of maintenance mode.""" diff --git a/python/pacemaker/_cts/tests/nearquorumpointtest.py b/python/pacemaker/_cts/tests/nearquorumpointtest.py index 7ed6f7e0746..3d48b446961 100644 --- a/python/pacemaker/_cts/tests/nearquorumpointtest.py +++ b/python/pacemaker/_cts/tests/nearquorumpointtest.py @@ -12,8 +12,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/tests/reattach.py b/python/pacemaker/_cts/tests/reattach.py index bf914fca48d..896921e92aa 100644 --- a/python/pacemaker/_cts/tests/reattach.py +++ b/python/pacemaker/_cts/tests/reattach.py @@ -15,14 +15,6 @@ from pacemaker._cts.tests.simulstoplite import SimulStopLite from pacemaker._cts.tests.starttest import StartTest -# Disable various pylint warnings that occur in so many places throughout this -# file it's easiest to just take care of them globally. This does introduce the -# possibility that we'll miss some other cause of the same warning, but we'll -# just have to be careful. - -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable - class Reattach(CTSTest): """Restart the cluster and verify that resources remain running throughout.""" diff --git a/python/pacemaker/_cts/tests/remotedriver.py b/python/pacemaker/_cts/tests/remotedriver.py index 706e41eb9f8..db1a10d6ef1 100644 --- a/python/pacemaker/_cts/tests/remotedriver.py +++ b/python/pacemaker/_cts/tests/remotedriver.py @@ -17,14 +17,6 @@ from pacemaker._cts.tests.stoptest import StopTest from pacemaker._cts.timer import Timer -# Disable various pylint warnings that occur in so many places throughout this -# file it's easiest to just take care of them globally. This does introduce the -# possibility that we'll miss some other cause of the same warning, but we'll -# just have to be careful. - -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable - class RemoteDriver(CTSTest): """ diff --git a/python/pacemaker/_cts/tests/resourcerecover.py b/python/pacemaker/_cts/tests/resourcerecover.py index f70147a8d9a..ffd2cd83d66 100644 --- a/python/pacemaker/_cts/tests/resourcerecover.py +++ b/python/pacemaker/_cts/tests/resourcerecover.py @@ -10,14 +10,6 @@ from pacemaker._cts.tests.starttest import StartTest from pacemaker._cts.timer import Timer -# Disable various pylint warnings that occur in so many places throughout this -# file it's easiest to just take care of them globally. This does introduce the -# possibility that we'll miss some other cause of the same warning, but we'll -# just have to be careful. - -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable - class ResourceRecover(CTSTest): """Fail a random resource.""" diff --git a/python/pacemaker/_cts/tests/resynccib.py b/python/pacemaker/_cts/tests/resynccib.py index 581ffdc4b4b..bac47376d0e 100644 --- a/python/pacemaker/_cts/tests/resynccib.py +++ b/python/pacemaker/_cts/tests/resynccib.py @@ -1,7 +1,7 @@ """Start the cluster without a CIB and verify it gets copied from another node.""" __all__ = ["ResyncCIB"] -__copyright__ = "Copyright 2000-2025 the Pacemaker project contributors" +__copyright__ = "Copyright 2000-2026 the Pacemaker project contributors" __license__ = "GNU General Public License version 2 or later (GPLv2+) WITHOUT ANY WARRANTY" from pacemaker import BuildOptions @@ -10,14 +10,6 @@ from pacemaker._cts.tests.simulstartlite import SimulStartLite from pacemaker._cts.tests.simulstoplite import SimulStopLite -# Disable various pylint warnings that occur in so many places throughout this -# file it's easiest to just take care of them globally. This does introduce the -# possibility that we'll miss some other cause of the same warning, but we'll -# just have to be careful. - -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable - class ResyncCIB(CTSTest): """Start the cluster on a node without a CIB and verify the CIB is copied over later.""" diff --git a/python/pacemaker/_cts/tests/simulstoplite.py b/python/pacemaker/_cts/tests/simulstoplite.py index 840ccaabd51..51a9182c402 100644 --- a/python/pacemaker/_cts/tests/simulstoplite.py +++ b/python/pacemaker/_cts/tests/simulstoplite.py @@ -12,8 +12,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/tests/splitbraintest.py b/python/pacemaker/_cts/tests/splitbraintest.py index e6edb83bd95..33bba3c7bc4 100644 --- a/python/pacemaker/_cts/tests/splitbraintest.py +++ b/python/pacemaker/_cts/tests/splitbraintest.py @@ -17,8 +17,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/tests/stonithdtest.py b/python/pacemaker/_cts/tests/stonithdtest.py index 415d76494f0..e77b2905a61 100644 --- a/python/pacemaker/_cts/tests/stonithdtest.py +++ b/python/pacemaker/_cts/tests/stonithdtest.py @@ -15,8 +15,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/tests/stoptest.py b/python/pacemaker/_cts/tests/stoptest.py index 74f7929459c..04e2f021f1c 100644 --- a/python/pacemaker/_cts/tests/stoptest.py +++ b/python/pacemaker/_cts/tests/stoptest.py @@ -12,8 +12,6 @@ # possibility that we'll miss some other cause of the same warning, but we'll # just have to be careful. -# pylint doesn't understand that self._rsh is callable. -# pylint: disable=not-callable # pylint doesn't understand that self._env is subscriptable. # pylint: disable=unsubscriptable-object diff --git a/python/pacemaker/_cts/watcher.py b/python/pacemaker/_cts/watcher.py index d1f1423b047..91058e10c75 100644 --- a/python/pacemaker/_cts/watcher.py +++ b/python/pacemaker/_cts/watcher.py @@ -12,7 +12,7 @@ from pacemaker.buildoptions import BuildOptions from pacemaker._cts.errors import OutputNotFoundError from pacemaker._cts import logging -from pacemaker._cts.remote import RemoteFactory +from pacemaker._cts.remote import RemoteExec CTS_SUPPORT_BIN = f"{BuildOptions.DAEMON_DIR}/cts-support" @@ -51,7 +51,7 @@ def __init__(self, filename, host=None, name=None): self.limit = None self.name = name self.offset = "EOF" - self.rsh = RemoteFactory().getInstance() + self.rsh = RemoteExec() if host: self.host = host @@ -188,7 +188,6 @@ def set_end(self): cmd = f"{CTS_SUPPORT_BIN} watch -p CTSwatcher: -l 2 -f {self.filename} -o EOF" - # pylint: disable=not-callable (_, lines) = self.rsh(self.host, cmd, verbose=0) for line in lines: @@ -323,7 +322,6 @@ def set_end(self): return # Seconds and nanoseconds since epoch - # pylint: disable=not-callable (rc, lines) = self.rsh(self.host, "date +%s.%N", verbose=0) if rc == 0 and len(lines) == 1: From ff52c2e88c1f967f2ef35b6f6af7cd2a409f76ca Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Thu, 12 Feb 2026 16:19:16 -0500 Subject: [PATCH 10/15] Refactor: python: Rename RemoteExec.__call__ to RemoteExec.call. I don't know if there's any reason to use one over the other, but personally I like the more explicit function name. Ref T680 --- python/pacemaker/_cts/CTS.py | 14 +++--- python/pacemaker/_cts/audits.py | 44 +++++++++---------- python/pacemaker/_cts/cib.py | 10 ++--- python/pacemaker/_cts/cibxml.py | 2 +- python/pacemaker/_cts/clustermanager.py | 38 ++++++++-------- python/pacemaker/_cts/environment.py | 22 +++++----- python/pacemaker/_cts/remote.py | 6 +-- python/pacemaker/_cts/tests/cibsecret.py | 34 +++++++------- python/pacemaker/_cts/tests/componentfail.py | 2 +- .../pacemaker/_cts/tests/maintenancemode.py | 8 ++-- .../_cts/tests/nearquorumpointtest.py | 2 +- python/pacemaker/_cts/tests/reattach.py | 14 +++--- python/pacemaker/_cts/tests/remotedriver.py | 38 ++++++++-------- .../pacemaker/_cts/tests/resourcerecover.py | 8 ++-- python/pacemaker/_cts/tests/resynccib.py | 2 +- python/pacemaker/_cts/tests/simulstoplite.py | 2 +- python/pacemaker/_cts/tests/splitbraintest.py | 4 +- python/pacemaker/_cts/tests/stonithdtest.py | 2 +- python/pacemaker/_cts/tests/stoptest.py | 4 +- python/pacemaker/_cts/watcher.py | 4 +- 20 files changed, 130 insertions(+), 130 deletions(-) diff --git a/python/pacemaker/_cts/CTS.py b/python/pacemaker/_cts/CTS.py index 01c602d3d13..c83b3d705f2 100644 --- a/python/pacemaker/_cts/CTS.py +++ b/python/pacemaker/_cts/CTS.py @@ -60,9 +60,9 @@ def __contains__(self, key): def __getitem__(self, key): """Return the given environment key, or raise KeyError if it does not exist.""" # Throughout this file, pylint has trouble understanding that EnvFactory - # and RemoteFactory are singleton instances that can be treated as callable - # and subscriptable objects. Various warnings are disabled because of this. - # See also a comment about self._rsh in environment.py. + # is a singleton instance that can be treated as a callable and subscriptable + # object. Various warnings are disabled because of this. See also a comment + # about self._rsh in environment.py. # pylint: disable=unsubscriptable-object return self._env[key] @@ -133,12 +133,12 @@ def __init__(self, env): def _node_booted(self, node): """Return True if the given node is booted (responds to pings).""" - (rc, _) = RemoteExec()("localhost", f"ping -nq -c1 -w1 {node}", verbose=0) + (rc, _) = RemoteExec().call("localhost", f"ping -nq -c1 -w1 {node}", verbose=0) return rc == 0 def _sshd_up(self, node): """Return true if sshd responds on the given node.""" - (rc, _) = RemoteExec()(node, "true", verbose=0) + (rc, _) = RemoteExec().call(node, "true", verbose=0) return rc == 0 def wait_for_node(self, node, timeout=300): @@ -223,7 +223,7 @@ def signal(self, sig, node): search_re = f"({word_begin}valgrind )?.*{word_begin}{self.name}{word_end}" if sig in ["SIGKILL", "KILL", 9, "SIGTERM", "TERM", 15]: - (rc, _) = self._cm.rsh(node, f"pgrep --full '{search_re}'") + (rc, _) = self._cm.rsh.call(node, f"pgrep --full '{search_re}'") if rc == 1: # No matching process, so nothing to kill/terminate return @@ -235,6 +235,6 @@ def signal(self, sig, node): # 0: One or more processes were successfully signaled. # 1: No processes matched or none of them could be signalled. # This is why we check for no matching process above. - (rc, _) = self._cm.rsh(node, f"pkill --signal {sig} --full '{search_re}'") + (rc, _) = self._cm.rsh.call(node, f"pkill --signal {sig} --full '{search_re}'") if rc != 0: self._cm.log(f"ERROR: Sending signal {sig} to {self.name} failed on node {node}") diff --git a/python/pacemaker/_cts/audits.py b/python/pacemaker/_cts/audits.py index 7669b94becb..489e073e196 100644 --- a/python/pacemaker/_cts/audits.py +++ b/python/pacemaker/_cts/audits.py @@ -82,16 +82,16 @@ def _restart_cluster_logging(self, nodes=None): for node in nodes: if self._cm.env["have_systemd"]: - (rc, _) = self._cm.rsh(node, "systemctl stop systemd-journald.socket") + (rc, _) = self._cm.rsh.call(node, "systemctl stop systemd-journald.socket") if rc != 0: self._cm.log(f"ERROR: Cannot stop 'systemd-journald' on {node}") - (rc, _) = self._cm.rsh(node, "systemctl start systemd-journald.service") + (rc, _) = self._cm.rsh.call(node, "systemctl start systemd-journald.service") if rc != 0: self._cm.log(f"ERROR: Cannot start 'systemd-journald' on {node}") if "syslogd" in self._cm.env: - (rc, _) = self._cm.rsh(node, f"service {self._cm.env['syslogd']} restart") + (rc, _) = self._cm.rsh.call(node, f"service {self._cm.env['syslogd']} restart") if rc != 0: self._cm.log(f"""ERROR: Cannot restart '{self._cm.env["syslogd"]}' on {node}""") @@ -210,7 +210,7 @@ def __call__(self): self._cm.ns.wait_for_all_nodes(self._cm.env["nodes"]) for node in self._cm.env["nodes"]: - (_, dfout) = self._cm.rsh(node, dfcmd, verbose=1) + (_, dfout) = self._cm.rsh.call(node, dfcmd, verbose=1) if not dfout: self._cm.log(f"ERROR: Cannot execute remote df command [{dfcmd}] on {node}") continue @@ -279,13 +279,13 @@ def _output_has_core(self, output, node): def _find_core_with_coredumpctl(self, node): """Use coredumpctl to find core dumps on the given node.""" - (_, lsout) = self._cm.rsh(node, "coredumpctl --no-legend --no-pager") + (_, lsout) = self._cm.rsh.call(node, "coredumpctl --no-legend --no-pager") return self._output_has_core(lsout, node) def _find_core_on_fs(self, node, paths): """Check for core dumps on the given node, under any of the given paths.""" - (_, lsout) = self._cm.rsh(node, f"ls -al {' '.join(paths)} | grep core.[0-9]", - verbose=1) + (_, lsout) = self._cm.rsh.call(node, f"ls -al {' '.join(paths)} | grep core.[0-9]", + verbose=1) return self._output_has_core(lsout, node) def __call__(self): @@ -317,7 +317,7 @@ def __call__(self): if self._cm.expected_status.get(node) == "down": clean = False - (_, lsout) = self._cm.rsh(node, "ls -al /dev/shm | grep qb-", verbose=1) + (_, lsout) = self._cm.rsh.call(node, "ls -al /dev/shm | grep qb-", verbose=1) for line in lsout: passed = False @@ -325,12 +325,12 @@ def __call__(self): self._cm.log(f"Warning: Stale IPC file on {node}: {line}") if clean: - (_, lsout) = self._cm.rsh(node, "ps axf | grep -e pacemaker -e corosync", verbose=1) + (_, lsout) = self._cm.rsh.call(node, "ps axf | grep -e pacemaker -e corosync", verbose=1) for line in lsout: self._cm.debug(f"ps[{node}]: {line}") - self._cm.rsh(node, "rm -rf /dev/shm/qb-*") + self._cm.rsh.call(node, "rm -rf /dev/shm/qb-*") else: self._cm.debug(f"Skipping {node}") @@ -508,8 +508,8 @@ def _setup(self): self.debug(f"No nodes active - skipping {self.name}") return False - (_, lines) = self._cm.rsh(self._target, "crm_resource --list-cts", - verbose=1) + (_, lines) = self._cm.rsh.call(self._target, "crm_resource --list-cts", + verbose=1) for line in lines: if re.search("^Resource", line): @@ -667,9 +667,9 @@ def __init__(self, cm): def _crm_location(self, resource): """Return a list of cluster nodes where a given resource is running.""" - (rc, lines) = self._cm.rsh(self._target, - f"crm_resource --locate -r {resource} -Q", - verbose=1) + (rc, lines) = self._cm.rsh.call(self._target, + f"crm_resource --locate -r {resource} -Q", + verbose=1) hosts = [] if rc == 0: @@ -820,7 +820,7 @@ def _audit_cib_contents(self, hostlist): passed = False else: - (rc, result) = self._cm.rsh( + (rc, result) = self._cm.rsh.call( node0, f"crm_diff -VV -cf --new {node_xml} --original {node0_xml}", verbose=1) if rc != 0: @@ -847,14 +847,14 @@ def _store_remote_cib(self, node, target): if not target: target = node - (rc, lines) = self._cm.rsh(node, self._cm.templates["CibQuery"], verbose=1) + (rc, lines) = self._cm.rsh.call(node, self._cm.templates["CibQuery"], verbose=1) if rc != 0: self._cm.log("Could not retrieve configuration") return None - self._cm.rsh("localhost", f"rm -f {filename}") + self._cm.rsh.call("localhost", f"rm -f {filename}") for line in lines: - self._cm.rsh("localhost", f"echo \'{line[:-1]}\' >> {filename}", verbose=0) + self._cm.rsh.call("localhost", f"echo \'{line[:-1]}\' >> {filename}", verbose=0) if self._cm.rsh.copy(filename, f"root@{target}:{filename}") != 0: self._cm.log("Could not store configuration") @@ -957,13 +957,13 @@ def _audit_partition(self, partition): # not in itself a reason to fail the audit (not what we're # checking for in this audit) - (_, out) = self._cm.rsh(node, self._cm.templates["StatusCmd"] % node, verbose=1) + (_, out) = self._cm.rsh.call(node, self._cm.templates["StatusCmd"] % node, verbose=1) self._node_state[node] = out[0].strip() - (_, out) = self._cm.rsh(node, self._cm.templates["EpochCmd"], verbose=1) + (_, out) = self._cm.rsh.call(node, self._cm.templates["EpochCmd"], verbose=1) self._node_epoch[node] = out[0].strip() - (_, out) = self._cm.rsh(node, self._cm.templates["QuorumCmd"], verbose=1) + (_, out) = self._cm.rsh.call(node, self._cm.templates["QuorumCmd"], verbose=1) self._node_quorum[node] = out[0].strip() self.debug(f"Node {node}: {self._node_state[node]} - {self._node_epoch[node]} - {self._node_quorum[node]}.") diff --git a/python/pacemaker/_cts/cib.py b/python/pacemaker/_cts/cib.py index 93f03cbf046..7e256847e2c 100644 --- a/python/pacemaker/_cts/cib.py +++ b/python/pacemaker/_cts/cib.py @@ -49,7 +49,7 @@ def __init__(self, cm, version, factory, tmpfile=None): def _show(self): """Query a cluster node for its generated CIB; log and return the result.""" output = "" - (_, result) = self._factory.rsh(self._factory.target, f"HOME=/root CIB_file={self._factory.tmpfile} cibadmin -Q", verbose=1) + (_, result) = self._factory.rsh.call(self._factory.target, f"HOME=/root CIB_file={self._factory.tmpfile} cibadmin -Q", verbose=1) for line in result: output += line @@ -105,7 +105,7 @@ def get_node_id(self, node_name): r"""{gsub(/.*nodeid:\s*/,"");gsub(/\s+.*$/,"");print}' %s""" \ % (node_name, BuildOptions.COROSYNC_CONFIG_FILE) - (rc, output) = self._factory.rsh(self._factory.target, awk, verbose=1) + (rc, output) = self._factory.rsh.call(self._factory.target, awk, verbose=1) if rc == 0 and len(output) == 1: try: @@ -124,7 +124,7 @@ def install(self, target): self._factory.tmpfile = f"{BuildOptions.CIB_DIR}/cib.xml" self.contents(target) - self._factory.rsh(self._factory.target, f"chown {BuildOptions.DAEMON_USER} {self._factory.tmpfile}") + self._factory.rsh.call(self._factory.target, f"chown {BuildOptions.DAEMON_USER} {self._factory.tmpfile}") self._factory.tmpfile = old @@ -138,7 +138,7 @@ def contents(self, target): if target: self._factory.target = target - self._factory.rsh(self._factory.target, f"HOME=/root cibadmin --empty {self.version} > {self._factory.tmpfile}") + self._factory.rsh.call(self._factory.target, f"HOME=/root cibadmin --empty {self.version} > {self._factory.tmpfile}") self._num_nodes = len(self._cm.env["nodes"]) no_quorum = "stop" @@ -300,7 +300,7 @@ def contents(self, target): self._cib = self._show() if self._factory.tmpfile != f"{BuildOptions.CIB_DIR}/cib.xml": - self._factory.rsh(self._factory.target, f"rm -f {self._factory.tmpfile}") + self._factory.rsh.call(self._factory.target, f"rm -f {self._factory.tmpfile}") return self._cib diff --git a/python/pacemaker/_cts/cibxml.py b/python/pacemaker/_cts/cibxml.py index 8c9179deb67..7e4ce0a5171 100644 --- a/python/pacemaker/_cts/cibxml.py +++ b/python/pacemaker/_cts/cibxml.py @@ -149,7 +149,7 @@ def _run(self, operation, xml, section, options=""): fixed = f"HOME=/root CIB_file={self._factory.tmpfile}" fixed += f" cibadmin --{operation} --scope {section} {options} --xml-text '{xml}'" - (rc, _) = self._factory.rsh(self._factory.target, fixed) + (rc, _) = self._factory.rsh.call(self._factory.target, fixed) if rc != 0: raise RuntimeError(f"Configure call failed: {fixed}") diff --git a/python/pacemaker/_cts/clustermanager.py b/python/pacemaker/_cts/clustermanager.py index 10d565d1374..5ba97790c34 100644 --- a/python/pacemaker/_cts/clustermanager.py +++ b/python/pacemaker/_cts/clustermanager.py @@ -101,7 +101,7 @@ def install_support(self, command="install"): This includes various init scripts and data, daemons, fencing agents, etc. """ for node in self.env["nodes"]: - self.rsh(node, f"{BuildOptions.DAEMON_DIR}/cts-support {command}") + self.rsh.call(node, f"{BuildOptions.DAEMON_DIR}/cts-support {command}") def prepare_fencing_watcher(self): """Return a LogWatcher object that watches for fencing log messages.""" @@ -227,7 +227,7 @@ def _install_config(self, node): return self._cib_sync[node] = True - self.rsh(node, f"rm -f {BuildOptions.CIB_DIR}/cib*") + self.rsh.call(node, f"rm -f {BuildOptions.CIB_DIR}/cib*") # Only install the CIB on the first node, all the other ones will pick it up from there if self._cib_installed: @@ -246,7 +246,7 @@ def _install_config(self, node): self.log(f"Installing Generated CIB on node {node}") self._cib.install(node) - self.rsh(node, f"chown {BuildOptions.DAEMON_USER} {BuildOptions.CIB_DIR}/cib.xml") + self.rsh.call(node, f"chown {BuildOptions.DAEMON_USER} {BuildOptions.CIB_DIR}/cib.xml") def start_cm(self, node, verbose=False): """Start up the cluster manager on a given node.""" @@ -284,7 +284,7 @@ def start_cm(self, node, verbose=False): stonith = self.prepare_fencing_watcher() watch.set_watch() - (rc, _) = self.rsh(node, self.templates["StartCmd"]) + (rc, _) = self.rsh.call(node, self.templates["StartCmd"]) if rc != 0: logging.log(f"Warn: Start command failed on node {node}") self.fencing_cleanup(node, stonith) @@ -325,7 +325,7 @@ def stop_cm(self, node, verbose=False, force=False): if self.expected_status[node] != "up" and not force: return True - (rc, _) = self.rsh(node, self.templates["StopCmd"]) + (rc, _) = self.rsh.call(node, self.templates["StopCmd"]) if rc == 0: # Make sure we can continue even if corosync leaks self.expected_status[node] = "down" @@ -428,7 +428,7 @@ def isolate_node(self, target, nodes=None): if node == target: continue - (rc, _) = self.rsh(target, self.templates["BreakCommCmd"] % node) + (rc, _) = self.rsh.call(target, self.templates["BreakCommCmd"] % node) if rc != 0: logging.log(f"Could not break the communication between {target} and {node}: {rc}") return False @@ -494,7 +494,7 @@ def test_node_cm(self, node): self.env["log_kind"], "ClusterIdle") idle_watch.set_watch() - (_, out) = self.rsh(node, self.templates["StatusCmd"] % node, verbose=1) + (_, out) = self.rsh.call(node, self.templates["StatusCmd"] % node, verbose=1) if not out: out = "" @@ -568,7 +568,7 @@ def _partition_stable(self, nodes, timeout=None): for node in nodes.split(): # have each node dump its current state - self.rsh(node, self.templates["StatusCmd"] % node, verbose=1) + self.rsh.call(node, self.templates["StatusCmd"] % node, verbose=1) ret = idle_watch.look() @@ -612,7 +612,7 @@ def is_node_dc(self, node, status_line=None): Check the given status_line, or query the cluster if None. """ if not status_line: - (_, out) = self.rsh(node, self.templates["StatusCmd"] % node, verbose=1) + (_, out) = self.rsh.call(node, self.templates["StatusCmd"] % node, verbose=1) if out: status_line = out[0].strip() @@ -639,7 +639,7 @@ def is_node_dc(self, node, status_line=None): def active_resources(self, node): """Return a list of primitive resources active on the given node.""" - (_, output) = self.rsh(node, "crm_resource -c", verbose=1) + (_, output) = self.rsh.call(node, "crm_resource -c", verbose=1) resources = [] for line in output: if not re.search("^Resource", line): @@ -659,7 +659,7 @@ def resource_location(self, rid): continue cmd = self.templates["RscRunning"] % rid - (rc, lines) = self.rsh(node, cmd) + (rc, lines) = self.rsh.call(node, cmd) if rc == 127: self.log(f"Command '{cmd}' failed. Binary or pacemaker-cts package not installed?") @@ -685,7 +685,7 @@ def find_partitions(self): self.debug(f"Node {node} is down... skipping") continue - (_, out) = self.rsh(node, self.templates["PartitionCmd"], verbose=1) + (_, out) = self.rsh.call(node, self.templates["PartitionCmd"], verbose=1) if not out: self.log(f"no partition details for {node}") @@ -728,7 +728,7 @@ def has_quorum(self, node_list): if self.expected_status[node] != "up": continue - (rc, quorum) = self.rsh(node, self.templates["QuorumCmd"], verbose=1) + (rc, quorum) = self.rsh.call(node, self.templates["QuorumCmd"], verbose=1) if rc != ExitStatus.OK: self.debug(f"WARN: Quorum check on {node} returned error ({rc})") continue @@ -753,7 +753,7 @@ def components(self): def in_standby_mode(self, node): """Return whether or not the node is in Standby.""" - (_, out) = self.rsh(node, self.templates["StandbyQueryCmd"] % node, verbose=1) + (_, out) = self.rsh.call(node, self.templates["StandbyQueryCmd"] % node, verbose=1) if not out: return False @@ -778,7 +778,7 @@ def set_standby_mode(self, node, status): else: cmd = self.templates["StandbyCmd"] % (node, "off") - (rc, _) = self.rsh(node, cmd) + (rc, _) = self.rsh.call(node, cmd) return rc == 0 def add_dummy_rsc(self, node, rid): @@ -795,13 +795,13 @@ def add_dummy_rsc(self, node, rid): '""" - self.rsh(node, self.templates['CibAddXml'] % rsc_xml) - self.rsh(node, self.templates['CibAddXml'] % constraint_xml) + self.rsh.call(node, self.templates['CibAddXml'] % rsc_xml) + self.rsh.call(node, self.templates['CibAddXml'] % constraint_xml) def remove_dummy_rsc(self, node, rid): """Remove the previously added dummy resource given by rid on the given node.""" constraint = f"\"//rsc_location[@rsc='{rid}']\"" rsc = f"\"//primitive[@id='{rid}']\"" - self.rsh(node, self.templates['CibDelXpath'] % constraint) - self.rsh(node, self.templates['CibDelXpath'] % rsc) + self.rsh.call(node, self.templates['CibDelXpath'] % constraint) + self.rsh.call(node, self.templates['CibDelXpath'] % rsc) diff --git a/python/pacemaker/_cts/environment.py b/python/pacemaker/_cts/environment.py index 525a8fb5f15..4dffc2cdeb6 100644 --- a/python/pacemaker/_cts/environment.py +++ b/python/pacemaker/_cts/environment.py @@ -108,7 +108,7 @@ def random_node(self): def _detect_systemd(self, node): """Detect whether systemd is in use on the target node.""" if "have_systemd" not in self.data: - (rc, _) = self._rsh(node, "systemctl list-units", verbose=0) + (rc, _) = self._rsh.call(node, "systemctl list-units", verbose=0) self["have_systemd"] = rc == 0 def _detect_syslog(self, node): @@ -118,10 +118,10 @@ def _detect_syslog(self, node): if self["have_systemd"]: # Systemd - (_, lines) = self._rsh(node, r"systemctl list-units | grep syslog.*\.service.*active.*running | sed 's:.service.*::'", verbose=1) + (_, lines) = self._rsh.call(node, r"systemctl list-units | grep syslog.*\.service.*active.*running | sed 's:.service.*::'", verbose=1) else: # SYS-V - (_, lines) = self._rsh(node, "chkconfig --list | grep syslog.*on | awk '{print $1}' | head -n 1", verbose=1) + (_, lines) = self._rsh.call(node, "chkconfig --list | grep syslog.*on | awk '{print $1}' | head -n 1", verbose=1) with suppress(IndexError): self["syslogd"] = lines[0].strip() @@ -130,22 +130,22 @@ def disable_service(self, node, service): """Disable the given service on the given node.""" if self["have_systemd"]: # Systemd - (rc, _) = self._rsh(node, f"systemctl disable {service}") + (rc, _) = self._rsh.call(node, f"systemctl disable {service}") return rc # SYS-V - (rc, _) = self._rsh(node, f"chkconfig {service} off") + (rc, _) = self._rsh.call(node, f"chkconfig {service} off") return rc def enable_service(self, node, service): """Enable the given service on the given node.""" if self["have_systemd"]: # Systemd - (rc, _) = self._rsh(node, f"systemctl enable {service}") + (rc, _) = self._rsh.call(node, f"systemctl enable {service}") return rc # SYS-V - (rc, _) = self._rsh(node, f"chkconfig {service} on") + (rc, _) = self._rsh.call(node, f"chkconfig {service} on") return rc def service_is_enabled(self, node, service): @@ -157,11 +157,11 @@ def service_is_enabled(self, node, service): # explicitly "enabled" instead of the return code. For example it returns # 0 if the service is "static" or "indirect", but they don't really count # as "enabled". - (rc, _) = self._rsh(node, f"systemctl is-enabled {service} | grep enabled") + (rc, _) = self._rsh.call(node, f"systemctl is-enabled {service} | grep enabled") return rc == 0 # SYS-V - (rc, _) = self._rsh(node, f"chkconfig --list | grep -e {service}.*on") + (rc, _) = self._rsh.call(node, f"chkconfig --list | grep -e {service}.*on") return rc == 0 def _detect_at_boot(self, node): @@ -172,10 +172,10 @@ def _detect_at_boot(self, node): def _detect_ip_offset(self, node): """Detect the offset for IPaddr resources.""" if self["create_resources"] and "IPBase" not in self.data: - (_, lines) = self._rsh(node, "ip addr | grep inet | grep -v -e link -e inet6 -e '/32' -e ' lo' | awk '{print $2}'", verbose=0) + (_, lines) = self._rsh.call(node, "ip addr | grep inet | grep -v -e link -e inet6 -e '/32' -e ' lo' | awk '{print $2}'", verbose=0) network = lines[0].strip() - (_, lines) = self._rsh(node, "nmap -sn -n %s | grep 'scan report' | awk '{print $NF}' | sed 's:(::' | sed 's:)::' | sort -V | tail -n 1" % network, verbose=0) + (_, lines) = self._rsh.call(node, "nmap -sn -n %s | grep 'scan report' | awk '{print $NF}' | sed 's:(::' | sed 's:)::' | sort -V | tail -n 1" % network, verbose=0) try: self["IPBase"] = lines[0].strip() diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 8ffd7888d1a..921d30e6963 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -124,7 +124,7 @@ def call_async(self, node, command, delegate=None): aproc.start() return aproc - def __call__(self, node, command, verbose=2): + def call(self, node, command, verbose=2): """ Run the given command on the given remote system. @@ -186,7 +186,7 @@ def copy(self, source, target): def exists_on_all(self, filename, hosts): """Return True if specified file exists on all specified hosts.""" for host in hosts: - (rc, _) = self(host, f"test -r {filename}") + (rc, _) = self.call(host, f"test -r {filename}") if rc != 0: return False @@ -195,7 +195,7 @@ def exists_on_all(self, filename, hosts): def exists_on_none(self, filename, hosts): """Return True if specified file does not exist on any specified host.""" for host in hosts: - (rc, _) = self(host, f"test -r {filename}") + (rc, _) = self.call(host, f"test -r {filename}") if rc == 0: return False diff --git a/python/pacemaker/_cts/tests/cibsecret.py b/python/pacemaker/_cts/tests/cibsecret.py index adb0de05267..62f47c63cb4 100644 --- a/python/pacemaker/_cts/tests/cibsecret.py +++ b/python/pacemaker/_cts/tests/cibsecret.py @@ -85,8 +85,8 @@ def _remove_dummy(self, node): def _check_cib_value(self, node, expected): """Check that the secret has the expected value.""" - (rc, lines) = self._rsh(node, f"crm_resource -r {self._rid} -g {self._secret}", - verbose=1) + (rc, lines) = self._rsh.call(node, f"crm_resource -r {self._rid} -g {self._secret}", + verbose=1) s = " ".join(lines).strip() if rc != 0 or s != expected: @@ -97,8 +97,8 @@ def _check_cib_value(self, node, expected): def _test_check(self, node): """Test the 'cibsecret check' subcommand.""" - (rc, _) = self._rsh(node, f"cibsecret check {self._rid} {self._secret}", - verbose=1) + (rc, _) = self._rsh.call(node, f"cibsecret check {self._rid} {self._secret}", + verbose=1) if rc != 0: return self.failure("Failed to check secret") @@ -107,8 +107,8 @@ def _test_check(self, node): def _test_delete(self, node): """Test the 'cibsecret delete' subcommand.""" - (rc, _) = self._rsh(node, f"cibsecret delete {self._rid} {self._secret}", - verbose=1) + (rc, _) = self._rsh.call(node, f"cibsecret delete {self._rid} {self._secret}", + verbose=1) if rc != 0: return self.failure("Failed to delete secret") @@ -117,8 +117,8 @@ def _test_delete(self, node): def _test_get(self, node, expected): """Test the 'cibsecret get' subcommand.""" - (rc, lines) = self._rsh(node, f"cibsecret get {self._rid} {self._secret}", - verbose=1) + (rc, lines) = self._rsh.call(node, f"cibsecret get {self._rid} {self._secret}", + verbose=1) s = " ".join(lines).strip() if rc != 0 or s != expected: @@ -129,8 +129,8 @@ def _test_get(self, node, expected): def _test_set(self, node): """Test the 'cibsecret set' subcommand.""" - (rc, _) = self._rsh(node, f"cibsecret set {self._rid} {self._secret} {self._secret_val}", - verbose=1) + (rc, _) = self._rsh.call(node, f"cibsecret set {self._rid} {self._secret} {self._secret_val}", + verbose=1) if rc != 0: return self.failure("Failed to set secret") @@ -139,8 +139,8 @@ def _test_set(self, node): def _test_stash(self, node): """Test the 'cibsecret stash' subcommand.""" - (rc, _) = self._rsh(node, f"cibsecret stash {self._rid} {self._secret}", - verbose=1) + (rc, _) = self._rsh.call(node, f"cibsecret stash {self._rid} {self._secret}", + verbose=1) if rc != 0: return self.failure(f"Failed to stash secret {self._secret}") @@ -149,7 +149,7 @@ def _test_stash(self, node): def _test_sync(self, node): """Test the 'cibsecret sync' subcommand.""" - (rc, _) = self._rsh(node, "cibsecret sync", verbose=1) + (rc, _) = self._rsh.call(node, "cibsecret sync", verbose=1) if rc != 0: return self.failure("Failed to sync secrets") @@ -158,8 +158,8 @@ def _test_sync(self, node): def _test_unstash(self, node): """Test the 'cibsecret unstash' subcommand.""" - (rc, _) = self._rsh(node, f"cibsecret unstash {self._rid} {self._secret}", - verbose=1) + (rc, _) = self._rsh.call(node, f"cibsecret unstash {self._rid} {self._secret}", + verbose=1) if rc != 0: return self.failure(f"Failed to unstash secret {self._secret}") @@ -261,8 +261,8 @@ def is_applicable(self): other = self._cm.env["nodes"][1:] for o in other: - (rc, _) = self._cm.rsh(node, f"{self._cm.rsh.command} {o} exit", - verbose=0) + (rc, _) = self._cm.rsh.call(node, f"{self._cm.rsh.command} {o} exit", + verbose=0) if rc != ExitStatus.OK: return False diff --git a/python/pacemaker/_cts/tests/componentfail.py b/python/pacemaker/_cts/tests/componentfail.py index 4f839bbe9a9..fb7f62eb9a1 100644 --- a/python/pacemaker/_cts/tests/componentfail.py +++ b/python/pacemaker/_cts/tests/componentfail.py @@ -80,7 +80,7 @@ def __call__(self, node): self._okerrpatterns = [ self._cm.templates["Pat:Resource_active"], ] - (_, lines) = self._rsh(node, "crm_resource -c", verbose=1) + (_, lines) = self._rsh.call(node, "crm_resource -c", verbose=1) for line in lines: if re.search("^Resource", line): diff --git a/python/pacemaker/_cts/tests/maintenancemode.py b/python/pacemaker/_cts/tests/maintenancemode.py index 3b1a6a5b37c..16ede1b9089 100644 --- a/python/pacemaker/_cts/tests/maintenancemode.py +++ b/python/pacemaker/_cts/tests/maintenancemode.py @@ -59,10 +59,10 @@ def _toggle_maintenance_mode(self, node, enabled): watch.set_watch() self.debug(f"Turning maintenance mode {action}") - self._rsh(node, self._cm.templates[f"MaintenanceMode{action}"]) + self._rsh.call(node, self._cm.templates[f"MaintenanceMode{action}"]) if enabled: - self._rsh(node, f"crm_resource -V -F -r {self._rid} -H {node} &>/dev/null") + self._rsh.call(node, f"crm_resource -V -F -r {self._rid} -H {node} &>/dev/null") with Timer(self.name, f"recover{action}"): watch.look_for_all() @@ -115,7 +115,7 @@ def _remove_maintenance_dummy(self, node): def _managed_rscs(self, node): """Return a list of all resources managed by the cluster.""" rscs = [] - (_, lines) = self._rsh(node, "crm_resource -c", verbose=1) + (_, lines) = self._rsh.call(node, "crm_resource -c", verbose=1) for line in lines: if re.search("^Resource", line): @@ -134,7 +134,7 @@ def _verify_resources(self, node, rscs, managed): if not managed: managed_str = "unmanaged" - (_, lines) = self._rsh(node, "crm_resource -c", verbose=1) + (_, lines) = self._rsh.call(node, "crm_resource -c", verbose=1) for line in lines: if re.search("^Resource", line): tmp = AuditResource(self._cm, line) diff --git a/python/pacemaker/_cts/tests/nearquorumpointtest.py b/python/pacemaker/_cts/tests/nearquorumpointtest.py index 3d48b446961..58e46039e4a 100644 --- a/python/pacemaker/_cts/tests/nearquorumpointtest.py +++ b/python/pacemaker/_cts/tests/nearquorumpointtest.py @@ -107,7 +107,7 @@ def __call__(self, dummy): # Make sure they're completely down with no residule for node in stopset: - self._rsh(node, self._cm.templates["StopCmd"]) + self._rsh.call(node, self._cm.templates["StopCmd"]) return self.success() diff --git a/python/pacemaker/_cts/tests/reattach.py b/python/pacemaker/_cts/tests/reattach.py index 896921e92aa..9a3cbce7bf5 100644 --- a/python/pacemaker/_cts/tests/reattach.py +++ b/python/pacemaker/_cts/tests/reattach.py @@ -35,19 +35,19 @@ def __init__(self, cm): def _is_managed(self, node): """Return whether resources are managed by the cluster.""" - (_, is_managed) = self._rsh(node, "crm_attribute -t rsc_defaults -n is-managed -q -G -d true", verbose=1) + (_, is_managed) = self._rsh.call(node, "crm_attribute -t rsc_defaults -n is-managed -q -G -d true", verbose=1) is_managed = is_managed[0].strip() return is_managed == "true" def _set_unmanaged(self, node): """Disable resource management.""" self.debug("Disable resource management") - self._rsh(node, "crm_attribute -t rsc_defaults -n is-managed -v false") + self._rsh.call(node, "crm_attribute -t rsc_defaults -n is-managed -v false") def _set_managed(self, node): """Enable resource management.""" self.debug("Re-enable resource management") - self._rsh(node, "crm_attribute -t rsc_defaults -n is-managed -D") + self._rsh.call(node, "crm_attribute -t rsc_defaults -n is-managed -D") def _disable_incompatible_rscs(self, node): """ @@ -67,13 +67,13 @@ def _disable_incompatible_rscs(self, node): ' --scope rsc_defaults""" - return self._rsh(node, self._cm.templates['CibAddXml'] % xml) + return self._rsh.call(node, self._cm.templates['CibAddXml'] % xml) def _enable_incompatible_rscs(self, node): """Re-enable resources that were incompatible with this test.""" self.debug("Re-enable incompatible resources") xml = """""" - return self._rsh(node, f"""cibadmin --delete --xml-text '{xml}'""") + return self._rsh.call(node, f"""cibadmin --delete --xml-text '{xml}'""") def _reprobe(self, node): """ @@ -84,7 +84,7 @@ def _reprobe(self, node): rules. An earlier test may have erased the relevant node attribute, so do a reprobe, which should add the attribute back. """ - return self._rsh(node, """crm_resource --refresh""") + return self._rsh.call(node, """crm_resource --refresh""") def setup(self, node): """Set up this test.""" @@ -178,7 +178,7 @@ def __call__(self, node): # Ignore actions for STONITH resources ignore = [] - (_, lines) = self._rsh(node, "crm_resource -c", verbose=1) + (_, lines) = self._rsh.call(node, "crm_resource -c", verbose=1) for line in lines: if re.search("^Resource", line): r = AuditResource(self._cm, line) diff --git a/python/pacemaker/_cts/tests/remotedriver.py b/python/pacemaker/_cts/tests/remotedriver.py index db1a10d6ef1..106cabf38e6 100644 --- a/python/pacemaker/_cts/tests/remotedriver.py +++ b/python/pacemaker/_cts/tests/remotedriver.py @@ -92,7 +92,7 @@ def _del_rsc(self, node, rsc): delete command. """ othernode = self._get_other_node(node) - (rc, _) = self._rsh(othernode, f"crm_resource -D -r {rsc} -t primitive") + (rc, _) = self._rsh.call(othernode, f"crm_resource -D -r {rsc} -t primitive") if rc != 0: self.fail(f"Removal of resource '{rsc}' failed") @@ -104,7 +104,7 @@ def _add_rsc(self, node, rsc_xml): add command. """ othernode = self._get_other_node(node) - (rc, _) = self._rsh(othernode, f"cibadmin -C -o resources -X '{rsc_xml}'") + (rc, _) = self._rsh.call(othernode, f"cibadmin -C -o resources -X '{rsc_xml}'") if rc != 0: self.fail("resource creation failed") @@ -180,7 +180,7 @@ def _enable_services(self, node): def _stop_pcmk_remote(self, node): """Stop the Pacemaker Remote service on the given node.""" for _ in range(10): - (rc, _) = self._rsh(node, "service pacemaker_remote stop") + (rc, _) = self._rsh.call(node, "service pacemaker_remote stop") if rc != 0: time.sleep(6) else: @@ -189,7 +189,7 @@ def _stop_pcmk_remote(self, node): def _start_pcmk_remote(self, node): """Start the Pacemaker Remote service on the given node.""" for _ in range(10): - (rc, _) = self._rsh(node, "service pacemaker_remote start") + (rc, _) = self._rsh.call(node, "service pacemaker_remote start") if rc != 0: time.sleep(6) else: @@ -221,8 +221,8 @@ def _start_metal(self, node): self._disable_services(node) # make sure the resource doesn't already exist for some reason - self._rsh(node, f"crm_resource -D -r {self._remote_rsc} -t primitive") - self._rsh(node, f"crm_resource -D -r {self._remote_node} -t primitive") + self._rsh.call(node, f"crm_resource -D -r {self._remote_rsc} -t primitive") + self._rsh.call(node, f"crm_resource -D -r {self._remote_node} -t primitive") if not self._stop(node): self.fail(f"Failed to shutdown cluster node {node}") @@ -266,7 +266,7 @@ def migrate_connection(self, node): watch = self.create_watch(pats, 120) watch.set_watch() - (rc, _) = self._rsh(node, f"crm_resource -M -r {self._remote_node}", verbose=1) + (rc, _) = self._rsh.call(node, f"crm_resource -M -r {self._remote_node}", verbose=1) if rc != 0: self.fail("failed to move remote node connection resource") return @@ -297,7 +297,7 @@ def fail_rsc(self, node): self.debug("causing dummy rsc to fail.") - self._rsh(node, "rm -f /var/run/resource-agents/Dummy*") + self._rsh.call(node, "rm -f /var/run/resource-agents/Dummy*") with Timer(self.name, "remoteRscFail"): watch.look_for_all() @@ -382,7 +382,7 @@ def _add_dummy_rsc(self, node): self._add_primitive_rsc(node) # force that rsc to prefer the remote node. - (rc, _) = self._cm.rsh(node, f"crm_resource -M -r {self._remote_rsc} -N {self._remote_node} -f", verbose=1) + (rc, _) = self._cm.rsh.call(node, f"crm_resource -M -r {self._remote_rsc} -N {self._remote_node} -f", verbose=1) if rc != 0: self.fail("Failed to place remote resource on remote node.") return @@ -400,17 +400,17 @@ def test_attributes(self, node): # This verifies permanent attributes can be set on a remote-node. It also # verifies the remote-node can edit its own cib node section remotely. - (rc, line) = self._cm.rsh(node, f"crm_attribute -l forever -n testattr -v testval -N {self._remote_node}", verbose=1) + (rc, line) = self._cm.rsh.call(node, f"crm_attribute -l forever -n testattr -v testval -N {self._remote_node}", verbose=1) if rc != 0: self.fail(f"Failed to set remote-node attribute. rc:{rc} output:{line}") return - (rc, _) = self._cm.rsh(node, f"crm_attribute -l forever -n testattr -q -N {self._remote_node}", verbose=1) + (rc, _) = self._cm.rsh.call(node, f"crm_attribute -l forever -n testattr -q -N {self._remote_node}", verbose=1) if rc != 0: self.fail("Failed to get remote-node attribute") return - (rc, _) = self._cm.rsh(node, f"crm_attribute -l forever -n testattr -D -N {self._remote_node}", verbose=1) + (rc, _) = self._cm.rsh.call(node, f"crm_attribute -l forever -n testattr -D -N {self._remote_node}", verbose=1) if rc != 0: self.fail("Failed to delete remote-node attribute") @@ -443,13 +443,13 @@ def cleanup_metal(self, node): if self._remote_rsc_added: # Remove dummy resource added for remote node tests self.debug("Cleaning up dummy rsc put on remote node") - self._rsh(self._get_other_node(node), f"crm_resource -U -r {self._remote_rsc}") + self._rsh.call(self._get_other_node(node), f"crm_resource -U -r {self._remote_rsc}") self._del_rsc(node, self._remote_rsc) if self._remote_node_added: # Remove remote node's connection resource self.debug("Cleaning up remote node connection resource") - self._rsh(self._get_other_node(node), f"crm_resource -U -r {self._remote_node}") + self._rsh.call(self._get_other_node(node), f"crm_resource -U -r {self._remote_node}") self._del_rsc(node, self._remote_node) watch.look_for_all() @@ -465,7 +465,7 @@ def cleanup_metal(self, node): if self._remote_node_added: # Remove remote node itself self.debug("Cleaning up node entry for remote node") - self._rsh(self._get_other_node(node), f"crm_node --force --remove {self._remote_node}") + self._rsh.call(self._get_other_node(node), f"crm_node --force --remove {self._remote_node}") def _setup_env(self, node): """ @@ -489,10 +489,10 @@ def _setup_env(self, node): # sync key throughout the cluster for n in self._env["nodes"]: - self._rsh(n, "mkdir -p --mode=0750 /etc/pacemaker") + self._rsh.call(n, "mkdir -p --mode=0750 /etc/pacemaker") self._rsh.copy(keyfile, f"root@{n}:/etc/pacemaker/authkey") - self._rsh(n, "chgrp haclient /etc/pacemaker /etc/pacemaker/authkey") - self._rsh(n, "chmod 0640 /etc/pacemaker/authkey") + self._rsh.call(n, "chgrp haclient /etc/pacemaker /etc/pacemaker/authkey") + self._rsh.call(n, "chmod 0640 /etc/pacemaker/authkey") os.unlink(keyfile) @@ -502,7 +502,7 @@ def is_applicable(self): return False for node in self._env["nodes"]: - (rc, _) = self._rsh(node, "which pacemaker-remoted >/dev/null 2>&1") + (rc, _) = self._rsh.call(node, "which pacemaker-remoted >/dev/null 2>&1") if rc != 0: return False diff --git a/python/pacemaker/_cts/tests/resourcerecover.py b/python/pacemaker/_cts/tests/resourcerecover.py index ffd2cd83d66..9ac87d63573 100644 --- a/python/pacemaker/_cts/tests/resourcerecover.py +++ b/python/pacemaker/_cts/tests/resourcerecover.py @@ -84,7 +84,7 @@ def _choose_resource(self, node, resourcelist): """Choose a random resource to target.""" self._rid = self._env.random_gen.choice(resourcelist) self._rid_alt = self._rid - (_, lines) = self._rsh(node, "crm_resource -c", verbose=1) + (_, lines) = self._rsh.call(node, "crm_resource -c", verbose=1) for line in lines: if line.startswith("Resource: "): @@ -100,8 +100,8 @@ def _choose_resource(self, node, resourcelist): def _get_failcount(self, node): """Check the fail count of targeted resource on given node.""" cmd = "crm_failcount --quiet --query --resource %s --operation %s --interval %d --node %s" - (rc, lines) = self._rsh(node, cmd % (self._rid, self._action, self._interval, node), - verbose=1) + (rc, lines) = self._rsh.call(node, cmd % (self._rid, self._action, self._interval, node), + verbose=1) if rc != 0 or len(lines) != 1: lines = [line.strip() for line in lines] @@ -125,7 +125,7 @@ def _fail_resource(self, rsc, node, pats): watch = self.create_watch(pats, 60) watch.set_watch() - self._rsh(node, f"crm_resource -V -F -r {self._rid} -H {node} &>/dev/null") + self._rsh.call(node, f"crm_resource -V -F -r {self._rid} -H {node} &>/dev/null") with Timer(self.name, "recover"): watch.look_for_all() diff --git a/python/pacemaker/_cts/tests/resynccib.py b/python/pacemaker/_cts/tests/resynccib.py index bac47376d0e..61e75a95cef 100644 --- a/python/pacemaker/_cts/tests/resynccib.py +++ b/python/pacemaker/_cts/tests/resynccib.py @@ -38,7 +38,7 @@ def __call__(self, node): return self.failure("Could not stop all nodes") # Test config recovery when the other nodes come up - self._rsh(node, f"rm -f {BuildOptions.CIB_DIR}/cib*") + self._rsh.call(node, f"rm -f {BuildOptions.CIB_DIR}/cib*") # Start the selected node if not self._restart1(node): diff --git a/python/pacemaker/_cts/tests/simulstoplite.py b/python/pacemaker/_cts/tests/simulstoplite.py index 51a9182c402..71364430dac 100644 --- a/python/pacemaker/_cts/tests/simulstoplite.py +++ b/python/pacemaker/_cts/tests/simulstoplite.py @@ -63,7 +63,7 @@ def __call__(self, dummy): if watch.look_for_all(): # Make sure they're completely down with no residule for node in self._env["nodes"]: - self._rsh(node, self._cm.templates["StopCmd"]) + self._rsh.call(node, self._cm.templates["StopCmd"]) return self.success() diff --git a/python/pacemaker/_cts/tests/splitbraintest.py b/python/pacemaker/_cts/tests/splitbraintest.py index 33bba3c7bc4..008839bab78 100644 --- a/python/pacemaker/_cts/tests/splitbraintest.py +++ b/python/pacemaker/_cts/tests/splitbraintest.py @@ -116,7 +116,7 @@ def __call__(self, node): self.debug(f"Partition[{key}]:\t{val!r}") # Disabling STONITH to reduce test complexity for now - self._rsh(node, "crm_attribute -V -n fencing-enabled -v false") + self._rsh.call(node, "crm_attribute -V -n fencing-enabled -v false") for val in partitions.values(): self._isolate_partition(val) @@ -183,7 +183,7 @@ def __call__(self, node): # Turn fencing back on if self._env["fencing_enabled"]: - self._rsh(node, "crm_attribute -V -D -n fencing-enabled") + self._rsh.call(node, "crm_attribute -V -D -n fencing-enabled") self._cm.cluster_stable() diff --git a/python/pacemaker/_cts/tests/stonithdtest.py b/python/pacemaker/_cts/tests/stonithdtest.py index e77b2905a61..7061260cbe4 100644 --- a/python/pacemaker/_cts/tests/stonithdtest.py +++ b/python/pacemaker/_cts/tests/stonithdtest.py @@ -66,7 +66,7 @@ def __call__(self, node): origin = self._env.random_gen.choice(self._env["nodes"]) - (rc, _) = self._rsh(origin, f"stonith_admin --reboot {node} -VVVVVV") + (rc, _) = self._rsh.call(origin, f"stonith_admin --reboot {node} -VVVVVV") if rc == ExitStatus.TIMEOUT: # Look for the patterns, usually this means the required diff --git a/python/pacemaker/_cts/tests/stoptest.py b/python/pacemaker/_cts/tests/stoptest.py index 04e2f021f1c..38b26cde82f 100644 --- a/python/pacemaker/_cts/tests/stoptest.py +++ b/python/pacemaker/_cts/tests/stoptest.py @@ -69,11 +69,11 @@ def __call__(self, node): unmatched_str = "||" if watch.unmatched: - (_, output) = self._rsh(node, "/bin/ps axf", verbose=1) + (_, output) = self._rsh.call(node, "/bin/ps axf", verbose=1) for line in output: self.debug(line) - (_, output) = self._rsh(node, "/usr/sbin/dlm_tool dump 2>/dev/null", verbose=1) + (_, output) = self._rsh.call(node, "/usr/sbin/dlm_tool dump 2>/dev/null", verbose=1) for line in output: self.debug(line) diff --git a/python/pacemaker/_cts/watcher.py b/python/pacemaker/_cts/watcher.py index 91058e10c75..779b0f71d34 100644 --- a/python/pacemaker/_cts/watcher.py +++ b/python/pacemaker/_cts/watcher.py @@ -188,7 +188,7 @@ def set_end(self): cmd = f"{CTS_SUPPORT_BIN} watch -p CTSwatcher: -l 2 -f {self.filename} -o EOF" - (_, lines) = self.rsh(self.host, cmd, verbose=0) + (_, lines) = self.rsh.call(self.host, cmd, verbose=0) for line in lines: match = re.search(r"^CTSwatcher:Last read: (\d+)", line) @@ -322,7 +322,7 @@ def set_end(self): return # Seconds and nanoseconds since epoch - (rc, lines) = self.rsh(self.host, "date +%s.%N", verbose=0) + (rc, lines) = self.rsh.call(self.host, "date +%s.%N", verbose=0) if rc == 0 and len(lines) == 1: self.limit = float(lines[0].strip()) From aed5b1582f5cae41d0d3d6e6f23dd057b8e1d16f Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 16 Feb 2026 09:13:21 -0500 Subject: [PATCH 11/15] Low: python: Don't pass the ssh command to rsh.call. What we really care about here is whether or not one system can ssh into another, so all we need to do is just try to ssh without all those extra options. Ref T680 --- python/pacemaker/_cts/tests/cibsecret.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/pacemaker/_cts/tests/cibsecret.py b/python/pacemaker/_cts/tests/cibsecret.py index 62f47c63cb4..bc79d04ae3a 100644 --- a/python/pacemaker/_cts/tests/cibsecret.py +++ b/python/pacemaker/_cts/tests/cibsecret.py @@ -261,8 +261,7 @@ def is_applicable(self): other = self._cm.env["nodes"][1:] for o in other: - (rc, _) = self._cm.rsh.call(node, f"{self._cm.rsh.command} {o} exit", - verbose=0) + (rc, _) = self._cm.rsh.call(node, f"ssh root@{o} exit", verbose=0) if rc != ExitStatus.OK: return False From 0cf338d58a38a351931c7044ce9a8d0161c6102b Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 16 Feb 2026 11:56:48 -0500 Subject: [PATCH 12/15] Low: python: Add "-o StrictHostKeyChecking=no" to ssh/scp args. In general, you definitely want to do host key checking and this option is set to "ask" by default. However, in CTS, this means that if you haven't ssh'd between systems first and done host key checking (or at least responded to the ssh prompt), it will stop and ask. This results in test case failures. So add this argument to just skip the host key checking and prevent those failures. I think this is reasonable to do in CTS, but obviously you wouldn't want to do this elsewhere. --- python/pacemaker/_cts/remote.py | 6 +++--- python/pacemaker/_cts/tests/cibsecret.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 921d30e6963..1e62f6c1dfd 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -89,7 +89,7 @@ def __init__(self): # stops responding self._command = "ssh -l root -n -x -o ServerAliveInterval=5 " \ "-o ConnectTimeout=10 -o TCPKeepAlive=yes " \ - "-o ServerAliveCountMax=3" + "-o ServerAliveCountMax=3 -o StrictHostKeyChecking=off" self._our_node = os.uname()[1].lower() def _fixcmd(self, cmd): @@ -178,8 +178,8 @@ def copy(self, source, target): Returns the return code of the copy process. """ # -B: batch mode, -q: no stats (quiet) - p = subprocess.run(["scp", "-B", "-q", f"'{source}'", f"'{target}'"], - check=False) + p = subprocess.run(["scp", "-B", "-q", "-o", "StrictHostKeyChecking=off", + f"'{source}'", f"'{target}'"], check=False) logging.debug(f"cmd: rc={p.returncode}: {p.args}") return p.returncode diff --git a/python/pacemaker/_cts/tests/cibsecret.py b/python/pacemaker/_cts/tests/cibsecret.py index bc79d04ae3a..01d85e68f18 100644 --- a/python/pacemaker/_cts/tests/cibsecret.py +++ b/python/pacemaker/_cts/tests/cibsecret.py @@ -261,7 +261,8 @@ def is_applicable(self): other = self._cm.env["nodes"][1:] for o in other: - (rc, _) = self._cm.rsh.call(node, f"ssh root@{o} exit", verbose=0) + (rc, _) = self._cm.rsh.call(node, f"ssh -o StrictHostKeyChecking=off root@{o} exit", + verbose=0) if rc != ExitStatus.OK: return False From 0194cbf8ea8ac88ed70045cb760b955ca88f6b89 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 16 Feb 2026 13:00:48 -0500 Subject: [PATCH 13/15] Refactor: python: Rename patternVariants to pattern_variants. This fixes a pylint warning regarding naming. --- python/pacemaker/_cts/patterns.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/python/pacemaker/_cts/patterns.py b/python/pacemaker/_cts/patterns.py index d8225f24af8..4b69cfe9a14 100644 --- a/python/pacemaker/_cts/patterns.py +++ b/python/pacemaker/_cts/patterns.py @@ -1,7 +1,7 @@ """Pattern-holding classes for Pacemaker's Cluster Test Suite (CTS).""" __all__ = ["PatternSelector"] -__copyright__ = "Copyright 2008-2025 the Pacemaker project contributors" +__copyright__ = "Copyright 2008-2026 the Pacemaker project contributors" __license__ = "GNU General Public License version 2 or later (GPLv2+)" from pacemaker.buildoptions import BuildOptions @@ -343,7 +343,7 @@ def __init__(self): ] -patternVariants = { +pattern_variants = { "crm-base": BasePatterns, "crm-corosync": Corosync2Patterns } @@ -363,11 +363,11 @@ def __init__(self, name="crm-corosync"): self._name = name # If no name was given, use the default. Otherwise, look up the appropriate - # class in patternVariants, instantiate it, and use that. + # class in pattern_variants, instantiate it, and use that. if not name: self._base = Corosync2Patterns() else: - self._base = patternVariants[name]() + self._base = pattern_variants[name]() def __getitem__(self, key): """ From 5342b2355d9395f63d034f20edf4406f79797a1a Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 16 Feb 2026 13:07:36 -0500 Subject: [PATCH 14/15] Refactor: python: Remove some unnecessary ssh options. TCPKeepAlive=yes and ServerAliveCountMax=3 are ssh defaults, and appear to have been ssh defaults for a very long time. The only reason to specify these would be if you had your ctslab machines configured differently, which I don't expect anyone is doing. Thus we can just remove these. --- python/pacemaker/_cts/remote.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/pacemaker/_cts/remote.py b/python/pacemaker/_cts/remote.py index 1e62f6c1dfd..015483d3fe7 100644 --- a/python/pacemaker/_cts/remote.py +++ b/python/pacemaker/_cts/remote.py @@ -88,8 +88,7 @@ def __init__(self): # -o ServerAliveInterval=5: disconnect after 3*5s if the server # stops responding self._command = "ssh -l root -n -x -o ServerAliveInterval=5 " \ - "-o ConnectTimeout=10 -o TCPKeepAlive=yes " \ - "-o ServerAliveCountMax=3 -o StrictHostKeyChecking=off" + "-o ConnectTimeout=10 -o StrictHostKeyChecking=off" self._our_node = os.uname()[1].lower() def _fixcmd(self, cmd): From 786ad5718e3c0897df73ff6c7c890d564ed1f217 Mon Sep 17 00:00:00 2001 From: Chris Lumens Date: Mon, 16 Feb 2026 16:16:18 -0500 Subject: [PATCH 15/15] Refactor: python: Remove unnecessary triple quotes. --- python/pacemaker/_cts/tests/reattach.py | 4 ++-- python/pacemaker/_cts/tests/remotedriver.py | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/python/pacemaker/_cts/tests/reattach.py b/python/pacemaker/_cts/tests/reattach.py index 9a3cbce7bf5..99070bc9557 100644 --- a/python/pacemaker/_cts/tests/reattach.py +++ b/python/pacemaker/_cts/tests/reattach.py @@ -73,7 +73,7 @@ def _enable_incompatible_rscs(self, node): """Re-enable resources that were incompatible with this test.""" self.debug("Re-enable incompatible resources") xml = """""" - return self._rsh.call(node, f"""cibadmin --delete --xml-text '{xml}'""") + return self._rsh.call(node, f"cibadmin --delete --xml-text '{xml}'") def _reprobe(self, node): """ @@ -84,7 +84,7 @@ def _reprobe(self, node): rules. An earlier test may have erased the relevant node attribute, so do a reprobe, which should add the attribute back. """ - return self._rsh.call(node, """crm_resource --refresh""") + return self._rsh.call(node, "crm_resource --refresh") def setup(self, node): """Set up this test.""" diff --git a/python/pacemaker/_cts/tests/remotedriver.py b/python/pacemaker/_cts/tests/remotedriver.py index 106cabf38e6..dc5500643d1 100644 --- a/python/pacemaker/_cts/tests/remotedriver.py +++ b/python/pacemaker/_cts/tests/remotedriver.py @@ -530,7 +530,7 @@ def __call__(self, node): def errors_to_ignore(self): """Return list of errors which should be ignored.""" return [ - r"""is running on remote.*which isn't allowed""", - r"""Connection terminated""", - r"""Could not send remote""" + r"is running on remote.*which isn't allowed", + r"Connection terminated", + r"Could not send remote" ]