Skip to content
This repository has been archived by the owner on Feb 2, 2021. It is now read-only.

Proposal: use context to control ExecTask timeout #90

Open
LucasRoesler opened this issue Mar 27, 2019 · 1 comment
Open

Proposal: use context to control ExecTask timeout #90

LucasRoesler opened this issue Mar 27, 2019 · 1 comment

Comments

@LucasRoesler
Copy link
Member

While reviewing #89 I thought about how to set a timeout on the rollout commands. Go provides a CommandContext that is designed to allow you to cancel a command via a context cancel. See https://golang.org/pkg/os/exec/#CommandContext

The provided context is used to kill the process (by calling os.Process.Kill) if the context becomes done before the command completes on its own.

We should update func (et ExecTask) Execute() (ExecResult, error) to accept a context so that the caller can set a timeout and explicitly control the max length of a command.

Expected Behaviour

Methods like certManagerReady() should look like

func certManagerReady() {
	ctx, cancel := context.WithTimeout(context.Background, 30*time.Second)
	defer cancel()
	task := execute.ExecTask{
		Command: "./scripts/get-cert-manager.sh",
		Shell:   true,
	}

	res, err := task.Execute(ctx)
	fmt.Println("cert-manager", res.ExitCode, res.Stdout, res.Stderr, err)
}

Possible Solution

  1. Update the ExecTask to accept a context
func (et ExecTask) Execute(ctx context.Context) (ExecResult, error)
  1. If the ctx is nil, then just use contetxt.Background()
  2. use exec.CommandContext instead of exec.Command
  3. update all calls to Execute to send at least a nil context
@alexellis
Copy link
Member

I thought about how to set a timeout on the rollout commands

^ this is part of kubectl

      --timeout=0s: The length of time to wait before ending watch, zero means never. Any other values should contain a
corresponding time unit (e.g. 1s, 2m, 3h).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants