* Add kcmp, a safe wrapper for google/go-cmp
* Returns an error instead of panicking.
* Holds common options that we use.
* Rename things
kcmp -> kmp
Diff -> SafeDiff
* Some clean ups in the FieldError code and its tests.
- rename FieldError.getNormalizedError to be normalized:
- Go kind of discourages GetA() vs A()
- it's an error, so normalizing itself is also an error, so
normalizedError() is a tautology at this point
- flatten was doing a[x]=fmt.Sprintf("%s%s", a[x],y), which is a very
expensive substitution for a[x]+=y
- some minor test cleanups (fix strings, streamline `got, want:=`
pattern, etc.
* Also make normalize() and merge() operate on pointers.
As the code currently is structured we are doing lots of duplication
and copies of the objects, since we're doing []FieldError throughout
normalize() and merge() even though normalize already returns a copy of
the initial error tree, rather than a list of pointers to existing
errors.
Hence list returned by normalize() and hence merge() can actually be pointer
lists which would save us some CPU and memory.
My prior change added sorting to the duck.CreatePatch method to try and stabilize the result of jsonpatch.CreatePatch, which is otherwise non-deterministic (probably walking a map?).
My bad assumption was that the patch operations this generated wouldn't conflict, e.g. it should use `replace` vs. `remove` and `add`.
Clearly this was bad because we start getting really strange errors trying to import this into knative/serving, e.g.
https://gubernator.knative.dev/build/knative-prow/pr-logs/pull/knative_serving/2646/pull-knative-serving-integration-tests/1070435951391543298/
Immutable fields with default values may now be changed iff they change is to populate their default value. This is to support defaulting in the scenario where an object was created long ago and a new field (with a default!) is added. When controllers attempt to mutate the object status today, this would create a webhook rejection! With this change, we compare against a freshly defaulted "old" object to exclude newly defaulted fields from the immutability check.
We saw this in knative/serving for the newly added TimeoutSeconds field in Revision (otherwise immutable), which I believe it leading to upgrade testing flakes since post-upgrade Revision status updates will fail.
* Use different reporting periods based on the metrics backend. Original implementation was setting this value to 1 min to accommodate the minimum allowed value for stackdriver, but that is a sub optimal behavior for Prometheus. The change also allows operator to override the default values.
* Fix the units of reconciler latency from nano seconds to milliseconds.
* Address PR comments.
* Add better logging for reconciliation.
* Remove unnecessary lock in the UT.
* Address PR comments.
I was hoping to switch from a simple string key to a struct to include additional metadata into our workqueue item (in particular the time at which it was queued), but the great unit tests here reminded me that this would not properly deduplicate keys (they would have different queue times!), so I'm settling for simplifying the loop in a few ways:
1. Keys can only be strings, so remove some error checking and a test.
1. We were wrapping a block in a `func` because it contained `defer`, but the outer function had nothing in it.
I'd still like to try and find a way to track queue times for keys, so that we can surface two interesting metrics:
1. How long are things spending queued?
1. How long between events being observed and us responding? (above + reconcile latency)
Old images cleanup is no longer part of e2e test, this functionality will be moved to a separate job runs periodically.
Proposed Changes
This part is done in knative/test-infra, updating dependencies in this repo to include this update.
Link to the issue in test-infra: knative/test-infra#276
* Update the ConditionSet to support non-terminal conditions.
This change updates the `ConditionSet` datatype to treat the set of conditions that it is initially supplied with as the set of "terminal" conditions, where as before:
1. If all `True`, results in `Ready=True`
2. If any `False`, results in `Ready=False`
3. Otherwise, `Ready=Unknown`
However, in additional to this initial "terminal" set, other conditions may be set that are "non-terminal" and have no influence over the "happy" condition (e.g. `Ready`).
Related to: https://github.com/knative/serving/issues/2394
* Add severity handling to ConditionSet.
This adds a Severity field to our common Condition type, as outlined [here](https://github.com/knative/serving/issues/2394#issuecomment-436806801).
The only way Severity is populated in this change is:
* Terminal conditions (passed at initial construction) always result in `severity: Error`,
* non-Terminal conditions (added outside initial set) always result in `severity: Info`.
We should think about how we want to update the `Condition{Set,Manager}` interfaces to surface more control (e.g. `severity: Warning`).
* Clean up eventing duck types.
* Introduce Addressable to unify delivery ducks.
* Remove Subscribable and Channelable (moved to knative/eventing).
* Mark Sinkable and Targetable as retired in favor of Addresable &
annotations for Callable.
See https://docs.google.com/document/d/1xu18lprM8EFknqrsyZeNgYX2sLE13hfT2UKlYfdxv1g/edit#
for interface/naming details.
* Keep targetable as retired until serving stops using it.
This promotes the private deletion-handling helper wrapping `meta.Accessor` into `kmeta`. The wordy name is based on the wordier `cache.DeletionHandlingMetaNamespaceKeyFunc`, since it serves a similar purpose.
* fix flaky test
* relax the requirement for stackdriver project id
* improve log
* add comment for stackdriverProjectIDKey not provided case
* minor format
* simplify the map access
* checkpoint.
* Tried a new way.
* Some more slimplifying based on feedback.
* use bang vs == false.
* not sure why I did not use go style on this forloop, fixed.
Markus Thömmes