Add schema validation to DB #86
Conversation
This ensures we don't open a DB with an earlier schema or a config that differs from ours Signed-off-by: Matthew Heon <matthew.heon@gmail.com>
| @@ -15,6 +15,137 @@ import ( | |||
| _ "github.com/mattn/go-sqlite3" | |||
| ) | |||
|
|
|||
| // Checks that the DB configuration matches the runtime's configuration | |||
| func checkDB(db *sql.DB, r *Runtime) (err error) { | |||
There was a problem hiding this comment.
The work that's going on in here for checkDB is not what I'd expect for a checkDB function. I thought I'd find just a simple query with a validation of the value(s) like you have starting at line 104. The rest of this code feel like it should be in a createRuntimeTable function to initially create the table, then another function to be called to insert/update values into it.
|
|
||
| }() | ||
|
|
||
| row := tx.QueryRow(checkRuntimeExists) |
There was a problem hiding this comment.
Too much code today. When you move from version 1 to version 2, will this code update itself appropriately? We need to ensure that there is only one row in the table with the latest schema version I think.
There was a problem hiding this comment.
There's a constraint in the database to ensure that there is only ever one row (the ID/primary key is forced to 0). We will never update this row, mainly because we don't have a good way to migate between database revisions, so for now if the schema changes this will just give you a nice, loud notice that you need to delete it and recreate it.
There was a problem hiding this comment.
Thanks for the 411. Just to be 100% sure, this notice will happen during one of our CI runs after a particular submit rather than on a customer's system right?
There was a problem hiding this comment.
It would happen on customer systems. We need to investigate proper schema migration at some point, definitely before we merge into CRI-O. This just clarifies errors so we know what's blowing up.
|
📌 Commit 98b1f0e has been approved by |
|
☀️ Test successful - status-papr |
github-actions
bot
added
the
locked - please file new issue/PR
This ensures we don't open a DB with an earlier schema or a config that differs from ours. Fixes an bug I found with @baude this morning.