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.

Sign up for GitHub

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

Jump to bottom

Address flakiness in command/exec tests #4517

Merged
merged 3 commits into from
Aug 10, 2018
Merged

Address flakiness in command/exec tests #4517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3015d28
Add fn to wait for TestAgent node and check registration
freddygv Aug 10, 2018
255e730
Add waits for TestAgent and retries before timeouts
freddygv Aug 10, 2018
d951d46
Restore original WaitForLeader signature
freddygv Aug 10, 2018
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
94 changes: 58 additions & 36 deletions command/exec/exec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"
"time"

"github.com/hashicorp/consul/testrpc"

"github.com/hashicorp/consul/agent"
consulapi "github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/testutil/retry"
Expand All @@ -25,6 +27,8 @@ func TestExecCommand(t *testing.T) {
`)
defer a.Shutdown()

testrpc.WaitForTestAgent(t, a.RPC, "dc1")

ui := cli.NewMockUi()
c := New(ui, nil)
args := []string{"-http-addr=" + a.HTTPAddr(), "-wait=1s", "uptime"}
Expand All @@ -46,6 +50,8 @@ func TestExecCommand_NoShell(t *testing.T) {
`)
defer a.Shutdown()

testrpc.WaitForTestAgent(t, a.RPC, "dc1")

ui := cli.NewMockUi()
c := New(ui, nil)
args := []string{"-http-addr=" + a.HTTPAddr(), "-shell=false", "-wait=1s", "uptime"}
Expand All @@ -67,26 +73,28 @@ func TestExecCommand_CrossDC(t *testing.T) {
`)
defer a1.Shutdown()

testrpc.WaitForTestAgent(t, a1.RPC, "dc1")

a2 := agent.NewTestAgent(t.Name(), `
datacenter = "dc2"
disable_remote_exec = false
`)
defer a2.Shutdown()

testrpc.WaitForTestAgent(t, a2.RPC, "dc2")

// Join over the WAN
_, err := a2.JoinWAN([]string{a1.Config.SerfBindAddrWAN.String()})
if err != nil {
t.Fatalf("err: %v", err)
}

retry.Run(t, func(r *retry.R) {
if got, want := len(a1.WANMembers()), 2; got != want {
r.Fatalf("got %d WAN members on a1 want %d", got, want)
}
if got, want := len(a2.WANMembers()), 2; got != want {
r.Fatalf("got %d WAN members on a2 want %d", got, want)
}
})
if got, want := len(a1.WANMembers()), 2; got != want {
t.Fatalf("got %d WAN members on a1 want %d", got, want)
}
if got, want := len(a2.WANMembers()), 2; got != want {
t.Fatalf("got %d WAN members on a2 want %d", got, want)
}

ui := cli.NewMockUi()
c := New(ui, nil)
Expand Down Expand Up @@ -145,6 +153,8 @@ func TestExecCommand_Sessions(t *testing.T) {
`)
defer a.Shutdown()

testrpc.WaitForTestAgent(t, a.RPC, "dc1")

ui := cli.NewMockUi()
c := New(ui, nil)
c.apiclient = a.Client()
Expand Down Expand Up @@ -233,6 +243,8 @@ func TestExecCommand_UploadDestroy(t *testing.T) {
`)
defer a.Shutdown()

testrpc.WaitForTestAgent(t, a.RPC, "dc1")

ui := cli.NewMockUi()

c := New(ui, nil)
Expand Down Expand Up @@ -288,6 +300,8 @@ func TestExecCommand_StreamResults(t *testing.T) {
`)
defer a.Shutdown()

testrpc.WaitForTestAgent(t, a.RPC, "dc1")

ui := cli.NewMockUi()
c := New(ui, nil)
c.apiclient = a.Client()
Expand Down Expand Up @@ -320,14 +334,16 @@ func TestExecCommand_StreamResults(t *testing.T) {
t.Fatalf("should be ok bro")
}

select {
case a := <-ackCh:
if a.Node != "foo" {
t.Fatalf("bad: %#v", a)
retry.Run(t, func(r *retry.R) {
select {
case a := <-ackCh:
if a.Node != "foo" {
r.Fatalf("bad: %#v", a)
}
case <-time.After(50 * time.Millisecond):
r.Fatalf("timeout")
}
case <-time.After(50 * time.Millisecond):
t.Fatalf("timeout")
}
})

ok, _, err = a.Client().KV().Acquire(&consulapi.KVPair{
Key: prefix + "foo/exit",
Expand All @@ -341,14 +357,16 @@ func TestExecCommand_StreamResults(t *testing.T) {
t.Fatalf("should be ok bro")
}

select {
case e := <-exitCh:
if e.Node != "foo" || e.Code != 127 {
t.Fatalf("bad: %#v", e)
retry.Run(t, func(r *retry.R) {
select {
case e := <-exitCh:
if e.Node != "foo" || e.Code != 127 {
r.Fatalf("bad: %#v", e)
}
case <-time.After(50 * time.Millisecond):
r.Fatalf("timeout")
}
case <-time.After(50 * time.Millisecond):
t.Fatalf("timeout")
}
})

// Random key, should ignore
ok, _, err = a.Client().KV().Acquire(&consulapi.KVPair{
Expand All @@ -374,14 +392,16 @@ func TestExecCommand_StreamResults(t *testing.T) {
t.Fatalf("should be ok bro")
}

select {
case h := <-heartCh:
if h.Node != "foo" {
t.Fatalf("bad: %#v", h)
retry.Run(t, func(r *retry.R) {
select {
case h := <-heartCh:
if h.Node != "foo" {
r.Fatalf("bad: %#v", h)
}
case <-time.After(50 * time.Millisecond):
r.Fatalf("timeout")
}
case <-time.After(50 * time.Millisecond):
t.Fatalf("timeout")
}
})

// Output value
ok, _, err = a.Client().KV().Acquire(&consulapi.KVPair{
Expand All @@ -396,12 +416,14 @@ func TestExecCommand_StreamResults(t *testing.T) {
t.Fatalf("should be ok bro")
}

select {
case o := <-outputCh:
if o.Node != "foo" || string(o.Output) != "test" {
t.Fatalf("bad: %#v", o)
retry.Run(t, func(r *retry.R) {
select {
case o := <-outputCh:
if o.Node != "foo" || string(o.Output) != "test" {
r.Fatalf("bad: %#v", o)
}
case <-time.After(50 * time.Millisecond):
r.Fatalf("timeout")
}
case <-time.After(50 * time.Millisecond):
t.Fatalf("timeout")
}
})
}
35 changes: 34 additions & 1 deletion testrpc/wait.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (

type rpcFn func(string, interface{}, interface{}) error

// WaitForLeader ensures we have a leader and a node registration.
func WaitForLeader(t *testing.T, rpc rpcFn, dc string) {
var out structs.IndexedNodes
retry.Run(t, func(r *retry.R) {
// Ensure we have a leader and a node registration.
args := &structs.DCSpecificRequest{Datacenter: dc}
if err := rpc("Catalog.ListNodes", args, &out); err != nil {
r.Fatalf("Catalog.ListNodes failed: %v", err)
Expand All @@ -25,3 +25,36 @@ func WaitForLeader(t *testing.T, rpc rpcFn, dc string) {
}
})
}

// WaitForTestAgent ensures we have a node with serfHealth check registered
func WaitForTestAgent(t *testing.T, rpc rpcFn, dc string) {
var nodes structs.IndexedNodes
var checks structs.IndexedHealthChecks

retry.Run(t, func(r *retry.R) {
dcReq := &structs.DCSpecificRequest{Datacenter: dc}
if err := rpc("Catalog.ListNodes", dcReq, &nodes); err != nil {
r.Fatalf("Catalog.ListNodes failed: %v", err)
}
if len(nodes.Nodes) == 0 {
r.Fatalf("No registered nodes")
}

// This assumes that there is a single agent per dc, typically a TestAgent
nodeReq := &structs.NodeSpecificRequest{Datacenter: dc, Node: nodes.Nodes[0].Node}
if err := rpc("Health.NodeChecks", nodeReq, &checks); err != nil {
r.Fatalf("Health.NodeChecks failed: %v", err)
}

var found bool
for _, check := range checks.HealthChecks {
if check.CheckID == "serfHealth" {
found = true
break
}
}
if !found {
r.Fatalf("serfHealth check not found")
}
})
}