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

Retain process logs on robustness tests #16077

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions tests/framework/e2e/cluster_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,14 @@ func (p *proxyEtcdProcess) Logs() LogsExpect {
return p.etcdProc.Logs()
}

func (p *proxyEtcdProcess) Lines() []string {
return p.etcdProc.Logs().Lines()
}

func (p *proxyEtcdProcess) LineCount() int {
return p.etcdProc.Logs().LineCount()
}

func (p *proxyEtcdProcess) Kill() error {
return p.etcdProc.Kill()
}
Expand Down
42 changes: 37 additions & 5 deletions tests/framework/e2e/etcd_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,14 @@ var (
EtcdServerReadyLines = []string{"ready to serve client requests"}
)

type LogLines interface {
Lines() []string
LineCount() int
}

// EtcdProcess is a process that serves etcd requests.
type EtcdProcess interface {
LogLines
EndpointsGRPC() []string
EndpointsHTTP() []string
EndpointsMetrics() []string
Expand All @@ -61,9 +67,8 @@ type EtcdProcess interface {
}

type LogsExpect interface {
LogLines
ExpectWithContext(context.Context, string) (string, error)
Lines() []string
LineCount() int
}

type EtcdServerProcess struct {
Expand All @@ -72,6 +77,7 @@ type EtcdServerProcess struct {
proxy proxy.Server
failpoints *BinaryFailpoints
donec chan struct{} // closed when Interact() terminates
logs *procLogs
Copy link
Member

@serathius serathius Jun 15, 2023

Choose a reason for hiding this comment

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

Suggested change
logs *procLogs
logs procLogs

It doesn't need to be a pointer.

}

type EtcdServerProcessConfig struct {
Expand Down Expand Up @@ -108,7 +114,7 @@ func NewEtcdServerProcess(cfg *EtcdServerProcessConfig) (*EtcdServerProcess, err
return nil, err
}
}
ep := &EtcdServerProcess{cfg: cfg, donec: make(chan struct{})}
ep := &EtcdServerProcess{cfg: cfg, donec: make(chan struct{}), logs: &procLogs{}}
if cfg.GoFailPort != 0 {
ep.failpoints = &BinaryFailpoints{member: ep}
}
Expand All @@ -124,8 +130,8 @@ func (ep *EtcdServerProcess) EndpointsHTTP() []string {
}
func (ep *EtcdServerProcess) EndpointsMetrics() []string { return []string{ep.cfg.MetricsURL} }

func (epc *EtcdServerProcess) Etcdctl(opts ...config.ClientOption) *EtcdctlV3 {
etcdctl, err := NewEtcdctl(epc.Config().Client, epc.EndpointsGRPC(), opts...)
func (ep *EtcdServerProcess) Etcdctl(opts ...config.ClientOption) *EtcdctlV3 {
etcdctl, err := NewEtcdctl(ep.Config().Client, ep.EndpointsGRPC(), opts...)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -177,6 +183,7 @@ func (ep *EtcdServerProcess) Stop() (err error) {
return nil
}
defer func() {
ep.logs.append(ep.proc.Lines())
ep.proc = nil
}()

Expand Down Expand Up @@ -235,6 +242,14 @@ func (ep *EtcdServerProcess) Logs() LogsExpect {
return ep.proc
}

func (ep *EtcdServerProcess) Lines() []string {
Copy link
Member

Choose a reason for hiding this comment

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

Proposed interface member.Logs() and member.Lines() seems unnatural. Recommendation:

  • Rename member.Logs() to member.ProcessLogs() to inform that those logs come only from the started process.
  • Rename member.Lines() to member.DumpLogs().

return ep.logs.Lines()
Copy link
Member

Choose a reason for hiding this comment

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

I find it pretty unnatural that calling member.Lines() will not return me any log lines until the process has exited.

}

func (ep *EtcdServerProcess) LineCount() int {
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't looks like it's used by anyone. Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just two places ultimately: https://github.com/search?q=repo%3Aetcd-io%2Fetcd%20LineCount&type=code

but looks like it could be removed

Copy link
Member

Choose a reason for hiding this comment

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

I mean it's not needed for EtcdServerProcess.

return ep.logs.LineCount()
}

func (ep *EtcdServerProcess) Kill() error {
ep.cfg.lg.Info("killing server...", zap.String("name", ep.cfg.Name))
return ep.proc.Signal(syscall.SIGKILL)
Expand Down Expand Up @@ -378,3 +393,20 @@ func GetVersionFromBinary(binaryPath string) (*semver.Version, error) {

return nil, fmt.Errorf("could not find version in binary output of %s, lines outputted were %v", binaryPath, lines)
}

type procLogs struct {
Copy link
Member

Choose a reason for hiding this comment

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

Don't think the name makes sense. Does "proc" refer to process or something else? As this merges log lines from multiple process lines it's definitely not processLogs.

procLogs []string
Copy link
Member

Choose a reason for hiding this comment

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

You know you don't need to create struct just have type logs []string ?

}

func (m *procLogs) append(lines []string) {
m.procLogs = append(m.procLogs, strings.Repeat("=", 50)+"\n")
m.procLogs = append(m.procLogs, lines...)
}

func (m *procLogs) Lines() []string {
return m.procLogs
}

func (m *procLogs) LineCount() int {
return len(m.procLogs)
}
11 changes: 11 additions & 0 deletions tests/robustness/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ func (r *report) Report(t *testing.T, force bool) {
for _, member := range r.clus.Procs {
memberDataDir := filepath.Join(path, fmt.Sprintf("server-%s", member.Config().Name))
persistMemberDataDir(t, r.lg, member, memberDataDir)

logsFile := filepath.Join(path, fmt.Sprintf("server-%s.log", member.Config().Name))
persistMemberLogs(t, r.lg, member, logsFile)
}
if r.clientReports != nil {
sort.Slice(r.clientReports, func(i, j int) bool {
Expand Down Expand Up @@ -118,6 +121,14 @@ func persistWatchResponses(t *testing.T, lg *zap.Logger, path string, responses
}
}

func persistMemberLogs(t *testing.T, lg *zap.Logger, member e2e.EtcdProcess, path string) {
lg.Info("Saving member logs dir", zap.String("member", member.Config().Name), zap.String("path", path))
err := os.WriteFile(path, []byte(strings.Join(member.Lines(), "")), 0644)
if err != nil {
t.Fatal(err)
}
}

func persistWatchEvents(t *testing.T, lg *zap.Logger, path string, events []traffic.TimedWatchEvent) {
lg.Info("Saving watch events", zap.String("path", path))
file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_TRUNC, 0755)
Expand Down
4 changes: 3 additions & 1 deletion tests/robustness/validate/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ import (
)

func validateOperationHistoryAndReturnVisualize(t *testing.T, lg *zap.Logger, operations []porcupine.Operation) (visualize func(basepath string)) {
linearizable, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, 5*time.Minute)
const timeout = 5 * time.Minute
lg.Info("Running porcupine to check operations", zap.String("model", "NonDeterministicModel"), zap.Duration("timeout", timeout))
linearizable, info := porcupine.CheckOperationsVerbose(model.NonDeterministicModel, operations, timeout)
if linearizable == porcupine.Illegal {
t.Error("Model is not linearizable")
}
Expand Down