Use Context in ssh client

[guettli]

/kind feature

Currently, the ssh client used by caph does not use the Context.

This means, when the ctx times out, the ssh client does not notice that.

Current code:

type Factory interface {
	NewClient(Input) Client
}

type Input struct {
	IP         string
	PrivateKey string
	Port       int
}

We could add Context to Input.

Using the ctx is not straight forward, because the ssh package was created before context was added to Go.

But it is not difficult either:

1. create conn with net.Dialer.DialContext(ctx, ...)
2. pass that conn into ssh.NewClientConn(...)
3. in a goroutine: select { case <-ctx.Done(): conn.Close() }

Additional benefit: We could get the logger from the context.

---

Alternative:

Currently:

type Client interface {
	GetHostName() Output
	GetHardwareDetailsRAM() Output
	GetHardwareDetailsNics() Output
	...

We add the ctx to each function of the interface:

type Client interface {
	GetHostName(ctx context.Context) Output
...
	

[abdullah599]

Proposed solution

I think we should go with option 2: add ctx context.Context as the first parameter on every method of the Client interface. Keep the Input struct as is.

Reason:

• The SSH connection is opened per method call inside runSSH, not at NewClient time. The ctx that matters is the one at the moment of the operation, so putting it on Input binds one cancellation signal to many logically independent operations and prevents per call deadlines

• The Go Code Review Comments wiki states: "Don't add a Context member to a struct type; instead add a ctx parameter to each method on that type that needs to pass it along." The containedctx linter, already enabled in .golangci.yaml, would flag a stored ctx. ref: https://go.dev/wiki/CodeReviewComments#contexts

• The interface is already halfway there. Four methods already take ctx (WipeDisk, CheckDisk, ExecutePreProvisionCommand, StartImageURLCommand). Option 2 finishes a migration that is already underway, option 1 reverses it.

@guettli what do you think?

[guettli]

@abdullah599 yes, sounds good.