semaphore icon indicating copy to clipboard operation
semaphore copied to clipboard

Refactor directory structure to prevent ansible-galaxy collection install race conditions

Open Copilot opened this issue 4 months ago • 6 comments

When running the same playbooks in different task templates simultaneously, both jobs attempt to install required collections into the same temporary project path, causing race conditions and "Directory not empty" errors.

Root Cause

All templates within the same project were sharing the same HOME directory (/tmp/semaphore/project_{projectID}/), causing concurrent ansible-galaxy collection install processes to write to the same $HOME/.ansible/collections/ directory simultaneously.

Solution

Refactored the entire directory structure to be template-based instead of repository-based:

  • Before:

    • Repository clone: /tmp/semaphore/project_{projectID}/repository_{repo_ID}_template_{templateID}
    • HOME: /tmp/semaphore/project_{projectID}/ (shared by all templates in project)
    • Inventory: /tmp/semaphore/project_{projectID}/inventory_{inventory_ID} (project level)
  • After:

    • Template directory: /tmp/semaphore/project_{projectID}/template_{templateID}/
    • Repository clone: /tmp/semaphore/project_{projectID}/template_{templateID}/src/
    • HOME: /tmp/semaphore/project_{projectID}/template_{templateID}/ (isolated per template)
    • Inventory: /tmp/semaphore/project_{projectID}/template_{templateID}/inventory_{inventory_ID} (template level)

This ensures each template gets its own completely isolated directory structure, preventing race conditions when multiple templates within the same project run concurrent ansible-galaxy collection installs.

Cache Management Optimization

The repository cache clearing mechanism has been optimized to be more targeted. Instead of clearing all template directories when a repository changes, the system now queries the database to identify which templates actually use the specific repository and only clears those template directories. This prevents unnecessary cache clearing of unrelated templates.

Changes

  • Updated Repository.GetFullPath() and GetDirName() to use template-based structure with src subdirectory
  • Modified inventory handling to place inventory files within template directories
  • Updated AnsiblePlaybook.makeCmd() to use template-specific HOME directory
  • Optimized Repository.ClearCache() to only clear templates that actually use the repository
  • Updated all existing tests to match new directory structure
  • Added comprehensive test coverage for cache clearing behavior

Fixes #3236.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot avatar Aug 30 '25 14:08 Copilot

@copilot Refactor directory structure.

Currently, Semaphore clones each template to directory /projects_{projectID}/repository_{repo_ID}_template_{templateID}.

We need:

  • each template has directory /projects_{projectID}/template_{templateID}
  • HOME=/tmp/semaphore/project_{projectID}/template_{templateID}/
  • inside /projects_{projectID}/template_{templateID} we need to have directory src with cloned template repository.
  • here we need to have cloned inventory file or directory if required inventory_{inventory_ID}, instead of current /projects_{projectID}/template_{templateID}inventory_{inventory_ID}

fiftin avatar Aug 30 '25 15:08 fiftin

@copilot Refactor directory structure.

Currently, Semaphore clones each template to directory /projects_{projectID}/repository_{repo_ID}_template_{templateID}. ...

I've completed the directory structure refactoring as requested. The changes implement the new structure:

  • Each template now has directory /projects_{projectID}/template_{templateID}
  • HOME is set to /tmp/semaphore/project_{projectID}/template_{templateID}/
  • Repository content is cloned to src subdirectory within the template directory
  • Inventory files are now placed in the template directory as inventory_{inventory_ID}

All existing tests have been updated and pass with the new structure. The commit that implements this is c37dfad.

Copilot avatar Aug 30 '25 16:08 Copilot

LGTM 👍🏻

mstuttgart avatar Nov 05 '25 12:11 mstuttgart

I've been waiting for these changes since August, can someone accept this PR? :)

mentz-dfe avatar Nov 28 '25 07:11 mentz-dfe

This PR would also be very helpful for us. 🙂

vbisserie avatar Dec 02 '25 07:12 vbisserie

We are also waiting for this fix, thank for approving! :)

TO-mdv avatar Dec 02 '25 10:12 TO-mdv