feat: Always wait for reconciliation

kubernetes-sigs / cli-utils

This repo contains binaries that built from libraries in cli-runtime.

Apache License 2.0

156 stars 78 forks source link

feat: Always wait for reconciliation #449

Closed karlkfi closed 2 years ago

karlkfi commented 3 years ago

Converted WaitTask to use a context for timeout/cancellation, to improve readability and reduce error cases. Now it only sends TaskResult events from one place, removing the need for a token to silence subsequent complete() calls.
Add pending object tracking to WaitTask to ensure all objects are accounted for at least one WaitEvent.
Upgraded applier tests to use full event comparison, for better signal on breaking changes.
Enhanced task tests to consume all events before validating and test actual event output.
Add set sorting to graph.SortObjs, to make wait event ordering more consistent, to make testing easier.

BREAKING CHANGE: wait tasks always execute after apply/prune/delete (except dry runs) BREAKING CHANGE: wait tasks default to waiting until cancelled (previously 1m default)

fixes: https://github.com/kubernetes-sigs/cli-utils/issues/438

karlkfi commented 3 years ago

Note: I used https://www.conventionalcommits.org for the commit message so we can easily spot the breaking change later.

karlkfi commented 3 years ago

If desired, I can pull the task_test.go refactor out to its own PR. That would reduce the size of this PR and prove that the only real behavior changes are wait task related.

karlkfi commented 3 years ago

/assign @mortent

mortent commented 3 years ago

/approve

k8s-ci-robot commented 3 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi, mortent

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/cli-utils/blob/master/OWNERS)~~ [mortent] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

karlkfi commented 3 years ago

Blocked by https://github.com/kubernetes-sigs/cli-utils/issues/452

karlkfi commented 3 years ago

Rebased to master to pull in #452

karlkfi commented 3 years ago

/hold

This overlaps with fixes needed to clean up Tech Debt from https://github.com/kubernetes-sigs/cli-utils/pull/451

Updating the Printer to accept WaitEvents is turning into a lot of work, which requires refactoring WaitTask and WaitEvent.

karlkfi commented 3 years ago

/unhold

https://github.com/kubernetes-sigs/cli-utils/pull/451 is merged

karlkfi commented 3 years ago

rebased

karlkfi commented 2 years ago

rebased

karlkfi commented 2 years ago

Tests are passing and unrelated changes have been pulled out to other PRs, which are now merged.

This is ready for review now.

haiyanmeng commented 2 years ago

/lgtm