From 7dceb35fd1afb197c16d018df4999e496ce5dd1d Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Mon, 4 May 2026 16:40:34 -0700 Subject: [PATCH 01/29] feat: add support for posixgroup feature creation based on repo GID --- ansible-runner/project | 2 +- modules/coactd.py | 52 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/ansible-runner/project b/ansible-runner/project index 9e3cd00..ac1bcee 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 9e3cd0040a6a03eec7a7648281b818d00058115a +Subproject commit ac1bceef6f5c12adaf96c4e49d9f60fe32c6402f diff --git a/modules/coactd.py b/modules/coactd.py index 94f226c..5e0cb5d 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -534,6 +534,22 @@ def do_new_repo( # run the facility tasks for this repo runner = self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) + # Extract repo GID if it was created for facilities that use grouper + repo_gid = None + repo_group_name = "" + if facility.lower() in ['lcls', 'cryoem']: + try: + # Get the repo_gid fact that was set in add_repo.yaml + repo_facts = self.playbook_task_res(runner, 'Create a facility repo', 'Expose group values for runner') + if repo_facts and 'ansible_facts' in repo_facts: + repo_gid = repo_facts['ansible_facts']['repo_gid'] + repo_group_name = repo_facts['ansible_facts']['repo_group_name'] + self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") + else: + self.logger.warning(f"No repo GID found in playbook results for {facility}:{repo}") + except Exception as e: + self.logger.warning(f"Failed to extract repo GID for {facility}:{repo}: {e}") + leaders = [principal] users = [principal] @@ -558,15 +574,43 @@ def do_new_repo( repo_upserted = self.back_channel.execute(REPO_UPSERT_GQL, repo_create_req) repo_id = repo_upserted['repoUpsert']['Id'] - feature_req = {'repo': {'Id': repo_id}} + # Create a parameterized feature upsert mutation FEATURE_UPSERT_GQL = gql(""" - mutation repoUpsert($repo: RepoInput! ) { - repoUpsertFeature(repo: $repo, feature: { name: "slurm", state: true, options: [] }) { + mutation repoUpsert($repo: RepoInput!, $feature: RepoFeatureInput!) { + repoUpsertFeature(repo: $repo, feature: $feature) { Id } } """) - feature_upserted = self.back_channel.execute(FEATURE_UPSERT_GQL, feature_req) + + # Create slurm feature + slurm_feature_req = { + 'repo': {'Id': repo_id}, + 'feature': {'name': 'slurm', 'state': True, 'options': []} + } + self.back_channel.execute(FEATURE_UPSERT_GQL, slurm_feature_req) + + # Create posixgroup feature if GID was obtained from grouper + if repo_gid is not None: + posixgroup_options = [json.dumps({ + "name": repo_group_name, + "gidNumber": int(repo_gid) + })] + + posixgroup_feature_req = { + 'repo': {'Id': repo_id}, + 'feature': { + 'name': 'posixgroup', + 'state': True, + 'options': posixgroup_options + } + } + + try: + self.back_channel.execute(FEATURE_UPSERT_GQL, posixgroup_feature_req) + self.logger.info(f"Created posixgroup feature for {facility}:{repo} with GID {repo_gid}") + except Exception as e: + self.logger.warning(f"Failed to create posixgroup feature for {facility}:{repo}: {e}") return True From f98f7251937763a941d38fdeb122687c3a72e5c0 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Mon, 4 May 2026 17:25:15 -0700 Subject: [PATCH 02/29] add unit tests --- tests/test_repo_registration_gid.py | 127 ++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 tests/test_repo_registration_gid.py diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py new file mode 100644 index 0000000..5d911a8 --- /dev/null +++ b/tests/test_repo_registration_gid.py @@ -0,0 +1,127 @@ +""" +Unit tests for repository registration GID/group name handling. + +Tests the new functionality in RepoRegistration.do_new_repo() that extracts +GID information from Ansible playbook results and creates posixgroup features +for facilities that use grouper (currently only CryoEM). +""" + +import sys +from unittest.mock import Mock, patch +import pytest + +# Mock ansible_runner module to avoid all sdf-ansible dependencies +sys.modules['ansible_runner'] = Mock() + +from modules.coactd import RepoRegistration + + +class TestRepoRegistrationGID: + """Test GID/group name handling in repository registration.""" + + @pytest.fixture + def repo_registration(self) -> RepoRegistration: + """Create a RepoRegistration instance with mocked dependencies.""" + with patch('modules.coactd.GraphQlSubscriber.__init__'), \ + patch('modules.coactd.AnsibleRunner.__init__'): + reg = RepoRegistration( + username='test-user', + password_file='/tmp/test-password', + client_name='test-client' + ) + reg.logger = Mock() + reg.back_channel = Mock() + reg.run_playbook = Mock() + reg.playbook_task_res = Mock() + return reg + + @pytest.fixture + def mock_ansible_runner(self) -> Mock: + """Mock ansible runner with realistic structure.""" + runner = Mock() + runner.events = [] + return runner + + @pytest.fixture + def sample_gid_facts(self) -> dict: + """Sample ansible facts with GID information.""" + return { + 'ansible_facts': { + 'repo_gid': '12345', + 'repo_group_name': 'test-repo-group' + } + } + + def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration, mock_ansible_runner: Mock, sample_gid_facts: dict): + """Test successful GID extraction for CryoEM facility.""" + # Setup + repo_registration.run_playbook.return_value = mock_ansible_runner + repo_registration.playbook_task_res.return_value = sample_gid_facts + repo_registration.back_channel.execute.side_effect = [ + {'repoUpsert': {'Id': 'repo-456'}}, + {'repoUpsertFeature': {'Id': 'feature-slurm'}}, + {'repoUpsertFeature': {'Id': 'feature-posix'}} + ] + + # Execute + result = repo_registration.do_new_repo( + repo='cryo-test', + facility='cryoem', # Test case-insensitive matching + principal='cryo-user' + ) + + # Verify + assert result is True + repo_registration.logger.info.assert_any_call( + "Retrieved repo GID for cryoem:cryo-test: 12345" + ) + + def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, mock_ansible_runner: Mock): + """Test handling when ansible facts are empty (triggers KeyError).""" + # Setup + repo_registration.run_playbook.return_value = mock_ansible_runner + repo_registration.playbook_task_res.return_value = {'ansible_facts': {}} # Empty facts + repo_registration.back_channel.execute.side_effect = [ + {'repoUpsert': {'Id': 'repo-123'}}, + {'repoUpsertFeature': {'Id': 'feature-slurm'}} + ] + + # Execute + result = repo_registration.do_new_repo( + repo='test-repo', + facility='cryoem', + principal='test-user' + ) + + # Verify + assert result is True + + # Should log warning about extraction failure (KeyError when accessing repo_gid) + repo_registration.logger.warning.assert_called_with( + "Failed to extract repo GID for cryoem:test-repo: 'repo_gid'" + ) + + def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_runner): + """Test that non-grouper facilities skip GID extraction.""" + # Setup + repo_registration.run_playbook.return_value = mock_ansible_runner + repo_registration.back_channel.execute.side_effect = [ + {'repoUpsert': {'Id': 'repo-123'}}, + {'repoUpsertFeature': {'Id': 'feature-slurm'}} + ] + + # Execute + result = repo_registration.do_new_repo( + repo='test-repo', + facility='OTHER', # Not CryoEM + principal='test-user' + ) + + # Verify + assert result is True + + # playbook_task_res should NOT be called + repo_registration.playbook_task_res.assert_not_called() + + # Should only create slurm feature + assert repo_registration.back_channel.execute.call_count == 2 From 266dc2bdaf86c6856d2cda6101447ad4075e4266 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 7 May 2026 13:22:23 -0700 Subject: [PATCH 03/29] cryoem only --- modules/coactd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/coactd.py b/modules/coactd.py index 5e0cb5d..c8a4a73 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -537,7 +537,7 @@ def do_new_repo( # Extract repo GID if it was created for facilities that use grouper repo_gid = None repo_group_name = "" - if facility.lower() in ['lcls', 'cryoem']: + if facility.lower() in ['cryoem']: try: # Get the repo_gid fact that was set in add_repo.yaml repo_facts = self.playbook_task_res(runner, 'Create a facility repo', 'Expose group values for runner') From 4ca32b83244c9890918c7f641cf12ba3ddcedfde Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 7 May 2026 14:19:11 -0700 Subject: [PATCH 04/29] move grouper run to Python Co-authored-by: Copilot --- modules/coactd.py | 47 +++++++++++++------ tests/test_repo_registration_gid.py | 72 +++++++++++++++++++++-------- 2 files changed, 87 insertions(+), 32 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index c8a4a73..34949fe 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -154,12 +154,13 @@ class Registration(GraphQlSubscriber, AnsibleRunner): } """ - def __init__(self, username: str, password_file: str, client_name: str, dry_run: bool = False): + def __init__(self, username: str, password_file: str, client_name: str, dry_run: bool = False, grouper_password_file: str = None): self.logger = logger self.username = username self.password_file = password_file self.client_name = client_name self.dry_run = dry_run + self.grouper_password_file = grouper_password_file def run(self): """Main entry point - connect and process subscription requests.""" @@ -231,6 +232,12 @@ def registration_options(f): type=click.Path(exists=True), help='Basic auth password for graphql service' )(f) + f = click.option( + '--grouper-password-file', + default=None, + type=click.Path(exists=True), + help='Path to file containing the Grouper service account password' + )(f) f = click.option( '--client-name', default=None, @@ -532,23 +539,36 @@ def do_new_repo( repo_allocation_end_delta = pdl.duration(years=5) # run the facility tasks for this repo - runner = self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) + self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) - # Extract repo GID if it was created for facilities that use grouper + # For CryoEM repos (ct* / ce*), create a POSIX group via Grouper repo_gid = None repo_group_name = "" - if facility.lower() in ['cryoem']: + uses_grouper = ( + facility.lower() == 'cryoem' + and (repo.lower().startswith('ct') or repo.lower().startswith('ce')) + ) + if uses_grouper: + grouper_name = f"sdf-{facility.lower()}-{repo.lower()}" try: - # Get the repo_gid fact that was set in add_repo.yaml - repo_facts = self.playbook_task_res(runner, 'Create a facility repo', 'Expose group values for runner') - if repo_facts and 'ansible_facts' in repo_facts: - repo_gid = repo_facts['ansible_facts']['repo_gid'] - repo_group_name = repo_facts['ansible_facts']['repo_group_name'] + if not self.grouper_password_file: + raise ValueError("Grouper password file must be provided for CryoEM ct/ce repos") + grouper_kwargs = dict( + grouper_name=grouper_name, + state="present", + grouper_description=f"POSIX group for {facility} {repo} repository access", + grouper_password_file=self.grouper_password_file + ) + grouper_runner = self.run_playbook("coact/grouper.yml", **grouper_kwargs) + grouper_facts = self.playbook_task_res(grouper_runner, 'Grouper', 'Export grouper params') + if grouper_facts and 'ansible_facts' in grouper_facts: + repo_gid = grouper_facts['ansible_facts']['gid'] + repo_group_name = grouper_name self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") else: - self.logger.warning(f"No repo GID found in playbook results for {facility}:{repo}") + self.logger.warning(f"No GID found in grouper playbook results for {facility}:{repo}") except Exception as e: - self.logger.warning(f"Failed to extract repo GID for {facility}:{repo}: {e}") + self.logger.warning(f"Failed to create grouper POSIX group for {facility}:{repo}: {e}") leaders = [principal] users = [principal] @@ -951,7 +971,7 @@ def do_feature(self, repo, facility, dry_run: bool = False) -> bool: @common_options @registration_options @click.pass_context -def repo_registration(ctx, verbose, username, password_file, client_name, dry_run): +def repo_registration(ctx, verbose, username, password_file, client_name, dry_run, grouper_password_file): """Workflow for repository maintenance. Handles NewRepo, RepoMembership, RepoRemoveUser, RepoChangeComputeRequirement, @@ -964,7 +984,8 @@ def repo_registration(ctx, verbose, username, password_file, client_name, dry_ru username=username, password_file=password_file, client_name=client_name or 'sdf-bot-RepoRegistration', - dry_run=dry_run + dry_run=dry_run, + grouper_password_file=grouper_password_file ) handler.run() diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index 5d911a8..0e58215 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -27,7 +27,8 @@ def repo_registration(self) -> RepoRegistration: reg = RepoRegistration( username='test-user', password_file='/tmp/test-password', - client_name='test-client' + client_name='test-client', + grouper_password_file='/tmp/test-grouper-password' ) reg.logger = Mock() reg.back_channel = Mock() @@ -44,17 +45,16 @@ def mock_ansible_runner(self) -> Mock: @pytest.fixture def sample_gid_facts(self) -> dict: - """Sample ansible facts with GID information.""" + """Sample ansible facts returned by grouper.yml 'Export grouper params' task.""" return { 'ansible_facts': { - 'repo_gid': '12345', - 'repo_group_name': 'test-repo-group' + 'gid': '12345', } } def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration, mock_ansible_runner: Mock, sample_gid_facts: dict): - """Test successful GID extraction for CryoEM facility.""" - # Setup + """Test successful GID extraction for CryoEM facility (ct* repo triggers grouper).""" + # Setup — run_playbook called twice: add_repo.yaml then grouper.yml repo_registration.run_playbook.return_value = mock_ansible_runner repo_registration.playbook_task_res.return_value = sample_gid_facts repo_registration.back_channel.execute.side_effect = [ @@ -63,32 +63,40 @@ def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration {'repoUpsertFeature': {'Id': 'feature-posix'}} ] - # Execute + # Execute — repo starts with 'ct' to satisfy grouper condition result = repo_registration.do_new_repo( - repo='cryo-test', - facility='cryoem', # Test case-insensitive matching + repo='ct-test', + facility='cryoem', principal='cryo-user' ) # Verify assert result is True + # grouper.yml should have been invoked + repo_registration.run_playbook.assert_any_call( + 'coact/grouper.yml', + grouper_name='sdf-cryoem-ct-test', + state='present', + grouper_description='POSIX group for cryoem ct-test repository access', + grouper_password_file='/tmp/test-grouper-password', + ) repo_registration.logger.info.assert_any_call( - "Retrieved repo GID for cryoem:cryo-test: 12345" + "Retrieved repo GID for cryoem:ct-test: 12345" ) def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, mock_ansible_runner: Mock): - """Test handling when ansible facts are empty (triggers KeyError).""" + """Test handling when grouper playbook returns empty ansible_facts (triggers KeyError).""" # Setup repo_registration.run_playbook.return_value = mock_ansible_runner - repo_registration.playbook_task_res.return_value = {'ansible_facts': {}} # Empty facts + repo_registration.playbook_task_res.return_value = {'ansible_facts': {}} # Empty facts — KeyError on 'gid' repo_registration.back_channel.execute.side_effect = [ {'repoUpsert': {'Id': 'repo-123'}}, {'repoUpsertFeature': {'Id': 'feature-slurm'}} ] - # Execute + # Execute — repo starts with 'ct' to trigger grouper result = repo_registration.do_new_repo( - repo='test-repo', + repo='ct-repo', facility='cryoem', principal='test-user' ) @@ -96,13 +104,13 @@ def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, m # Verify assert result is True - # Should log warning about extraction failure (KeyError when accessing repo_gid) + # KeyError on 'gid' is caught and logged as warning repo_registration.logger.warning.assert_called_with( - "Failed to extract repo GID for cryoem:test-repo: 'repo_gid'" + "Failed to create grouper POSIX group for cryoem:ct-repo: 'gid'" ) def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_runner): - """Test that non-grouper facilities skip GID extraction.""" + """Test that non-cryoem facilities skip grouper entirely.""" # Setup repo_registration.run_playbook.return_value = mock_ansible_runner repo_registration.back_channel.execute.side_effect = [ @@ -120,8 +128,34 @@ def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_ru # Verify assert result is True - # playbook_task_res should NOT be called + # Only add_repo.yaml should run — grouper.yml should NOT be called + repo_registration.run_playbook.assert_called_once_with( + 'coact/add_repo.yaml', facility='OTHER', repo='test-repo' + ) repo_registration.playbook_task_res.assert_not_called() - # Should only create slurm feature + # Should only create slurm feature (2 back_channel calls: repoUpsert + feature) assert repo_registration.back_channel.execute.call_count == 2 + + def test_cryoem_non_ct_ce_repo_skips_grouper(self, repo_registration, mock_ansible_runner): + """Test that cryoem repos not starting with ct/ce skip grouper.""" + # Setup + repo_registration.run_playbook.return_value = mock_ansible_runner + repo_registration.back_channel.execute.side_effect = [ + {'repoUpsert': {'Id': 'repo-123'}}, + {'repoUpsertFeature': {'Id': 'feature-slurm'}} + ] + + # Execute — repo does not start with ct or ce + result = repo_registration.do_new_repo( + repo='other-repo', + facility='cryoem', + principal='test-user' + ) + + # Verify + assert result is True + repo_registration.run_playbook.assert_called_once_with( + 'coact/add_repo.yaml', facility='cryoem', repo='other-repo' + ) + repo_registration.playbook_task_res.assert_not_called() From 23bb3415e6366c95656bf2e78ada39a077fe22ec Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 26 May 2026 11:14:43 -0700 Subject: [PATCH 05/29] add --grouper-password-file to daemon commands --- coact-facility-overage-daemon.sh | 2 +- coact-reporegistration-daemon.sh | 2 +- coact-userregistration-daemon.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coact-facility-overage-daemon.sh b/coact-facility-overage-daemon.sh index 7124ea1..8386a30 100755 --- a/coact-facility-overage-daemon.sh +++ b/coact-facility-overage-daemon.sh @@ -5,6 +5,6 @@ export SDF_COACT_URI=coact.slac.stanford.edu:443/graphql-service while [ 1 ]; do date - ./venv/bin/python3 ./sdf_click.py coact overage --password-file ./etc/.secrets/password --windows 5 --windows 15 --windows 60 --windows 180 --windows 1440 --verbose --influxdb-url=https://influxdb.slac.stanford.edu:443 + ./venv/bin/python3 ./sdf_click.py coact overage --password-file ./etc/.secrets/password --grouper-password-file ./etc/.secrets/grouper_password --windows 5 --windows 15 --windows 60 --windows 180 --windows 1440 --verbose --influxdb-url=https://influxdb.slac.stanford.edu:443 sleep 300 done diff --git a/coact-reporegistration-daemon.sh b/coact-reporegistration-daemon.sh index 3dfaa86..0204091 100755 --- a/coact-reporegistration-daemon.sh +++ b/coact-reporegistration-daemon.sh @@ -1,6 +1,6 @@ #!/bin/sh while [ 1 ]; do - SDF_COACT_URI=coact.slac.stanford.edu/graphql-service ./venv/bin/python3 ./sdf_click.py coactd reporegistration --username=sdf-bot --password-file=etc/.secrets/password -vv + SDF_COACT_URI=coact.slac.stanford.edu/graphql-service ./venv/bin/python3 ./sdf_click.py coactd reporegistration --username=sdf-bot --password-file=etc/.secrets/password --grouper-password-file ./etc/.secrets/grouper_password -vv sleep 1 done diff --git a/coact-userregistration-daemon.sh b/coact-userregistration-daemon.sh index 6e29712..4e4f3af 100755 --- a/coact-userregistration-daemon.sh +++ b/coact-userregistration-daemon.sh @@ -1,6 +1,6 @@ #!/bin/sh while [ 1 ]; do - SDF_COACT_URI=coact.slac.stanford.edu:443/graphql-service ./venv/bin/python3 ./sdf_click.py coactd userregistration --username sdf-bot --password-file ./etc/.secrets/password -vv + SDF_COACT_URI=coact.slac.stanford.edu:443/graphql-service ./venv/bin/python3 ./sdf_click.py coactd userregistration --username sdf-bot --password-file ./etc/.secrets/password --grouper-password-file ./etc/.secrets/grouper_password -vv sleep 5 done From 001ebaba13536b8b293af9e16f8abe44627e018f Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 10:03:46 -0700 Subject: [PATCH 06/29] only pass grouper password file into repo registration --- modules/coactd.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index 34949fe..c0f2c32 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -232,12 +232,6 @@ def registration_options(f): type=click.Path(exists=True), help='Basic auth password for graphql service' )(f) - f = click.option( - '--grouper-password-file', - default=None, - type=click.Path(exists=True), - help='Path to file containing the Grouper service account password' - )(f) f = click.option( '--client-name', default=None, @@ -970,6 +964,12 @@ def do_feature(self, repo, facility, dry_run: bool = False) -> bool: @coactd.command(name='reporegistration') @common_options @registration_options +@click.option( + '--grouper-password-file', + default=None, + type=click.Path(exists=True), + help='Path to file containing the Grouper service account password' +) @click.pass_context def repo_registration(ctx, verbose, username, password_file, client_name, dry_run, grouper_password_file): """Workflow for repository maintenance. From 2bc8337857d38d9dbe9fe6b4d21a169f19262301 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 10:15:34 -0700 Subject: [PATCH 07/29] query grouper for gid before repo creation --- modules/coactd.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index c0f2c32..899a75d 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -532,9 +532,6 @@ def do_new_repo( if repo_allocation_end_delta is None: repo_allocation_end_delta = pdl.duration(years=5) - # run the facility tasks for this repo - self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) - # For CryoEM repos (ct* / ce*), create a POSIX group via Grouper repo_gid = None repo_group_name = "" @@ -564,6 +561,9 @@ def do_new_repo( except Exception as e: self.logger.warning(f"Failed to create grouper POSIX group for {facility}:{repo}: {e}") + # run the facility tasks for this repo + self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) + leaders = [principal] users = [principal] From 0245dfeaf6e232969f6da8387c87dec691f11ba4 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 11:19:06 -0700 Subject: [PATCH 08/29] feed grouper-generated gid to new repo ansible workflow --- ansible-runner/project | 2 +- modules/coactd.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/ansible-runner/project b/ansible-runner/project index ac1bcee..48a4956 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit ac1bceef6f5c12adaf96c4e49d9f60fe32c6402f +Subproject commit 48a49560c5c700cebe0223e754ec2f31370ae127 diff --git a/modules/coactd.py b/modules/coactd.py index 899a75d..74619e2 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -554,15 +554,23 @@ def do_new_repo( grouper_facts = self.playbook_task_res(grouper_runner, 'Grouper', 'Export grouper params') if grouper_facts and 'ansible_facts' in grouper_facts: repo_gid = grouper_facts['ansible_facts']['gid'] - repo_group_name = grouper_name self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") else: self.logger.warning(f"No GID found in grouper playbook results for {facility}:{repo}") except Exception as e: self.logger.warning(f"Failed to create grouper POSIX group for {facility}:{repo}: {e}") + repo_gid = repo_gid or None + # run the facility tasks for this repo - self.run_playbook("coact/add_repo.yaml", facility=facility, repo=repo) + self.run_playbook( + "coact/add_repo.yaml", + facility=facility, + repo=repo, + principal=principal, + gidNumber=repo_gid, + groupName=grouper_name + ) leaders = [principal] users = [principal] From 2fdcfe79ee69c52129b9c7cb70deb00fc9a4e801 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 11:33:34 -0700 Subject: [PATCH 09/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 48a4956..f7bab81 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 48a49560c5c700cebe0223e754ec2f31370ae127 +Subproject commit f7bab811160678434a0d3429166c859e608d41e7 From ff81d5a0d630f564ba5d35ed6f11869abfd979c5 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 11:42:49 -0700 Subject: [PATCH 10/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index f7bab81..0d66b79 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit f7bab811160678434a0d3429166c859e608d41e7 +Subproject commit 0d66b7979ac09bd264debf473e9e968ce60c9966 From 5f5ad5369565d567433b7fa3de12064c2bd210c9 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 12:26:45 -0700 Subject: [PATCH 11/29] print complete grouper_facts --- ansible-runner/project | 2 +- modules/coactd.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 0d66b79..af24b04 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 0d66b7979ac09bd264debf473e9e968ce60c9966 +Subproject commit af24b04c1ed5d93e7d77c40bc3018af8a1494888 diff --git a/modules/coactd.py b/modules/coactd.py index 74619e2..947ed80 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -552,6 +552,7 @@ def do_new_repo( ) grouper_runner = self.run_playbook("coact/grouper.yml", **grouper_kwargs) grouper_facts = self.playbook_task_res(grouper_runner, 'Grouper', 'Export grouper params') + self.logger.info(f"Grouper playbook facts for {facility}:{repo}: {grouper_facts}") if grouper_facts and 'ansible_facts' in grouper_facts: repo_gid = grouper_facts['ansible_facts']['gid'] self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") From dab3f009a8d691248a1d0d460da76c0df4bafd50 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 12:31:55 -0700 Subject: [PATCH 12/29] aggressively scan ansible_facts --- modules/coactd.py | 74 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 5 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index 947ed80..86f2de8 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -535,6 +535,7 @@ def do_new_repo( # For CryoEM repos (ct* / ce*), create a POSIX group via Grouper repo_gid = None repo_group_name = "" + grouper_name = "" uses_grouper = ( facility.lower() == 'cryoem' and (repo.lower().startswith('ct') or repo.lower().startswith('ce')) @@ -551,10 +552,14 @@ def do_new_repo( grouper_password_file=self.grouper_password_file ) grouper_runner = self.run_playbook("coact/grouper.yml", **grouper_kwargs) - grouper_facts = self.playbook_task_res(grouper_runner, 'Grouper', 'Export grouper params') - self.logger.info(f"Grouper playbook facts for {facility}:{repo}: {grouper_facts}") - if grouper_facts and 'ansible_facts' in grouper_facts: - repo_gid = grouper_facts['ansible_facts']['gid'] + repo_gid, repo_group_name = self.extract_grouper_values( + grouper_runner, + default_group_name=grouper_name + ) + self.logger.info( + f"Retrieved grouper values for {facility}:{repo}: gid={repo_gid}, group_name={repo_group_name}" + ) + if repo_gid is not None: self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") else: self.logger.warning(f"No GID found in grouper playbook results for {facility}:{repo}") @@ -570,7 +575,7 @@ def do_new_repo( repo=repo, principal=principal, gidNumber=repo_gid, - groupName=grouper_name + groupName=repo_group_name ) leaders = [principal] @@ -637,6 +642,65 @@ def do_new_repo( return True + def extract_grouper_values(self, runner: ansible_runner.runner.Runner, default_group_name: str = ""): + """Extract gid and group name from grouper playbook events. + + Ansible callbacks can emit task results in different shapes depending on + task type (`s3df_grouper`, `debug`, `set_fact`). This method scans all + events and picks the first non-empty gid and group name found. + """ + + def _clean(v): + if v is None: + return None + if isinstance(v, str): + v = v.strip() + return v if v else None + return str(v) + + gid = None + group_name = _clean(default_group_name) + + for event_data in self.playbook_events(runner): + res = event_data.get('res', None) + if not isinstance(res, dict): + continue + + facts = res.get('ansible_facts', {}) + facts = facts if isinstance(facts, dict) else {} + msg = res.get('msg', None) + msg = msg if isinstance(msg, dict) else {} + + candidates_gid = [ + res.get('gid', None), + facts.get('gid', None), + msg.get('gid', None), + (res.get('group', {}) if isinstance(res.get('group', {}), dict) else {}).get('idIndex', None), + ] + candidates_group_name = [ + res.get('group_name', None), + facts.get('group_name', None), + msg.get('group_name', None), + (res.get('group', {}) if isinstance(res.get('group', {}), dict) else {}).get('name', None), + ] + + for candidate in candidates_gid: + cleaned = _clean(candidate) + if cleaned is not None: + gid = cleaned + break + + for candidate in candidates_group_name: + cleaned = _clean(candidate) + if cleaned is not None: + group_name = cleaned + break + + if gid is not None and group_name is not None: + break + + return gid, group_name or "" + def upsert_repo_compute_allocation( self, repo_id: str, From 5066773e07403c28ddeb59137df692c107d72ca7 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 12:39:40 -0700 Subject: [PATCH 13/29] done pass fqn to posix group creation --- modules/coactd.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index 86f2de8..47d3622 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -534,7 +534,6 @@ def do_new_repo( # For CryoEM repos (ct* / ce*), create a POSIX group via Grouper repo_gid = None - repo_group_name = "" grouper_name = "" uses_grouper = ( facility.lower() == 'cryoem' @@ -552,12 +551,12 @@ def do_new_repo( grouper_password_file=self.grouper_password_file ) grouper_runner = self.run_playbook("coact/grouper.yml", **grouper_kwargs) - repo_gid, repo_group_name = self.extract_grouper_values( + repo_gid, __dict__ = self.extract_grouper_values( grouper_runner, default_group_name=grouper_name ) self.logger.info( - f"Retrieved grouper values for {facility}:{repo}: gid={repo_gid}, group_name={repo_group_name}" + f"Retrieved grouper values for {facility}:{repo}: gid={repo_gid}" ) if repo_gid is not None: self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") @@ -575,7 +574,7 @@ def do_new_repo( repo=repo, principal=principal, gidNumber=repo_gid, - groupName=repo_group_name + groupName=grouper_name ) leaders = [principal] From 404260cfa6edb0e1a0be0e4c4cc36c8f4fa4ba4f Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 12:42:38 -0700 Subject: [PATCH 14/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index af24b04..f4ab346 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit af24b04c1ed5d93e7d77c40bc3018af8a1494888 +Subproject commit f4ab3468e11ada52d88e235a83e5b2766b723a40 From 11e46a0236fb5e8179ea9040bf238e3ef075d72d Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Fri, 29 May 2026 12:44:48 -0700 Subject: [PATCH 15/29] missed repo_group_name --- modules/coactd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/coactd.py b/modules/coactd.py index 47d3622..5aa842e 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -620,7 +620,7 @@ def do_new_repo( # Create posixgroup feature if GID was obtained from grouper if repo_gid is not None: posixgroup_options = [json.dumps({ - "name": repo_group_name, + "name": grouper_name, "gidNumber": int(repo_gid) })] From a5f116d99de56683ec008ffb99f4083391af1d41 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 12:16:02 -0700 Subject: [PATCH 16/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index f4ab346..b46d057 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit f4ab3468e11ada52d88e235a83e5b2766b723a40 +Subproject commit b46d057d231eda8b7c51f8cc4f654841b87534a4 From 6943b6d91b53de5bd19183a6aa650bfc92de7167 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 12:21:26 -0700 Subject: [PATCH 17/29] update tests ot hanel empty runbook values --- tests/test_repo_registration_gid.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index 0e58215..e47bb14 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -56,7 +56,8 @@ def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration """Test successful GID extraction for CryoEM facility (ct* repo triggers grouper).""" # Setup — run_playbook called twice: add_repo.yaml then grouper.yml repo_registration.run_playbook.return_value = mock_ansible_runner - repo_registration.playbook_task_res.return_value = sample_gid_facts + # Mock extract_grouper_values to return the GID + repo_registration.extract_grouper_values = Mock(return_value=('12345', 'sdf-cryoem-ct-test')) repo_registration.back_channel.execute.side_effect = [ {'repoUpsert': {'Id': 'repo-456'}}, {'repoUpsertFeature': {'Id': 'feature-slurm'}}, @@ -85,10 +86,11 @@ def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration ) def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, mock_ansible_runner: Mock): - """Test handling when grouper playbook returns empty ansible_facts (triggers KeyError).""" + """Test handling when grouper playbook returns empty ansible_facts (no GID found).""" # Setup repo_registration.run_playbook.return_value = mock_ansible_runner - repo_registration.playbook_task_res.return_value = {'ansible_facts': {}} # Empty facts — KeyError on 'gid' + # Mock extract_grouper_values to return None for GID (empty facts scenario) + repo_registration.extract_grouper_values = Mock(return_value=(None, 'sdf-cryoem-ct-repo')) repo_registration.back_channel.execute.side_effect = [ {'repoUpsert': {'Id': 'repo-123'}}, {'repoUpsertFeature': {'Id': 'feature-slurm'}} @@ -104,9 +106,9 @@ def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, m # Verify assert result is True - # KeyError on 'gid' is caught and logged as warning + # When GID is None, it should log "No GID found..." repo_registration.logger.warning.assert_called_with( - "Failed to create grouper POSIX group for cryoem:ct-repo: 'gid'" + "No GID found in grouper playbook results for cryoem:ct-repo" ) def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_runner): @@ -129,10 +131,13 @@ def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_ru assert result is True # Only add_repo.yaml should run — grouper.yml should NOT be called + # The new implementation passes principal, gidNumber=None, and groupName='' repo_registration.run_playbook.assert_called_once_with( - 'coact/add_repo.yaml', facility='OTHER', repo='test-repo' + 'coact/add_repo.yaml', facility='OTHER', repo='test-repo', principal='test-user', gidNumber=None, groupName='' ) - repo_registration.playbook_task_res.assert_not_called() + # extract_grouper_values should not be called for non-grouper facilities + if hasattr(repo_registration, 'extract_grouper_values') and isinstance(repo_registration.extract_grouper_values, Mock): + repo_registration.extract_grouper_values.assert_not_called() # Should only create slurm feature (2 back_channel calls: repoUpsert + feature) assert repo_registration.back_channel.execute.call_count == 2 @@ -155,7 +160,10 @@ def test_cryoem_non_ct_ce_repo_skips_grouper(self, repo_registration, mock_ansib # Verify assert result is True + # The new implementation passes principal, gidNumber=None, and groupName='' repo_registration.run_playbook.assert_called_once_with( - 'coact/add_repo.yaml', facility='cryoem', repo='other-repo' + 'coact/add_repo.yaml', facility='cryoem', repo='other-repo', principal='test-user', gidNumber=None, groupName='' ) - repo_registration.playbook_task_res.assert_not_called() + # extract_grouper_values should not be called since this repo doesn't match the pattern + if hasattr(repo_registration, 'extract_grouper_values') and isinstance(repo_registration.extract_grouper_values, Mock): + repo_registration.extract_grouper_values.assert_not_called() From fba5af6e7f7005b88deaf336ec6950bdcd1e05a5 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 12:36:32 -0700 Subject: [PATCH 18/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index b46d057..0c07435 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit b46d057d231eda8b7c51f8cc4f654841b87534a4 +Subproject commit 0c074351c77ed47698c2e4cc618f7f3414552752 From c92c01ee7e47b0ec5d27682dcf3cbd50bff7ff6f Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 13:56:06 -0700 Subject: [PATCH 19/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 0c07435..3b4ccd1 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 0c074351c77ed47698c2e4cc618f7f3414552752 +Subproject commit 3b4ccd164356fe571c3fc3bd49ed0a46c65f1490 From 3f3dc727209df012ea244e262722ea28cd38bc6a Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:31:12 -0700 Subject: [PATCH 20/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 3b4ccd1..96b4016 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 3b4ccd164356fe571c3fc3bd49ed0a46c65f1490 +Subproject commit 96b4016ed2fa2a166508f0c1e6b648a5673a66a5 From 1eaee68ecb60c46da4043ceaecf45a7a043d89f1 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:34:35 -0700 Subject: [PATCH 21/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 96b4016..29ea221 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 96b4016ed2fa2a166508f0c1e6b648a5673a66a5 +Subproject commit 29ea221a5cac7b0388213f5097b64ded61ec8434 From f2c12c6de5d6cf57c04229a16ca547d1df16cbb7 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:40:43 -0700 Subject: [PATCH 22/29] principal -> repo_principal to avoid naming conflict with ansible-role-ldap-auth --- ansible-runner/project | 2 +- modules/coactd.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible-runner/project b/ansible-runner/project index 29ea221..62fe30b 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 29ea221a5cac7b0388213f5097b64ded61ec8434 +Subproject commit 62fe30bdf154bfd7b212df4eb2525d941c668f49 diff --git a/modules/coactd.py b/modules/coactd.py index 5aa842e..746e7fe 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -572,7 +572,7 @@ def do_new_repo( "coact/add_repo.yaml", facility=facility, repo=repo, - principal=principal, + repo_principal=principal, gidNumber=repo_gid, groupName=grouper_name ) From dee10544ec20d8f2dc1785962ae70c36a60f7792 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:43:41 -0700 Subject: [PATCH 23/29] update submodule --- ansible-runner/project | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ansible-runner/project b/ansible-runner/project index 62fe30b..e48b1a2 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit 62fe30bdf154bfd7b212df4eb2525d941c668f49 +Subproject commit e48b1a2299d1c568d6c2640f189e6d38c6fad40e From a818953e16104ce7b37da759f6cbcaae01794d1f Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:45:14 -0700 Subject: [PATCH 24/29] principal -> repo_principal in tests --- tests/test_repo_registration_gid.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index e47bb14..3b845e8 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -68,7 +68,7 @@ def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration result = repo_registration.do_new_repo( repo='ct-test', facility='cryoem', - principal='cryo-user' + repo_principal='cryo-user' ) # Verify @@ -100,7 +100,7 @@ def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, m result = repo_registration.do_new_repo( repo='ct-repo', facility='cryoem', - principal='test-user' + repo_principal='test-user' ) # Verify @@ -124,16 +124,16 @@ def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_ru result = repo_registration.do_new_repo( repo='test-repo', facility='OTHER', # Not CryoEM - principal='test-user' + repo_principal='test-user' ) # Verify assert result is True # Only add_repo.yaml should run — grouper.yml should NOT be called - # The new implementation passes principal, gidNumber=None, and groupName='' + # The new implementation passes repo_principal, gidNumber=None, and groupName='' repo_registration.run_playbook.assert_called_once_with( - 'coact/add_repo.yaml', facility='OTHER', repo='test-repo', principal='test-user', gidNumber=None, groupName='' + 'coact/add_repo.yaml', facility='OTHER', repo='test-repo', repo_principal='test-user', gidNumber=None, groupName='' ) # extract_grouper_values should not be called for non-grouper facilities if hasattr(repo_registration, 'extract_grouper_values') and isinstance(repo_registration.extract_grouper_values, Mock): @@ -155,14 +155,14 @@ def test_cryoem_non_ct_ce_repo_skips_grouper(self, repo_registration, mock_ansib result = repo_registration.do_new_repo( repo='other-repo', facility='cryoem', - principal='test-user' + repo_principal='test-user' ) # Verify assert result is True - # The new implementation passes principal, gidNumber=None, and groupName='' + # The new implementation passes repo_principal, gidNumber=None, and groupName='' repo_registration.run_playbook.assert_called_once_with( - 'coact/add_repo.yaml', facility='cryoem', repo='other-repo', principal='test-user', gidNumber=None, groupName='' + 'coact/add_repo.yaml', facility='cryoem', repo='other-repo', repo_principal='test-user', gidNumber=None, groupName='' ) # extract_grouper_values should not be called since this repo doesn't match the pattern if hasattr(repo_registration, 'extract_grouper_values') and isinstance(repo_registration.extract_grouper_values, Mock): From 3d8a2899cbdfac64abba7dc4f33987ba297be5c9 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Tue, 2 Jun 2026 14:46:59 -0700 Subject: [PATCH 25/29] wrong principal corrected --- tests/test_repo_registration_gid.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index 3b845e8..aaf0ac0 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -68,7 +68,7 @@ def test_gid_extraction_success_cryoem(self, repo_registration: RepoRegistration result = repo_registration.do_new_repo( repo='ct-test', facility='cryoem', - repo_principal='cryo-user' + principal='cryo-user' ) # Verify @@ -100,7 +100,7 @@ def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, m result = repo_registration.do_new_repo( repo='ct-repo', facility='cryoem', - repo_principal='test-user' + principal='test-user' ) # Verify @@ -124,7 +124,7 @@ def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_ru result = repo_registration.do_new_repo( repo='test-repo', facility='OTHER', # Not CryoEM - repo_principal='test-user' + principal='test-user' ) # Verify @@ -155,7 +155,7 @@ def test_cryoem_non_ct_ce_repo_skips_grouper(self, repo_registration, mock_ansib result = repo_registration.do_new_repo( repo='other-repo', facility='cryoem', - repo_principal='test-user' + principal='test-user' ) # Verify From 659f495e692bdd6169ff92a39e6dbd852c08819e Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 4 Jun 2026 11:17:56 -0700 Subject: [PATCH 26/29] error if gid fetching failed --- modules/coactd.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/coactd.py b/modules/coactd.py index 746e7fe..63f19d3 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -565,7 +565,8 @@ def do_new_repo( except Exception as e: self.logger.warning(f"Failed to create grouper POSIX group for {facility}:{repo}: {e}") - repo_gid = repo_gid or None + if repo_gid is None: + raise RuntimeError("Unanable to fetch gid from grouper.") # run the facility tasks for this repo self.run_playbook( From 7eb69e9da9b0aad83b1bacc20335751f0d507631 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 4 Jun 2026 11:24:47 -0700 Subject: [PATCH 27/29] raise if fail to fetch grouper --- modules/coactd.py | 5 ++--- tests/test_repo_registration_gid.py | 19 +++++++++---------- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/modules/coactd.py b/modules/coactd.py index 63f19d3..de7bad4 100644 --- a/modules/coactd.py +++ b/modules/coactd.py @@ -562,11 +562,10 @@ def do_new_repo( self.logger.info(f"Retrieved repo GID for {facility}:{repo}: {repo_gid}") else: self.logger.warning(f"No GID found in grouper playbook results for {facility}:{repo}") + raise RuntimeError("Unable to fetch gid from grouper.") except Exception as e: self.logger.warning(f"Failed to create grouper POSIX group for {facility}:{repo}: {e}") - - if repo_gid is None: - raise RuntimeError("Unanable to fetch gid from grouper.") + raise # run the facility tasks for this repo self.run_playbook( diff --git a/tests/test_repo_registration_gid.py b/tests/test_repo_registration_gid.py index aaf0ac0..659d07d 100644 --- a/tests/test_repo_registration_gid.py +++ b/tests/test_repo_registration_gid.py @@ -97,18 +97,17 @@ def test_gid_extraction_empty_facts(self, repo_registration: RepoRegistration, m ] # Execute — repo starts with 'ct' to trigger grouper - result = repo_registration.do_new_repo( - repo='ct-repo', - facility='cryoem', - principal='test-user' - ) - - # Verify - assert result is True + # This should now raise a RuntimeError when GID is None + with pytest.raises(RuntimeError, match="Unable to fetch gid from grouper"): + repo_registration.do_new_repo( + repo='ct-repo', + facility='cryoem', + principal='test-user' + ) - # When GID is None, it should log "No GID found..." + # Verify the exception was logged by the outer exception handler repo_registration.logger.warning.assert_called_with( - "No GID found in grouper playbook results for cryoem:ct-repo" + "Failed to create grouper POSIX group for cryoem:ct-repo: Unable to fetch gid from grouper." ) def test_non_grouper_facility_skips_gid(self, repo_registration, mock_ansible_runner): From 2f8693a9f97acd648c22c94e9bcb1409592907a5 Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 4 Jun 2026 13:17:29 -0700 Subject: [PATCH 28/29] update submodule + cap gql version --- ansible-runner/project | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ansible-runner/project b/ansible-runner/project index e48b1a2..5f3c7c6 160000 --- a/ansible-runner/project +++ b/ansible-runner/project @@ -1 +1 @@ -Subproject commit e48b1a2299d1c568d6c2640f189e6d38c6fad40e +Subproject commit 5f3c7c6741626a859ca509af8be55dc0ca340c07 diff --git a/pyproject.toml b/pyproject.toml index 29f26e6..42c3731 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ requires-python = ">=3.12,<3.13" dependencies = [ "cliff==3.10.1", "click>=8.0.0", - "gql[all]>=3.4.1", + "gql[all]>=3.4.1,<4", "ansible-runner==2.3.1", "Jinja2>=3.1.2", "pendulum>=3.2.0", From 86ce77881bfbcc6c08b85a0a7c3e2306c6cd111f Mon Sep 17 00:00:00 2001 From: Ryan Waldheim Date: Thu, 4 Jun 2026 13:22:12 -0700 Subject: [PATCH 29/29] update uv.lock --- uv.lock | 45 ++++++++++++--------------------------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/uv.lock b/uv.lock index f24215a..c1c5e12 100644 --- a/uv.lock +++ b/uv.lock @@ -2,15 +2,6 @@ version = 1 revision = 3 requires-python = "==3.12.*" -[[package]] -name = "aiofiles" -version = "25.1.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/41/c3/534eac40372d8ee36ef40df62ec129bee4fdb5ad9706e58a29be53b2c970/aiofiles-25.1.0.tar.gz", hash = "sha256:a8d728f0a29de45dc521f18f07297428d56992a742f0cd2701ba86e44d23d5b2", size = 46354, upload-time = "2025-10-09T20:51:04.358Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/bc/8a/340a1555ae33d7354dbca4faa54948d76d89a27ceef032c8c3bc661d003e/aiofiles-25.1.0-py3-none-any.whl", hash = "sha256:abe311e527c862958650f9438e859c1fa7568a141b22abcd015e120e86a85695", size = 14668, upload-time = "2025-10-09T20:51:03.174Z" }, -] - [[package]] name = "aiohappyeyeballs" version = "2.6.1" @@ -263,7 +254,7 @@ wheels = [ [[package]] name = "gql" -version = "4.0.0" +version = "3.5.3" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "anyio" }, @@ -271,14 +262,13 @@ dependencies = [ { name = "graphql-core" }, { name = "yarl" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/06/9f/cf224a88ed71eb223b7aa0b9ff0aa10d7ecc9a4acdca2279eb046c26d5dc/gql-4.0.0.tar.gz", hash = "sha256:f22980844eb6a7c0266ffc70f111b9c7e7c7c13da38c3b439afc7eab3d7c9c8e", size = 215644, upload-time = "2025-08-17T14:32:35.397Z" } +sdist = { url = "https://files.pythonhosted.org/packages/34/ed/44ffd30b06b3afc8274ee2f38c3c1b61fe4740bf03d92083e43d2c17ac77/gql-3.5.3.tar.gz", hash = "sha256:393b8c049d58e0d2f5461b9d738a2b5f904186a40395500b4a84dd092d56e42b", size = 180504, upload-time = "2025-05-20T12:34:08.954Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/ac/94/30bbd09e8d45339fa77a48f5778d74d47e9242c11b3cd1093b3d994770a5/gql-4.0.0-py3-none-any.whl", hash = "sha256:f3beed7c531218eb24d97cb7df031b4a84fdb462f4a2beb86e2633d395937479", size = 89900, upload-time = "2025-08-17T14:32:34.029Z" }, + { url = "https://files.pythonhosted.org/packages/cb/50/2f4e99b216821ac921dbebf91c644ba95818f5d07857acadee17220221f3/gql-3.5.3-py2.py3-none-any.whl", hash = "sha256:e1fcbde2893fcafdd28114ece87ff47f1cc339a31db271fc4e1d528f5a1d4fbc", size = 74348, upload-time = "2025-05-20T12:34:07.687Z" }, ] [package.optional-dependencies] all = [ - { name = "aiofiles" }, { name = "aiohttp" }, { name = "botocore" }, { name = "httpx" }, @@ -289,11 +279,11 @@ all = [ [[package]] name = "graphql-core" -version = "3.2.8" +version = "3.2.6" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/68/c5/36aa96205c3ecbb3d34c7c24189e4553c7ca2ebc7e1dd07432339b980272/graphql_core-3.2.8.tar.gz", hash = "sha256:015457da5d996c924ddf57a43f4e959b0b94fb695b85ed4c29446e508ed65cf3", size = 513181, upload-time = "2026-03-05T19:55:37.332Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c4/16/7574029da84834349b60ed71614d66ca3afe46e9bf9c7b9562102acb7d4f/graphql_core-3.2.6.tar.gz", hash = "sha256:c08eec22f9e40f0bd61d805907e3b3b1b9a320bc606e23dc145eebca07c8fbab", size = 505353, upload-time = "2025-01-26T16:36:27.374Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/86/41/cb887d9afc5dabd78feefe6ccbaf83ff423c206a7a1b7aeeac05120b2125/graphql_core-3.2.8-py3-none-any.whl", hash = "sha256:cbee07bee1b3ed5e531723685369039f32ff815ef60166686e0162f540f1520c", size = 207349, upload-time = "2026-03-05T19:55:35.911Z" }, + { url = "https://files.pythonhosted.org/packages/ae/4f/7297663840621022bc73c22d7d9d80dbc78b4db6297f764b545cd5dd462d/graphql_core-3.2.6-py3-none-any.whl", hash = "sha256:78b016718c161a6fb20a7d97bbf107f331cd1afe53e45566c59f776ed7f0b45f", size = 203416, upload-time = "2025-01-26T16:36:24.868Z" }, ] [[package]] @@ -756,7 +746,7 @@ requires-dist = [ { name = "ansible-runner", specifier = "==2.3.1" }, { name = "click", specifier = ">=8.0.0" }, { name = "cliff", specifier = "==3.10.1" }, - { name = "gql", extras = ["all"], specifier = ">=3.4.1" }, + { name = "gql", extras = ["all"], specifier = ">=3.4.1,<4" }, { name = "jinja2", specifier = ">=3.1.2" }, { name = "loguru", specifier = ">=0.7.0" }, { name = "pendulum", specifier = ">=3.2.0" }, @@ -843,22 +833,11 @@ wheels = [ [[package]] name = "websockets" -version = "15.0.1" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/21/e6/26d09fab466b7ca9c7737474c52be4f76a40301b08362eb2dbc19dcc16c1/websockets-15.0.1.tar.gz", hash = "sha256:82544de02076bafba038ce055ee6412d68da13ab47f0c60cab827346de828dee", size = 177016, upload-time = "2025-03-05T20:03:41.606Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/51/6b/4545a0d843594f5d0771e86463606a3988b5a09ca5123136f8a76580dd63/websockets-15.0.1-cp312-cp312-macosx_10_13_universal2.whl", hash = "sha256:3e90baa811a5d73f3ca0bcbf32064d663ed81318ab225ee4f427ad4e26e5aff3", size = 175437, upload-time = "2025-03-05T20:02:16.706Z" }, - { url = "https://files.pythonhosted.org/packages/f4/71/809a0f5f6a06522af902e0f2ea2757f71ead94610010cf570ab5c98e99ed/websockets-15.0.1-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:592f1a9fe869c778694f0aa806ba0374e97648ab57936f092fd9d87f8bc03665", size = 173096, upload-time = "2025-03-05T20:02:18.832Z" }, - { url = "https://files.pythonhosted.org/packages/3d/69/1a681dd6f02180916f116894181eab8b2e25b31e484c5d0eae637ec01f7c/websockets-15.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:0701bc3cfcb9164d04a14b149fd74be7347a530ad3bbf15ab2c678a2cd3dd9a2", size = 173332, upload-time = "2025-03-05T20:02:20.187Z" }, - { url = "https://files.pythonhosted.org/packages/a6/02/0073b3952f5bce97eafbb35757f8d0d54812b6174ed8dd952aa08429bcc3/websockets-15.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:e8b56bdcdb4505c8078cb6c7157d9811a85790f2f2b3632c7d1462ab5783d215", size = 183152, upload-time = "2025-03-05T20:02:22.286Z" }, - { url = "https://files.pythonhosted.org/packages/74/45/c205c8480eafd114b428284840da0b1be9ffd0e4f87338dc95dc6ff961a1/websockets-15.0.1-cp312-cp312-manylinux_2_5_i686.manylinux1_i686.manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:0af68c55afbd5f07986df82831c7bff04846928ea8d1fd7f30052638788bc9b5", size = 182096, upload-time = "2025-03-05T20:02:24.368Z" }, - { url = "https://files.pythonhosted.org/packages/14/8f/aa61f528fba38578ec553c145857a181384c72b98156f858ca5c8e82d9d3/websockets-15.0.1-cp312-cp312-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:64dee438fed052b52e4f98f76c5790513235efaa1ef7f3f2192c392cd7c91b65", size = 182523, upload-time = "2025-03-05T20:02:25.669Z" }, - { url = "https://files.pythonhosted.org/packages/ec/6d/0267396610add5bc0d0d3e77f546d4cd287200804fe02323797de77dbce9/websockets-15.0.1-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:d5f6b181bb38171a8ad1d6aa58a67a6aa9d4b38d0f8c5f496b9e42561dfc62fe", size = 182790, upload-time = "2025-03-05T20:02:26.99Z" }, - { url = "https://files.pythonhosted.org/packages/02/05/c68c5adbf679cf610ae2f74a9b871ae84564462955d991178f95a1ddb7dd/websockets-15.0.1-cp312-cp312-musllinux_1_2_i686.whl", hash = "sha256:5d54b09eba2bada6011aea5375542a157637b91029687eb4fdb2dab11059c1b4", size = 182165, upload-time = "2025-03-05T20:02:30.291Z" }, - { url = "https://files.pythonhosted.org/packages/29/93/bb672df7b2f5faac89761cb5fa34f5cec45a4026c383a4b5761c6cea5c16/websockets-15.0.1-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:3be571a8b5afed347da347bfcf27ba12b069d9d7f42cb8c7028b5e98bbb12597", size = 182160, upload-time = "2025-03-05T20:02:31.634Z" }, - { url = "https://files.pythonhosted.org/packages/ff/83/de1f7709376dc3ca9b7eeb4b9a07b4526b14876b6d372a4dc62312bebee0/websockets-15.0.1-cp312-cp312-win32.whl", hash = "sha256:c338ffa0520bdb12fbc527265235639fb76e7bc7faafbb93f6ba80d9c06578a9", size = 176395, upload-time = "2025-03-05T20:02:33.017Z" }, - { url = "https://files.pythonhosted.org/packages/7d/71/abf2ebc3bbfa40f391ce1428c7168fb20582d0ff57019b69ea20fa698043/websockets-15.0.1-cp312-cp312-win_amd64.whl", hash = "sha256:fcd5cf9e305d7b8338754470cf69cf81f420459dbae8a3b40cee57417f4614a7", size = 176841, upload-time = "2025-03-05T20:02:34.498Z" }, - { url = "https://files.pythonhosted.org/packages/fa/a8/5b41e0da817d64113292ab1f8247140aac61cbf6cfd085d6a0fa77f4984f/websockets-15.0.1-py3-none-any.whl", hash = "sha256:f7a866fbc1e97b5c617ee4116daaa09b722101d4a3c170c787450ba409f9736f", size = 169743, upload-time = "2025-03-05T20:03:39.41Z" }, +version = "11.0.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/d8/3b/2ed38e52eed4cf277f9df5f0463a99199a04d9e29c9e227cfafa57bd3993/websockets-11.0.3.tar.gz", hash = "sha256:88fc51d9a26b10fc331be344f1781224a375b78488fc343620184e95a4b27016", size = 104235, upload-time = "2023-05-07T14:25:20.083Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/47/96/9d5749106ff57629b54360664ae7eb9afd8302fad1680ead385383e33746/websockets-11.0.3-py3-none-any.whl", hash = "sha256:6681ba9e7f8f3b19440921e99efbb40fc89f26cd71bf539e45d8c8a25c976dc6", size = 118056, upload-time = "2023-05-07T14:25:18.508Z" }, ] [[package]]