refactor: plumb a single context throughout the code

[hbagdi]

This patch is an effort to improve the context propagation in decK, down
to every single operation.
In order to do that, a single root context is now being created in the
main function, and all contexts should be derived from it.

[codecov-commenter]

Codecov Report

Merging #350 (79c1fd6) into main (fc24613) will decrease coverage by 0.14%.
The diff coverage is 0.00%.

❗ Current head 79c1fd6 differs from pull request most recent head 92aafdf. Consider uploading reports for the commit 92aafdf to get more accurate results

@@            Coverage Diff             @@
##             main     #350      +/-   ##
==========================================
- Coverage   53.66%   53.51%   -0.15%     
==========================================
  Files          60       59       -1     
  Lines        4877     4890      +13     
==========================================
  Hits         2617     2617              
- Misses       1962     1975      +13     
  Partials      298      298              

Impacted Files	Coverage Δ
cmd/common.go	0.00% <0.00%> (ø)
cmd/common_konnect.go	15.21% <0.00%> (ø)
cmd/diff.go	72.72% <0.00%> (-2.28%)	⬇️
cmd/dump.go	24.24% <0.00%> (-0.25%)	⬇️
cmd/konnect_dump.go	27.08% <0.00%> (ø)
cmd/ping.go	35.71% <0.00%> (-2.75%)	⬇️
cmd/reset.go	27.86% <0.00%> (-0.47%)	⬇️
cmd/root.go	55.96% <0.00%> (ø)
cmd/sync.go	72.72% <0.00%> (-2.28%)	⬇️
dump/dump.go	1.56% <0.00%> (ø)
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fc24613...92aafdf. Read the comment docs.

[hbagdi]

Will wait for @mflendrich for a review on this one.

[mmorel-35]

The contexts of ping command seems to be missing is it deliberately?

[hbagdi]

The contexts of ping command seems to be missing is it deliberately?

Good catch. Added that.

Code updated again

[mmorel-35]

Another one, the last one 😉?

deck/cmd/dump.go

Line 38 in 76d64ae

_, err = client.Do(nil, req, &response)

[mmorel-35]

Shall those lines be removed with this pr?

deck/.golangci.yml

Lines 18 to 20 in 64a98cc

	- linters:
	- staticcheck
	text: "SA1012" # do not pass a nil Context

[hbagdi]

Shall those lines be removed with this pr?

We can't because solver package has a bunch of nil ctx invocations.
This PR wasn't meant to be an exhaustive squash of all issues with ctx.

[hbagdi]

Conflicts have been fixed. PTAL.

[mflendrich]

I'm happy to see this change 👍

LGTM! With one optional comment.

[mflendrich]

Consider the following:

https://pkg.go.dev/os/signal#NotifyContext

ctx, _ := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM)

[hbagdi]

included this, this is a handy simplification

[hbagdi]

Said too soon, @mflendrich the only problem here is that when the signal arrives, there wouldn't be a message to stdout stating that the signal was actually received. This is quite helpful in debugging. Thoughts?

[mflendrich]

Hacky way (not recommended):

go func() {
  select {
    case <-ctx.Done():
      fmt.Printf("context cancelled (%w), terminating...", ctx.Err())
  }
}

Clean way:

• Replace os.Exit(1) throughout the codebase with errors plumbed all the way through to RunE and to the return value of cmd.Execute()
• After cmd.Execute() returns, check if ctx.Err() is not nil. If it isn't, then the context got cancelled by the signal.

Both are somewhat tedious and may defeat the purpose of this simplification.

[hbagdi]

Thought and explored a few different ways to do it but I haven't found a good way to deal with it in this PR.
This also feels minor enough that we can leave it be for now. If it is a bigger issue, please LMK and I'll address with a quick follow up PR.
I'm going to go ahead and merge this in.

[mmorel-35]

Part 1of #325