Skip to content

Commit

Permalink
Improve playground and test (#715)
Browse files Browse the repository at this point in the history
* Improve playground and test

Do not call the Cmd.Process.Wait directly or we can not get the
Cmd.ProcessState and the Uptime show in display can't not find out
the process is quit. Also, Cmd.Process return the ProcessState and
nil if the process exit with not 0 code, so if the instance quit
unexpected will tail the last log of the instance will never work.

Setup signal handler before we boot the cluster. Before this PR, if
return before we setup the signal handler and receive signal(like user
press ctrl-c), playground will quit directly, but we may have started
some subprocess which become orphaned process.

Improve the integration test to make sure scale-in/out, display
basically work.
  • Loading branch information
july2993 authored Aug 27, 2020
1 parent 7724a71 commit e8a7b45
Show file tree
Hide file tree
Showing 10 changed files with 327 additions and 205 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/integrate.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ jobs:
# if: steps.test.outputs.exit_code != 0
if: always()
run: |
find ${{env.working-directory}}/tests/tiup-playground/_tmp/data -type f | grep -E '*/ti.*/ti.*.log$' | xargs tar czvf ${{env.working-directory}}/playground.logs.tar.gz
find ${{env.working-directory}}/tests/tiup-playground/_tmp/home/data -type f | grep -E '*/ti.*/ti.*.log$' | xargs tar czvf ${{env.working-directory}}/playground.logs.tar.gz
- name: Upload component log
# if: steps.test.outputs.exit_code != 0
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ bin/
/tests/tiup_mirrors/*.sha1
/tests/tiup_mirrors/
/logs
docker/secret/
14 changes: 13 additions & 1 deletion components/playground/grafana.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"path/filepath"
"regexp"
"strings"
"sync"

"github.com/pingcap/errors"
"github.com/pingcap/tiup/pkg/environment"
Expand All @@ -36,7 +37,9 @@ type grafana struct {
port int
version string

cmd *exec.Cmd
waitErr error
waitOnce sync.Once
cmd *exec.Cmd
}

func newGrafana(version string, host string) *grafana {
Expand Down Expand Up @@ -149,6 +152,7 @@ func makeSureDir(fname string) error {
var clusterName string = "playground"

// dir should contains files untar the grafana.
// return not error iff the Cmd is started successfully.
func (g *grafana) start(ctx context.Context, dir string, p8sURL string) (err error) {
g.port, err = utils.GetFreePort(g.host, 3000)
if err != nil {
Expand Down Expand Up @@ -208,3 +212,11 @@ http_port = %d

return g.cmd.Start()
}

func (g *grafana) wait() error {
g.waitOnce.Do(func() {
g.waitErr = g.cmd.Wait()
})

return g.waitErr
}
4 changes: 3 additions & 1 deletion components/playground/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type instance struct {
type Instance interface {
Pid() int
// Start the instance process.
// Will kill the process once the context is done.
Start(ctx context.Context, version v0manifest.Version) error
// Component Return the component name.
Component() string
Expand All @@ -52,7 +53,8 @@ type Instance interface {
// StatusAddrs return the address to pull metrics.
StatusAddrs() []string
// Wait Should only call this if the instance is started successfully.
Wait(ctx context.Context) error
// The implementation should be safe to call Wait multi times.
Wait() error
}

func (inst *instance) StatusAddrs() (addrs []string) {
Expand Down
20 changes: 14 additions & 6 deletions components/playground/instance/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import (
"io"
"os"
"os/exec"
"sync"
"time"

"github.com/pingcap/errors"
"github.com/pingcap/tiup/pkg/environment"
tiupexec "github.com/pingcap/tiup/pkg/exec"
"github.com/pingcap/tiup/pkg/repository/v0manifest"
"github.com/pingcap/tiup/pkg/utils"
)

// Process represent process to be run by playground
type Process interface {
Start() error
Wait(ctx context.Context) error
Wait() error
Pid() int
Uptime() string
SetOutputFile(fname string) error
Expand All @@ -28,6 +28,9 @@ type Process interface {
type process struct {
cmd *exec.Cmd
startTime time.Time

waitOnce sync.Once
waitErr error
}

// Start the process
Expand All @@ -38,8 +41,12 @@ func (p *process) Start() error {
}

// Wait implements Instance interface.
func (p *process) Wait(ctx context.Context) error {
return utils.WaitContext(ctx, p.cmd)
func (p *process) Wait() error {
p.waitOnce.Do(func() {
p.waitErr = p.cmd.Wait()
})

return p.waitErr
}

// Pid implements Instance interface.
Expand All @@ -50,8 +57,9 @@ func (p *process) Pid() int {
// Uptime implements Instance interface.
func (p *process) Uptime() string {
s := p.cmd.ProcessState
if s != nil && s.Exited() {
return "exited"

if s != nil {
return s.String()
}

duration := time.Since(p.startTime)
Expand Down
69 changes: 66 additions & 3 deletions components/playground/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ import (
"net/http"
_ "net/http/pprof"
"os"
"os/signal"
"os/user"
"path/filepath"
"strconv"
"strings"
"sync/atomic"
"syscall"
"time"

"github.com/fatih/color"
Expand Down Expand Up @@ -101,7 +104,8 @@ Examples:
$ tiup playground nightly --monitor=false # Start a local cluster and disable monitor system
$ tiup playground --pd.config ~/config/pd.toml # Start a local cluster with specified configuration file,
$ tiup playground --db.binpath /xx/tidb-server # Start a local cluster with component binary path`,
SilenceUsage: true,
SilenceUsage: true,
SilenceErrors: true,
Args: func(cmd *cobra.Command, args []string) error {
return nil
},
Expand All @@ -126,7 +130,66 @@ Examples:
}
environment.SetGlobalEnv(env)

return p.bootCluster(env, opt)
var booted uint32
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

go func() {
sc := make(chan os.Signal, 1)
signal.Notify(sc,
syscall.SIGHUP,
syscall.SIGINT,
syscall.SIGTERM,
syscall.SIGQUIT,
)

sig := (<-sc).(syscall.Signal)
atomic.StoreInt32(&p.curSig, int32(sig))
fmt.Println("Playground receive signal: ", sig)

// if bootCluster is not done we just cancel context to make it
// clean up and return ASAP and exit directly after timeout.
// Note now bootCluster can not learn the context is done and return quickly now
// like while it's downloading component.
if atomic.LoadUint32(&booted) == 0 {
cancel()
time.AfterFunc(time.Second, func() {
os.Exit(0)
})
return
}

go p.terminate(sig)
// If user try double ctrl+c, force quit
sig = (<-sc).(syscall.Signal)
atomic.StoreInt32(&p.curSig, int32(syscall.SIGKILL))
if sig == syscall.SIGINT {
p.terminate(syscall.SIGKILL)
}
}()

// TODO: we can set Pdeathsig of Cmd.SysProcAttr(linux only) in all the Cmd we started to kill
// all the process we start instead of let the orphaned child process adopted by init,
// this can make sure we kill all process event if
// playground is killed -9.
// ref: https://medium.com/@ganeshmaharaj/clean-exit-of-golangs-exec-command-897832ac3fa5
bootErr := p.bootCluster(ctx, env, opt)
if bootErr != nil {
// always kill all process started and wait before quit.
atomic.StoreInt32(&p.curSig, int32(syscall.SIGKILL))
p.terminate(syscall.SIGKILL)
_ = p.wait()
return errors.Annotate(bootErr, "Playground bootstrapping failed")
}

atomic.StoreUint32(&booted, 1)

waitErr := p.wait()
if waitErr != nil {
return errors.AddStack(waitErr)
}

return nil
},
}

Expand Down Expand Up @@ -292,7 +355,7 @@ func newEtcdClient(endpoint string) (*clientv3.Client, error) {

func main() {
if err := execute(); err != nil {
fmt.Printf("Playground bootstrapping failed: %v\n", err)
fmt.Println(color.RedString("Error: %v", err))
os.Exit(1)
}
}
27 changes: 19 additions & 8 deletions components/playground/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"os/exec"
"path/filepath"
"sync"

"github.com/pingcap/errors"
"github.com/pingcap/tiup/pkg/environment"
Expand Down Expand Up @@ -69,20 +70,28 @@ type monitor struct {
cmd *exec.Cmd

sdFname string

waitErr error
waitOnce sync.Once
}

func newMonitor() *monitor {
return &monitor{}
func (m *monitor) wait() error {
m.waitOnce.Do(func() {
m.waitErr = m.cmd.Wait()
})

return m.waitErr
}

func (m *monitor) startMonitor(ctx context.Context, version string, host, dir string) (int, *exec.Cmd, error) {
// the cmd is not started after return
func newMonitor(ctx context.Context, version string, host, dir string) (*monitor, error) {
if err := os.MkdirAll(dir, 0755); err != nil {
return 0, nil, err
return nil, errors.AddStack(err)
}

port, err := utils.GetFreePort(host, 9090)
if err != nil {
return 0, nil, err
return nil, errors.AddStack(err)
}
addr := fmt.Sprintf("%s:%d", host, port)

Expand Down Expand Up @@ -114,10 +123,11 @@ scrape_configs:
`

m := new(monitor)
m.sdFname = filepath.Join(dir, "targets.json")

if err := ioutil.WriteFile(filepath.Join(dir, "prometheus.yml"), []byte(tmpl), os.ModePerm); err != nil {
return 0, nil, err
return nil, errors.AddStack(err)
}

args := []string{
Expand All @@ -130,11 +140,12 @@ scrape_configs:
env := environment.GlobalEnv()
cmd, err := tiupexec.PrepareCommand(ctx, "prometheus", v0manifest.Version(version), "", "", dir, dir, args, env)
if err != nil {
return 0, nil, err
return nil, err
}

m.port = port
m.cmd = cmd
m.host = host
return port, cmd, nil

return m, nil
}
Loading

0 comments on commit e8a7b45

Please sign in to comment.