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

api/projects/keys.go

@@ -143,7 +143,7 @@ func (c *KeyController) UpdateKey(w http.ResponseWriter, r *http.Request) {
 		if repo.SSHKeyID != key.ID {
 			continue
 		}
-		err = repo.ClearCache()
+		err = repo.ClearCache(helpers.Store(r))
 		if err != nil {
 			helpers.WriteError(w, err)
 			return

api/projects/repository.go

@@ -169,7 +169,7 @@ func UpdateRepository(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if oldRepo.GitURL != repository.GitURL {
-		util.LogWarning(oldRepo.ClearCache())
+		util.LogWarning(oldRepo.ClearCache(helpers.Store(r)))
 	}
 
 	helpers.EventLog(r, helpers.EventLogUpdate, helpers.EventLogItem{
@@ -201,7 +201,7 @@ func RemoveRepository(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	util.LogWarning(repository.ClearCache())
+	util.LogWarning(repository.ClearCache(helpers.Store(r)))
 
 	helpers.EventLog(r, helpers.EventLogDelete, helpers.EventLogItem{
 		UserID:      helpers.UserFromContext(r).ID,

db/Repository.go

@@ -2,6 +2,7 @@ package db
 
 import (
 	"fmt"
+	"os"
 	"path"
 	"regexp"
 	"strconv"
@@ -32,16 +33,33 @@ type Repository struct {
 	SSHKey AccessKey `db:"-" json:"-" backup:"-"`
 }
 
-func (r Repository) ClearCache() error {
-	return util.ClearDir(util.Config.GetProjectTmpDir(r.ProjectID), true, r.getDirNamePrefix())
+func (r Repository) ClearCache(templateManager TemplateManager) error {
+	// Find templates that use this repository and clear only their cache directories
+	templates, err := templateManager.GetTemplates(r.ProjectID, TemplateFilter{}, RetrieveQueryParams{})
+	if err != nil {
+		return err
+	}
+
+	// Clear cache only for templates that use this repository
+	for _, template := range templates {
+		if template.RepositoryID == r.ID {
+			templateDir := path.Join(util.Config.GetProjectTmpDir(r.ProjectID), "template_" + strconv.Itoa(template.ID))
+			err := os.RemoveAll(templateDir)
+			if err != nil && !os.IsNotExist(err) {
+				return err
+			}
+		}
+	}
+
+	return nil
 }
 
 func (r Repository) getDirNamePrefix() string {
 	return "repository_" + strconv.Itoa(r.ID) + "_"
 }
 
 func (r Repository) GetDirName(templateID int) string {
-	return r.getDirNamePrefix() + "template_" + strconv.Itoa(templateID)
+	return path.Join("template_" + strconv.Itoa(templateID), "src")
 }
 
 func (r Repository) GetFullPath(templateID int) string {

db/Repository_test.go

@@ -21,19 +21,101 @@ func TestRepository_ClearCache(t *testing.T) {
 	util.Config = &util.ConfigType{
 		TmpPath: path.Join(os.TempDir(), util.RandString(rand.Intn(10-4)+4)),
 	}
-	repoDir := path.Join(util.Config.TmpPath, "project_0", "repository_123_55")
+	repoDir := path.Join(util.Config.TmpPath, "project_0", "template_55")
 	err := os.MkdirAll(repoDir, 0755)
 	require.NoError(t, err)
 
-	repo := Repository{ID: 123}
-	err = repo.ClearCache()
+	// Create a mock template manager that returns a template using this repository
+	mockTemplateManager := &mockTemplateManager{
+		templates: []Template{
+			{ID: 55, ProjectID: 0, RepositoryID: 123},
+		},
+	}
+
+	repo := Repository{ID: 123, ProjectID: 0}
+	err = repo.ClearCache(mockTemplateManager)
 	require.NoError(t, err)
 
 	_, err = os.Stat(repoDir)
 	require.Error(t, err, "repo directory not deleted")
 	assert.True(t, os.IsNotExist(err))
 }
 
+func TestRepository_ClearCache_NoTemplatesUsingRepo(t *testing.T) {
+	util.Config = &util.ConfigType{
+		TmpPath: path.Join(os.TempDir(), util.RandString(rand.Intn(10-4)+4)),
+	}
+	repoDir := path.Join(util.Config.TmpPath, "project_0", "template_55")
+	err := os.MkdirAll(repoDir, 0755)
+	require.NoError(t, err)
+
+	// Create a mock template manager with no templates using this repository
+	mockTemplateManager := &mockTemplateManager{
+		templates: []Template{
+			{ID: 55, ProjectID: 0, RepositoryID: 999}, // Different repository ID
+		},
+	}
+
+	repo := Repository{ID: 123, ProjectID: 0}
+	err = repo.ClearCache(mockTemplateManager)
+	require.NoError(t, err)
+
+	// Directory should still exist since no templates use this repository
+	_, err = os.Stat(repoDir)
+	require.NoError(t, err, "repo directory should not be deleted")
+}
+
+// Mock template manager for Repository tests
+type mockTemplateManager struct {
+	templates []Template
+}
+
+func (m *mockTemplateManager) GetTemplates(projectID int, filter TemplateFilter, params RetrieveQueryParams) ([]Template, error) {
+	var result []Template
+	for _, template := range m.templates {
+		if template.ProjectID == projectID {
+			result = append(result, template)
+		}
+	}
+	return result, nil
+}
+
+func (m *mockTemplateManager) GetTemplateRefs(projectID int, templateID int) (ObjectReferrers, error) {
+	return ObjectReferrers{}, nil
+}
+
+func (m *mockTemplateManager) CreateTemplate(template Template) (Template, error) {
+	return Template{}, nil
+}
+
+func (m *mockTemplateManager) UpdateTemplate(template Template) error {
+	return nil
+}
+
+func (m *mockTemplateManager) GetTemplate(projectID int, templateID int) (Template, error) {
+	return Template{}, nil
+}
+
+func (m *mockTemplateManager) DeleteTemplate(projectID int, templateID int) error {
+	return nil
+}
+
+func (m *mockTemplateManager) SetTemplateDescription(projectID int, templateID int, description string) error {
+	return nil
+}
+
+func (m *mockTemplateManager) GetTemplateVaults(projectID int, templateID int) ([]TemplateVault, error) {
+	return nil, nil
+}
+
+func (m *mockTemplateManager) CreateTemplateVault(vault TemplateVault) (TemplateVault, error) {
+	return TemplateVault{}, nil
+}
+
+func (m *mockTemplateManager) UpdateTemplateVaults(projectID int, templateID int, vaults []TemplateVault) error {
+	return nil
+}
+
 func TestRepository_GetGitURL(t *testing.T) {
 	for _, v := range []struct {
 		Repository     Repository

db_lib/AnsiblePlaybook.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"path"
 	"strings"
 
 	"github.com/creack/pty"
@@ -27,7 +28,7 @@ func (p AnsiblePlaybook) makeCmd(command string, args []string, environmentVars
 	cmd.Env = append(cmd.Env, "ANSIBLE_HOST_KEY_CHECKING=False")
 	//cmd.Env = append(cmd.Env, "ANSIBLE_SSH_ARGS=-o UserKnownHostsFile=/dev/null")
 	cmd.Env = append(cmd.Env, getEnvironmentVars()...)
-	cmd.Env = append(cmd.Env, fmt.Sprintf("HOME=%s", util.Config.GetProjectTmpDir(p.Repository.ProjectID)))
+	cmd.Env = append(cmd.Env, fmt.Sprintf("HOME=%s", path.Join(util.Config.GetProjectTmpDir(p.Repository.ProjectID), fmt.Sprintf("template_%d", p.TemplateID))))
 	cmd.Env = append(cmd.Env, fmt.Sprintf("PWD=%s", cmd.Dir))
 	cmd.Env = append(cmd.Env, environmentVars...)
 

db_lib/LocalApp_test.go

@@ -2,9 +2,13 @@ package db_lib
 
 import (
 	"os"
+	"os/exec"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/semaphoreui/semaphore/db"
+	"github.com/semaphoreui/semaphore/pkg/task_logger"
 	"github.com/semaphoreui/semaphore/util"
 )
 
@@ -48,3 +52,77 @@ func TestGetEnvironmentVars(t *testing.T) {
 		}
 	}
 }
+
+func TestAnsiblePlaybookTemplateSpecificHome(t *testing.T) {
+	util.Config = &util.ConfigType{
+		TmpPath: "/tmp",
+		Process: &util.ConfigProcess{}, // Empty process config to avoid nil pointer
+	}
+
+	// Create two different playbooks with different template IDs
+	playbook1 := AnsiblePlaybook{
+		TemplateID: 123,
+		Repository: db.Repository{
+			ProjectID: 42,
+		},
+		Logger: &mockLogger{},
+	}
+
+	playbook2 := AnsiblePlaybook{
+		TemplateID: 456,
+		Repository: db.Repository{
+			ProjectID: 42, // Same project but different template
+		},
+		Logger: &mockLogger{},
+	}
+
+	// Test that both playbooks get different HOME directories
+	cmd1 := playbook1.makeCmd("test-command", []string{}, nil)
+	cmd2 := playbook2.makeCmd("test-command", []string{}, nil)
+
+	// Extract HOME environment variables
+	var home1, home2 string
+	for _, env := range cmd1.Env {
+		if strings.HasPrefix(env, "HOME=") {
+			home1 = env[5:] // Remove "HOME=" prefix
+			break
+		}
+	}
+	for _, env := range cmd2.Env {
+		if strings.HasPrefix(env, "HOME=") {
+			home2 = env[5:] // Remove "HOME=" prefix
+			break
+		}
+	}
+
+	// Verify HOME directories are different
+	if home1 == home2 {
+		t.Errorf("Expected different HOME directories for different templates, but got same: %s", home1)
+	}
+
+	// Verify HOME directories are template-specific
+	expectedHome1 := "/tmp/project_42/template_123"
+	expectedHome2 := "/tmp/project_42/template_456"
+
+	if home1 != expectedHome1 {
+		t.Errorf("Expected HOME for template 123 to be %s, got %s", expectedHome1, home1)
+	}
+
+	if home2 != expectedHome2 {
+		t.Errorf("Expected HOME for template 456 to be %s, got %s", expectedHome2, home2)
+	}
+}
+
+// mockLogger implements task_logger.Logger for testing
+type mockLogger struct{}
+
+func (l *mockLogger) Log(msg string)                                      {}
+func (l *mockLogger) Logf(format string, a ...any)                        {}
+func (l *mockLogger) LogWithTime(now time.Time, msg string)               {}
+func (l *mockLogger) LogfWithTime(now time.Time, format string, a ...any) {}
+func (l *mockLogger) LogCmd(cmd *exec.Cmd)                                 {}
+func (l *mockLogger) SetStatus(status task_logger.TaskStatus)             {}
+func (l *mockLogger) AddStatusListener(l2 task_logger.StatusListener)     {}
+func (l *mockLogger) AddLogListener(l2 task_logger.LogListener)           {}
+func (l *mockLogger) SetCommit(hash, message string)                      {}
+func (l *mockLogger) WaitLog()                                            {}

services/tasks/LocalJob_inventory.go

@@ -1,6 +1,7 @@
 package tasks
 
 import (
+	"fmt"
 	"os"
 	"path"
 	"strconv"
@@ -38,17 +39,16 @@ func (t *LocalJob) installInventory() (err error) {
 }
 
 func (t *LocalJob) tmpInventoryFilename() string {
-	if t.Inventory.Repository == nil {
-		return "inventory_" + strconv.Itoa(t.Inventory.ID)
-	}
-	return t.Inventory.Repository.GetDirName(t.Template.ID) + "_inventory_" + strconv.Itoa(t.Inventory.ID)
+	return "inventory_" + strconv.Itoa(t.Inventory.ID)
 }
 
 func (t *LocalJob) tmpInventoryFullPath() string {
 	if t.Inventory.Repository != nil && t.Inventory.Repository.GetType() == db.RepositoryLocal {
 		return t.Inventory.Repository.GetGitURL(true)
 	}
-	pathname := path.Join(util.Config.GetProjectTmpDir(t.Template.ProjectID), t.tmpInventoryFilename())
+	// Place inventory in template directory: /project_{projectID}/template_{templateID}/inventory_{inventoryID}
+	templateDir := path.Join(util.Config.GetProjectTmpDir(t.Template.ProjectID), fmt.Sprintf("template_%d", t.Template.ID))
+	pathname := path.Join(templateDir, t.tmpInventoryFilename())
 	if t.Inventory.Type == db.InventoryStaticYaml {
 		pathname += ".yml"
 	}

services/tasks/TaskRunner_test.go

@@ -164,7 +164,7 @@ func TestGetRepoPath(t *testing.T) {
 	}
 
 	dir := tsk.job.(*LocalJob).App.(*db_lib.AnsibleApp).GetPlaybookDir()
-	if dir != "/tmp/project_0/repository_0_template_0/deploy" {
+	if dir != "/tmp/project_0/template_0/src/deploy" {
 		t.Fatal("Invalid playbook dir: " + dir)
 	}
 }
@@ -210,7 +210,7 @@ func TestGetRepoPath_whenStartsWithSlash(t *testing.T) {
 	}
 
 	dir := tsk.job.(*LocalJob).App.(*db_lib.AnsibleApp).GetPlaybookDir()
-	if dir != "/tmp/project_0/repository_0_template_0/deploy" {
+	if dir != "/tmp/project_0/template_0/src/deploy" {
 		t.Fatal("Invalid playbook dir: " + dir)
 	}
 }
@@ -567,7 +567,7 @@ func TestTaskGetPlaybookArgs(t *testing.T) {
 	}
 
 	res := strings.Join(args, " ")
-	if res != "-i /tmp/project_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
+	if res != "-i /tmp/project_0/template_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
 		t.Fatal("incorrect result")
 	}
 }
@@ -623,7 +623,7 @@ func TestTaskGetPlaybookArgs2(t *testing.T) {
 	}
 
 	res := strings.Join(args, " ")
-	if res != "-i /tmp/project_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
+	if res != "-i /tmp/project_0/template_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
 		t.Fatal("incorrect result")
 	}
 }
@@ -680,7 +680,7 @@ func TestTaskGetPlaybookArgs3(t *testing.T) {
 	}
 
 	res := strings.Join(args, " ")
-	if res != "-i /tmp/project_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
+	if res != "-i /tmp/project_0/template_0/inventory_0 --extra-vars {\"semaphore_vars\":{\"task_details\":{\"id\":0,\"url\":null,\"username\":\"\"}}} test.yml" {
 		t.Fatal("incorrect result")
 	}
 }