From 080f2e834b5524dea8060dd2fd285c4ebe7de26a Mon Sep 17 00:00:00 2001 From: lucklove Date: Tue, 28 Jul 2020 14:48:08 +0800 Subject: [PATCH 1/8] Handle yanked version in update Fix https://github.com/pingcap/tiup/issues/177 Signed-off-by: lucklove --- cmd/list.go | 2 +- cmd/mirror.go | 4 +- components/playground/playground.go | 2 +- pkg/exec/run.go | 2 +- pkg/repository/clone_mirror.go | 2 +- pkg/repository/v1_repository.go | 29 +++++++----- pkg/repository/v1_repository_test.go | 67 ++++++++++++++++++++++++---- tests/tiup/tiup.toml | 2 +- 8 files changed, 83 insertions(+), 27 deletions(-) diff --git a/cmd/list.go b/cmd/list.go index 6619e039b3..3ffc8e12f9 100644 --- a/cmd/list.go +++ b/cmd/list.go @@ -178,7 +178,7 @@ func showComponentList(env *environment.Environment, opt listOptions) (*listResu func showComponentVersions(env *environment.Environment, component string, opt listOptions) (*listResult, error) { var comp *v1manifest.Component var err error - comp, err = env.V1Repository().FetchComponentManifest(component) + comp, err = env.V1Repository().FetchComponentManifest(component, false) if err != nil { return nil, errors.Annotate(err, "failed to fetch component") } diff --git a/cmd/mirror.go b/cmd/mirror.go index 290ca4ea90..abfedaa024 100644 --- a/cmd/mirror.go +++ b/cmd/mirror.go @@ -254,7 +254,7 @@ func newMirrorModifyCmd() *cobra.Command { } comp, ver := environment.ParseCompVersion(args[0]) - m, err := env.V1Repository().FetchComponentManifest(comp) + m, err := env.V1Repository().FetchComponentManifest(comp, true) if err != nil { return err } @@ -334,7 +334,7 @@ func newMirrorPublishCmd() *cobra.Command { cmd.Flags().Visit(func(f *pflag.Flag) { flagSet.Insert(f.Name) }) - m, err := env.V1Repository().FetchComponentManifest(args[0]) + m, err := env.V1Repository().FetchComponentManifest(args[0], true) if err != nil { fmt.Printf("Fetch local manifest: %s\n", err.Error()) fmt.Printf("Failed to load component manifest, create a new one\n") diff --git a/components/playground/playground.go b/components/playground/playground.go index 59f9c1a210..4393ac8958 100755 --- a/components/playground/playground.go +++ b/components/playground/playground.go @@ -642,7 +642,7 @@ func (p *Playground) bootCluster(env *environment.Environment, options *bootOpti } if options.version == "" { - version, _, err := env.V1Repository().LatestStableVersion("tidb") + version, _, err := env.V1Repository().LatestStableVersion("tidb", false) if err != nil { return err } diff --git a/pkg/exec/run.go b/pkg/exec/run.go index 8b29c96914..9b19b15948 100644 --- a/pkg/exec/run.go +++ b/pkg/exec/run.go @@ -146,7 +146,7 @@ func PrepareCommand( } if len(checkUpdate) > 0 && checkUpdate[0] { - latestV, _, err := env.V1Repository().LatestStableVersion(component) + latestV, _, err := env.V1Repository().LatestStableVersion(component, true) if err != nil { return nil, err } diff --git a/pkg/repository/clone_mirror.go b/pkg/repository/clone_mirror.go index 05523f6035..bf9eb0bb08 100644 --- a/pkg/repository/clone_mirror.go +++ b/pkg/repository/clone_mirror.go @@ -247,7 +247,7 @@ func cloneComponents(repo *V1Repository, options CloneOptions) (map[string]*v1manifest.Component, error) { compManifests := map[string]*v1manifest.Component{} for _, name := range components { - manifest, err := repo.FetchComponentManifest(name) + manifest, err := repo.FetchComponentManifest(name, true) if err != nil { return nil, errors.Annotatef(err, "fetch component '%s' manifest failed", name) } diff --git a/pkg/repository/v1_repository.go b/pkg/repository/v1_repository.go index bdbc667a16..0f61849aa8 100644 --- a/pkg/repository/v1_repository.go +++ b/pkg/repository/v1_repository.go @@ -103,7 +103,7 @@ func (r *V1Repository) UpdateComponents(specs []ComponentSpec) error { var errs []string for _, spec := range specs { - manifest, err := r.updateComponentManifest(spec.ID) + manifest, err := r.updateComponentManifest(spec.ID, false) if err != nil { if err == errUnknownComponent { fmt.Println(color.YellowString("The component `%s` not found (may be deleted from repository); skipped", spec.ID)) @@ -416,7 +416,7 @@ func (r *V1Repository) updateLocalIndex(snapshot *v1manifest.Snapshot) error { } // Precondition: the snapshot and index manifests exist and are up to date. -func (r *V1Repository) updateComponentManifest(id string) (*v1manifest.Component, error) { +func (r *V1Repository) updateComponentManifest(id string, withYanked bool) (*v1manifest.Component, error) { defer func(start time.Time) { verbose.Log("update component '%s' manifest finished in %s", id, time.Since(start)) }(time.Now()) @@ -427,7 +427,14 @@ func (r *V1Repository) updateComponentManifest(id string) (*v1manifest.Component if err != nil { return nil, errors.Trace(err) } - item, ok := index.Components[id] + components := map[string]v1manifest.ComponentItem{} + if withYanked { + components = index.ComponentListWithYanked() + } else { + components = index.ComponentList() + } + + item, ok := components[id] if !ok { return nil, errUnknownComponent } @@ -639,8 +646,8 @@ func (r *V1Repository) UpdateComponentManifests() error { } for name := range index.Components { - _, err = r.updateComponentManifest(name) - if err != nil { + _, err = r.updateComponentManifest(name, false) + if err != nil && err != errUnknownComponent { return err } } @@ -649,18 +656,18 @@ func (r *V1Repository) UpdateComponentManifests() error { } // FetchComponentManifest fetch the component manifest. -func (r *V1Repository) FetchComponentManifest(id string) (com *v1manifest.Component, err error) { +func (r *V1Repository) FetchComponentManifest(id string, withYanked bool) (com *v1manifest.Component, err error) { err = r.ensureManifests() if err != nil { return nil, errors.AddStack(err) } - return r.updateComponentManifest(id) + return r.updateComponentManifest(id, withYanked) } // ComponentVersion returns version item of a component func (r *V1Repository) ComponentVersion(id, version string, includeYanked bool) (*v1manifest.VersionItem, error) { - manifest, err := r.FetchComponentManifest(id) + manifest, err := r.FetchComponentManifest(id, includeYanked) if err != nil { return nil, err } @@ -675,8 +682,8 @@ func (r *V1Repository) ComponentVersion(id, version string, includeYanked bool) } // LatestStableVersion returns the latest stable version of specific component -func (r *V1Repository) LatestStableVersion(id string) (v0manifest.Version, *v1manifest.VersionItem, error) { - com, err := r.FetchComponentManifest(id) +func (r *V1Repository) LatestStableVersion(id string, withYanked bool) (v0manifest.Version, *v1manifest.VersionItem, error) { + com, err := r.FetchComponentManifest(id, withYanked) if err != nil { return "", nil, err } @@ -722,7 +729,7 @@ func (r *V1Repository) BinaryPath(installPath string, componentID string, versio return "", err } if component == nil { - component, err = r.FetchComponentManifest(componentID) + component, err = r.FetchComponentManifest(componentID, true) if err != nil { return "", err } diff --git a/pkg/repository/v1_repository_test.go b/pkg/repository/v1_repository_test.go index b02d2a81c8..05e3d549d8 100644 --- a/pkg/repository/v1_repository_test.go +++ b/pkg/repository/v1_repository_test.go @@ -275,6 +275,33 @@ func TestUpdateIndex(t *testing.T) { // TODO test that invalid signature of snapshot causes an error } +func TestYanked(t *testing.T) { + mirror := MockMirror{ + Resources: map[string]string{}, + } + local := v1manifest.NewMockManifests() + _ = setNewRoot(t, local) + repo := NewV1Repo(&mirror, Options{}, local) + + index, indexPriv := indexManifest(t) + snapshot := snapshotManifest() + bar := componentManifest() + serBar := serialize(t, bar, indexPriv) + mirror.Resources["/7.bar.json"] = serBar + local.Manifests[v1manifest.ManifestFilenameSnapshot] = &v1manifest.Manifest{Signed: snapshot} + local.Manifests[v1manifest.ManifestFilenameIndex] = &v1manifest.Manifest{Signed: index} + + // Test yanked version + updated, err := repo.updateComponentManifest("bar", true) + assert.Nil(t, err) + _, ok := updated.VersionList("plat/form")["v2.0.3"] + assert.False(t, ok) + + updated, err = repo.updateComponentManifest("bar", false) + assert.NotNil(t, err) + assert.Equal(t, err, errUnknownComponent) +} + func TestUpdateComponent(t *testing.T) { mirror := MockMirror{ Resources: map[string]string{}, @@ -292,7 +319,7 @@ func TestUpdateComponent(t *testing.T) { local.Manifests[v1manifest.ManifestFilenameIndex] = &v1manifest.Manifest{Signed: index} // Test happy path - updated, err := repo.updateComponentManifest("foo") + updated, err := repo.updateComponentManifest("foo", false) assert.Nil(t, err) t.Logf("%+v", err) assert.NotNil(t, updated) @@ -303,13 +330,13 @@ func TestUpdateComponent(t *testing.T) { oldFoo.Version = 8 local.Manifests["foo.json"] = &v1manifest.Manifest{Signed: oldFoo} local.Saved = []string{} - updated, err = repo.updateComponentManifest("foo") + updated, err = repo.updateComponentManifest("foo", false) assert.NotNil(t, err) assert.Nil(t, updated) assert.Empty(t, local.Saved) // Test that id missing from index causes an error - updated, err = repo.updateComponentManifest("bar") + updated, err = repo.updateComponentManifest("bar", false) assert.NotNil(t, err) assert.Nil(t, updated) assert.Empty(t, local.Saved) @@ -598,6 +625,7 @@ func snapshotManifest() *v1manifest.Snapshot { v1manifest.ManifestURLRoot: {Version: 42}, v1manifest.ManifestURLIndex: {Version: 5}, "/foo.json": {Version: 7}, + "/bar.json": {Version: 7}, }, } } @@ -613,7 +641,10 @@ func componentManifest() *v1manifest.Component { ID: "foo", Description: "foo does stuff", Platforms: map[string]map[string]v1manifest.VersionItem{ - "plat/form": {"v2.0.1": versionItem()}, + "plat/form": { + "v2.0.1": versionItem(), + "v2.0.3": versionItem3(), + }, }, } } @@ -638,6 +669,17 @@ func versionItem2() v1manifest.VersionItem { } } +func versionItem3() v1manifest.VersionItem { + return v1manifest.VersionItem{ + URL: "/foo-2.0.3.tar.gz", + Yanked: true, + FileHash: v1manifest.FileHash{ + Hashes: map[string]string{v1manifest.SHA256: "5abe91bc22039c15c05580062357be7ab0bfd7968582a118fbb4eb817ddc2e76"}, + Length: 12, + }, + } +} + func indexManifest(t *testing.T) (*v1manifest.Index, crypto.PrivKey) { info, keyID, priv, err := v1manifest.FreshKeyInfo() assert.Nil(t, err) @@ -661,11 +703,18 @@ func indexManifest(t *testing.T) (*v1manifest.Index, crypto.PrivKey) { Keys: map[string]*v1manifest.KeyInfo{keyID: info}, Threshold: 1, }}, - Components: map[string]v1manifest.ComponentItem{"foo": { - Yanked: false, - Owner: "bar", - URL: "/foo.json", - }}, + Components: map[string]v1manifest.ComponentItem{ + "foo": { + Yanked: false, + Owner: "bar", + URL: "/foo.json", + }, + "bar": { + Yanked: true, + Owner: "bar", + URL: "/bar.json", + }, + }, DefaultComponents: []string{}, }, priv } diff --git a/tests/tiup/tiup.toml b/tests/tiup/tiup.toml index 598fc52376..708458b920 100644 --- a/tests/tiup/tiup.toml +++ b/tests/tiup/tiup.toml @@ -1 +1 @@ -mirror = "https://tiup-mirrors.pingcap.com" +mirror = "http://172.16.5.139:8988" From 620ecb3f229e4c2c988e4c19777189b8fa7b85cc Mon Sep 17 00:00:00 2001 From: lucklove Date: Tue, 28 Jul 2020 17:21:36 +0800 Subject: [PATCH 2/8] Revert test config Signed-off-by: lucklove --- tests/tiup/tiup.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tiup/tiup.toml b/tests/tiup/tiup.toml index 708458b920..153720a0ac 100644 --- a/tests/tiup/tiup.toml +++ b/tests/tiup/tiup.toml @@ -1 +1 @@ -mirror = "http://172.16.5.139:8988" +mirror = "https://tiup-mirrors.pingcap.com" \ No newline at end of file From 344bfb7641cb25cd3370919b663bebed8ddffdf0 Mon Sep 17 00:00:00 2001 From: lucklove Date: Tue, 28 Jul 2020 17:28:11 +0800 Subject: [PATCH 3/8] Fix lint Signed-off-by: lucklove --- pkg/repository/v1_repository.go | 2 +- pkg/repository/v1_repository_test.go | 2 +- tests/tiup/tiup.toml | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/repository/v1_repository.go b/pkg/repository/v1_repository.go index 0f61849aa8..dfe680d0bf 100644 --- a/pkg/repository/v1_repository.go +++ b/pkg/repository/v1_repository.go @@ -427,7 +427,7 @@ func (r *V1Repository) updateComponentManifest(id string, withYanked bool) (*v1m if err != nil { return nil, errors.Trace(err) } - components := map[string]v1manifest.ComponentItem{} + var components map[string]v1manifest.ComponentItem if withYanked { components = index.ComponentListWithYanked() } else { diff --git a/pkg/repository/v1_repository_test.go b/pkg/repository/v1_repository_test.go index 05e3d549d8..90f75c6055 100644 --- a/pkg/repository/v1_repository_test.go +++ b/pkg/repository/v1_repository_test.go @@ -297,7 +297,7 @@ func TestYanked(t *testing.T) { _, ok := updated.VersionList("plat/form")["v2.0.3"] assert.False(t, ok) - updated, err = repo.updateComponentManifest("bar", false) + _, err = repo.updateComponentManifest("bar", false) assert.NotNil(t, err) assert.Equal(t, err, errUnknownComponent) } diff --git a/tests/tiup/tiup.toml b/tests/tiup/tiup.toml index 153720a0ac..71adc823a8 100644 --- a/tests/tiup/tiup.toml +++ b/tests/tiup/tiup.toml @@ -1 +1,2 @@ -mirror = "https://tiup-mirrors.pingcap.com" \ No newline at end of file +mirror = "http://172.16.5.139:8988" + From 1866adbac34f23c26848c0f08bdc64a17c5fa272 Mon Sep 17 00:00:00 2001 From: lucklove Date: Thu, 30 Jul 2020 14:42:30 +0800 Subject: [PATCH 4/8] Fix test config Signed-off-by: lucklove --- tests/tiup/tiup.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tiup/tiup.toml b/tests/tiup/tiup.toml index 71adc823a8..5997c9e7de 100644 --- a/tests/tiup/tiup.toml +++ b/tests/tiup/tiup.toml @@ -1,2 +1,2 @@ -mirror = "http://172.16.5.139:8988" +mirror = "https://tiup-mirrors.pingcap.com" From 31713e4f357d5f6e68c6bed8b48d64fb48a9ab6a Mon Sep 17 00:00:00 2001 From: lucklove Date: Thu, 30 Jul 2020 14:45:27 +0800 Subject: [PATCH 5/8] Fix new line Signed-off-by: lucklove --- tests/tiup/tiup.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tiup/tiup.toml b/tests/tiup/tiup.toml index 5997c9e7de..598fc52376 100644 --- a/tests/tiup/tiup.toml +++ b/tests/tiup/tiup.toml @@ -1,2 +1 @@ mirror = "https://tiup-mirrors.pingcap.com" - From 11d8f3b0b620b722254f0543207ac5938c8d1550 Mon Sep 17 00:00:00 2001 From: lucklove Date: Thu, 30 Jul 2020 15:00:17 +0800 Subject: [PATCH 6/8] Wrap error Signed-off-by: lucklove --- pkg/repository/v1_repository.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/repository/v1_repository.go b/pkg/repository/v1_repository.go index dfe680d0bf..193e87c555 100644 --- a/pkg/repository/v1_repository.go +++ b/pkg/repository/v1_repository.go @@ -105,7 +105,7 @@ func (r *V1Repository) UpdateComponents(specs []ComponentSpec) error { for _, spec := range specs { manifest, err := r.updateComponentManifest(spec.ID, false) if err != nil { - if err == errUnknownComponent { + if errors.Cause(err) == errUnknownComponent { fmt.Println(color.YellowString("The component `%s` not found (may be deleted from repository); skipped", spec.ID)) } else { errs = append(errs, err.Error()) @@ -647,7 +647,7 @@ func (r *V1Repository) UpdateComponentManifests() error { for name := range index.Components { _, err = r.updateComponentManifest(name, false) - if err != nil && err != errUnknownComponent { + if err != nil && errors.Cause(err) != errUnknownComponent { return err } } From c88ae7024a3b9e501b1e106be61bfe95bdcb2554 Mon Sep 17 00:00:00 2001 From: SIGSEGV Date: Thu, 30 Jul 2020 15:02:14 +0800 Subject: [PATCH 7/8] Update pkg/repository/v1_repository.go Co-authored-by: Lonng --- pkg/repository/v1_repository.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/repository/v1_repository.go b/pkg/repository/v1_repository.go index 193e87c555..f56a3166c4 100644 --- a/pkg/repository/v1_repository.go +++ b/pkg/repository/v1_repository.go @@ -436,7 +436,7 @@ func (r *V1Repository) updateComponentManifest(id string, withYanked bool) (*v1m item, ok := components[id] if !ok { - return nil, errUnknownComponent + return nil, errors.AddStack(errUnknownComponent) } var snapshot v1manifest.Snapshot _, _, err = r.local.LoadManifest(&snapshot) From 15d2c8460ea2c0423f6c3de19edcb5ae7fd07980 Mon Sep 17 00:00:00 2001 From: lucklove Date: Thu, 30 Jul 2020 15:39:33 +0800 Subject: [PATCH 8/8] fix test Signed-off-by: lucklove --- pkg/repository/v1_repository_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/repository/v1_repository_test.go b/pkg/repository/v1_repository_test.go index 90f75c6055..896662fa58 100644 --- a/pkg/repository/v1_repository_test.go +++ b/pkg/repository/v1_repository_test.go @@ -25,6 +25,7 @@ import ( cjson "github.com/gibson042/canonicaljson-go" "github.com/google/uuid" + "github.com/pingcap/errors" "github.com/pingcap/tiup/pkg/localdata" "github.com/pingcap/tiup/pkg/repository/crypto" "github.com/pingcap/tiup/pkg/repository/v1manifest" @@ -299,7 +300,7 @@ func TestYanked(t *testing.T) { _, err = repo.updateComponentManifest("bar", false) assert.NotNil(t, err) - assert.Equal(t, err, errUnknownComponent) + assert.Equal(t, errors.Cause(err), errUnknownComponent) } func TestUpdateComponent(t *testing.T) {