Add initial schema, storage provider and quota manager for CockroachDB #2834
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
JAORMX
force-pushed
the
crdb
branch
3 times, most recently
from
9d77cf0 to
9bf155f
Compare
JAORMX
force-pushed
the
crdb
branch
9 times, most recently
from
59b4026 to
c1d7038
Compare
|
Current status: The driver spawns a single node crdb instance when the tests start, and these are passing. The tests and some basic linting is running on GH actions. I'm currently looking into getting the integration tests to run on GH actions too. One caveat is that in order to easily parse database URI's, I did use a golang 1.19 function: https://github.com/equinixmetal-security/trillian/blob/crdb/storage/testdb/testdb.go#L191 Don't know how much of an issue this would be 😕 , but it's sure a convenient function! 😄 Looking forward for initial feedback. |
JAORMX
force-pushed
the
crdb
branch
13 times, most recently
from
d95c343 to
cb4973f
Compare
|
@roger2hk ah! I used a golang 1.19 construct. I'll refactoring so it works in 1.17. |
JAORMX
added a commit
to JAORMX/helm-charts
that referenced
this pull request
Nov 17, 2022
Currently, the Trillian installation is hard-coded to use the mysql storage system. While this is mostly fine (it is the most common and used storage system for Trillian after all!), this makes it non trivial to try custom backends for Trillian. There is work on the way to introduce [a CockroachDB storage system for Trillian](google/trillian#2834). And, while this is currently not usable using the mainline Trillian image, it is possible to try out if one specifies a custom image for trillian. To enable this, some variables were introduced: * `storageSystem.driver`: defines the storage backend to use for both the log signer and the log server. Defaults to `mysql`. * `storageSystem.envCredentials`: is the kubernetes deployment definition of environment variables to aide the log signer in connecting to the desired database. When set to `null` it defaults to the environment variables used by MySQL: A values file that would take this work into use would look as follows: ```yaml storageSystem: driver: crdb envCredentials: - name: CRDB_USER valueFrom: secretKeyRef: name: mySecret key: crdb-user - name: CRDB_PASSWORD valueFrom: secretKeyRef: name: mySecret key: crdb-password - name: CRDB_HOST value: crdb.trillian.svc.cluster.local logServer: image: registry: ghcr.io repository: equinixmetal-security/trillian-log-server version: latest extraArgs: - "--crdb-uri=postgres://$(CRDB_USER):$(CRDB_PASSWORD)@$(CRDB_HOST):26257/" logSigner: image: registry: ghcr.io repository: equinixmetal-security/trillian-log-signer version: latest extraArgs: - "--crdb-uri=postgres://$(CRDB_USER):$(CRDB_PASSWORD)@$(CRDB_HOST):26257/" mysql: enabled: false ``` Signed-off-by: Juan Antonio Osorio <[email protected]>
|
/gcbrun |
1 similar comment
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
This creates an initial schema for CRDB by mirroring a lot of the existing constructs from the MySQL storage provider. It also adds scripts and test code to run unit and integration tests with CockRoachDB While this lead to a lot of code duplication, the intention is to address that in a separate PR in order to keep the work separated into logical batches. Doing all the refactoring to create common constructs for SQL backends would lead to a patch too big for folks to review in a reasonable time and would create more risk. Signed-off-by: Juan Antonio Osorio <[email protected]>
This adds a simple GitHub actions based test that runs unit and lint tests. Signed-off-by: Juan Antonio Osorio <[email protected]>
This ensures that the tests run faster. This is possible since we have a dedicated database per test. Signed-off-by: Juan Antonio Osorio <[email protected]>
These are not a problem in CRDB Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
This sets up integration tests in GitHub actions. Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
|
/gcbrun |
1 similar comment
|
/gcbrun |
|
Can you add a note in |
Signed-off-by: Juan Antonio Osorio <[email protected]>
done. |
|
/gcbrun |
AlCutter
approved these changes
Nov 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This creates an initial schema for CRDB by mirroring a lot of the existing constructs from the MySQL storage provider. It also adds scripts and test code to run unit and integration tests with CockroachDB
While this lead to a lot of code duplication, the intention is to address that in a separate PR in order to keep the work separated into logical batches. Doing all the refactoring to create common constructs for SQL backends would lead to a patch too big for folks to review in a reasonable time and would create more risk.
Checklist