From 964b40bad5d5a37bbc91164d3f1bf9168e001ebf Mon Sep 17 00:00:00 2001 From: xhe Date: Wed, 16 Oct 2024 15:16:57 +0800 Subject: [PATCH] *: try to fix download racing bugs (#2458) * *: try to fix download racing bugs Signed-off-by: xhe * fix workflow Signed-off-by: xhe * fix UT Signed-off-by: xhe * fix UT Signed-off-by: xhe * stable test Signed-off-by: xhe * stable test Signed-off-by: xhe * stable test Signed-off-by: xhe * stable test Signed-off-by: xhe * stable test Signed-off-by: xhe --------- Signed-off-by: xhe --- .github/workflows/integrate-dm.yaml | 4 ++-- .github/workflows/integrate-playground.yaml | 2 +- docker/control/Dockerfile | 2 +- pkg/cluster/operation/download.go | 2 +- pkg/repository/v1_repository.go | 12 +++++++++++- pkg/repository/v1_repository_test.go | 11 ++++++++--- pkg/repository/v1manifest/local_manifests.go | 4 ++++ tests/tiup-playground/test_playground.sh | 15 ++++++++++++--- 8 files changed, 40 insertions(+), 12 deletions(-) diff --git a/.github/workflows/integrate-dm.yaml b/.github/workflows/integrate-dm.yaml index dd898379eb..cb18ae3387 100644 --- 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 index c8c407f0d7..48d075c22c 100644 --- 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 index fe2901c994..b70c7939cd 100644 --- 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 index a2dfb6a6f5..fe7f431df7 100644 --- 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 index 9eab0529a4..ca443d9066 100644 --- 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 index 80606149a2..14bb50cbe4 100644 --- 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 index 10bbd97e0b..a7c32396b5 100644 --- 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 index 9e9f5284d0..dd431b41de 100755 --- 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