From 67a6ba5e02fe7e36a4116ebcc1646667ff3b9bd2 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Fri, 4 Mar 2022 08:55:36 -0500 Subject: [PATCH] e2e: use context for executing external commands (#12185) If any E2E test hangs, it'll eventually timeout and panic, causing the all the remaining tests to fail. External commands should use a short context whenever possible so we can fail the test quickly and move on to the next test. --- e2e/csi/csi.go | 12 +++++++++++- e2e/e2eutil/cli.go | 6 +++++- e2e/e2eutil/job.go | 27 ++++++++++++++++++++------- e2e/vaultsecrets/vaultsecrets.go | 5 ++++- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/e2e/csi/csi.go b/e2e/csi/csi.go index dc2b36473..3fdc15c49 100644 --- a/e2e/csi/csi.go +++ b/e2e/csi/csi.go @@ -215,7 +215,17 @@ func waitForPluginStatusCompare(pluginID string, compare func(got string) (bool, // cleanup. func volumeRegister(volID, volFilePath, createOrRegister string) error { - cmd := exec.Command("nomad", "volume", createOrRegister, "-") + // a CSI RPC to create a volume can take a long time because we + // have to wait on the AWS API to provision a disk, but a register + // should not because it only has to check the API for compatibility + timeout := time.Second * 30 + if createOrRegister == "create" { + timeout = time.Minute * 2 + } + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + cmd := exec.CommandContext(ctx, "nomad", "volume", createOrRegister, "-") stdin, err := cmd.StdinPipe() if err != nil { return fmt.Errorf("could not open stdin?: %w", err) diff --git a/e2e/e2eutil/cli.go b/e2e/e2eutil/cli.go index 9d5293fd9..46694663b 100644 --- a/e2e/e2eutil/cli.go +++ b/e2e/e2eutil/cli.go @@ -1,16 +1,20 @@ package e2eutil import ( + "context" "fmt" "os/exec" "regexp" "strings" + "time" ) // Command sends a command line argument to Nomad and returns the unbuffered // stdout as a string (or, if there's an error, the stderr) func Command(cmd string, args ...string) (string, error) { - bytes, err := exec.Command(cmd, args...).CombinedOutput() + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + bytes, err := exec.CommandContext(ctx, cmd, args...).CombinedOutput() out := string(bytes) if err != nil { return out, fmt.Errorf("command %v %v failed: %v\nOutput: %v", cmd, args, err, out) diff --git a/e2e/e2eutil/job.go b/e2e/e2eutil/job.go index a11419ddc..7b7f14a57 100644 --- a/e2e/e2eutil/job.go +++ b/e2e/e2eutil/job.go @@ -1,18 +1,22 @@ package e2eutil import ( + "context" "fmt" "io" "io/ioutil" "os/exec" "regexp" "strings" + "time" ) // Register registers a jobspec from a file but with a unique ID. // The caller is responsible for recording that ID for later cleanup. func Register(jobID, jobFilePath string) error { - return register(jobID, jobFilePath, exec.Command("nomad", "job", "run", "-detach", "-")) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + return register(jobID, jobFilePath, exec.CommandContext(ctx, "nomad", "job", "run", "-detach", "-")) } // RegisterWithArgs registers a jobspec from a file but with a unique ID. The @@ -25,8 +29,10 @@ func RegisterWithArgs(jobID, jobFilePath string, args ...string) error { baseArgs = append(baseArgs, args[i]) } baseArgs = append(baseArgs, "-") + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() - return register(jobID, jobFilePath, exec.Command("nomad", baseArgs...)) + return register(jobID, jobFilePath, exec.CommandContext(ctx, "nomad", baseArgs...)) } func register(jobID, jobFilePath string, cmd *exec.Cmd) error { @@ -60,7 +66,9 @@ func register(jobID, jobFilePath string, cmd *exec.Cmd) error { // PeriodicForce forces a periodic job to dispatch func PeriodicForce(jobID string) error { // nomad job periodic force - cmd := exec.Command("nomad", "job", "periodic", "force", jobID) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + cmd := exec.CommandContext(ctx, "nomad", "job", "periodic", "force", jobID) out, err := cmd.CombinedOutput() if err != nil { @@ -82,7 +90,9 @@ func Dispatch(jobID string, meta map[string]string, payload string) error { args = append(args, "-") } - cmd := exec.Command("nomad", args...) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + cmd := exec.CommandContext(ctx, "nomad", args...) cmd.Stdin = strings.NewReader(payload) out, err := cmd.CombinedOutput() @@ -96,7 +106,9 @@ func Dispatch(jobID string, meta map[string]string, payload string) error { // JobInspectTemplate runs nomad job inspect and formats the output // using the specified go template func JobInspectTemplate(jobID, template string) (string, error) { - cmd := exec.Command("nomad", "job", "inspect", "-t", template, jobID) + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + cmd := exec.CommandContext(ctx, "nomad", "job", "inspect", "-t", template, jobID) out, err := cmd.CombinedOutput() if err != nil { return "", fmt.Errorf("could not inspect job: %w\n%v", err, string(out)) @@ -109,8 +121,9 @@ func JobInspectTemplate(jobID, template string) (string, error) { // RegisterFromJobspec registers a jobspec from a string, also with a unique // ID. The caller is responsible for recording that ID for later cleanup. func RegisterFromJobspec(jobID, jobspec string) error { - - cmd := exec.Command("nomad", "job", "run", "-detach", "-") + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + cmd := exec.CommandContext(ctx, "nomad", "job", "run", "-detach", "-") stdin, err := cmd.StdinPipe() if err != nil { return fmt.Errorf("could not open stdin?: %w", err) diff --git a/e2e/vaultsecrets/vaultsecrets.go b/e2e/vaultsecrets/vaultsecrets.go index 7f53a3792..8a13611d2 100644 --- a/e2e/vaultsecrets/vaultsecrets.go +++ b/e2e/vaultsecrets/vaultsecrets.go @@ -1,6 +1,7 @@ package vaultsecrets import ( + "context" "fmt" "io" "io/ioutil" @@ -226,7 +227,9 @@ func writePolicy(policyID, policyPath, testID string) (string, error) { policyDoc := string(raw) policyDoc = strings.ReplaceAll(policyDoc, "TESTID", testID) - cmd := exec.Command("vault", "policy", "write", policyID, "-") + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + cmd := exec.CommandContext(ctx, "vault", "policy", "write", policyID, "-") stdin, err := cmd.StdinPipe() if err != nil { return "", err