Add CRDB support #116
Add CRDB support #116
Conversation
In a number of tests, the underlying storage backend interaction will return the revision (logical clock underpinning the MVCC implementation) at the call-time of the RPC. Previously, the tests validated that this returned revision was exactly equal to some previously seen revision. This assertion is only true in systems where no other events are advancing the logical clock. For instance, when using a single etcd cluster as a shared fixture for these tests, the assertion is not valid any longer. By checking that the returned revision is no older than the previously seen revision, the validation logic is correct in all cases. Signed-off-by: Steve Kuznetsov <[email protected]>
It is not possible for the nil-check to ever return anything different from what the explicit boolean used to, but this is only something that a reader can come to the conclusion on if they very, very carefuly read the code. Instead of having this implicit flow that is difficult to follow, let's keep the boolean. Signed-off-by: Steve Kuznetsov <[email protected]>
Signed-off-by: Steve Kuznetsov <[email protected]>
In order to be able to import these tests in the future, we need them to not be in `_test.go` files. Signed-off-by: Steve Kuznetsov <[email protected]>
Signed-off-by: Steve Kuznetsov <[email protected]>
Signed-off-by: Steve Kuznetsov <[email protected]>
The current etcd3 implementation is correct for any storage backend that allows clients to interact with the logcal clock timestamps used to drive MVCC semantics and provide a robust watching mechanism. This commit adapts the current implementation to use a generic interface (albeit this amount of genericism is small, few databases allow such control). Signed-off-by: Steve Kuznetsov <[email protected]>
This commit isolates etcd3-specific logic into a driver/client, which can simply be used as the backing implementation for the generic store. Signed-off-by: Steve Kuznetsov <[email protected]>
CockroachDB allows clients to access the logical clock that underpins the MVCC implementation in the database. Furthermore, changefeeds allow users to efficently watch for changes to keys. These characteristics enable CRDB to be used as a backing store for k8s as they expose the same interaction mechanisms as etcd. Signed-off-by: Steve Kuznetsov <[email protected]>
TODO: - there's a disagreement between caching indices (generic) and selectors, which can be fields or labels ... we need to reconcile this - for CRDs we'd need to plumb in CEL-based expressions to the current index support Signed-off-by: Steve Kuznetsov <[email protected]>
It is likely better to use `store.Interface` in all cases, but that is a larger clean-up here. Especially the use of etcd compaction is extremely questionable at this level of abstraction - even the storage implementation does not use that part of the etcd API. Signed-off-by: Steve Kuznetsov <[email protected]>
Signed-off-by: Steve Kuznetsov <[email protected]>
This reverts commit b6fb555. Signed-off-by: Steve Kuznetsov <[email protected]>
Signed-off-by: andreyod <[email protected]>
Fix conflicts in go.sum Signed-off-by: andreyod <[email protected]>
Signed-off-by: Ezra Silvera <[email protected]>
|
This is based on @stevekuznetsov code in https://github.com/stevekuznetsov/kubernetes/tree/skuznets/generic-storage |
|
/assign @stevekuznetsov |
|
do we have an idea how an out-of-tree implementation would look like that uses e.g. kine? |
by "look like" , do you mean performance related (as it doesn't rquire any code changes)? |
|
I mean feasibility & maintenance cost via kine versus reduced value compared to the in-tree implementation. |
This would mean a permanent and extensive fork of kube. With that it's out of scope of what we want in kcp-dev/k. |
Strongly depends, there was broad consensus around an interface that is etcd-agnostic to more concretely define the semantics k8s needs from etcd and allow for better testability - the epic we have linked to provisional work from Han on this subject. Such an interface would enable us to simply implement that alongside, no extensive or challenging fork required. |
|
The reasons I want to try and merge this version ASAP are:
We can work next on converting to the design according to the provisional work from Han and push that as a future PR. |
|
/hold |
|
On hold |
|
@ezrasilvera we don't have Prow enabled on this repo (for |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: