From 0b216c815abb14bc16d48fe9b0dd74f78ad4aad2 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 10:52:55 +0000 Subject: [PATCH 1/6] CI: Consolidate `apt-get install`s --- .ci/ansible_tests.py | 5 ----- .github/workflows/tests.yml | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.ci/ansible_tests.py b/.ci/ansible_tests.py index 4a7bedaeb..6c78bb0c6 100755 --- a/.ci/ansible_tests.py +++ b/.ci/ansible_tests.py @@ -71,11 +71,6 @@ def pause_if_interactive(): ci_lib.dump_file(inventory_path) - if not ci_lib.exists_in_path('sshpass'): - ci_lib.run("sudo apt-get update") - ci_lib.run("sudo apt-get install -y sshpass") - - with ci_lib.Fold('ansible'): playbook = os.environ.get('PLAYBOOK', 'all.yml') try: diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 9a275a00c..bbbc33e8b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -96,7 +96,7 @@ jobs: set -o errexit -o nounset -o pipefail sudo apt-get update - sudo apt-get install -y python2-dev python3-pip virtualenv + sudo apt-get install -y python2-dev python3-pip sshpass virtualenv - name: Show Python versions run: | set -o errexit -o nounset -o pipefail From 2c7eda1dc101cc6346b8120c5ce8fb2cac37fe75 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 10:53:45 +0000 Subject: [PATCH 2/6] CI: Fix NameError in ci_lib._have_cmd() --- .ci/ci_lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index afb62e023..25648bbe8 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -56,7 +56,7 @@ def _have_cmd(args): if exc.errno == errno.ENOENT: return False raise - except subprocess.CallProcessError: + except subprocess.CalledProcessError: return False return True From 2095342245a2af83926bf09eb726555e31cbfd49 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 10:55:07 +0000 Subject: [PATCH 3/6] CI: Remove unused ci_lib.have_*() functions --- .ci/ci_lib.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index 25648bbe8..c078791f3 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -61,14 +61,6 @@ def _have_cmd(args): return True -def have_apt(): - return _have_cmd(['apt', '--help']) - - -def have_brew(): - return _have_cmd(['brew', 'help']) - - def have_docker(): return _have_cmd(['docker', 'info']) From 620bc3a944a788a4f645edbcbbc940eee3e9e764 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 11:44:00 +0000 Subject: [PATCH 4/6] CI: Don't copy SSH private key to temporary dir Running tests aren't using the copy & it wasn't being cleaned up. --- .ci/ansible_tests.py | 2 +- .ci/ci_lib.py | 13 ++----------- .ci/debops_common_tests.py | 3 ++- .ci/localhost_ansible_tests.py | 3 +-- docs/changelog.rst | 1 + 5 files changed, 7 insertions(+), 15 deletions(-) diff --git a/.ci/ansible_tests.py b/.ci/ansible_tests.py index 6c78bb0c6..01a012f5e 100755 --- a/.ci/ansible_tests.py +++ b/.ci/ansible_tests.py @@ -40,8 +40,8 @@ def pause_if_interactive(): with ci_lib.Fold('job_setup'): + os.chmod(ci_lib.TESTS_SSH_PRIVATE_KEY_FILE, int('0600', 8)) os.chdir(TESTS_DIR) - os.chmod('../data/docker/mitogen__has_sudo_pubkey.key', int('0600', 8)) ci_lib.run("mkdir %s", HOSTS_DIR) for path in glob.glob(TESTS_DIR + '/hosts/*'): diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index c078791f3..b0c9c04a9 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -28,6 +28,7 @@ ) +GIT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) DISTRO_SPECS = os.environ.get( 'MITOGEN_TEST_DISTRO_SPECS', 'centos6 centos8 debian9 debian11 ubuntu1604 ubuntu2004', @@ -36,6 +37,7 @@ 'MITOGEN_TEST_IMAGE_TEMPLATE', 'public.ecr.aws/n5z0e8q9/%(distro)s-test', ) +TESTS_SSH_PRIVATE_KEY_FILE = os.path.join(GIT_ROOT, 'tests/data/docker/mitogen__has_sudo_pubkey.key') _print = print @@ -191,20 +193,9 @@ def __enter__(self): pass def __exit__(self, _1, _2, _3): pass -GIT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) TMP = TempDir().path -# We copy this out of the way to avoid random stuff modifying perms in the Git -# tree (like git pull). -src_key_file = os.path.join(GIT_ROOT, - 'tests/data/docker/mitogen__has_sudo_pubkey.key') -key_file = os.path.join(TMP, - 'mitogen__has_sudo_pubkey.key') -shutil.copyfile(src_key_file, key_file) -os.chmod(key_file, int('0600', 8)) - - os.environ['PYTHONDONTWRITEBYTECODE'] = 'x' os.environ['PYTHONPATH'] = '%s:%s' % ( os.environ.get('PYTHONPATH', ''), diff --git a/.ci/debops_common_tests.py b/.ci/debops_common_tests.py index b065486f8..b9b0030c4 100755 --- a/.ci/debops_common_tests.py +++ b/.ci/debops_common_tests.py @@ -22,6 +22,7 @@ with ci_lib.Fold('job_setup'): + os.chmod(ci_lib.TESTS_SSH_PRIVATE_KEY_FILE, int('0600', 8)) ci_lib.run('debops-init %s', project_dir) os.chdir(project_dir) @@ -45,7 +46,7 @@ "\n" # Speed up slow DH generation. "dhparam__bits: ['128', '64']\n" - % (ci_lib.key_file,) + % (ci_lib.TESTS_SSH_PRIVATE_KEY_FILE,) ) with open(inventory_path, 'a') as fp: diff --git a/.ci/localhost_ansible_tests.py b/.ci/localhost_ansible_tests.py index e4b8329ba..29022bf7d 100755 --- a/.ci/localhost_ansible_tests.py +++ b/.ci/localhost_ansible_tests.py @@ -13,7 +13,6 @@ TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible') IMAGE_PREP_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/image_prep') HOSTS_DIR = os.path.join(TESTS_DIR, 'hosts') -KEY_PATH = os.path.join(TESTS_DIR, '../data/docker/mitogen__has_sudo_pubkey.key') with ci_lib.Fold('unit_tests'): @@ -22,7 +21,7 @@ with ci_lib.Fold('job_setup'): - os.chmod(KEY_PATH, int('0600', 8)) + os.chmod(ci_lib.TESTS_SSH_PRIVATE_KEY_FILE, int('0600', 8)) # NOTE: sshpass v1.06 causes errors so pegging to 1.05 -> "msg": "Error when changing password","out": "passwd: DS error: eDSAuthFailed\n", # there's a checksum error with "brew install http://git.io/sshpass.rb" though, so installing manually if not ci_lib.exists_in_path('sshpass'): diff --git a/docs/changelog.rst b/docs/changelog.rst index 7ae6e5da3..1fbee883e 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -24,6 +24,7 @@ In progress (unreleased) * :gh:issue:`1121` :mod:`mitogen`: Log skipped :py:mod:`termios` attributes * :gh:issue:`1238` packaging: Avoid :py:mod:`ast`, requires Python = 2.6 * :gh:issue:`1118` CI: Statically specify test usernames and group names +* :gh:issue:`1118` CI: Don't copy SSH private key to temporary dir v0.3.22 (2025-02-04) From f659213159db7fd7d556a5ef0038f775d5e89af3 Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 12:19:16 +0000 Subject: [PATCH 5/6] CI: Don't share temporary directory between test groupings Each grouping gets an independant dir, e.g. - ansible -> /tmp/mitogen_ci_ansible - debops -> /tmp/mitogen_ci_debops Importing ci_lib no longer creates a temporary directory as a side effect. --- .ci/ansible_tests.py | 12 +++++++----- .ci/ci_lib.py | 7 ++----- .ci/debops_common_tests.py | 3 ++- docs/changelog.rst | 1 + 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.ci/ansible_tests.py b/.ci/ansible_tests.py index 01a012f5e..0d67c3b24 100755 --- a/.ci/ansible_tests.py +++ b/.ci/ansible_tests.py @@ -14,7 +14,8 @@ TEMPLATES_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible/templates') TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible') -HOSTS_DIR = os.path.join(ci_lib.TMP, 'hosts') +TMP = ci_lib.TempDir(prefix='mitogen_ci_ansible') +TMP_HOSTS_DIR = os.path.join(TMP.path, 'hosts') def pause_if_interactive(): @@ -43,10 +44,10 @@ def pause_if_interactive(): os.chmod(ci_lib.TESTS_SSH_PRIVATE_KEY_FILE, int('0600', 8)) os.chdir(TESTS_DIR) - ci_lib.run("mkdir %s", HOSTS_DIR) + os.mkdir(TMP_HOSTS_DIR) for path in glob.glob(TESTS_DIR + '/hosts/*'): if not path.endswith('default.hosts'): - ci_lib.run("ln -s %s %s", path, HOSTS_DIR) + os.symlink(path, os.path.join(TMP_HOSTS_DIR, os.path.basename(path))) distros = collections.defaultdict(list) families = collections.defaultdict(list) @@ -60,7 +61,7 @@ def pause_if_interactive(): trim_blocks=True, # Remove first newline after a block ) inventory_template = jinja_env.get_template('test-targets.j2') - inventory_path = os.path.join(HOSTS_DIR, 'target') + inventory_path = os.path.join(TMP_HOSTS_DIR, 'test-targets.ini') with open(inventory_path, 'w') as fp: fp.write(inventory_template.render( @@ -75,7 +76,8 @@ def pause_if_interactive(): playbook = os.environ.get('PLAYBOOK', 'all.yml') try: ci_lib.run('./run_ansible_playbook.py %s -i "%s" %s', - playbook, HOSTS_DIR, ' '.join(sys.argv[1:])) + playbook, TMP_HOSTS_DIR, ' '.join(sys.argv[1:]), + ) except: pause_if_interactive() raise diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index b0c9c04a9..b37472a26 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -179,8 +179,8 @@ def exists_in_path(progname): class TempDir(object): - def __init__(self): - self.path = tempfile.mkdtemp(prefix='mitogen_ci_lib') + def __init__(self, prefix='mitogen_ci_lib'): + self.path = tempfile.mkdtemp(prefix=prefix) atexit.register(self.destroy) def destroy(self, rmtree=shutil.rmtree): @@ -193,9 +193,6 @@ def __enter__(self): pass def __exit__(self, _1, _2, _3): pass -TMP = TempDir().path - - os.environ['PYTHONDONTWRITEBYTECODE'] = 'x' os.environ['PYTHONPATH'] = '%s:%s' % ( os.environ.get('PYTHONPATH', ''), diff --git a/.ci/debops_common_tests.py b/.ci/debops_common_tests.py index b9b0030c4..6fd50124a 100755 --- a/.ci/debops_common_tests.py +++ b/.ci/debops_common_tests.py @@ -6,7 +6,8 @@ import ci_lib -project_dir = os.path.join(ci_lib.TMP, 'project') +TMP = ci_lib.TempDir(prefix='mitogen_ci_debops') +project_dir = os.path.join(TMP.path, 'project') vars_path = 'ansible/inventory/group_vars/debops_all_hosts.yml' inventory_path = 'ansible/inventory/hosts' docker_hostname = ci_lib.get_docker_hostname() diff --git a/docs/changelog.rst b/docs/changelog.rst index 1fbee883e..ba4a608be 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -25,6 +25,7 @@ In progress (unreleased) * :gh:issue:`1238` packaging: Avoid :py:mod:`ast`, requires Python = 2.6 * :gh:issue:`1118` CI: Statically specify test usernames and group names * :gh:issue:`1118` CI: Don't copy SSH private key to temporary dir +* :gh:issue:`1118` CI: Don't share temporary directory between test groupings v0.3.22 (2025-02-04) From a376daa04db22cb04e455298692ceb5dc6c5b7de Mon Sep 17 00:00:00 2001 From: Alex Willmer Date: Fri, 7 Feb 2025 12:27:11 +0000 Subject: [PATCH 6/6] CI: Consolidate directory path constants --- .ci/ansible_tests.py | 10 +++++----- .ci/ci_lib.py | 4 ++++ .ci/localhost_ansible_tests.py | 11 +++-------- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/.ci/ansible_tests.py b/.ci/ansible_tests.py index 0d67c3b24..4bf86bcaa 100755 --- a/.ci/ansible_tests.py +++ b/.ci/ansible_tests.py @@ -12,8 +12,6 @@ import ci_lib -TEMPLATES_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible/templates') -TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible') TMP = ci_lib.TempDir(prefix='mitogen_ci_ansible') TMP_HOSTS_DIR = os.path.join(TMP.path, 'hosts') @@ -42,10 +40,10 @@ def pause_if_interactive(): with ci_lib.Fold('job_setup'): os.chmod(ci_lib.TESTS_SSH_PRIVATE_KEY_FILE, int('0600', 8)) - os.chdir(TESTS_DIR) + os.chdir(ci_lib.ANSIBLE_TESTS_DIR) os.mkdir(TMP_HOSTS_DIR) - for path in glob.glob(TESTS_DIR + '/hosts/*'): + for path in glob.glob(os.path.join(ci_lib.ANSIBLE_TESTS_HOSTS_DIR, '*')): if not path.endswith('default.hosts'): os.symlink(path, os.path.join(TMP_HOSTS_DIR, os.path.basename(path))) @@ -56,7 +54,9 @@ def pause_if_interactive(): families[container['family']].append(container['name']) jinja_env = jinja2.Environment( - loader=jinja2.FileSystemLoader(searchpath=TEMPLATES_DIR), + loader=jinja2.FileSystemLoader( + searchpath=ci_lib.ANSIBLE_TESTS_TEMPLATES_DIR, + ), lstrip_blocks=True, # Remove spaces and tabs from before a block trim_blocks=True, # Remove first newline after a block ) diff --git a/.ci/ci_lib.py b/.ci/ci_lib.py index b37472a26..9f333dde3 100644 --- a/.ci/ci_lib.py +++ b/.ci/ci_lib.py @@ -29,10 +29,14 @@ GIT_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), '..')) +ANSIBLE_TESTS_DIR = os.path.join(GIT_ROOT, 'tests/ansible') +ANSIBLE_TESTS_HOSTS_DIR = os.path.join(GIT_ROOT, 'tests/ansible/hosts') +ANSIBLE_TESTS_TEMPLATES_DIR = os.path.join(GIT_ROOT, 'tests/ansible/templates') DISTRO_SPECS = os.environ.get( 'MITOGEN_TEST_DISTRO_SPECS', 'centos6 centos8 debian9 debian11 ubuntu1604 ubuntu2004', ) +IMAGE_PREP_DIR = os.path.join(GIT_ROOT, 'tests/image_prep') IMAGE_TEMPLATE = os.environ.get( 'MITOGEN_TEST_IMAGE_TEMPLATE', 'public.ecr.aws/n5z0e8q9/%(distro)s-test', diff --git a/.ci/localhost_ansible_tests.py b/.ci/localhost_ansible_tests.py index 29022bf7d..359dc195f 100755 --- a/.ci/localhost_ansible_tests.py +++ b/.ci/localhost_ansible_tests.py @@ -10,11 +10,6 @@ import ci_lib -TESTS_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/ansible') -IMAGE_PREP_DIR = os.path.join(ci_lib.GIT_ROOT, 'tests/image_prep') -HOSTS_DIR = os.path.join(TESTS_DIR, 'hosts') - - with ci_lib.Fold('unit_tests'): os.environ['SKIP_MITOGEN'] = '1' ci_lib.run('./run_tests -v') @@ -50,11 +45,11 @@ subprocess.check_call('sudo chmod 700 ~root/.ssh', shell=True) subprocess.check_call('sudo chmod 600 ~root/.ssh/authorized_keys', shell=True) - os.chdir(IMAGE_PREP_DIR) + os.chdir(ci_lib.IMAGE_PREP_DIR) ci_lib.run("ansible-playbook -c local -i localhost, macos_localhost.yml") if os.path.expanduser('~mitogen__user1') == '~mitogen__user1': - os.chdir(IMAGE_PREP_DIR) + os.chdir(ci_lib.IMAGE_PREP_DIR) ci_lib.run("ansible-playbook -c local -i localhost, _user_accounts.yml") cmd = ';'.join([ @@ -79,7 +74,7 @@ with ci_lib.Fold('ansible'): - os.chdir(TESTS_DIR) + os.chdir(ci_lib.ANSIBLE_TESTS_DIR) playbook = os.environ.get('PLAYBOOK', 'all.yml') ci_lib.run('./run_ansible_playbook.py %s %s', playbook, ' '.join(sys.argv[1:]))