From 1918d8784241b5a350e11eff33ca862e1be9935f Mon Sep 17 00:00:00 2001 From: Owen Cliffe Date: Tue, 24 Apr 2018 17:07:15 +0100 Subject: [PATCH] make annotations nullable to fix migration (#917) make app.annotations and routes.annotations non-null always This resolves a schema ambiguity that arrose in the annotations layer that would result in different schemas depending on whether you started afresh or migrated up. --- api/datastore/sql/migrations/10_add_app_id.go | 4 +- .../11_fix_annotations_migrations_route.go | 86 +++++++++++++++++++ .../12_fix_annotations_migrations_app.go | 77 +++++++++++++++++ 3 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 api/datastore/sql/migrations/11_fix_annotations_migrations_route.go create mode 100644 api/datastore/sql/migrations/12_fix_annotations_migrations_app.go diff --git a/api/datastore/sql/migrations/10_add_app_id.go b/api/datastore/sql/migrations/10_add_app_id.go index facc23830..5c0917c3a 100644 --- a/api/datastore/sql/migrations/10_add_app_id.go +++ b/api/datastore/sql/migrations/10_add_app_id.go @@ -10,6 +10,8 @@ import ( "github.com/jmoiron/sqlx" ) +// routes.annotations is retconned to NULLABLE here to allow migrations to proceed +// see migrations 11 and 12 that fix this back up to NOT NULL var sqlStatements = [...]string{`CREATE TABLE IF NOT EXISTS routes ( app_id varchar(256) NOT NULL, path varchar(256) NOT NULL, @@ -22,7 +24,7 @@ var sqlStatements = [...]string{`CREATE TABLE IF NOT EXISTS routes ( type varchar(16) NOT NULL, headers text NOT NULL, config text NOT NULL, - annotations text NOT NULL, + annotations text, created_at text, updated_at varchar(256), PRIMARY KEY (app_id, path) diff --git a/api/datastore/sql/migrations/11_fix_annotations_migrations_route.go b/api/datastore/sql/migrations/11_fix_annotations_migrations_route.go new file mode 100644 index 000000000..99cdbaac0 --- /dev/null +++ b/api/datastore/sql/migrations/11_fix_annotations_migrations_route.go @@ -0,0 +1,86 @@ +package migrations + +import ( + "context" + + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/jmoiron/sqlx" +) + +func up11(ctx context.Context, tx *sqlx.Tx) error { + + // clear out any old null values + // on mysql this may result in some in-flight changes being missed and an error on the alter table below + _, err := tx.ExecContext(ctx, `UPDATE routes set annotations=(CASE WHEN annotations IS NULL THEN '' ELSE annotations END);`) + if err != nil { + return err + } + + switch tx.DriverName() { + + case "mysql": + // this implicitly commits but its the last command so should be safe. + _, err := tx.ExecContext(ctx, "ALTER TABLE routes MODIFY annotations TEXT NOT NULL;") + return err + case "postgres", "pgx": + _, err := tx.ExecContext(ctx, "ALTER TABLE routes ALTER COLUMN annotations DROP NOT NULL;") + return err + default: + _, err := tx.ExecContext(ctx, "ALTER TABLE routes RENAME TO old_routes;") + + if err != nil { + return err + } + + newTable := `CREATE TABLE routes ( + app_id varchar(256) NOT NULL, + path varchar(256) NOT NULL, + image varchar(256) NOT NULL, + format varchar(16) NOT NULL, + memory int NOT NULL, + cpus int, + timeout int NOT NULL, + idle_timeout int NOT NULL, + type varchar(16) NOT NULL, + headers text NOT NULL, + config text NOT NULL, + annotations text NOT NULL, + created_at text, + updated_at varchar(256), + PRIMARY KEY (app_id, path) +);` + _, err = tx.ExecContext(ctx, newTable) + if err != nil { + return err + } + insertQuery := `INSERT INTO routes(app_id,path,image,format,memory,cpus,timeout,idle_timeout,type,headers,config,annotations,created_at,updated_at) + SELECT app_id,path,image,format,memory,cpus,timeout,idle_timeout,type,headers,config,annotations,created_at,updated_at FROM old_routes;` + + _, err = tx.ExecContext(ctx, insertQuery) + if err != nil { + return err + } + + _, err = tx.ExecContext(ctx, "DROP TABLE old_routes;") + if err != nil { + return err + } + + return err + + } + +} + +func down11(ctx context.Context, tx *sqlx.Tx) error { + // annotations are in an indeterminate state so we leave this change as it is + return nil +} + +func init() { + Migrations = append(Migrations, &migratex.MigFields{ + VersionFunc: vfunc(11), + UpFunc: up11, + DownFunc: down11, + }) +} diff --git a/api/datastore/sql/migrations/12_fix_annotations_migrations_app.go b/api/datastore/sql/migrations/12_fix_annotations_migrations_app.go new file mode 100644 index 000000000..7b8cce8e8 --- /dev/null +++ b/api/datastore/sql/migrations/12_fix_annotations_migrations_app.go @@ -0,0 +1,77 @@ +package migrations + +import ( + "context" + + "github.com/fnproject/fn/api/datastore/sql/migratex" + "github.com/jmoiron/sqlx" +) + +func up12(ctx context.Context, tx *sqlx.Tx) error { + + // clear out any old null values + // on mysql this may result in some in-flight changes being missed and an error on the alter table below + _, err := tx.ExecContext(ctx, `UPDATE apps set annotations=(CASE WHEN annotations IS NULL THEN '' ELSE annotations END);`) + if err != nil { + return err + } + + switch tx.DriverName() { + + case "mysql": + // this implicitly commits but its the last command so should be safe. + _, err := tx.ExecContext(ctx, "ALTER TABLE apps MODIFY annotations TEXT NOT NULL;") + return err + case "postgres", "pgx": + _, err := tx.ExecContext(ctx, "ALTER TABLE apps ALTER COLUMN annotations DROP NOT NULL;") + return err + default: // nuclear option, replace the table using sqlite safe DDL + _, err := tx.ExecContext(ctx, "ALTER TABLE apps RENAME TO old_apps;") + + if err != nil { + return err + } + + newTable := `CREATE TABLE apps ( + id varchar(256), + name varchar(256) NOT NULL PRIMARY KEY, + config text NOT NULL, + annotations text NOT NULL, + created_at varchar(256), + updated_at varchar(256) +);` + _, err = tx.ExecContext(ctx, newTable) + if err != nil { + return err + } + insertQuery := `INSERT INTO apps(id,name,config,annotations,created_at,updated_at) + SELECT id,name,config,annotations,created_at,updated_at FROM old_apps;` + + _, err = tx.ExecContext(ctx, insertQuery) + if err != nil { + return err + } + + _, err = tx.ExecContext(ctx, "DROP TABLE old_apps;") + if err != nil { + return err + } + + return err + + } + +} + +func down12(ctx context.Context, tx *sqlx.Tx) error { + // annotations are in an indeterminate state so we leave this change as it is + return nil +} + +func init() { + Migrations = append(Migrations, &migratex.MigFields{ + VersionFunc: vfunc(12), + UpFunc: up12, + DownFunc: down12, + }) +}