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 <[email protected]>
pingcap · Feb 1, 2021 · dc3a20e · dc3a20e
1 parent 55a58ff
commit dc3a20e
Show file tree

Hide file tree

Showing 4 changed files with 197 additions and 143 deletions.
diff --git a/components/cluster/command/check.go b/components/cluster/command/check.go
@@ -174,137 +174,156 @@ 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,
 						task.CheckTypeFIO,
 						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
@@ -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
@@ -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
 }