Repository: kubernetes
Issue: apiserver/storage: test caching and indexing layers at the unit level
Labels: sig/api-machinery,kind/feature,triage/accepted

### What would you like to be added?

Today, unit-level tests only exist for `*etcd3.store{}`, and cannot be run against any other implementation of `storage.Interface`. In general, these tests define what it means to be a correct implementation of `storage.Interface`, and only use exported functionality. We aim to refactor the tests to accept any `storage.Interface`. This allows for testing the entire exported function space for an etcd storage layer with whichever caching and indexing layers on top.

The following tests can be ported as-is, since they use only `storage.Interface` methods:

In #109833:
- [x] `TestCreateWithTTL`
- [x] `TestCreateWithKeyExist`
- [x] `TestGet`
- [x] `TestUnconditionalDelete`
- [x] `TestConditionalDelete`
- [x] `TestDeleteWithSuggestion`
- [x] `TestDeleteWithSuggestionAndConflict`
- [x] `TestDeleteWithSuggestionOfDeletedObject`
- [x] `TestValidateDeletionWithSuggestion`
- [x] `TestPreconditionalDeleteWithSuggestion`
- [x] `TestGetListNonRecursive`
- [x] `TestGuaranteedUpdateWithTTL`
- [x] `TestGuaranteedUpdateWithConflict`
- [x] `TestGuaranteedUpdateWithSuggestionAndConflict`
- [x] `TestCount`
- [x] `TestWatch`
- [x] `TestDeleteTriggerWatch`
- [x] `TestWatchFromNoneZero`
- [x] `TestWatchInitializationSignal`

Other:
- [x] `TestProgressNotify` (requires that the underlying etcd is configured to send bookmarks)

The following tests currently require the etcd client, and need to be re-thought or adapted:

- [x] `TestCreate` uses the etcd client to: https://github.com/kubernetes/kubernetes/pull/109909
    - make sure nothing exists at the key before creation. Unclear what this is enforcing, and perhaps could use `storage.Interface.Get()` instead?
    - check invariants about data in the store, should be relegated to an `*etcd3.store{}` specific test only
- [x]  `TestGuaranteedUpdate` uses the etcd client to:
   - check invariants about data in the store, should be relegated to an `*etcd3.store{}` specific test only
- [x] `TestGuaranteedUpdateChecksStoredData` uses the etcd client to:
  - put data into the store using a different codec
- [x] `TestList` uses the etcd client to: https://github.com/kubernetes/kubernetes/pull/110024
  - share a single underlying storage between two `*etcd3.store{}` implementations (with and without paging)
- [x] `TestListContinuation` uses the etcd client to:
  - track the number of calls made to etcd
- [x] `TestListPaginationRareObject` uses the etcd client to:
  - track the number of calls made to etcd
- [x] `TestListContinuationWithFilter` uses the etcd client to:
  - track the number of calls made to etcd
- [x] `TestListInconsistentContinuation` uses the etcd client to:
  - cause a compaction in order to test clients asking for compacted revisions
- [x] `TestWatchFromZero` uses the etcd client to:
  - cause a compaction in order to test clients asking for compacted revisions
- [x] `TestWatchError` uses the etcd client to:
  - share a single underlying storage between two `*etcd3.store{}` implementations (with a valid and invalid codec)
- [x] `TestWatchDeleteEventObjectHaveLatestRV` uses the etcd client to:
  - create a raw watch, we check that the mod revision at which the raw watch shows us the delete is the same as the revision the `storage.Interface` uses to report the deletion ... but if we know nothing else is touching our storage, why not just use `storage.Interface.Get()` (or the returned value from creating) to check that the last RV we saw is what it's deleted at?


The following tests currently touch internals of `*etcd3.store{}`:

- [x]  `TestGuaranteedUpdate` touches:
   - `store.transformer` in order to inject staleness to cause an update when the semantic value of the data does not change
- [x] `TestGuaranteedUpdateChecksStoredData` touches:
  - `store.transformer` in order to inject staleness to cause an update when the semantic value of the data does not change
- [x] `TestTransformationFailure` touches:
  - `store.transformer` in order to test behavior under transformation failure
- [x] `TestListContinuation` touches:
  - `store.transformer` to count the number of items read from etcd
- [x] `TestListPaginationRareObject` touches:
  - `store.transformer` to count the number of items read from etcd
- [x] `TestListContinuationWithFilter` touches:
  - `store.transformer` to count the number of items read from etcd
- [x] `TestConsistentList` touches:
  - `store.transformer` to ... cause side-effect reads during a paginated LIST call ? @wojtek-t you wrote this, perhaps there's a less invasive way to do this
- [x] `TestLeaseMaxObjectCount` touches:
  - `store.leaseManager` - this is etcd-specific, so perhaps it remains in the `*etcd3.store{}` level?
- [x] `TestWatchFromZero` touches:
  - `store.versioner`, but that is a singleton and there's no real reason that needs to be an instance variable of the store as opposed to an exported package-level function?
- [x] `TestWatchContextCancel` touches: https://github.com/kubernetes/kubernetes/pull/109914
  - `store.watcher.Watch()`, but we're testing user-facing behavior of the watch method, so it's not clear why calling the watcher specifically is necessary here
- [x] `TestWatchErrResultNotBlockAfterCancel` touches:
  - `store.watcher.createWatchChan()` - this looks like an implementation detail of the watcher?
- [x] `TestWatchDeleteEventObjectHaveLatestRV` touches:
  - `store.versioner`, but that is a singleton and there's no real reason that needs to be an instance variable of the store as opposed to an exported package-level function?

Next steps:

- [x] migrate all of the tests that can be used as-is into `*_tests.go` files and change their signature to accept a `storage.Interface`, import these and use them to test caching and indexing layers (#109833)
- [x] make a decision for all of the tests that touch the `etcd` client whether they could instead use the `storage.Interface` itself, etc
- [x] similarly figure out if the tests that touch internals of the `*etcd3.store{}` are useful to test other `storage.Interface` implementations, and, if so, how they can be adapted

/sig api-machinery
/assign @stevekuznetsov @wojtek-t 
/cc @liggitt @lavalamp 

### Why is this needed?

To improve our ability to test the caching and indexing layers that sit on top of the etcd storage implementation.