-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add schema validation to DB #86
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) { | ||
// Create a table to hold runtime information | ||
// TODO: Include UID/GID mappings | ||
const runtimeTable = ` | ||
CREATE TABLE runtime( | ||
Id INTEGER NOT NULL PRIMARY KEY, | ||
SchemaVersion INTEGER NOT NULL, | ||
StaticDir TEXT NOT NULL, | ||
TmpDir TEXT NOT NULL, | ||
RunRoot TEXT NOT NULL, | ||
GraphRoot TEXT NOT NULL, | ||
GraphDriverName TEXT NOT NULL, | ||
CHECK (Id=0) | ||
); | ||
` | ||
const fillRuntimeTable = `INSERT INTO runtime VALUES ( | ||
?, ?, ?, ?, ?, ?, ? | ||
);` | ||
|
||
const selectRuntimeTable = `SELECT SchemaVersion, | ||
StaticDir, | ||
TmpDir, | ||
RunRoot, | ||
GraphRoot, | ||
GraphDriverName | ||
FROM runtime WHERE id=0;` | ||
|
||
const checkRuntimeExists = "SELECT name FROM sqlite_master WHERE type='table' AND name='runtime';" | ||
|
||
tx, err := db.Begin() | ||
if err != nil { | ||
return errors.Wrapf(err, "error beginning database transaction") | ||
} | ||
defer func() { | ||
if err != nil { | ||
if err2 := tx.Rollback(); err2 != nil { | ||
logrus.Errorf("Error rolling back transaction to check runtime table: %v", err2) | ||
} | ||
} | ||
|
||
}() | ||
|
||
row := tx.QueryRow(checkRuntimeExists) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
var table string | ||
if err := row.Scan(&table); err != nil { | ||
// There is no runtime table | ||
// Create and populate the runtime table | ||
if err == sql.ErrNoRows { | ||
if _, err := tx.Exec(runtimeTable); err != nil { | ||
return errors.Wrapf(err, "error creating runtime table in database") | ||
} | ||
|
||
_, err := tx.Exec(fillRuntimeTable, | ||
0, | ||
DBSchema, | ||
r.config.StaticDir, | ||
r.config.TmpDir, | ||
r.config.StorageConfig.RunRoot, | ||
r.config.StorageConfig.GraphRoot, | ||
r.config.StorageConfig.GraphDriverName) | ||
if err != nil { | ||
return errors.Wrapf(err, "error populating runtime table in database") | ||
} | ||
|
||
if err := tx.Commit(); err != nil { | ||
return errors.Wrapf(err, "error committing runtime table transaction in database") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
return errors.Wrapf(err, "error checking for presence of runtime table in database") | ||
} | ||
|
||
// There is a runtime table | ||
// Retrieve its contents | ||
var ( | ||
schemaVersion int | ||
staticDir string | ||
tmpDir string | ||
runRoot string | ||
graphRoot string | ||
graphDriverName string | ||
) | ||
|
||
row = tx.QueryRow(selectRuntimeTable) | ||
err = row.Scan( | ||
&schemaVersion, | ||
&staticDir, | ||
&tmpDir, | ||
&runRoot, | ||
&graphRoot, | ||
&graphDriverName) | ||
if err != nil { | ||
return errors.Wrapf(err, "error retrieving runtime information from database") | ||
} | ||
|
||
// Compare the information in the database against our runtime config | ||
if schemaVersion != DBSchema { | ||
return errors.Wrapf(ErrDBBadConfig, "database schema version %d does not match our schema version %d", | ||
schemaVersion, DBSchema) | ||
} | ||
if staticDir != r.config.StaticDir { | ||
return errors.Wrapf(ErrDBBadConfig, "database static directory %s does not match our static directory %s", | ||
staticDir, r.config.StaticDir) | ||
} | ||
if tmpDir != r.config.TmpDir { | ||
return errors.Wrapf(ErrDBBadConfig, "database temp directory %s does not match our temp directory %s", | ||
tmpDir, r.config.TmpDir) | ||
} | ||
if runRoot != r.config.StorageConfig.RunRoot { | ||
return errors.Wrapf(ErrDBBadConfig, "database runroot directory %s does not match our runroot directory %s", | ||
runRoot, r.config.StorageConfig.RunRoot) | ||
} | ||
if graphRoot != r.config.StorageConfig.GraphRoot { | ||
return errors.Wrapf(ErrDBBadConfig, "database graph root directory %s does not match our graph root directory %s", | ||
graphRoot, r.config.StorageConfig.GraphRoot) | ||
} | ||
if graphDriverName != r.config.StorageConfig.GraphDriverName { | ||
return errors.Wrapf(ErrDBBadConfig, "database runroot directory %s does not match our runroot directory %s", | ||
graphDriverName, r.config.StorageConfig.GraphDriverName) | ||
} | ||
|
||
if err := tx.Commit(); err != nil { | ||
return errors.Wrapf(err, "error committing runtime table transaction in database") | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// Performs database setup including by not limited to initializing tables in | ||
// the database | ||
func prepareDB(db *sql.DB) (err error) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.