Linter complains that the expected event lists are too similar,
which is deliberate and should not be abstracted away. Explicit
test expectations in the test makes it easier to debug failures.
- WaitEvent can be Pending, Reconciled, Skipped, or Timeout.
Skipped, Pending, and Reconciled events are sent at task start.
Reconciled events are sent later as status updates are recieved.
Timeout events are sent for remaining events on timeout.
- Rewrite WaitTask.Start to use context.WithTimeout and a goroutine to
handle task completion (replacing setTimer and the token hack).
- Replaced Task.ClearTimeout with Task.Cancel.
- Replaced WaitTask.complete & checkCondition with Task.StatusUpdate.
- Replaced WaitTask.startAndComplete with a check in WaitTask.Start.
- Replaced WaitTask.amendTimeoutError with WaitTask.sendTimeoutEvents
to send multiple timeout events, instead of one event with a list of
TimedOutResources.
- Updated all printers to handle WaitEvent.
Event printer now includes reconcile events.
JSON printer now includes resourceReconciled eventType.
Table printer not includes reconcile column.
- Added JSON printer tests for error handling.
- Updated Formatter.FormatActionGroupEvent to collect WaitStats.
- Enable status events by default for kapply with table output
BREAKING CHANGE: WaitEvents now sent for each object
The Pruner modifies the input object when the deletion prevention
annotation is present. This in turn modifies the object map, which
creates a race condition when reading the object map from the task
runner. To fix the race, a copy of the object is made, but only if
the deletion prevention filter prevents deletion.
Historically, the objects in ApplyTask were being modified in the
Start func by BuildInfo to remove the path annotation, but this can
cause a race condition in the task runner if it tried to read the
task objects.
Now a DeepCopy is made in BuildInfo instead. This avoids the race
condition but broke mutation, which executes after BuildInfo on the
original object. So we now extract the object from the into after
BuildInfo and use that instead for mutations, filters, and events.
Today, when a WaitTask timeout happens, the WaitTask sends the
TimeoutError on the TaskChannel. After receiving the TimeoutError,
`baseRunner.run` terminates immediately by returning the error to its
caller (Applier.Run or Destroyer.Run). The caller then sends the error
onto the EventChannel and terminates.
With this PR, when a WaitTask timeout happens, the WaitTask sends a
WaitType Event including the TimeoutError on the EventChannel, and then
sends an empty TaskResult on the TaskChannel. An empty TaskResult
suggests that the task finished successfully, and therefore
`baseRunner.run` would continue instead of terminate.
The motivation of this change is to make sure that cli-utils only
terminates on fatal errors (such as inventory-related errors, and
ApplyOptions creation errors). A WaitTask timeout may not always mean a
fatal error (it may happen because the StatusPoller has not finished
polling everything, or some but not all the resources have not reached
the desired status), and therefore should not terminate cli-utils.
- Added SkippedApplies and SkippedDeletes to the TaskContext
- Modified tasks to use the new skipped tracking, replacing usage
of failure tracking, where skipped is more accurate.
- Renamed some TaskContext methods for consistency
- Added ObjMetadataSetFromMap for use by TaskContext
- Added ObjectMetadataSet.Intersection for use by InvSetTask
- Cleaned up InvSetTask to be more readable with comments explaining
intended behavior, including handling of skips.
- Added apply and prune tests for skipped, failure, and abandoned
- Rename PruneOptions to Pruner (it does the work itself)
- Rename variables and internal methods for clarity/consistency
- Extract deleteObject method to improve readability
- Rename GetObject to getObject (only used internally and by test)
- Modify log messages for readability
- Add log support for prune tests
- Make log messages more consistent
If an object with the deletion prevention annotation is removed from the
inventory, the config.k8s.io/owning-inventory annotation should be
removed from the object, and the object should be removed from the
inventory.
- Destroyer context added, with cancel test.
- Fix Applier cancel test not using specified timeouts.
- Added a few logs to help with debugging test failures
- Add test for applier.Run context cancel/timeout
- Add AssertEqual & AssertNotEqual to wrap testutil.Equal matcher
- Move common test utils to common_test.go
- Use yaml artifacts, instead of Unstructured
- Use apply.Options as test input
- Use UnstructuredSet as test input
- Extract Applier construction with fake inputs to newTestApplier
- Extract nil inventory to its own test to simplify test inputs
to TestReadAndPrepareObjects
Kubernetes Prow Robot
Mikhail Mazurskiy
Karl Isenberg
Sean Sullivan
Haiyan Meng
Morten Torkildsen