[WIP] Add context and timeouts

[karlkfi]

• Add timeout flag to apply, destory, and preview cmds
• Default to no timeout
• Add context to TaskContext
• Pass context through to all functions that use it
• Rename prune-timeout flag to delete-timeout and add actual impl
• Rename PruneOptions to Pruner (there's already prune.Options)
• Rename PropagationPolicy to DeletionPropagationPolicy consistently
• Wrap func calls and defs for readability
• Remove Prune bool from solver.Options to simplify
  AppendPruneWaitTasks to always prune.
  Change Applier to only call AppendPruneWaitTasks if NoPrune.
  Change Destroyer to always call AppendPruneWaitTasks.
• Replace task.ClearTimeout with context.Cancel
• Add task.OnStatusEvent to fix WaitTask leaky abstraction.
  baseRunner was calling private funcs from WaitTask.
• Move ResourceStatusCollector into TaskContext. It doesn't need to
  persist between runs. Moving it simplifies baseRunner.
  This required making some of the collector funcs public.
• Remove Identifier from collector.ResourceStatus. Redundant with id.
• Add collector.Get/Put funcs for encapsulation and thread-safety
• Add locking to ResourceStatusCollector because a test was complaining
  about a race condition when updating status from another goroutine,
  like the poller does.
• Add logging to taskrunner tests for easier debugging concurrency
• Update task tests for cancel and timeout
• Rename options vars to "opts" for consistency
• Rewrite WaitTask context/timeout handling.
  TaskChannel is now only written to from one place, gated by an
  internal error channel and sync.Once.
  WaitTask now distinguishes between wait task timeout and parent
  context timeout.
• Update task tests to reflect new behavior with context cancel/timeout

Note: This PR is probably too big. I started cleaning things up and kept finding things to fix. Let me know what I should leave out or punt to another PR.

[k8s-ci-robot]

Hi @karlkfi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: karlkfi
To complete the pull request process, please assign liujingfang1 after the PR has been reviewed.
You can assign the PR to them by writing /assign @liujingfang1 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[seans3]

/ok-to-test

[k8s-ci-robot]

@karlkfi: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
cli-utils-presubmit-master-e2e	52130d8	link	true	/test cli-utils-presubmit-master-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[seans3]

This is a massive PR, and it needs to be broken up in to safer, more digestible chunks. Renaming variables or functions should almost always be done in separate PR's to reduce risk and complexity. Smaller PR's are faster to merge and safer (as long as they are also well-tested).

On another note, do not worry about substantive changes to any code under the cmd hierarchy, since these commands are not exported; they are only used for testing and demonstration purposes.

[karlkfi]

Sure. I'll chop this into smaller PRs. Just wanted a place to stash a bunch of the tech debt fixes that I came up with while writing the apply-time-mutation PR. Was hoping to get some feedback on the stuff being done, but if it's too much, I'll pull stuff out, after I have time to figure out what's actually blocking the apply-time-mutation PR.

I think the ResourceCache and ResourceStatusCollector probably need to get merged.

But of course, the e2e tests here are failing, so I must have broken something. I couldn't figure out how to even debug the failures on this branch, so I spent some time improving the tests and testutils to make it easier: #405

Hopefully once that's merged I can come back to this and figure out what went wrong.

[karlkfi]

/hold

This PR needs a lot of work. It's just here for staging until I have time to break it up.

[k8s-ci-robot]

@karlkfi: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[karlkfi]

A large chunk of this PR was extracted to #423

I'm gonna leave this open for a while tho, to remind me to extract some of the other bits that were refactored in this PR.

[karlkfi]

Global timeout option extracted to #439

Still TODO:

• Change default behavior to always wait for reconciliation & deletion confirmation #438
• Feature Request: Allow cancellation of running applier/destroyer tasks #440
• Tech Debt: Rename prune/delete timeout to reconcile timeout #441
• Clean up prune package #445
• TechDebt: Merge Prune & Delete code & names #450
• Clean up handling of optional Pruning (remove unused NoPrune option)
• Remove Identifier from collector.ResourceStatus. Redundant with id. (needed for direct kstatus usage)
• Rename options vars to "opts" for consistency (as needed, not important)

[karlkfi]

This is obsolete. The improvements have been made in other PRs and logged as backlog issues.