Cleanup of initializing working dirs

Subsumes #726, with a unit test fix and a tweak to make sure that artifact_bucket_test doesn't try to run in parallel, since that messes up various PipelineRun tests. Signed-off-by: Andrew Bayer <[email protected]>
tektoncd · May 8, 2019 · e2b53c8 · e2b53c8
1 parent b81e22d
commit e2b53c8
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 9 deletions.
diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod.go
@@ -26,6 +26,7 @@ import (
 	"io"
 	"io/ioutil"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	"go.uber.org/zap"
@@ -169,15 +170,22 @@ func makeCredentialInitializer(serviceAccountName, namespace string, kubeclient
 
 func makeWorkingDirScript(workingDirs map[string]bool) string {
 	script := ""
+	var orderedDirs []string
+
 	for wd := range workingDirs {
 		if wd != "" {
-			p := filepath.Clean(wd)
-			if rel, err := filepath.Rel(workspaceDir, p); err == nil && !strings.HasPrefix(rel, ".") {
-				if script == "" {
-					script = fmt.Sprintf("mkdir -p %s", p)
-				} else {
-					script = fmt.Sprintf("%s %s", script, p)
-				}
+			orderedDirs = append(orderedDirs, wd)
+		}
+	}
+	sort.Strings(orderedDirs)
+
+	for _, wd := range orderedDirs {
+		p := filepath.Clean(wd)
+		if rel, err := filepath.Rel(workspaceDir, p); err == nil && !strings.HasPrefix(rel, ".") {
+			if script == "" {
+				script = fmt.Sprintf("mkdir -p %s", p)
+			} else {
+				script = fmt.Sprintf("%s %s", script, p)
 			}
 		}
 	}

diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/pod_test.go
@@ -434,7 +434,7 @@ func TestMakeWorkingDirScript(t *testing.T) {
 	}, {
 		desc:        "simple",
 		workingDirs: map[string]bool{"/workspace/foo": true, "/workspace/bar": true, "/baz": true},
-		want:        "mkdir -p /workspace/foo /workspace/bar",
+		want:        "mkdir -p /workspace/bar /workspace/foo",
 	}, {
 		desc:        "empty",
 		workingDirs: map[string]bool{"/workspace": true, "": true, "/baz": true, "/workspacedir": true},

diff --git a/test/artifact_bucket_test.go b/test/artifact_bucket_test.go
@@ -48,7 +48,7 @@ func TestStorageBucketPipelineRun(t *testing.T) {
 		t.Skip("GCP_SERVICE_ACCOUNT_KEY_PATH variable is not set.")
 	}
 	c, namespace := setup(t)
-	t.Parallel()
+	// Bucket tests can't run in parallel without causing issues with other tests.
 
 	knativetest.CleanupOnInterrupt(func() { tearDown(t, c, namespace) }, t.Logf)
 	defer tearDown(t, c, namespace)