*: try to fix download racing bugs (#2458)

* *: try to fix download racing bugs Signed-off-by: xhe <[email protected]> * fix workflow Signed-off-by: xhe <[email protected]> * fix UT Signed-off-by: xhe <[email protected]> * fix UT Signed-off-by: xhe <[email protected]> * stable test Signed-off-by: xhe <[email protected]> * stable test Signed-off-by: xhe <[email protected]> * stable test Signed-off-by: xhe <[email protected]> * stable test Signed-off-by: xhe <[email protected]> * stable test Signed-off-by: xhe <[email protected]> --------- Signed-off-by: xhe <[email protected]>
pingcap · Oct 16, 2024 · 964b40b · 964b40b
1 parent c1a14a5
commit 964b40b
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 12 deletions.
diff --git a/.github/workflows/integrate-dm.yaml b/.github/workflows/integrate-dm.yaml
@@ -67,7 +67,7 @@ jobs:
         # with --dev the first run will fail for unknow reason, just retry it and will success now..
         run: |
           cd ${{ env.working-directory }}/docker
-          TIUP_CLUSTER_ROOT=${{ env.working-directory }} ./up.sh --daemon --dev --compose ./docker-compose.dm.yml || TIUP_CLUSTER_ROOT=${{ env.working-directory }} ./up.sh --daemon --dev --compose ./docker-compose.dm.yml
+          TIUP_CLUSTER_ROOT=${{ env.working-directory }} ./up.sh --daemon --dev --compose ./docker-compose.dm.yml
 
       - name: Check running containers
         run: |
@@ -97,7 +97,7 @@ jobs:
       - name: Upload component log
         if: ${{ failure() }}
         # if: always()
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v3
         with:
           name: dm_logs
           path: ${{ env.working-directory }}/dm.logs.tar.gz

diff --git a/.github/workflows/integrate-playground.yaml b/.github/workflows/integrate-playground.yaml
@@ -77,7 +77,7 @@ jobs:
       - name: Upload component log
         if: ${{ failure() }}
         # if: always()
-        uses: actions/upload-artifact@v1
+        uses: actions/upload-artifact@v3
         with:
           name: playground_logs
           path: ${{ env.working-directory }}/playground.logs.tar.gz

diff --git a/docker/control/Dockerfile b/docker/control/Dockerfile
@@ -1,4 +1,4 @@
-FROM golang:1.17-bullseye
+FROM golang:1.21
 
 
 # Use mirrors for poor network...

diff --git a/pkg/cluster/operation/download.go b/pkg/cluster/operation/download.go
@@ -51,7 +51,7 @@ func Download(component, nodeOS, arch string, version string) error {
 		if err := repo.DownloadComponent(component, version, targetPath); err != nil {
 			return err
 		}
-	} else {
+	} else if version != "nightly" {
 		if err := repo.VerifyComponent(component, version, targetPath); err != nil {
 			os.Remove(targetPath)
 		}

diff --git a/pkg/repository/v1_repository.go b/pkg/repository/v1_repository.go
@@ -483,9 +483,19 @@ func (r *V1Repository) updateComponentManifest(id string, withYanked bool) (*v1m
 // DownloadComponent downloads the component specified by item into local file,
 // the component will be removed if hash is not correct
 func (r *V1Repository) DownloadComponent(item *v1manifest.VersionItem, target string) error {
+	// make a tempdir such that every download will not inference each other
 	targetDir := filepath.Dir(target)
-	err := r.mirror.Download(item.URL, targetDir)
+	err := os.MkdirAll(targetDir, 0755)
 	if err != nil {
+		return errors.Trace(err)
+	}
+
+	targetDir, err = os.MkdirTemp(targetDir, "download")
+	if err != nil {
+		return errors.Trace(err)
+	}
+
+	if err := r.mirror.Download(item.URL, targetDir); err != nil {
 		return err
 	}
 

diff --git a/pkg/repository/v1_repository_test.go b/pkg/repository/v1_repository_test.go
@@ -20,6 +20,7 @@ import (
 	"io"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -502,10 +503,14 @@ func TestLatestStableVersionWithPrerelease(t *testing.T) {
 }
 
 func TestUpdateComponents(t *testing.T) {
+	t1 := t.TempDir()
+	t2 := t.TempDir()
+
 	mirror := MockMirror{
 		Resources: map[string]string{},
 	}
 	local := v1manifest.NewMockManifests()
+	local.RootDir = t1
 	priv := setNewRoot(t, local)
 
 	repo := NewV1Repo(&mirror, Options{GOOS: "plat", GOARCH: "form"}, local)
@@ -533,7 +538,7 @@ func TestUpdateComponents(t *testing.T) {
 	assert.Equal(t, 1, len(local.Installed))
 	assert.Equal(t, "v2.0.1", local.Installed["foo"].Version)
 	assert.Equal(t, "foo201", local.Installed["foo"].Contents)
-	assert.Equal(t, "/tmp/mock/components/foo/v2.0.1/foo-2.0.1.tar.gz", local.Installed["foo"].BinaryPath)
+	assert.Equal(t, filepath.Join(t1, "components/foo/v2.0.1/foo-2.0.1.tar.gz"), local.Installed["foo"].BinaryPath)
 
 	// Update
 	foo.Version = 8
@@ -552,13 +557,13 @@ func TestUpdateComponents(t *testing.T) {
 	repo.PurgeTimestamp()
 	err = repo.UpdateComponents([]ComponentSpec{{
 		ID:        "foo",
-		TargetDir: "/tmp/mock-mock",
+		TargetDir: t2,
 	}})
 	assert.Nil(t, err)
 	assert.Equal(t, 1, len(local.Installed))
 	assert.Equal(t, "v2.0.2", local.Installed["foo"].Version)
 	assert.Equal(t, "foo202", local.Installed["foo"].Contents)
-	assert.Equal(t, "/tmp/mock-mock/foo-2.0.2.tar.gz", local.Installed["foo"].BinaryPath)
+	assert.Equal(t, filepath.Join(t2, "foo-2.0.2.tar.gz"), local.Installed["foo"].BinaryPath)
 
 	// Update; already up to date
 	err = repo.UpdateComponents([]ComponentSpec{{

diff --git a/pkg/repository/v1manifest/local_manifests.go b/pkg/repository/v1manifest/local_manifests.go
@@ -292,6 +292,7 @@ type MockManifests struct {
 	Saved     []string
 	Installed map[string]MockInstalled
 	Ks        *KeyStore
+	RootDir   string
 }
 
 // MockInstalled is used by MockManifests to remember what was installed for a component.
@@ -401,6 +402,9 @@ func (ms *MockManifests) KeyStore() *KeyStore {
 
 // TargetRootDir implements LocalManifests.
 func (ms *MockManifests) TargetRootDir() string {
+	if ms.RootDir != "" {
+		return ms.RootDir
+	}
 	return "/tmp/mock"
 }
 

diff --git a/tests/tiup-playground/test_playground.sh b/tests/tiup-playground/test_playground.sh
@@ -20,12 +20,14 @@ mkdir -p $TIUP_INSTANCE_DATA_DIR
 mkdir -p $TEST_DIR/cover
 
 function tiup-playground() {
+set +x
     # echo "in function"
     if [ -f "$TEST_DIR/bin/tiup-playground.test" ]; then
         $TEST_DIR/bin/tiup-playground.test -test.coverprofile=$TEST_DIR/cover/cov.itest-$(date +'%s')-$RANDOM.out __DEVEL--i-heard-you-like-tests "$@"
     else
         $TEST_DIR/../../bin/tiup-playground "$@"
     fi
+set -x
 }
 
 # usage: check_instance_num tidb 1
@@ -63,9 +65,16 @@ sleep 3
 trap "kill_all" EXIT
 
 # wait start cluster successfully
-timeout 300 grep -q "TiDB Playground Cluster is started" <(tail -f $outfile)
-
-tiup-playground display | grep -qv "exit"
+n=0
+while [ "$n" -lt 300 ] && ! grep -q "TiDB Playground Cluster is started" $outfile; do
+	n=$(( n + 1 ))
+	sleep 1
+done
+n=0
+while [ "$n" -lt 10 ] && ! tiup-playground display; do
+	n=$(( n + 1 ))
+	sleep 1
+done
 tiup-playground scale-out --db 2
 sleep 5