Fix dependences version binding when cloning mirror (#779)

cmd/mirror.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/pingcap/errors"
+	"github.com/pingcap/tiup/pkg/cluster/spec"
 	"github.com/pingcap/tiup/pkg/environment"
 	"github.com/pingcap/tiup/pkg/localdata"
 	"github.com/pingcap/tiup/pkg/repository"
@@ -669,7 +670,11 @@ func newMirrorCloneCmd() *cobra.Command {
 			}
 			defer repo.Mirror().Close()
 
-			return repository.CloneMirror(repo, components, args[0], args[1:], options)
+			var versionMapper = func(ver string) string {
+				return spec.TiDBComponentVersion(ver, "")
+			}
+
+			return repository.CloneMirror(repo, components, versionMapper, args[0], args[1:], options)
 		},
 	}
 

pkg/cluster/manager.go

@@ -39,10 +39,12 @@ import (
 	"github.com/pingcap/tiup/pkg/cluster/spec"
 	"github.com/pingcap/tiup/pkg/cluster/task"
 	"github.com/pingcap/tiup/pkg/crypto"
+	"github.com/pingcap/tiup/pkg/environment"
 	"github.com/pingcap/tiup/pkg/errutil"
 	"github.com/pingcap/tiup/pkg/file"
 	"github.com/pingcap/tiup/pkg/logger/log"
 	"github.com/pingcap/tiup/pkg/meta"
+	"github.com/pingcap/tiup/pkg/repository/v0manifest"
 	"github.com/pingcap/tiup/pkg/set"
 	"github.com/pingcap/tiup/pkg/utils"
 	"github.com/pingcap/tiup/pkg/version"
@@ -846,7 +848,12 @@ func (m *Manager) Upgrade(clusterName string, clusterVersion string, opt operato
 				// copy dependency component if needed
 				switch inst.ComponentName() {
 				case spec.ComponentTiSpark:
-					tb = tb.DeploySpark(inst, version, "" /* default srcPath */, deployDir, m.bindVersion)
+					env := environment.GlobalEnv()
+					sparkVer, _, err := env.V1Repository().LatestStableVersion(spec.ComponentSpark, false)
+					if err != nil {
+						return err
+					}
+					tb = tb.DeploySpark(inst, sparkVer.String(), "" /* default srcPath */, deployDir)
 				default:
 					tb = tb.CopyComponent(
 						inst.ComponentName(),
@@ -1195,7 +1202,12 @@ func (m *Manager) Deploy(
 			// copy dependency component if needed
 			switch inst.ComponentName() {
 			case spec.ComponentTiSpark:
-				t = t.DeploySpark(inst, version, "" /* default srcPath */, deployDir, m.bindVersion)
+				env := environment.GlobalEnv()
+				var sparkVer v0manifest.Version
+				if sparkVer, _, iterErr = env.V1Repository().LatestStableVersion(spec.ComponentSpark, false); iterErr != nil {
+					return
+				}
+				t = t.DeploySpark(inst, sparkVer.String(), "" /* default srcPath */, deployDir)
 			default:
 				t = t.CopyComponent(
 					inst.ComponentName(),
@@ -1238,6 +1250,10 @@ func (m *Manager) Deploy(
 		)
 	})
 
+	if iterErr != nil {
+		return iterErr
+	}
+
 	// Deploy monitor relevant components to remote
 	dlTasks, dpTasks := buildMonitoredDeployTask(
 		m.bindVersion,
@@ -1946,7 +1962,12 @@ func buildScaleOutTask(
 			// copy dependency component if needed
 			switch inst.ComponentName() {
 			case spec.ComponentTiSpark:
-				tb = tb.DeploySpark(inst, version, srcPath, deployDir, m.bindVersion)
+				env := environment.GlobalEnv()
+				var sparkVer v0manifest.Version
+				if sparkVer, _, iterErr = env.V1Repository().LatestStableVersion(spec.ComponentSpark, false); iterErr != nil {
+					return
+				}
+				tb = tb.DeploySpark(inst, sparkVer.String(), srcPath, deployDir)
 			default:
 				tb.CopyComponent(
 					inst.ComponentName(),

pkg/cluster/prepare.go

@@ -26,20 +26,23 @@ type InstanceIter interface {
 }
 
 // BuildDownloadCompTasks build download component tasks
-func BuildDownloadCompTasks(version string, instanceIter InstanceIter, bindVersion spec.BindVersion) []*task.StepDisplay {
+func BuildDownloadCompTasks(clusterVersion string, instanceIter InstanceIter, bindVersion spec.BindVersion) []*task.StepDisplay {
 	var tasks []*task.StepDisplay
 	uniqueTaskList := make(map[string]struct{}) // map["comp-os-arch"]{}
 	instanceIter.IterInstance(func(inst spec.Instance) {
 		key := fmt.Sprintf("%s-%s-%s", inst.ComponentName(), inst.OS(), inst.Arch())
 		if _, found := uniqueTaskList[key]; !found {
 			uniqueTaskList[key] = struct{}{}
 
-			// download spark as dependency of tispark
+			// we don't set version for tispark, so the lastest tispark will be used
+			var version string
 			if inst.ComponentName() == spec.ComponentTiSpark {
-				tasks = append(tasks, buildDownloadSparkTask(version, inst, bindVersion))
+				// download spark as dependency of tispark
+				tasks = append(tasks, buildDownloadSparkTask(inst))
+			} else {
+				version = bindVersion(inst.ComponentName(), clusterVersion)
 			}
 
-			version := bindVersion(inst.ComponentName(), version)
 			t := task.NewBuilder().
 				Download(inst.ComponentName(), inst.OS(), inst.Arch(), version).
 				BuildAsStep(fmt.Sprintf("  - Download %s:%s (%s/%s)",
@@ -52,10 +55,9 @@ func BuildDownloadCompTasks(version string, instanceIter InstanceIter, bindVersi
 
 // buildDownloadSparkTask build download task for spark, which is a dependency of tispark
 // FIXME: this is a hack and should be replaced by dependency handling in manifest processing
-func buildDownloadSparkTask(version string, inst spec.Instance, bindVersion spec.BindVersion) *task.StepDisplay {
-	ver := bindVersion(spec.ComponentSpark, version)
+func buildDownloadSparkTask(inst spec.Instance) *task.StepDisplay {
 	return task.NewBuilder().
-		Download(spec.ComponentSpark, inst.OS(), inst.Arch(), ver).
-		BuildAsStep(fmt.Sprintf("  - Download %s:%s (%s/%s)",
-			spec.ComponentSpark, version, inst.OS(), inst.Arch()))
+		Download(spec.ComponentSpark, inst.OS(), inst.Arch(), "").
+		BuildAsStep(fmt.Sprintf("  - Download %s: (%s/%s)",
+			spec.ComponentSpark, inst.OS(), inst.Arch()))
 }

pkg/cluster/spec/bindversion.go

@@ -31,10 +31,6 @@ func TiDBComponentVersion(comp, version string) string {
 		return "v0.7.0"
 	case ComponentCheckCollector:
 		return "v0.3.1"
-	case ComponentTiSpark:
-		return "v2.3.1"
-	case ComponentSpark:
-		return "v2.4.3"
 	default:
 		return version
 	}

pkg/cluster/task/builder.go

@@ -346,14 +346,13 @@ func (b *Builder) CheckSys(host, dataDir, checkType string, topo *spec.Specifica
 }
 
 // DeploySpark deployes spark as dependency of TiSpark
-func (b *Builder) DeploySpark(inst spec.Instance, version, srcPath, deployDir string, bindVersion spec.BindVersion) *Builder {
-	sparkSubPath := spec.ComponentSubDir(spec.ComponentSpark,
-		bindVersion(spec.ComponentSpark, version))
+func (b *Builder) DeploySpark(inst spec.Instance, sparkVersion, srcPath, deployDir string) *Builder {
+	sparkSubPath := spec.ComponentSubDir(spec.ComponentSpark, sparkVersion)
 	return b.CopyComponent(
 		spec.ComponentSpark,
 		inst.OS(),
 		inst.Arch(),
-		bindVersion(spec.ComponentSpark, version),
+		sparkVersion,
 		srcPath,
 		inst.GetHost(),
 		deployDir,
@@ -370,7 +369,7 @@ func (b *Builder) DeploySpark(inst spec.Instance, version, srcPath, deployDir st
 		inst.ComponentName(),
 		inst.OS(),
 		inst.Arch(),
-		version,
+		"", // use the latest stable version
 		srcPath,
 		inst.GetHost(),
 		deployDir,

pkg/cluster/task/copy_component.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 
 	"github.com/pingcap/tiup/pkg/cluster/spec"
+	"github.com/pingcap/tiup/pkg/environment"
 )
 
 // CopyComponent is used to copy all files related the specific version a component
@@ -39,6 +40,16 @@ func PackagePath(comp string, version string, os string, arch string) string {
 
 // Execute implements the Task interface
 func (c *CopyComponent) Execute(ctx *Context) error {
+	// If the version is not specified, the last stable one will be used
+	if c.version == "" {
+		env := environment.GlobalEnv()
+		ver, _, err := env.V1Repository().LatestStableVersion(c.component, false)
+		if err != nil {
+			return err
+		}
+		c.version = string(ver)
+	}
+
 	// Copy to remote server
 	srcPath := c.srcPath
 	if srcPath == "" {

pkg/cluster/task/download.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 
 	operator "github.com/pingcap/tiup/pkg/cluster/operation"
+	"github.com/pingcap/tiup/pkg/environment"
 )
 
 // Downloader is used to download the specific version of a component from
@@ -40,6 +41,15 @@ func NewDownloader(component string, os string, arch string, version string) *Do
 
 // Execute implements the Task interface
 func (d *Downloader) Execute(_ *Context) error {
+	// If the version is not specified, the last stable one will be used
+	if d.version == "" {
+		env := environment.GlobalEnv()
+		ver, _, err := env.V1Repository().LatestStableVersion(d.component, false)
+		if err != nil {
+			return err
+		}
+		d.version = string(ver)
+	}
 	return operator.Download(d.component, d.os, d.arch, d.version)
 }
 

pkg/repository/clone_mirror.go

@@ -44,7 +44,13 @@ type CloneOptions struct {
 }
 
 // CloneMirror clones a local mirror from the remote repository
-func CloneMirror(repo *V1Repository, components []string, targetDir string, selectedVersions []string, options CloneOptions) error {
+func CloneMirror(repo *V1Repository,
+	components []string,
+	tidbClusterVersionMapper func(string) string,
+	targetDir string,
+	selectedVersions []string,
+	options CloneOptions) error {
+
 	fmt.Printf("Start to clone mirror, targetDir is %s, selectedVersions are [%s]\n", targetDir, strings.Join(selectedVersions, ","))
 	fmt.Println("If this does not meet expectations, please abort this process, read `tiup mirror clone --help` and run again")
 
@@ -146,7 +152,7 @@ func CloneMirror(repo *V1Repository, components []string, targetDir string, sele
 	snapshot := v1manifest.NewSnapshot(initTime)
 	snapshot.SetExpiresAt(expirsAt)
 
-	componentManifests, err := cloneComponents(repo, components, selectedVersions, targetDir, tmpDir, options)
+	componentManifests, err := cloneComponents(repo, components, selectedVersions, tidbClusterVersionMapper, targetDir, tmpDir, options)
 	if err != nil {
 		return err
 	}
@@ -243,16 +249,18 @@ func CloneMirror(repo *V1Repository, components []string, targetDir string, sele
 
 func cloneComponents(repo *V1Repository,
 	components, selectedVersions []string,
+	tidbClusterVersionMapper func(string) string,
 	targetDir, tmpDir string,
 	options CloneOptions) (map[string]*v1manifest.Component, error) {
+
 	compManifests := map[string]*v1manifest.Component{}
 	for _, name := range components {
 		manifest, err := repo.FetchComponentManifest(name, true)
 		if err != nil {
 			return nil, errors.Annotatef(err, "fetch component '%s' manifest failed", name)
 		}
 
-		vs := combineVersions(options.Components[name], manifest, options.OSs, options.Archs, selectedVersions)
+		vs := combineVersions(options.Components[name], tidbClusterVersionMapper, manifest, options.OSs, options.Archs, selectedVersions)
 		var newManifest *v1manifest.Component
 		if options.Full {
 			newManifest = manifest
@@ -391,20 +399,17 @@ func checkVersion(options CloneOptions, versions set.StringSet, version string)
 	return false
 }
 
-func combineVersions(versions *[]string, manifest *v1manifest.Component, oss, archs, selectedVersions []string) set.StringSet {
+func combineVersions(versions *[]string,
+	tidbClusterVersionMapper func(string) string,
+	manifest *v1manifest.Component, oss, archs,
+	selectedVersions []string) set.StringSet {
+
 	if (versions == nil || len(*versions) < 1) && len(selectedVersions) < 1 {
 		return nil
 	}
 
-	switch manifest.ID {
-	case "alertmanager":
-		return set.NewStringSet("v0.17.0")
-	case "blackbox_exporter":
-		return set.NewStringSet("v0.12.0")
-	case "node_exporter":
-		return set.NewStringSet("v0.17.0")
-	case "pushgateway":
-		return set.NewStringSet("v0.7.0")
+	if bindver := tidbClusterVersionMapper(manifest.ID); bindver != "" {
+		return set.NewStringSet(bindver)
 	}
 
 	result := set.NewStringSet()

pkg/repository/v1manifest/key_store.go

@@ -15,19 +15,22 @@ package v1manifest
 
 import (
 	"fmt"
+	"sync"
 
 	"github.com/pingcap/errors"
 	"github.com/pingcap/tiup/pkg/crypto"
 )
 
-// KeyStore tracks roles, keys, etc. and verifies signatures against this metadata.
-type KeyStore map[string]roleKeys
+// KeyStore tracks roles, keys, etc. and verifies signatures against this metadata. (map[string]roleKeys)
+type KeyStore struct {
+	sync.Map
+}
 
 type roleKeys struct {
 	threshold uint
 	expiry    string
-	// key id -> public key
-	keys map[string]crypto.PubKey
+	// key id -> public key (map[string]crypto.PubKey)
+	keys *sync.Map
 }
 
 // NewKeyStore return a KeyStore
@@ -41,15 +44,17 @@ func (s *KeyStore) AddKeys(role string, threshold uint, expiry string, keys map[
 		return errors.Errorf("invalid threshold (0)")
 	}
 
-	(*s)[role] = roleKeys{threshold: threshold, expiry: expiry, keys: map[string]crypto.PubKey{}}
+	rk := roleKeys{threshold: threshold, expiry: expiry, keys: &sync.Map{}}
 
 	for id, info := range keys {
 		pub, err := info.publicKey()
 		if err != nil {
 			return err
 		}
-		(*s)[role].keys[id] = pub
+
+		rk.keys.Store(id, pub)
 	}
+	s.Store(role, rk)
 
 	return nil
 }
@@ -80,7 +85,7 @@ func (s *SignatureError) Error() string {
 // transitionRoot checks that signed is verified by signatures using newThreshold, and if so, updates the keys for the root
 // role in the key store.
 func (s *KeyStore) transitionRoot(signed []byte, newThreshold uint, expiry string, signatures []Signature, newKeys map[string]*KeyInfo) error {
-	oldKeys := (*s)[ManifestTypeRoot]
+	oldKeys, hasOldKeys := s.Load(ManifestTypeRoot)
 
 	err := s.AddKeys(ManifestTypeRoot, newThreshold, expiry, newKeys)
 	if err != nil {
@@ -90,7 +95,9 @@ func (s *KeyStore) transitionRoot(signed []byte, newThreshold uint, expiry strin
 	err = s.verifySignature(signed, ManifestTypeRoot, signatures, ManifestFilenameRoot)
 	if err != nil {
 		// Restore the old root keys.
-		(*s)[ManifestTypeRoot] = oldKeys
+		if hasOldKeys {
+			s.Store(ManifestTypeRoot, oldKeys)
+		}
 		return err
 	}
 
@@ -114,18 +121,19 @@ func (s *KeyStore) verifySignature(signed []byte, role string, signatures []Sign
 		has[sig.KeyID] = struct{}{}
 	}
 
-	keys, ok := (*s)[role]
+	ks, ok := s.Load(role)
 	if !ok {
 		return errors.Errorf("Unknown role %s", role)
 	}
+	keys := ks.(roleKeys)
 
 	var validSigs uint = 0
 	for _, sig := range signatures {
-		key, ok := keys.keys[sig.KeyID]
+		key, ok := keys.keys.Load(sig.KeyID)
 		if !ok {
 			continue
 		}
-		err := key.VerifySignature(signed, sig.Sig)
+		err := key.(crypto.PubKey).VerifySignature(signed, sig.Sig)
 		if err != nil {
 			return newSignatureError(filename, err)
 		}