Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: reuse cluster.DeployOption and spec.PDScaleScript #1253

Merged
merged 8 commits into from
Mar 31, 2021
2 changes: 1 addition & 1 deletion components/cluster/command/scale_out.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
)

func newScaleOutCmd() *cobra.Command {
opt := manager.ScaleOutOptions{
opt := manager.DeployOptions{
IdentityFile: filepath.Join(utils.UserHome(), ".ssh", "id_rsa"),
}
cmd := &cobra.Command{
Expand Down
2 changes: 1 addition & 1 deletion components/dm/command/scale_out.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
)

func newScaleOutCmd() *cobra.Command {
opt := manager.ScaleOutOptions{
opt := manager.DeployOptions{
IdentityFile: filepath.Join(utils.UserHome(), ".ssh", "id_rsa"),
}
cmd := &cobra.Command{
Expand Down
75 changes: 39 additions & 36 deletions pkg/cluster/manager/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func buildScaleOutTask(
name string,
metadata spec.Metadata,
mergedTopo spec.Topology,
opt ScaleOutOptions,
opt DeployOptions,
sshConnProps *cliutil.SSHConnectionProps,
newPart spec.Topology,
patchedComponents set.StringSet,
Expand Down Expand Up @@ -92,44 +92,46 @@ func buildScaleOutTask(
// uninitializedHosts are hosts which haven't been initialized yet
uninitializedHosts := make(map[string]hostInfo) // host -> ssh-port, os, arch
newPart.IterInstance(func(instance spec.Instance) {
if host := instance.GetHost(); !initializedHosts.Exist(host) {
if _, found := uninitializedHosts[host]; found {
return
}
host := instance.GetHost()
if initializedHosts.Exist(host) {
return
}
if _, found := uninitializedHosts[host]; found {
return
}

uninitializedHosts[host] = hostInfo{
ssh: instance.GetSSHPort(),
os: instance.OS(),
arch: instance.Arch(),
}
uninitializedHosts[host] = hostInfo{
ssh: instance.GetSSHPort(),
os: instance.OS(),
arch: instance.Arch(),
}

var dirs []string
globalOptions := metadata.GetTopology().BaseTopo().GlobalOptions
for _, dir := range []string{globalOptions.DeployDir, globalOptions.DataDir, globalOptions.LogDir} {
for _, dirname := range strings.Split(dir, ",") {
if dirname == "" {
continue
}
dirs = append(dirs, spec.Abs(globalOptions.User, dirname))
var dirs []string
globalOptions := metadata.GetTopology().BaseTopo().GlobalOptions
for _, dir := range []string{globalOptions.DeployDir, globalOptions.DataDir, globalOptions.LogDir} {
for _, dirname := range strings.Split(dir, ",") {
if dirname == "" {
continue
}
dirs = append(dirs, spec.Abs(globalOptions.User, dirname))
}
t := task.NewBuilder().
RootSSH(
instance.GetHost(),
instance.GetSSHPort(),
opt.User,
sshConnProps.Password,
sshConnProps.IdentityFile,
sshConnProps.IdentityFilePassphrase,
gOpt.SSHTimeout,
gOpt.SSHType,
globalOptions.SSHType,
).
EnvInit(instance.GetHost(), base.User, base.Group, opt.SkipCreateUser || globalOptions.User == opt.User).
Mkdir(globalOptions.User, instance.GetHost(), dirs...).
Build()
envInitTasks = append(envInitTasks, t)
}
t := task.NewBuilder().
RootSSH(
instance.GetHost(),
instance.GetSSHPort(),
opt.User,
sshConnProps.Password,
sshConnProps.IdentityFile,
sshConnProps.IdentityFilePassphrase,
gOpt.SSHTimeout,
gOpt.SSHType,
globalOptions.SSHType,
).
EnvInit(instance.GetHost(), base.User, base.Group, opt.SkipCreateUser || globalOptions.User == opt.User).
Mkdir(globalOptions.User, instance.GetHost(), dirs...).
Build()
envInitTasks = append(envInitTasks, t)
})

// Download missing component
Expand All @@ -146,7 +148,7 @@ func buildScaleOutTask(
logDir := spec.Abs(base.User, inst.LogDir())

deployDirs := []string{
deployDir, logDir,
deployDir,
filepath.Join(deployDir, "bin"),
filepath.Join(deployDir, "conf"),
filepath.Join(deployDir, "scripts"),
Expand All @@ -158,7 +160,8 @@ func buildScaleOutTask(
tb := task.NewBuilder().
UserSSH(inst.GetHost(), inst.GetSSHPort(), base.User, gOpt.SSHTimeout, gOpt.SSHType, topo.BaseTopo().GlobalOptions.SSHType).
Mkdir(base.User, inst.GetHost(), deployDirs...).
Mkdir(base.User, inst.GetHost(), dataDirs...)
Mkdir(base.User, inst.GetHost(), dataDirs...).
Mkdir(base.User, inst.GetHost(), logDir)

srcPath := ""
if patchedComponents.Exist(inst.ComponentName()) {
Expand Down
1 change: 0 additions & 1 deletion pkg/cluster/manager/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
)

// DeployOptions contains the options for scale out.
// TODO: merge ScaleOutOptions, should check config too when scale out.
type DeployOptions struct {
User string // username to login to the SSH server
SkipCreateUser bool // don't create the user
Expand Down
11 changes: 1 addition & 10 deletions pkg/cluster/manager/scale_out.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,13 @@ import (
"github.com/pingcap/tiup/pkg/utils"
)

// ScaleOutOptions contains the options for scale out.
type ScaleOutOptions struct {
User string // username to login to the SSH server
SkipCreateUser bool // don't create user
IdentityFile string // path to the private key file
UsePassword bool // use password instead of identity file for ssh connection
NoLabels bool // don't check labels for TiKV instance
}

// ScaleOut scale out the cluster.
func (m *Manager) ScaleOut(
name string,
topoFile string,
afterDeploy func(b *task.Builder, newPart spec.Topology),
final func(b *task.Builder, name string, meta spec.Metadata),
opt ScaleOutOptions,
opt DeployOptions,
skipConfirm bool,
gOpt operator.Options,
) error {
Expand Down
5 changes: 3 additions & 2 deletions pkg/cluster/spec/pd.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ func (i *PDInstance) ScaleConfig(
cluster := mustBeClusterTopo(topo)

spec := i.InstanceSpec.(*PDSpec)
cfg := scripts.NewPDScaleScript(
cfg0 := scripts.NewPDScript(
i.Name,
i.GetHost(),
paths.Deploy,
Expand All @@ -272,8 +272,9 @@ func (i *PDInstance) ScaleConfig(
AppendEndpoints(cluster.Endpoints(deployUser)...).
WithListenHost(i.GetListenHost())
if topo.BaseTopo().GlobalOptions.TLSEnabled {
cfg = cfg.WithScheme("https")
cfg0 = cfg0.WithScheme("https")
}
cfg := scripts.NewPDScaleScript(cfg0)

fp := filepath.Join(paths.Cache, fmt.Sprintf("run_pd_%s_%d.sh", i.GetHost(), i.GetPort()))
log.Infof("script path: %s", fp)
Expand Down
55 changes: 8 additions & 47 deletions pkg/cluster/template/scripts/pd.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"text/template"

"github.com/pingcap/tiup/embed"
"github.com/pingcap/tiup/pkg/logger/log"
)

// PDScript represent the data to generate pd config
Expand Down Expand Up @@ -91,7 +90,11 @@ func (c *PDScript) AppendEndpoints(ends ...*PDScript) *PDScript {

// Config generate the config file data.
func (c *PDScript) Config() ([]byte, error) {
fp := path.Join("templates", "scripts", "run_pd.sh.tpl")
return c.configWithScript("run_pd.sh.tpl")
}

func (c *PDScript) configWithScript(script string) ([]byte, error) {
fp := path.Join("templates", "scripts", script)
tpl, err := embed.ReadFile(fp)
if err != nil {
return nil, err
Expand Down Expand Up @@ -138,55 +141,13 @@ type PDScaleScript struct {
}

// NewPDScaleScript return a new PDScaleScript
func NewPDScaleScript(name, ip, deployDir, dataDir, logDir string) *PDScaleScript {
return &PDScaleScript{*NewPDScript(name, ip, deployDir, dataDir, logDir)}
}

// WithScheme set Scheme field of PDScaleScript
func (c *PDScaleScript) WithScheme(scheme string) *PDScaleScript {
c.Scheme = scheme
return c
}

// WithClientPort set ClientPort field of PDScaleScript
func (c *PDScaleScript) WithClientPort(port int) *PDScaleScript {
c.ClientPort = port
return c
}

// WithPeerPort set PeerPort field of PDScript
func (c *PDScaleScript) WithPeerPort(port int) *PDScaleScript {
c.PeerPort = port
return c
}

// WithNumaNode set NumaNode field of PDScaleScript
func (c *PDScaleScript) WithNumaNode(numa string) *PDScaleScript {
c.NumaNode = numa
return c
}

// AppendEndpoints add new PDScaleScript to Endpoints field
func (c *PDScaleScript) AppendEndpoints(ends ...*PDScript) *PDScaleScript {
c.Endpoints = append(c.Endpoints, ends...)
return c
}

// WithListenHost set listenHost field of PDScript
func (c *PDScaleScript) WithListenHost(listenHost string) *PDScaleScript {
c.ListenHost = listenHost
return c
func NewPDScaleScript(pdScript *PDScript) *PDScaleScript {
return &PDScaleScript{*pdScript}
}

// Config generate the config file data.
func (c *PDScaleScript) Config() ([]byte, error) {
fp := path.Join("templates", "scripts", "run_pd_scale.sh.tpl")
log.Infof("script path: %s", fp)
tpl, err := embed.ReadFile(fp)
if err != nil {
return nil, err
}
return c.ConfigWithTemplate(string(tpl))
return c.configWithScript("run_pd_scale.sh.tpl")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may not work as you expected since you removed func (c *PDScaleScript) WithXXX.

When we call WithXXX in scale out stage, the WithXXX will return a PDScript pointer because you removed WithXXX on PDScaleScript (it will fallback to PDScript's WithXXX). Then when you call ConfigToFile or Config, it will call on PDScript, so the script generated in scale-out stage is not correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your professional advice 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not work because PDScaleScript not implements ConfigToFile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, please take another look.

}

// ConfigToFile write config content to specific path
Expand Down
62 changes: 62 additions & 0 deletions pkg/cluster/template/scripts/pd_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2021 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.
package scripts

import (
"os"
"path"
"strings"
"testing"

. "github.com/pingcap/check"
)

type pdSuite struct{}

var _ = Suite(&pdSuite{})

func Test(t *testing.T) { TestingT(t) }

func (s *pdSuite) TestScaleConfig(c *C) {
confDir, err := os.MkdirTemp("", "tiup-*")
c.Assert(err, IsNil)
defer os.RemoveAll(confDir)

conf := path.Join(confDir, "pd0.conf")
pdScript := NewPDScript("pd", "1.1.1.1", "/home/deploy/pd-2379", "/home/pd-data", "/home/pd-log")
pdConfig, err := pdScript.Config()
c.Assert(err, IsNil)
c.Assert(strings.Contains(string(pdConfig), "--initial-cluster"), IsTrue)
c.Assert(strings.Contains(string(pdConfig), "--join"), IsFalse)

err = pdScript.ConfigToFile(conf)
c.Assert(err, IsNil)
content, err := os.ReadFile(conf)
c.Assert(err, IsNil)
c.Assert(strings.Contains(string(content), "--initial-cluster"), IsTrue)
c.Assert(strings.Contains(string(content), "--join"), IsFalse)

scScript := NewPDScaleScript(pdScript)
scConfig, err := scScript.Config()
c.Assert(err, IsNil)
c.Assert(pdConfig, Not(DeepEquals), scConfig)
c.Assert(strings.Contains(string(scConfig), "--initial-cluster"), IsFalse)
c.Assert(strings.Contains(string(scConfig), "--join"), IsTrue)

err = scScript.ConfigToFile(conf)
c.Assert(err, IsNil)
content, err = os.ReadFile(conf)
c.Assert(err, IsNil)
c.Assert(strings.Contains(string(content), "--initial-cluster"), IsFalse)
c.Assert(strings.Contains(string(content), "--join"), IsTrue)
}