From dc3a20efe48dc8a61de86c137594642edb955579 Mon Sep 17 00:00:00 2001 From: Allen Zhong Date: Mon, 1 Feb 2021 19:51:46 +0800 Subject: [PATCH] cluster: check permission for deploy/data dir for exist clusters (#1107) * cluster: check write permission for deploy_dir/data_dir for exist clusters * cluster/check: fix for checking dir permissions for each instance * remove redudant uniqueInsts set Co-authored-by: Ti Chi Robot <71242396+ti-chi-bot@users.noreply.github.com> --- components/cluster/command/check.go | 243 +++++++++++++++------------- pkg/cluster/operation/check.go | 62 +++++-- pkg/cluster/task/builder.go | 12 +- pkg/cluster/task/check.go | 23 ++- 4 files changed, 197 insertions(+), 143 deletions(-) diff --git a/components/cluster/command/check.go b/components/cluster/command/check.go index b6a3c901a8..16d23c8b8f 100644 --- a/components/cluster/command/check.go +++ b/components/cluster/command/check.go @@ -174,126 +174,23 @@ func checkSystemInfo(s *cliutil.SSHConnectionProps, topo *spec.Specification, gO downloadTasks = append(downloadTasks, t0) } - if _, found := uniqueHosts[inst.GetHost()]; found { - continue - } - - uniqueHosts[inst.GetHost()] = inst.GetSSHPort() - - // build system info collecting tasks - t1 := task.NewBuilder(). - RootSSH( - inst.GetHost(), - inst.GetSSHPort(), - opt.user, - s.Password, - s.IdentityFile, - s.IdentityFilePassphrase, - gOpt.SSHTimeout, - gOpt.SSHType, - topo.GlobalOptions.SSHType, - ). - Mkdir(opt.user, inst.GetHost(), filepath.Join(task.CheckToolsPathDir, "bin")). - CopyComponent( - spec.ComponentCheckCollector, - inst.OS(), - inst.Arch(), - insightVer, - "", // use default srcPath + t1 := task.NewBuilder() + // checks that applies to each instance + if opt.existCluster { + t1 = t1.CheckSys( inst.GetHost(), - task.CheckToolsPathDir, - ). - Shell( - inst.GetHost(), - filepath.Join(task.CheckToolsPathDir, "bin", "insight"), - "", - false, - ). - BuildAsStep(fmt.Sprintf(" - Getting system info of %s:%d", inst.GetHost(), inst.GetSSHPort())) - collectTasks = append(collectTasks, t1) - - // build checking tasks - t2 := task.NewBuilder(). - // check for general system info - CheckSys( - inst.GetHost(), - "", - task.CheckTypeSystemInfo, - topo, - opt.opr, - ). - CheckSys( - inst.GetHost(), - "", - task.CheckTypePartitions, - topo, - opt.opr, - ). - // check for listening port - Shell( - inst.GetHost(), - "ss -lnt", - "", - false, - ). - CheckSys( - inst.GetHost(), - "", - task.CheckTypePort, - topo, - opt.opr, - ). - // check for system limits - Shell( - inst.GetHost(), - "cat /etc/security/limits.conf", - "", - false, - ). - CheckSys( - inst.GetHost(), - "", - task.CheckTypeSystemLimits, - topo, - opt.opr, - ). - // check for kernel params - Shell( - inst.GetHost(), - "sysctl -a", - "", - true, - ). - CheckSys( - inst.GetHost(), - "", - task.CheckTypeSystemConfig, - topo, - opt.opr, - ). - // check for needed system service - CheckSys( - inst.GetHost(), - "", - task.CheckTypeService, - topo, - opt.opr, - ). - // check for needed packages - CheckSys( - inst.GetHost(), - "", - task.CheckTypePackage, + inst.DeployDir(), + task.CheckTypePermission, topo, opt.opr, ) - + } // if the data dir set in topology is relative, and the home dir of deploy user // and the user run the check command is on different partitions, the disk detection // may be using incorrect partition for validations. for _, dataDir := range spec.MultiDirAbs(opt.user, inst.DataDir()) { // build checking tasks - t2 = t2. + t1 = t1. CheckSys( inst.GetHost(), dataDir, @@ -301,10 +198,132 @@ func checkSystemInfo(s *cliutil.SSHConnectionProps, topo *spec.Specification, gO topo, opt.opr, ) + if opt.existCluster { + t1 = t1.CheckSys( + inst.GetHost(), + dataDir, + task.CheckTypePermission, + topo, + opt.opr, + ) + } } + + // checks that applies to each host + if _, found := uniqueHosts[inst.GetHost()]; !found { + uniqueHosts[inst.GetHost()] = inst.GetSSHPort() + // build system info collecting tasks + t2 := task.NewBuilder(). + RootSSH( + inst.GetHost(), + inst.GetSSHPort(), + opt.user, + s.Password, + s.IdentityFile, + s.IdentityFilePassphrase, + gOpt.SSHTimeout, + gOpt.SSHType, + topo.GlobalOptions.SSHType, + ). + Mkdir(opt.user, inst.GetHost(), filepath.Join(task.CheckToolsPathDir, "bin")). + CopyComponent( + spec.ComponentCheckCollector, + inst.OS(), + inst.Arch(), + insightVer, + "", // use default srcPath + inst.GetHost(), + task.CheckToolsPathDir, + ). + Shell( + inst.GetHost(), + filepath.Join(task.CheckToolsPathDir, "bin", "insight"), + "", + false, + ). + BuildAsStep(fmt.Sprintf(" - Getting system info of %s:%d", inst.GetHost(), inst.GetSSHPort())) + collectTasks = append(collectTasks, t2) + + // build checking tasks + t1 = t1. + // check for general system info + CheckSys( + inst.GetHost(), + "", + task.CheckTypeSystemInfo, + topo, + opt.opr, + ). + CheckSys( + inst.GetHost(), + "", + task.CheckTypePartitions, + topo, + opt.opr, + ). + // check for listening port + Shell( + inst.GetHost(), + "ss -lnt", + "", + false, + ). + CheckSys( + inst.GetHost(), + "", + task.CheckTypePort, + topo, + opt.opr, + ). + // check for system limits + Shell( + inst.GetHost(), + "cat /etc/security/limits.conf", + "", + false, + ). + CheckSys( + inst.GetHost(), + "", + task.CheckTypeSystemLimits, + topo, + opt.opr, + ). + // check for kernel params + Shell( + inst.GetHost(), + "sysctl -a", + "", + true, + ). + CheckSys( + inst.GetHost(), + "", + task.CheckTypeSystemConfig, + topo, + opt.opr, + ). + // check for needed system service + CheckSys( + inst.GetHost(), + "", + task.CheckTypeService, + topo, + opt.opr, + ). + // check for needed packages + CheckSys( + inst.GetHost(), + "", + task.CheckTypePackage, + topo, + opt.opr, + ) + } + checkSysTasks = append( checkSysTasks, - t2.BuildAsStep(fmt.Sprintf(" - Checking node %s", inst.GetHost())), + t1.BuildAsStep(fmt.Sprintf(" - Checking node %s", inst.GetHost())), ) t3 := task.NewBuilder(). diff --git a/pkg/cluster/operation/check.go b/pkg/cluster/operation/check.go index b5d24a22dc..ae56fcc1a6 100644 --- a/pkg/cluster/operation/check.go +++ b/pkg/cluster/operation/check.go @@ -43,23 +43,24 @@ type CheckOptions struct { // Names of checks var ( - CheckNameGeneral = "general" // errors that don't fit any specific check - CheckNameNTP = "ntp" - CheckNameOSVer = "os-version" - CheckNameSwap = "swap" - CheckNameSysctl = "sysctl" - CheckNameCPUThreads = "cpu-cores" - CheckNameCPUGovernor = "cpu-governor" - CheckNameDisks = "disk" - CheckNamePortListen = "listening-port" - CheckNameEpoll = "epoll-exclusive" - CheckNameMem = "memory" - CheckNameLimits = "limits" - CheckNameSysService = "service" - CheckNameSELinux = "selinux" - CheckNameCommand = "command" - CheckNameFio = "fio" - CheckNameTHP = "thp" + CheckNameGeneral = "general" // errors that don't fit any specific check + CheckNameNTP = "ntp" + CheckNameOSVer = "os-version" + CheckNameSwap = "swap" + CheckNameSysctl = "sysctl" + CheckNameCPUThreads = "cpu-cores" + CheckNameCPUGovernor = "cpu-governor" + CheckNameDisks = "disk" + CheckNamePortListen = "listening-port" + CheckNameEpoll = "epoll-exclusive" + CheckNameMem = "memory" + CheckNameLimits = "limits" + CheckNameSysService = "service" + CheckNameSELinux = "selinux" + CheckNameCommand = "command" + CheckNameFio = "fio" + CheckNameTHP = "thp" + CheckNameDirPermission = "permission" ) // CheckResult is the result of a check @@ -766,3 +767,30 @@ func CheckJRE(ctx context.Context, e ctxt.Executor, host string, topo *spec.Spec return results } + +// CheckDirPermission checks if the user can write to given path +func CheckDirPermission(ctx context.Context, e ctxt.Executor, user, path string) []*CheckResult { + var results []*CheckResult + + _, stderr, err := e.Execute(ctx, + fmt.Sprintf( + "sudo -u %[1]s touch %[2]s/.tiup_cluster_check_file && rm -f %[2]s/.tiup_cluster_check_file", + user, + path, + ), + false) + if err != nil || len(stderr) > 0 { + results = append(results, &CheckResult{ + Name: CheckNameDirPermission, + Err: fmt.Errorf("unable to write to dir %s: %s", path, strings.Split(string(stderr), "\n")[0]), + Msg: fmt.Sprintf("%s: %s", path, err), + }) + } else { + results = append(results, &CheckResult{ + Name: CheckNameDirPermission, + Msg: fmt.Sprintf("%s is writable", path), + }) + } + + return results +} diff --git a/pkg/cluster/task/builder.go b/pkg/cluster/task/builder.go index 243e9d62a7..5edb00ec1c 100644 --- a/pkg/cluster/task/builder.go +++ b/pkg/cluster/task/builder.go @@ -336,13 +336,13 @@ func (b *Builder) Limit(host, domain, limit, item, value string) *Builder { } // CheckSys checks system information of deploy server -func (b *Builder) CheckSys(host, dataDir, checkType string, topo *spec.Specification, opt *operator.CheckOptions) *Builder { +func (b *Builder) CheckSys(host, dir, checkType string, topo *spec.Specification, opt *operator.CheckOptions) *Builder { b.tasks = append(b.tasks, &CheckSys{ - host: host, - topo: topo, - opt: opt, - dataDir: dataDir, - check: checkType, + host: host, + topo: topo, + opt: opt, + checkDir: dir, + check: checkType, }) return b } diff --git a/pkg/cluster/task/check.go b/pkg/cluster/task/check.go index 43f0fcecbd..4a835346f2 100644 --- a/pkg/cluster/task/check.go +++ b/pkg/cluster/task/check.go @@ -35,6 +35,7 @@ var ( CheckTypePackage = "package" CheckTypePartitions = "partitions" CheckTypeFIO = "fio" + CheckTypePermission = "permission" ) // place the check utilities are stored @@ -44,11 +45,11 @@ const ( // CheckSys performs checks of system information type CheckSys struct { - host string - topo *spec.Specification - opt *operator.CheckOptions - check string // check type name - dataDir string + host string + topo *spec.Specification + opt *operator.CheckOptions + check string // check type name + checkDir string } func storeResults(ctx context.Context, host string, results []*operator.CheckResult) { @@ -130,7 +131,7 @@ func (c *CheckSys) Execute(ctx context.Context) error { // check partition mount options for data_dir storeResults(ctx, c.host, operator.CheckPartitions(c.opt, c.host, c.topo, stdout)) case CheckTypeFIO: - if !c.opt.EnableDisk || c.dataDir == "" { + if !c.opt.EnableDisk || c.checkDir == "" { break } @@ -140,6 +141,12 @@ func (c *CheckSys) Execute(ctx context.Context) error { } storeResults(ctx, c.host, operator.CheckFIOResult(rr, rw, lat)) + case CheckTypePermission: + e, ok := ctxt.GetInner(ctx).GetExecutor(c.host) + if !ok { + return ErrNoExecutor + } + storeResults(ctx, c.host, operator.CheckDirPermission(ctx, e, c.topo.GlobalOptions.User, c.checkDir)) } return nil @@ -163,8 +170,8 @@ func (c *CheckSys) runFIO(ctx context.Context) (outRR []byte, outRW []byte, outL return } - dataDir := spec.Abs(c.topo.GlobalOptions.User, c.dataDir) - testWd := filepath.Join(dataDir, "tiup-fio-test") + checkDir := spec.Abs(c.topo.GlobalOptions.User, c.checkDir) + testWd := filepath.Join(checkDir, "tiup-fio-test") fioBin := filepath.Join(CheckToolsPathDir, "bin", "fio") var stderr []byte