All of lore.kernel.org
 help / color / mirror / Atom feed
* [OSSTEST PATCH 0/7] Support database schema updates
@ 2015-12-10 13:51 Ian Jackson
  2015-12-10 13:51 ` [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX Ian Jackson
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

With this series it becomes possible to update the database schema in
a controlled and manageable way.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:04   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 2/7] Schema: Rename schema file Ian Jackson
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 mg-schema-test-database |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mg-schema-test-database b/mg-schema-test-database
index d704950..9791d84 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -244,7 +244,7 @@ create)
 				-v type="${rhs%% *}"		\
 				-v refkey="${rhs#* }"
 			;;
-		_)	suffix="$arg"
+		_*)	suffix="$arg"
 			;;
 		-f*)	minflight="${arg#-f}"
 			;;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 2/7] Schema: Rename schema file
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
  2015-12-10 13:51 ` [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:04   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql Ian Jackson
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

We are going to have multiple schema snippets and this is going be
just the initial baseline.

Rename the file and change references to it.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 INSTALL.production                                |    4 ++--
 mg-schema-test-database                           |    2 +-
 executive-postgresql-schema => schema/initial.sql |    0
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename executive-postgresql-schema => schema/initial.sql (100%)

diff --git a/INSTALL.production b/INSTALL.production
index 2db8bd3..2114ccc 100644
--- a/INSTALL.production
+++ b/INSTALL.production
@@ -29,8 +29,8 @@ DATABASE SERVER
 ---------------
 
 osstest requires a Postgres database server and a database configured
-with the schema described in executive-postgresql-schema which should
-be accessible to an osstest role account.
+with the schema described in schema/initial.sql should be accessible
+to an osstest role account.
 
 The hostname and dbname are configured via the "ExecutiveDbnamePat"
 config option.
diff --git a/mg-schema-test-database b/mg-schema-test-database
index 9791d84..0c4dab7 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -387,7 +387,7 @@ END
 	tables=$(tsort <$t.tablesortlist)
 
 	# We don't want to set the permissions
-	perl <executive-postgresql-schema >$t.new-schema -pe '
+	perl <schema/initial.sql >$t.new-schema -pe '
 		s/^/--/ if
 			m/^ALTER TABLE .* OWNER TO / ||
 			m/^GRANT |^REVOKE /
diff --git a/executive-postgresql-schema b/schema/initial.sql
similarity index 100%
rename from executive-postgresql-schema
rename to schema/initial.sql
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
  2015-12-10 13:51 ` [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX Ian Jackson
  2015-12-10 13:51 ` [OSSTEST PATCH 2/7] Schema: Rename schema file Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:07   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create Ian Jackson
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

Really, we don't want the initial schema setup to mess about with
permissions.  Instead, we simply expect to run the creation as the
correct role user.

So:
 - Remove the code in mg-schema-test-database to remove the
   permission settings from initial.sql;
 - Instead, run exactly that code on initial.sql and commit the
   result.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 mg-schema-test-database |    9 +---
 schema/initial.sql      |  132 +++++++++++++++++++++++------------------------
 2 files changed, 67 insertions(+), 74 deletions(-)

diff --git a/mg-schema-test-database b/mg-schema-test-database
index 0c4dab7..c68b1d2 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -386,19 +386,12 @@ END
 
 	tables=$(tsort <$t.tablesortlist)
 
-	# We don't want to set the permissions
-	perl <schema/initial.sql >$t.new-schema -pe '
-		s/^/--/ if
-			m/^ALTER TABLE .* OWNER TO / ||
-			m/^GRANT |^REVOKE /
-	'
-
 	#---------- create test db ----------
 
 	psql_do <<END
 		CREATE DATABASE $dbname;
 END
-	$(withtest get_psql_cmd) -q -f $t.new-schema
+	$(withtest get_psql_cmd) -q -f schema/initial.sql
 
 	printf ".\n"
 
diff --git a/schema/initial.sql b/schema/initial.sql
index 7bd6c55..4feeeb2 100644
--- a/schema/initial.sql
+++ b/schema/initial.sql
@@ -28,7 +28,7 @@ CREATE TABLE flights (
 );
 
 
-ALTER TABLE public.flights OWNER TO osstest;
+--ALTER TABLE public.flights OWNER TO osstest;
 
 --
 -- Name: flights_flight_seq; Type: SEQUENCE; Schema: public; Owner: osstest
@@ -42,7 +42,7 @@ CREATE SEQUENCE flights_flight_seq
     CACHE 1;
 
 
-ALTER TABLE public.flights_flight_seq OWNER TO osstest;
+--ALTER TABLE public.flights_flight_seq OWNER TO osstest;
 
 --
 -- Name: flights_flight_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: osstest
@@ -61,7 +61,7 @@ CREATE TABLE flights_harness_touched (
 );
 
 
-ALTER TABLE public.flights_harness_touched OWNER TO osstest;
+--ALTER TABLE public.flights_harness_touched OWNER TO osstest;
 
 --
 -- Name: hostflags; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -73,7 +73,7 @@ CREATE TABLE hostflags (
 );
 
 
-ALTER TABLE public.hostflags OWNER TO osstest;
+--ALTER TABLE public.hostflags OWNER TO osstest;
 
 --
 -- Name: jobs; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -87,7 +87,7 @@ CREATE TABLE jobs (
 );
 
 
-ALTER TABLE public.jobs OWNER TO osstest;
+--ALTER TABLE public.jobs OWNER TO osstest;
 
 --
 -- Name: resource_log_evid_seq; Type: SEQUENCE; Schema: public; Owner: iwj
@@ -101,7 +101,7 @@ CREATE SEQUENCE resource_log_evid_seq
     CACHE 1;
 
 
-ALTER TABLE public.resource_log_evid_seq OWNER TO iwj;
+--ALTER TABLE public.resource_log_evid_seq OWNER TO iwj;
 
 --
 -- Name: resource_log; Type: TABLE; Schema: public; Owner: iwj; Tablespace: 
@@ -122,7 +122,7 @@ CREATE TABLE resource_log (
 );
 
 
-ALTER TABLE public.resource_log OWNER TO iwj;
+--ALTER TABLE public.resource_log OWNER TO iwj;
 
 --
 -- Name: resource_properties; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -136,7 +136,7 @@ CREATE TABLE resource_properties (
 );
 
 
-ALTER TABLE public.resource_properties OWNER TO osstest;
+--ALTER TABLE public.resource_properties OWNER TO osstest;
 
 --
 -- Name: resource_sharing; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -152,7 +152,7 @@ CREATE TABLE resource_sharing (
 );
 
 
-ALTER TABLE public.resource_sharing OWNER TO osstest;
+--ALTER TABLE public.resource_sharing OWNER TO osstest;
 
 --
 -- Name: resources; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -168,7 +168,7 @@ CREATE TABLE resources (
 );
 
 
-ALTER TABLE public.resources OWNER TO osstest;
+--ALTER TABLE public.resources OWNER TO osstest;
 
 --
 -- Name: runvars; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -183,7 +183,7 @@ CREATE TABLE runvars (
 );
 
 
-ALTER TABLE public.runvars OWNER TO osstest;
+--ALTER TABLE public.runvars OWNER TO osstest;
 
 --
 -- Name: steps; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -201,7 +201,7 @@ CREATE TABLE steps (
 );
 
 
-ALTER TABLE public.steps OWNER TO osstest;
+--ALTER TABLE public.steps OWNER TO osstest;
 
 --
 -- Name: tasks_taskid_seq; Type: SEQUENCE; Schema: public; Owner: osstest
@@ -215,7 +215,7 @@ CREATE SEQUENCE tasks_taskid_seq
     CACHE 1;
 
 
-ALTER TABLE public.tasks_taskid_seq OWNER TO osstest;
+--ALTER TABLE public.tasks_taskid_seq OWNER TO osstest;
 
 --
 -- Name: tasks; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
@@ -232,7 +232,7 @@ CREATE TABLE tasks (
 );
 
 
-ALTER TABLE public.tasks OWNER TO osstest;
+--ALTER TABLE public.tasks OWNER TO osstest;
 
 --
 -- Name: flight; Type: DEFAULT; Schema: public; Owner: osstest
@@ -409,130 +409,130 @@ ALTER TABLE ONLY steps
 -- Name: public; Type: ACL; Schema: -; Owner: postgres
 --
 
-REVOKE ALL ON SCHEMA public FROM PUBLIC;
-REVOKE ALL ON SCHEMA public FROM postgres;
-GRANT ALL ON SCHEMA public TO postgres;
-GRANT ALL ON SCHEMA public TO PUBLIC;
+--REVOKE ALL ON SCHEMA public FROM PUBLIC;
+--REVOKE ALL ON SCHEMA public FROM postgres;
+--GRANT ALL ON SCHEMA public TO postgres;
+--GRANT ALL ON SCHEMA public TO PUBLIC;
 
 
 --
 -- Name: flights; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE flights FROM PUBLIC;
-REVOKE ALL ON TABLE flights FROM osstest;
-GRANT ALL ON TABLE flights TO osstest;
-GRANT SELECT ON TABLE flights TO osstest_ro;
+--REVOKE ALL ON TABLE flights FROM PUBLIC;
+--REVOKE ALL ON TABLE flights FROM osstest;
+--GRANT ALL ON TABLE flights TO osstest;
+--GRANT SELECT ON TABLE flights TO osstest_ro;
 
 
 --
 -- Name: flights_flight_seq; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON SEQUENCE flights_flight_seq FROM PUBLIC;
-REVOKE ALL ON SEQUENCE flights_flight_seq FROM osstest;
-GRANT ALL ON SEQUENCE flights_flight_seq TO osstest;
-GRANT SELECT ON SEQUENCE flights_flight_seq TO osstest_ro;
+--REVOKE ALL ON SEQUENCE flights_flight_seq FROM PUBLIC;
+--REVOKE ALL ON SEQUENCE flights_flight_seq FROM osstest;
+--GRANT ALL ON SEQUENCE flights_flight_seq TO osstest;
+--GRANT SELECT ON SEQUENCE flights_flight_seq TO osstest_ro;
 
 
 --
 -- Name: flights_harness_touched; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE flights_harness_touched FROM PUBLIC;
-REVOKE ALL ON TABLE flights_harness_touched FROM osstest;
-GRANT ALL ON TABLE flights_harness_touched TO osstest;
-GRANT SELECT ON TABLE flights_harness_touched TO osstest_ro;
+--REVOKE ALL ON TABLE flights_harness_touched FROM PUBLIC;
+--REVOKE ALL ON TABLE flights_harness_touched FROM osstest;
+--GRANT ALL ON TABLE flights_harness_touched TO osstest;
+--GRANT SELECT ON TABLE flights_harness_touched TO osstest_ro;
 
 
 --
 -- Name: hostflags; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE hostflags FROM PUBLIC;
-REVOKE ALL ON TABLE hostflags FROM osstest;
-GRANT ALL ON TABLE hostflags TO osstest;
-GRANT SELECT ON TABLE hostflags TO osstest_ro;
+--REVOKE ALL ON TABLE hostflags FROM PUBLIC;
+--REVOKE ALL ON TABLE hostflags FROM osstest;
+--GRANT ALL ON TABLE hostflags TO osstest;
+--GRANT SELECT ON TABLE hostflags TO osstest_ro;
 
 
 --
 -- Name: jobs; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE jobs FROM PUBLIC;
-REVOKE ALL ON TABLE jobs FROM osstest;
-GRANT ALL ON TABLE jobs TO osstest;
-GRANT SELECT ON TABLE jobs TO osstest_ro;
+--REVOKE ALL ON TABLE jobs FROM PUBLIC;
+--REVOKE ALL ON TABLE jobs FROM osstest;
+--GRANT ALL ON TABLE jobs TO osstest;
+--GRANT SELECT ON TABLE jobs TO osstest_ro;
 
 
 --
 -- Name: resource_properties; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE resource_properties FROM PUBLIC;
-REVOKE ALL ON TABLE resource_properties FROM osstest;
-GRANT ALL ON TABLE resource_properties TO osstest;
-GRANT SELECT ON TABLE resource_properties TO osstest_ro;
+--REVOKE ALL ON TABLE resource_properties FROM PUBLIC;
+--REVOKE ALL ON TABLE resource_properties FROM osstest;
+--GRANT ALL ON TABLE resource_properties TO osstest;
+--GRANT SELECT ON TABLE resource_properties TO osstest_ro;
 
 
 --
 -- Name: resource_sharing; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE resource_sharing FROM PUBLIC;
-REVOKE ALL ON TABLE resource_sharing FROM osstest;
-GRANT ALL ON TABLE resource_sharing TO osstest;
-GRANT SELECT ON TABLE resource_sharing TO osstest_ro;
+--REVOKE ALL ON TABLE resource_sharing FROM PUBLIC;
+--REVOKE ALL ON TABLE resource_sharing FROM osstest;
+--GRANT ALL ON TABLE resource_sharing TO osstest;
+--GRANT SELECT ON TABLE resource_sharing TO osstest_ro;
 
 
 --
 -- Name: resources; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE resources FROM PUBLIC;
-REVOKE ALL ON TABLE resources FROM osstest;
-GRANT ALL ON TABLE resources TO osstest;
-GRANT SELECT ON TABLE resources TO osstest_ro;
+--REVOKE ALL ON TABLE resources FROM PUBLIC;
+--REVOKE ALL ON TABLE resources FROM osstest;
+--GRANT ALL ON TABLE resources TO osstest;
+--GRANT SELECT ON TABLE resources TO osstest_ro;
 
 
 --
 -- Name: runvars; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE runvars FROM PUBLIC;
-REVOKE ALL ON TABLE runvars FROM osstest;
-GRANT ALL ON TABLE runvars TO osstest;
-GRANT SELECT ON TABLE runvars TO osstest_ro;
+--REVOKE ALL ON TABLE runvars FROM PUBLIC;
+--REVOKE ALL ON TABLE runvars FROM osstest;
+--GRANT ALL ON TABLE runvars TO osstest;
+--GRANT SELECT ON TABLE runvars TO osstest_ro;
 
 
 --
 -- Name: steps; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE steps FROM PUBLIC;
-REVOKE ALL ON TABLE steps FROM osstest;
-GRANT ALL ON TABLE steps TO osstest;
-GRANT SELECT ON TABLE steps TO osstest_ro;
+--REVOKE ALL ON TABLE steps FROM PUBLIC;
+--REVOKE ALL ON TABLE steps FROM osstest;
+--GRANT ALL ON TABLE steps TO osstest;
+--GRANT SELECT ON TABLE steps TO osstest_ro;
 
 
 --
 -- Name: tasks_taskid_seq; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM PUBLIC;
-REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM osstest;
-GRANT ALL ON SEQUENCE tasks_taskid_seq TO osstest;
-GRANT SELECT ON SEQUENCE tasks_taskid_seq TO osstest_ro;
+--REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM PUBLIC;
+--REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM osstest;
+--GRANT ALL ON SEQUENCE tasks_taskid_seq TO osstest;
+--GRANT SELECT ON SEQUENCE tasks_taskid_seq TO osstest_ro;
 
 
 --
 -- Name: tasks; Type: ACL; Schema: public; Owner: osstest
 --
 
-REVOKE ALL ON TABLE tasks FROM PUBLIC;
-REVOKE ALL ON TABLE tasks FROM osstest;
-GRANT ALL ON TABLE tasks TO osstest;
-GRANT SELECT ON TABLE tasks TO osstest_ro;
+--REVOKE ALL ON TABLE tasks FROM PUBLIC;
+--REVOKE ALL ON TABLE tasks FROM osstest;
+--GRANT ALL ON TABLE tasks TO osstest;
+--GRANT SELECT ON TABLE tasks TO osstest_ro;
 
 
 --
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
                   ` (2 preceding siblings ...)
  2015-12-10 13:51 ` [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:08   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 5/7] Schema: Support database schema updates Ian Jackson
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

There is a fair amount of option parsing clobber here that will be
relevant shortly.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 mg-schema-create        |   61 +++++++++++++++++++++++++++++++++++++++++++++++
 mg-schema-test-database |    2 +-
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100755 mg-schema-create

diff --git a/mg-schema-create b/mg-schema-create
new file mode 100755
index 0000000..54f1c76
--- /dev/null
+++ b/mg-schema-create
@@ -0,0 +1,61 @@
+#!/bin/bash
+
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2015 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+# Usages:
+#
+#  ./mg-schema-create [<options>]
+#
+#  Database must already exist.  (Ie, mg-schema-create does not
+#  do CREATE DATABASE.)
+#
+#  When setting up a production database, mg-schema-create should
+#  be run *AS THE ROLE USER* who is to own all the resources.
+#
+# Options:
+#
+#  -q                            don't print progress messages
+
+set -e
+set -o posix
+set -o pipefail
+
+progress () { printf "%s\n" "$*"; }
+progress=progress
+quietopt=''
+
+while [ $# != 0 ]; do
+    arg=$1; shift
+    case "$arg" in
+    -q)
+        progress=:
+        quietopt=-q
+        ;;
+    *)
+        echo >&2 "bad usage ($arg)"; exit 127
+        ;;
+    esac
+done
+
+. ./cri-getconfig
+
+$progress "Populating database..."
+
+$(get_psql_cmd) $quietopt -f schema/initial.sql
+
+$progress "Database set up."
diff --git a/mg-schema-test-database b/mg-schema-test-database
index c68b1d2..3616c4d 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -391,7 +391,7 @@ END
 	psql_do <<END
 		CREATE DATABASE $dbname;
 END
-	$(withtest get_psql_cmd) -q -f schema/initial.sql
+	withtest ./mg-schema-create -q
 
 	printf ".\n"
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 5/7] Schema: Support database schema updates
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
                   ` (3 preceding siblings ...)
  2015-12-10 13:51 ` [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:43   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user Ian Jackson
  2015-12-10 13:51 ` [OSSTEST PATCH 7/7] Schema: drop old resource_log table Ian Jackson
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

See schema/README.schema, introduced in this patch, for the design.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm      |   82 +++++++++++++++
 mg-schema-create          |   20 ++++
 mg-schema-test-database   |    6 +-
 mg-schema-update          |  255 +++++++++++++++++++++++++++++++++++++++++++++
 schema/README.updates     |  122 ++++++++++++++++++++++
 schema/schema-updates.sql |    6 ++
 6 files changed, 490 insertions(+), 1 deletion(-)
 create mode 100755 mg-schema-update
 create mode 100644 schema/README.updates
 create mode 100644 schema/schema-updates.sql

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index fcef83f..e1fbe3b 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -56,6 +56,7 @@ BEGIN {
                       resource_check_allocated resource_shared_mark_ready
                       duration_estimator
                       db_pg_dsn opendb opendb_state
+                      db_schema_updates_applied db_schema_updates_intree
                       );
     %EXPORT_TAGS = ( colours => [qw($green $red $yellow $purple $blue)] );
 
@@ -128,6 +129,87 @@ sub grabrepolock_reexec {
     }
 }
 
+sub db_schema_updates_applied (;$) {
+    my ($cond) = @_;
+    my $r;
+    $cond //= '1=1';
+    eval {
+	local $dbh_tests->{PrintError} = 0;
+	$r = $dbh_tests->selectall_arrayref(<<END);
+	    SELECT updatename, applytime
+              FROM schema_updates WHERE $cond
+END
+    };
+    if ($@) {
+	die unless
+	    $dbh_tests->err()==7 && # DBD::Pg(3pm)
+	    $dbh_tests->state() eq '42P01';
+	# http://www.postgresql.org/docs/current/static/errcodes-appendix.html
+	$r = [ ];
+    }
+    my @r;
+    foreach (@$r) {
+	push @r, { Name => $_->[0], Applied => $_->[1] };
+    }
+    return \@r;
+}
+
+sub db_schema_updates_intree (;$) {
+    my ($incommit) = @_;
+    # ->[]{Name}
+    # ->[]{Seq}
+    # ->[]{State}
+
+    my @results;
+
+    my @files;
+    if (!$incommit) {
+	@files = <schema/*.sql>;
+    } else {
+	local $/ = "\0";
+	open GLF, "-|", qw(git ls-tree -z), $incommit, "schema/" or die $!;
+	while (<GLF>) {
+	    chomp;
+	    next unless s/^\d+ blob \w+\t//;
+	    push @files, $_;
+	}
+	$!=0; $?=0; close GLF or die "$! $? ($incommit)";
+    }
+
+  FILE: foreach my $f (@files) {
+        $f =~ m/\.sql$/ or next;
+	$f =~ m#/([a-z][0-9a-z-]+)\.sql$# or die "badly named .sql file $f\n";
+	my $name = $1;
+	next if $name eq 'initial';
+	if ($incommit) {
+	    open SQLF, "-|", qw(git cat-file blob), "$incommit:$f" or die $!;
+	} else {
+	    open SQLF, "<", $f or die "$f $!";
+	}
+	while (<SQLF>) {
+	    chomp;
+	    my $origl = $_;
+	    next unless s/^\s*--\s*##OSSTEST##\s+//;
+	    m/^0*([1-9]\d*)\s+(Harmless|Preparatory|Unfinished|Ready|Needed)\b/
+		or die "$origl ?";
+	    push @results, {
+		Name => $name,
+		Seq => $1+0,
+		State => $2,
+	    };
+	    next FILE;
+	}
+	$!=0; $?=0; close SQLF; die "$f \`$name' no token ($! $?)";
+    }
+
+    @results = sort {
+	$a->{Seq} <=> $b->{Seq} ||
+	    die "$a->{Name} $a->{Seq} == $b->{Name} $b->{Seq}"
+    } @results;
+
+    return \@results;
+}
+
 #---------- database access ----------#
 
 sub opendb_state () {
diff --git a/mg-schema-create b/mg-schema-create
index 54f1c76..1ee007b 100755
--- a/mg-schema-create
+++ b/mg-schema-create
@@ -30,6 +30,9 @@
 # Options:
 #
 #  -q                            don't print progress messages
+#  --no-updates                  apply no schema updates
+#  --stop-before --stop-after    only apply some schema updates -
+#                                 see mg-schema-update
 
 set -e
 set -o posix
@@ -38,6 +41,8 @@ set -o pipefail
 progress () { printf "%s\n" "$*"; }
 progress=progress
 quietopt=''
+do_updates=true
+updates=()
 
 while [ $# != 0 ]; do
     arg=$1; shift
@@ -46,16 +51,31 @@ while [ $# != 0 ]; do
         progress=:
         quietopt=-q
         ;;
+    --stop-before|--stop-after)
+    	updates+=("$arg" "$1"); shift
+	;;
+    --stop-before=*|--stop-after=*)
+    	updates+=("$arg"); shift
+	;;
+    --no-updates)
+    	do_updates=false
+	;;
     *)
         echo >&2 "bad usage ($arg)"; exit 127
         ;;
     esac
 done
 
+export OSSTEST_DB_USEREAL_IGNORETEST='.*'
+
 . ./cri-getconfig
 
 $progress "Populating database..."
 
 $(get_psql_cmd) $quietopt -f schema/initial.sql
 
+if $do_updates; then
+    ./mg-schema-update $quietopt apply-all "${updates[@]}"
+fi
+
 $progress "Database set up."
diff --git a/mg-schema-test-database b/mg-schema-test-database
index 3616c4d..5c6a935 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -344,6 +344,8 @@ END
 	# Keep a copy as it came from dump, for comparison
 	cp $t.schema $t.schema.orig
 
+	wantupdates=$(./mg-schema-update list-applied)
+
 	# http://www.postgresql.org/message-id/26790.1306355327@sss.pgh.pa.us
 	perl -i~ -pe '
 		s/^/--/ if
@@ -391,7 +393,9 @@ END
 	psql_do <<END
 		CREATE DATABASE $dbname;
 END
-	withtest ./mg-schema-create -q
+	withtest ./mg-schema-create -q --no-updates
+
+	withtest ./mg-schema-update -q apply $wantupdates
 
 	printf ".\n"
 
diff --git a/mg-schema-update b/mg-schema-update
new file mode 100755
index 0000000..a617c3b
--- /dev/null
+++ b/mg-schema-update
@@ -0,0 +1,255 @@
+#!/usr/bin/perl -w
+
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2015 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+# Usages:
+#
+#  ./mg-schema-update [<options>] apply [<updatename>...]
+#  ./mg-schema-update [<options>] show
+#  ./mg-schema-update [<options>] apply-all
+#
+# Usual rune for applying updates:
+#
+#  ./mg-schema-update -o <oldest-running-git-ref-spec> apply-all
+#
+# Options:
+#
+#  -o<git-ref-spec> --oldest=<git-ref-spec>
+#     Specify the oldest version of osstest that is currently running
+#     anywhere against this DB.  Used to determine compatibility of
+#     updates.
+#
+#  -f
+#     Force.  May be repeated for greater effect:
+#
+#     Force installation of updates which break some old code, even
+#     though -o was not specified so it is not known whether such old
+#     code is still running anywhere.
+#
+#  -ff
+#     Force installation of updates which are known to break running
+#     code (including, perhaps, this very code right here).
+#
+#  -fff
+#     Force an attempt to install an already-installed update.  This
+#     is not likely to work.
+
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use Osstest::Executive;
+
+use Getopt::Long qw(:config bundling gnu_compat require_order no_ignore_case);
+
+csreadconfig();
+
+our (%state, @state);
+our $there;
+our $force=0;
+our $quiet=0;
+
+sub getstate () {
+    my $record = sub {
+	my ($key,$list) = @_;
+	foreach my $entry (@$list) {
+	    $state{ $entry->{Name} }{ $key } = $entry;
+	}
+    };
+
+    $record->('Db', db_schema_updates_applied());
+    $record->('Here', db_schema_updates_intree());
+    $record->('There', db_schema_updates_intree($there)) if $there;
+
+    foreach my $name (keys %state) {
+	my $out = $state{$name};
+	$out->{Name} = $name;
+	$out->{Sortkey} =
+	    $out->{Here}{Seq} //
+	    $out->{There}{Seq} //
+	    0;
+    }
+
+    foreach my $out (values %state) {
+	my $st = $out->{Here}{State} // 'missing';
+	# ->{Todo} = 0: no; 1: maybe (?old code); 2: always
+	#             -1: already applied
+	if ($out->{Db}{Applied}) {
+	    $out->{Todo} = -1;
+	    $out->{Msg} = "already applied";
+	} elsif ($st =~ m/Harmless|Preparatory/) {
+	    $out->{Todo} = 2;
+	    $out->{Msg} = "right away";
+	} elsif ($st =~ m/Ready|Needed/) {
+	    my $tst = $out->{There}{State} // 'missing';
+	    if (!$there) {
+		$out->{Todo} = 1;
+		$out->{Msg} = "would break any old code";
+	    } elsif ($tst =~ m/Harmless|Preparatory/) {
+		$out->{Todo} = 2;
+		$out->{Msg} = "specified revision can cope";
+	    } else {
+		$out->{Todo} = 0;
+		$out->{Msg} = "specified revision would break";
+	    }
+	} else {
+	    $out->{Todo} = 0;
+	    $out->{Msg} = "not ready";
+	}
+	die unless defined $out->{Todo} && defined $out->{Msg};
+
+	$out->{File} = "schema/$out->{Name}.sql";
+    }
+
+    @state = sort { $a->{Sortkey} <=> $b->{Sortkey} } values %state;
+}
+
+sub cmd_list_applied () {
+    die if @ARGV;
+    getstate();
+    foreach my $v (@state) {
+	next unless $v->{Db}{Applied};
+	print $v->{Name}, "\n" or die $!;
+    }
+}
+
+sub cmd_show () {
+    die if @ARGV;
+
+    getstate();
+
+    printf "%-20s", "Name";
+    printf "    %-9s", "Worktree";
+    printf "  %-11.11s", (sprintf "%8.11s", $there) if $there;
+    printf "  DB update\n";
+
+    foreach my $v (@state) {
+	printf(" %-20s %5s %-5.5s",
+	       $v->{Name},
+	       $v->{Here}{Seq} // '',
+	       $v->{Here}{State} // '-');
+	printf("  %5s %-5.5s",
+	       $v->{There}{Seq} // '',
+	       $v->{There}{State} // '-') if $there;
+	my $app;
+	if ($v->{Db}{Applied}) {
+	    $app = show_abs_time($v->{Db}{Applied});
+	} else {
+	    $app = (qw(No: Maybe: Ready!))[$v->{Todo}];
+	    $app .= " ";
+	    $app .= $v->{Msg};
+	}
+	printf("  %s\n", $app);
+    }
+
+    STDOUT->error and die $!;
+}
+
+sub want_apply ($) {
+    my ($v) = @_;
+    $v->{Todo} >= 2-$force;
+}
+
+sub applyone ($) {
+    my ($v) = @_;
+    die "Will not apply $v->{Name}.sql: $v->{Msg}\n"
+	unless want_apply($v);
+
+    my $fn = $v->{File};
+
+    db_retry($dbh_tests, \@all_lock_tables, sub {
+	print "Applying $fn...\n" unless $quiet;
+	open F, "<", $fn or die "$fn: $!";
+	local $/ = undef;
+	my $sql = <F>;
+	F->error and die $!;
+	close F or die $!;
+
+	$dbh_tests->do(<<END) if $quiet;
+            SET client_min_messages = warning;
+END
+
+	$dbh_tests->do($sql);
+
+	$dbh_tests->do(<<END, {}, $v->{Name}, time);
+	    INSERT INTO schema_updates
+		(updatename, applytime)
+		VALUES (?, ?)
+END
+    });
+
+    print "Applying $fn done.\n" unless $quiet;
+}
+
+sub cmd_apply () {
+    print "No updates applied by calling apply with no update names.\n"
+	unless $quiet;
+    foreach my $name (@ARGV) {
+	getstate();
+	my $ent = $state{$name};
+	die "unknown update \`$name'\n" unless $ent;
+	die "update \`$name' not in this tree\n" unless $ent->{Here}{Seq};
+	applyone($ent);
+    }
+}
+
+sub cmd_apply_all () {
+    my (%stopafter, %stopbefore);
+
+    GetOptions('--stop-after=s' => sub { $stopafter{$_[1]} = 1 },
+	       '--stop-before=s' => sub { $stopbefore{$_[1]} = 1 });
+    die "further arguments to apply-all prohibited\n" if @ARGV;
+
+    getstate();
+    foreach my $v (@state) {
+	next unless $v->{Here}{Seq};
+	my $stop = sub {
+	    my ($map) = @_;
+	    return 0 unless
+		$map->{ $v->{Name} } ||
+		$map->{ $v->{Here}{Seq} };
+	    print "Stopping at $v->{Name} ($v->{Here}{Seq}).\n"
+		unless $quiet;
+	    return 1;
+	};
+
+	last if $stop->(\%stopbefore);
+
+	if (want_apply($v)) {
+	    applyone($v);
+	} else {
+	    print "Skipping $v->{File}: $v->{Msg}\n" unless $quiet;
+	}
+
+	last if $stop->(\%stopafter);
+    }
+
+    print "Appropriate updates applied.\n" unless $quiet;
+}
+
+GetOptions('f|force+' => \$force,
+	   'q+' => \$quiet,
+	   'o|oldest=s' => \$there);
+
+die "need operation\n" unless @ARGV;
+
+my $subcmd= shift @ARGV;
+$subcmd =~ s/-/_/g;
+my $subcmdproc = ${*::}{"cmd_$subcmd"};
+die "unknown subcommand" unless $subcmdproc;
+$subcmdproc->();
diff --git a/schema/README.updates b/schema/README.updates
new file mode 100644
index 0000000..0aa349d
--- /dev/null
+++ b/schema/README.updates
@@ -0,0 +1,122 @@
+SCHEMA DEFINITION AND SCHEMA UPDATES (PRODUCTION `EXECUTIVE' MODE)
+==================================================================
+
+To generate a new DB, we apply the original schema (in initial.sql)
+and then apply all the updates, in order.
+
+We maintain a table in the DB which records which updates are applied.
+
+
+Schema update snippet format
+----------------------------
+
+Schema update snippets should be called
+   schema/<updatename>.sql
+
+They should contain DDL commands (ALTER TABLE etc.) to make whatever
+changes are needed.
+
+They MUST NOT contain BEGIN or COMMIT.
+
+They must contain a special comment near the top:
+
+  -- ##OSSTEST## <sequence> <status>
+
+<updatename> is a string (/^[a-z][0-9a-z-]+$/) which uniquely identifies
+the update.  It must not be changed because existing installations
+rely on updates having stable names.
+
+<sequence> is a positive integer, which should be unique.  Updates are
+applied in order.
+
+<status> reflects the compatibility of various schema versions, see
+below.
+
+
+Update orders
+-------------
+
+There are three reasonable plans for schema changes:
+
+ * Fully intercompatible: both old code and new code are each
+   compatible with both old schema and new schema.  The code and
+   schema updates may be done in any order.
+
+   Such a schema change is always in state:
+      Harmless
+
+ * Explicit conditional: first update the code to understand both
+   versions of the schema; then update the schema; then drop the
+   compatibility code.
+
+   Such a schema change is in state:
+      Unfinished (or absent)   in old code
+      Ready                    in intermediate code
+      Needed                   in the final code
+
+ * Code first: the new code works with either old or new schema,
+   but the old code cannot cope with the new schema.
+
+   Such a schema change is in state:
+      Unfinished (or absent)   in old code
+      Ready'                   in new code
+
+ * Schema first: the new schema works with any code; but the old
+   schema does not work with new code.
+
+   Such a schema change is in state
+      Preparatory              in old code
+      Needed                   in the new code
+
+
+Update order for Populate-then-rely
+-----------------------------------
+
+This is for when we want to record new information and then later rely
+on it.  There are typically two schema changes: to add the column(s)
+(`add') and then to add appropriate constraints (`constraint') to
+prevent it being left blank.
+
+1. Commit: new schema update `add', in state Preparatory.
+
+1. Commit: new schema update `constraint', in state Unfinished.
+
+2. Apply: `add'.
+
+2. Commit: code to populate new column; changing `add' to state
+   Needed and `constraint' to state Ready.
+
+3. Optionally commit: idempotent utility script to populate missing
+   data.  (This can be done with DML in the `constraint' update.)
+
+3. Wait for all executions of old code to finish.
+
+5. Apply: `constraint'.
+
+
+States and rules for push and db update
+---------------------------------------
+
+  Harmless
+  Preparatory
+     No restrictions
+
+  Unfinished
+  (sql fragment entirely missing is equivalent to Unfinished)
+     Schema update: prevented
+     Code push: unrestricted
+
+  Ready
+     Schema update: need all live code to be Preparatory/Ready/Needed
+     Code push: unrestricted
+
+  Needed
+     Schema update: need all live code to be Preparatory/Ready/Needed
+     Code push: depends on schema update
+
+
+"Push depends on schema update" is not currently implemented.
+
+"Checks for live old code" means to look for the state of this schema
+update in other running versions of osstest.  This is not currently
+implemented.
diff --git a/schema/schema-updates.sql b/schema/schema-updates.sql
new file mode 100644
index 0000000..cd8dc0c
--- /dev/null
+++ b/schema/schema-updates.sql
@@ -0,0 +1,6 @@
+-- ##OSSTEST## 001 Harmless
+
+CREATE TABLE schema_updates (
+    updatename TEXT PRIMARY KEY,
+    applytime integer NOT NULL
+);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
                   ` (4 preceding siblings ...)
  2015-12-10 13:51 ` [OSSTEST PATCH 5/7] Schema: Support database schema updates Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:45   ` Ian Campbell
  2015-12-10 13:51 ` [OSSTEST PATCH 7/7] Schema: drop old resource_log table Ian Jackson
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 Osstest/Executive.pm    |    1 +
 README                  |    5 +++++
 mg-schema-create        |    4 +++-
 mg-schema-test-database |    1 +
 mg-schema-update        |   16 ++++++++++++++++
 5 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index e1fbe3b..f2d29ef 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -105,6 +105,7 @@ augmentconfigdefaults(
     QueuePlanUpdateInterval => 300, # seconds
     Repos => "$ENV{'HOME'}/repos",
     BisectionRevisonGraphSize => '600x300',
+    ExecutiveDbOwningRoleRegexp => 'osstest',
 );
 
 augmentconfigdefaults(
diff --git a/README b/README
index 5740ac0..0a346dc 100644
--- a/README
+++ b/README
@@ -571,6 +571,11 @@ ExecutiveDbname_<DB>
    PostgreSQL dbname string for the database <DB>.  Default is to use
    ExecutiveDbnamePat.
 
+ExecutiveDbOwningRoleRegexp
+   Regexp which is supposed to match the database user used for schema
+   changes - because, that role will end up owning the database objects.
+   Defaults to `osstest'.
+
 Adhoc/Custom Flights
 ====================
 
diff --git a/mg-schema-create b/mg-schema-create
index 1ee007b..df5e215 100755
--- a/mg-schema-create
+++ b/mg-schema-create
@@ -25,7 +25,7 @@
 #  do CREATE DATABASE.)
 #
 #  When setting up a production database, mg-schema-create should
-#  be run *AS THE ROLE USER* who is to own all the resources.
+#  be run as the role user who is to own all the resources.
 #
 # Options:
 #
@@ -70,6 +70,8 @@ export OSSTEST_DB_USEREAL_IGNORETEST='.*'
 
 . ./cri-getconfig
 
+./mg-schema-update $quietopt check-user
+
 $progress "Populating database..."
 
 $(get_psql_cmd) $quietopt -f schema/initial.sql
diff --git a/mg-schema-test-database b/mg-schema-test-database
index 5c6a935..bf82c75 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -335,6 +335,7 @@ OwnerDaemonHost $ctrlhost
 QueueDaemonHost $ctrlhost
 OwnerDaemonPort ${ctrlports%,*}
 QueueDaemonPort ${ctrlports#*,}
+ExecutiveDbOwningRoleRegexp .*
 END
 	mv -f $tcfg.tmp $tcfg
 
diff --git a/mg-schema-update b/mg-schema-update
index a617c3b..f699180 100755
--- a/mg-schema-update
+++ b/mg-schema-update
@@ -22,6 +22,7 @@
 #  ./mg-schema-update [<options>] apply [<updatename>...]
 #  ./mg-schema-update [<options>] show
 #  ./mg-schema-update [<options>] apply-all
+#  ./mg-schema-update [<options>] check-user
 #
 # Usual rune for applying updates:
 #
@@ -119,6 +120,13 @@ sub getstate () {
     @state = sort { $a->{Sortkey} <=> $b->{Sortkey} } values %state;
 }
 
+sub check_user () {
+    my $user = $dbh_tests->{pg_user};
+    my $re = $c{ExecutiveDbOwningRoleRegexp};
+    return if $user =~ m/^$re$/o;
+    die "running as wrong user \`$user', expected to match \`$re'\n";
+}
+
 sub cmd_list_applied () {
     die if @ARGV;
     getstate();
@@ -170,6 +178,8 @@ sub applyone ($) {
     die "Will not apply $v->{Name}.sql: $v->{Msg}\n"
 	unless want_apply($v);
 
+    check_user();
+
     my $fn = $v->{File};
 
     db_retry($dbh_tests, \@all_lock_tables, sub {
@@ -242,6 +252,12 @@ sub cmd_apply_all () {
     print "Appropriate updates applied.\n" unless $quiet;
 }
 
+sub cmd_check_user () {
+    die "too many arguments\n" if @ARGV>1;
+    $c{ExecutiveDbOwningRoleRegexp} = shift @ARGV if @ARGV;
+    check_user();
+}
+
 GetOptions('f|force+' => \$force,
 	   'q+' => \$quiet,
 	   'o|oldest=s' => \$there);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 7/7] Schema: drop old resource_log table
  2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
                   ` (5 preceding siblings ...)
  2015-12-10 13:51 ` [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user Ian Jackson
@ 2015-12-10 13:51 ` Ian Jackson
  2015-12-10 15:46   ` Ian Campbell
  6 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 13:51 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 schema/drop-old-resource-log.sql |    8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 schema/drop-old-resource-log.sql

diff --git a/schema/drop-old-resource-log.sql b/schema/drop-old-resource-log.sql
new file mode 100644
index 0000000..9494e1b
--- /dev/null
+++ b/schema/drop-old-resource-log.sql
@@ -0,0 +1,8 @@
+-- ##OSSTEST## 002 Harmless
+--
+-- This table and associated constraints and indices is from a previous
+-- aborted attempt at resource logging.  Nothing actually reads or
+-- writes it.
+
+DROP TABLE resource_log;
+DROP SEQUENCE resource_log_evid_seq;
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX
  2015-12-10 13:51 ` [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX Ian Jackson
@ 2015-12-10 15:04   ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:04 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 2/7] Schema: Rename schema file
  2015-12-10 13:51 ` [OSSTEST PATCH 2/7] Schema: Rename schema file Ian Jackson
@ 2015-12-10 15:04   ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:04 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> We are going to have multiple schema snippets and this is going be
> just the initial baseline.
> 
> Rename the file and change references to it.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql
  2015-12-10 13:51 ` [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql Ian Jackson
@ 2015-12-10 15:07   ` Ian Campbell
  2015-12-10 15:16     ` Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:07 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> Really, we don't want the initial schema setup to mess about with
> permissions.  Instead, we simply expect to run the creation as the
> correct role user.
> 
> So:
>  - Remove the code in mg-schema-test-database to remove the
>    permission settings from initial.sql;
>  - Instead, run exactly that code on initial.sql and commit the
>    result.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

I suppose the resulting comments in initial.sql are useful even for
illustration (i.e. not much point in removing).

I noticed that owner is not always osstest, I see instances of iwj,
postgres and osstest_ro. I guess iwj is some historical baggage which the
expectation to run as the role user supercedes, but I'm not sure about
osstest_ro or postgres?

> ---
>  mg-schema-test-database |    9 +---
>  schema/initial.sql      |  132 +++++++++++++++++++++++----------------
> --------
>  2 files changed, 67 insertions(+), 74 deletions(-)
> 
> diff --git a/mg-schema-test-database b/mg-schema-test-database
> index 0c4dab7..c68b1d2 100755
> --- a/mg-schema-test-database
> +++ b/mg-schema-test-database
> @@ -386,19 +386,12 @@ END
>  
>  	tables=$(tsort <$t.tablesortlist)
>  
> -	# We don't want to set the permissions
> -	perl <schema/initial.sql >$t.new-schema -pe '
> -		s/^/--/ if
> -			m/^ALTER TABLE .* OWNER TO / ||
> -			m/^GRANT |^REVOKE /
> -	'
> -
>  	#---------- create test db ----------
>  
>  	psql_do <<END
>  		CREATE DATABASE $dbname;
>  END
> -	$(withtest get_psql_cmd) -q -f $t.new-schema
> +	$(withtest get_psql_cmd) -q -f schema/initial.sql
>  
>  	printf ".\n"
>  
> diff --git a/schema/initial.sql b/schema/initial.sql
> index 7bd6c55..4feeeb2 100644
> --- a/schema/initial.sql
> +++ b/schema/initial.sql
> @@ -28,7 +28,7 @@ CREATE TABLE flights (
>  );
>  
>  
> -ALTER TABLE public.flights OWNER TO osstest;
> +--ALTER TABLE public.flights OWNER TO osstest;
>  
>  --
>  -- Name: flights_flight_seq; Type: SEQUENCE; Schema: public; Owner:
> osstest
> @@ -42,7 +42,7 @@ CREATE SEQUENCE flights_flight_seq
>      CACHE 1;
>  
>  
> -ALTER TABLE public.flights_flight_seq OWNER TO osstest;
> +--ALTER TABLE public.flights_flight_seq OWNER TO osstest;
>  
>  --
>  -- Name: flights_flight_seq; Type: SEQUENCE OWNED BY; Schema: public;
> Owner: osstest
> @@ -61,7 +61,7 @@ CREATE TABLE flights_harness_touched (
>  );
>  
>  
> -ALTER TABLE public.flights_harness_touched OWNER TO osstest;
> +--ALTER TABLE public.flights_harness_touched OWNER TO osstest;
>  
>  --
>  -- Name: hostflags; Type: TABLE; Schema: public; Owner: osstest;
> Tablespace: 
> @@ -73,7 +73,7 @@ CREATE TABLE hostflags (
>  );
>  
>  
> -ALTER TABLE public.hostflags OWNER TO osstest;
> +--ALTER TABLE public.hostflags OWNER TO osstest;
>  
>  --
>  -- Name: jobs; Type: TABLE; Schema: public; Owner: osstest; Tablespace: 
> @@ -87,7 +87,7 @@ CREATE TABLE jobs (
>  );
>  
>  
> -ALTER TABLE public.jobs OWNER TO osstest;
> +--ALTER TABLE public.jobs OWNER TO osstest;
>  
>  --
>  -- Name: resource_log_evid_seq; Type: SEQUENCE; Schema: public; Owner:
> iwj
> @@ -101,7 +101,7 @@ CREATE SEQUENCE resource_log_evid_seq
>      CACHE 1;
>  
>  
> -ALTER TABLE public.resource_log_evid_seq OWNER TO iwj;
> +--ALTER TABLE public.resource_log_evid_seq OWNER TO iwj;
>  
>  --
>  -- Name: resource_log; Type: TABLE; Schema: public; Owner: iwj;
> Tablespace: 
> @@ -122,7 +122,7 @@ CREATE TABLE resource_log (
>  );
>  
>  
> -ALTER TABLE public.resource_log OWNER TO iwj;
> +--ALTER TABLE public.resource_log OWNER TO iwj;
>  
>  --
>  -- Name: resource_properties; Type: TABLE; Schema: public; Owner:
> osstest; Tablespace: 
> @@ -136,7 +136,7 @@ CREATE TABLE resource_properties (
>  );
>  
>  
> -ALTER TABLE public.resource_properties OWNER TO osstest;
> +--ALTER TABLE public.resource_properties OWNER TO osstest;
>  
>  --
>  -- Name: resource_sharing; Type: TABLE; Schema: public; Owner: osstest;
> Tablespace: 
> @@ -152,7 +152,7 @@ CREATE TABLE resource_sharing (
>  );
>  
>  
> -ALTER TABLE public.resource_sharing OWNER TO osstest;
> +--ALTER TABLE public.resource_sharing OWNER TO osstest;
>  
>  --
>  -- Name: resources; Type: TABLE; Schema: public; Owner: osstest;
> Tablespace: 
> @@ -168,7 +168,7 @@ CREATE TABLE resources (
>  );
>  
>  
> -ALTER TABLE public.resources OWNER TO osstest;
> +--ALTER TABLE public.resources OWNER TO osstest;
>  
>  --
>  -- Name: runvars; Type: TABLE; Schema: public; Owner: osstest;
> Tablespace: 
> @@ -183,7 +183,7 @@ CREATE TABLE runvars (
>  );
>  
>  
> -ALTER TABLE public.runvars OWNER TO osstest;
> +--ALTER TABLE public.runvars OWNER TO osstest;
>  
>  --
>  -- Name: steps; Type: TABLE; Schema: public; Owner: osstest; Tablespace:
> @@ -201,7 +201,7 @@ CREATE TABLE steps (
>  );
>  
>  
> -ALTER TABLE public.steps OWNER TO osstest;
> +--ALTER TABLE public.steps OWNER TO osstest;
>  
>  --
>  -- Name: tasks_taskid_seq; Type: SEQUENCE; Schema: public; Owner:
> osstest
> @@ -215,7 +215,7 @@ CREATE SEQUENCE tasks_taskid_seq
>      CACHE 1;
>  
>  
> -ALTER TABLE public.tasks_taskid_seq OWNER TO osstest;
> +--ALTER TABLE public.tasks_taskid_seq OWNER TO osstest;
>  
>  --
>  -- Name: tasks; Type: TABLE; Schema: public; Owner: osstest; Tablespace:
> @@ -232,7 +232,7 @@ CREATE TABLE tasks (
>  );
>  
>  
> -ALTER TABLE public.tasks OWNER TO osstest;
> +--ALTER TABLE public.tasks OWNER TO osstest;
>  
>  --
>  -- Name: flight; Type: DEFAULT; Schema: public; Owner: osstest
> @@ -409,130 +409,130 @@ ALTER TABLE ONLY steps
>  -- Name: public; Type: ACL; Schema: -; Owner: postgres
>  --
>  
> -REVOKE ALL ON SCHEMA public FROM PUBLIC;
> -REVOKE ALL ON SCHEMA public FROM postgres;
> -GRANT ALL ON SCHEMA public TO postgres;
> -GRANT ALL ON SCHEMA public TO PUBLIC;
> +--REVOKE ALL ON SCHEMA public FROM PUBLIC;
> +--REVOKE ALL ON SCHEMA public FROM postgres;
> +--GRANT ALL ON SCHEMA public TO postgres;
> +--GRANT ALL ON SCHEMA public TO PUBLIC;
>  
>  
>  --
>  -- Name: flights; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE flights FROM PUBLIC;
> -REVOKE ALL ON TABLE flights FROM osstest;
> -GRANT ALL ON TABLE flights TO osstest;
> -GRANT SELECT ON TABLE flights TO osstest_ro;
> +--REVOKE ALL ON TABLE flights FROM PUBLIC;
> +--REVOKE ALL ON TABLE flights FROM osstest;
> +--GRANT ALL ON TABLE flights TO osstest;
> +--GRANT SELECT ON TABLE flights TO osstest_ro;
>  
>  
>  --
>  -- Name: flights_flight_seq; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON SEQUENCE flights_flight_seq FROM PUBLIC;
> -REVOKE ALL ON SEQUENCE flights_flight_seq FROM osstest;
> -GRANT ALL ON SEQUENCE flights_flight_seq TO osstest;
> -GRANT SELECT ON SEQUENCE flights_flight_seq TO osstest_ro;
> +--REVOKE ALL ON SEQUENCE flights_flight_seq FROM PUBLIC;
> +--REVOKE ALL ON SEQUENCE flights_flight_seq FROM osstest;
> +--GRANT ALL ON SEQUENCE flights_flight_seq TO osstest;
> +--GRANT SELECT ON SEQUENCE flights_flight_seq TO osstest_ro;
>  
>  
>  --
>  -- Name: flights_harness_touched; Type: ACL; Schema: public; Owner:
> osstest
>  --
>  
> -REVOKE ALL ON TABLE flights_harness_touched FROM PUBLIC;
> -REVOKE ALL ON TABLE flights_harness_touched FROM osstest;
> -GRANT ALL ON TABLE flights_harness_touched TO osstest;
> -GRANT SELECT ON TABLE flights_harness_touched TO osstest_ro;
> +--REVOKE ALL ON TABLE flights_harness_touched FROM PUBLIC;
> +--REVOKE ALL ON TABLE flights_harness_touched FROM osstest;
> +--GRANT ALL ON TABLE flights_harness_touched TO osstest;
> +--GRANT SELECT ON TABLE flights_harness_touched TO osstest_ro;
>  
>  
>  --
>  -- Name: hostflags; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE hostflags FROM PUBLIC;
> -REVOKE ALL ON TABLE hostflags FROM osstest;
> -GRANT ALL ON TABLE hostflags TO osstest;
> -GRANT SELECT ON TABLE hostflags TO osstest_ro;
> +--REVOKE ALL ON TABLE hostflags FROM PUBLIC;
> +--REVOKE ALL ON TABLE hostflags FROM osstest;
> +--GRANT ALL ON TABLE hostflags TO osstest;
> +--GRANT SELECT ON TABLE hostflags TO osstest_ro;
>  
>  
>  --
>  -- Name: jobs; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE jobs FROM PUBLIC;
> -REVOKE ALL ON TABLE jobs FROM osstest;
> -GRANT ALL ON TABLE jobs TO osstest;
> -GRANT SELECT ON TABLE jobs TO osstest_ro;
> +--REVOKE ALL ON TABLE jobs FROM PUBLIC;
> +--REVOKE ALL ON TABLE jobs FROM osstest;
> +--GRANT ALL ON TABLE jobs TO osstest;
> +--GRANT SELECT ON TABLE jobs TO osstest_ro;
>  
>  
>  --
>  -- Name: resource_properties; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE resource_properties FROM PUBLIC;
> -REVOKE ALL ON TABLE resource_properties FROM osstest;
> -GRANT ALL ON TABLE resource_properties TO osstest;
> -GRANT SELECT ON TABLE resource_properties TO osstest_ro;
> +--REVOKE ALL ON TABLE resource_properties FROM PUBLIC;
> +--REVOKE ALL ON TABLE resource_properties FROM osstest;
> +--GRANT ALL ON TABLE resource_properties TO osstest;
> +--GRANT SELECT ON TABLE resource_properties TO osstest_ro;
>  
>  
>  --
>  -- Name: resource_sharing; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE resource_sharing FROM PUBLIC;
> -REVOKE ALL ON TABLE resource_sharing FROM osstest;
> -GRANT ALL ON TABLE resource_sharing TO osstest;
> -GRANT SELECT ON TABLE resource_sharing TO osstest_ro;
> +--REVOKE ALL ON TABLE resource_sharing FROM PUBLIC;
> +--REVOKE ALL ON TABLE resource_sharing FROM osstest;
> +--GRANT ALL ON TABLE resource_sharing TO osstest;
> +--GRANT SELECT ON TABLE resource_sharing TO osstest_ro;
>  
>  
>  --
>  -- Name: resources; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE resources FROM PUBLIC;
> -REVOKE ALL ON TABLE resources FROM osstest;
> -GRANT ALL ON TABLE resources TO osstest;
> -GRANT SELECT ON TABLE resources TO osstest_ro;
> +--REVOKE ALL ON TABLE resources FROM PUBLIC;
> +--REVOKE ALL ON TABLE resources FROM osstest;
> +--GRANT ALL ON TABLE resources TO osstest;
> +--GRANT SELECT ON TABLE resources TO osstest_ro;
>  
>  
>  --
>  -- Name: runvars; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE runvars FROM PUBLIC;
> -REVOKE ALL ON TABLE runvars FROM osstest;
> -GRANT ALL ON TABLE runvars TO osstest;
> -GRANT SELECT ON TABLE runvars TO osstest_ro;
> +--REVOKE ALL ON TABLE runvars FROM PUBLIC;
> +--REVOKE ALL ON TABLE runvars FROM osstest;
> +--GRANT ALL ON TABLE runvars TO osstest;
> +--GRANT SELECT ON TABLE runvars TO osstest_ro;
>  
>  
>  --
>  -- Name: steps; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE steps FROM PUBLIC;
> -REVOKE ALL ON TABLE steps FROM osstest;
> -GRANT ALL ON TABLE steps TO osstest;
> -GRANT SELECT ON TABLE steps TO osstest_ro;
> +--REVOKE ALL ON TABLE steps FROM PUBLIC;
> +--REVOKE ALL ON TABLE steps FROM osstest;
> +--GRANT ALL ON TABLE steps TO osstest;
> +--GRANT SELECT ON TABLE steps TO osstest_ro;
>  
>  
>  --
>  -- Name: tasks_taskid_seq; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM PUBLIC;
> -REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM osstest;
> -GRANT ALL ON SEQUENCE tasks_taskid_seq TO osstest;
> -GRANT SELECT ON SEQUENCE tasks_taskid_seq TO osstest_ro;
> +--REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM PUBLIC;
> +--REVOKE ALL ON SEQUENCE tasks_taskid_seq FROM osstest;
> +--GRANT ALL ON SEQUENCE tasks_taskid_seq TO osstest;
> +--GRANT SELECT ON SEQUENCE tasks_taskid_seq TO osstest_ro;
>  
>  
>  --
>  -- Name: tasks; Type: ACL; Schema: public; Owner: osstest
>  --
>  
> -REVOKE ALL ON TABLE tasks FROM PUBLIC;
> -REVOKE ALL ON TABLE tasks FROM osstest;
> -GRANT ALL ON TABLE tasks TO osstest;
> -GRANT SELECT ON TABLE tasks TO osstest_ro;
> +--REVOKE ALL ON TABLE tasks FROM PUBLIC;
> +--REVOKE ALL ON TABLE tasks FROM osstest;
> +--GRANT ALL ON TABLE tasks TO osstest;
> +--GRANT SELECT ON TABLE tasks TO osstest_ro;
>  
>  
>  --

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create
  2015-12-10 13:51 ` [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create Ian Jackson
@ 2015-12-10 15:08   ` Ian Campbell
  2015-12-10 15:19     ` Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:08 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> There is a fair amount of option parsing clobber here that will be
> relevant shortly.

How dangerous is this script if you just run it e.g. without with_test?

Specifically, would it nuke an existing database?

> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> ---
>  mg-schema-create        |   61
> +++++++++++++++++++++++++++++++++++++++++++++++
>  mg-schema-test-database |    2 +-
>  2 files changed, 62 insertions(+), 1 deletion(-)
>  create mode 100755 mg-schema-create
> 
> diff --git a/mg-schema-create b/mg-schema-create
> new file mode 100755
> index 0000000..54f1c76
> --- /dev/null
> +++ b/mg-schema-create
> @@ -0,0 +1,61 @@
> +#!/bin/bash
> +
> +# This is part of "osstest", an automated testing framework for Xen.
> +# Copyright (C) 2009-2015 Citrix Inc.
> +# 
> +# This program is free software: you can redistribute it and/or modify
> +# it under the terms of the GNU Affero General Public License as
> published by
> +# the Free Software Foundation, either version 3 of the License, or
> +# (at your option) any later version.
> +# 
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU Affero General Public License for more details.
> +# 
> +# You should have received a copy of the GNU Affero General Public
> License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +
> +# Usages:
> +#
> +#  ./mg-schema-create [<options>]
> +#
> +#  Database must already exist.  (Ie, mg-schema-create does not
> +#  do CREATE DATABASE.)
> +#
> +#  When setting up a production database, mg-schema-create should
> +#  be run *AS THE ROLE USER* who is to own all the resources.
> +#
> +# Options:
> +#
> +#  -q                            don't print progress messages
> +
> +set -e
> +set -o posix
> +set -o pipefail
> +
> +progress () { printf "%s\n" "$*"; }
> +progress=progress
> +quietopt=''
> +
> +while [ $# != 0 ]; do
> +    arg=$1; shift
> +    case "$arg" in
> +    -q)
> +        progress=:
> +        quietopt=-q
> +        ;;
> +    *)
> +        echo >&2 "bad usage ($arg)"; exit 127
> +        ;;
> +    esac
> +done
> +
> +. ./cri-getconfig
> +
> +$progress "Populating database..."
> +
> +$(get_psql_cmd) $quietopt -f schema/initial.sql
> +
> +$progress "Database set up."
> diff --git a/mg-schema-test-database b/mg-schema-test-database
> index c68b1d2..3616c4d 100755
> --- a/mg-schema-test-database
> +++ b/mg-schema-test-database
> @@ -391,7 +391,7 @@ END
>  	psql_do <<END
>  		CREATE DATABASE $dbname;
>  END
> -	$(withtest get_psql_cmd) -q -f schema/initial.sql
> +	withtest ./mg-schema-create -q
>  
>  	printf ".\n"
>  

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql
  2015-12-10 15:07   ` Ian Campbell
@ 2015-12-10 15:16     ` Ian Jackson
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 15:16 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("Re: [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql"):
> I suppose the resulting comments in initial.sql are useful even for
> illustration (i.e. not much point in removing).

Yes.  Also they reflect actual stuff in at least the Cambridge
instance.

> I noticed that owner is not always osstest, I see instances of iwj,
> postgres and osstest_ro. I guess iwj is some historical baggage which the
> expectation to run as the role user supercedes, but I'm not sure about
> osstest_ro or postgres?

osstest_ro is a user that can only read things which seemed like a
good idea at the time but in practice isn't ever used for anything.

iwj is historical baggage.

If we decide we want the schema to have an access control scheme that
is different to what we have now, other than the default, I think we
would now do this by writing an idempotent schema update script to set
things right.

I forget what the `public' and `postgresql' stuff is for.  The GRANTs
and REVOKEs.  I'm removing are evidently not important because the
database creation works fine without :-).

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create
  2015-12-10 15:08   ` Ian Campbell
@ 2015-12-10 15:19     ` Ian Jackson
  2015-12-10 15:34       ` [OSSTEST PATCH 8/7] Schema: When creating, check that no updates are applied Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 15:19 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("Re: [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create"):
> On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> > There is a fair amount of option parsing clobber here that will be
> > relevant shortly.
> 
> How dangerous is this script if you just run it e.g. without with_test?

Not particularly:

(osstest)mariner:testing.git> OSSTEST_CONFIG=/u/iwj/.xen-osstest/config:local-config.test-database_iwj ./mg-schema-create
Populating database...
SET
SET
SET
SET
SET
SET
SET
SET
SET
psql:schema/initial.sql:28: ERROR:  relation "flights" already exists
(osstest)mariner:testing.git> 

> Specifically, would it nuke an existing database?

So, no.  If we ever delete the `flights' table in a schema update,
this script might recreate it before falling over later.  So if we do
that we ought to add a safety catch to mg-schema-create, for example
to have it check that no schema updates have yet been applied.

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [OSSTEST PATCH 8/7] Schema: When creating, check that no updates are applied
  2015-12-10 15:19     ` Ian Jackson
@ 2015-12-10 15:34       ` Ian Jackson
  2015-12-10 15:46         ` Ian Campbell
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 15:34 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Ian Campbell

If you try to run mg-schema-create on an existing instance it bombs
out right at the beginning because it tries to create the `flights'
table, which already exists.

But in the future the `flights' table might be removed in an update,
which would remove this safety catch.  Then running the create might
partially succeed, leaving debris a production instance.

Detect this situation by looking for applied schema updates, and
bombing out if there are any.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
 mg-schema-create |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mg-schema-create b/mg-schema-create
index df5e215..e15ecb9 100755
--- a/mg-schema-create
+++ b/mg-schema-create
@@ -70,6 +70,13 @@ export OSSTEST_DB_USEREAL_IGNORETEST='.*'
 
 . ./cri-getconfig
 
+updates_applied=$(./mg-schema-update list-applied)
+if [ "x$updates_applied" != x ]; then
+    ./mg-schema-update show
+    echo >&2 'Database already exists with applied updates!'
+    exit 127
+fi
+
 ./mg-schema-update $quietopt check-user
 
 $progress "Populating database..."
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 5/7] Schema: Support database schema updates
  2015-12-10 13:51 ` [OSSTEST PATCH 5/7] Schema: Support database schema updates Ian Jackson
@ 2015-12-10 15:43   ` Ian Campbell
  2015-12-10 16:46     ` Ian Jackson
  0 siblings, 1 reply; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:43 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> See schema/README.schema, introduced in this patch, for the design.

That is all I have done here. Some of the questions I had might be answered
in the code.

[...]
> diff --git a/schema/README.updates b/schema/README.updates
> new file mode 100644
> index 0000000..0aa349d
> --- /dev/null
> +++ b/schema/README.updates
> @@ -0,0 +1,122 @@
> +SCHEMA DEFINITION AND SCHEMA UPDATES (PRODUCTION `EXECUTIVE' MODE)
> +==================================================================
> +
> +To generate a new DB, we apply the original schema (in initial.sql)
> +and then apply all the updates, in order.
> +
> +We maintain a table in the DB which records which updates are applied.
> +
> +
> +Schema update snippet format
> +----------------------------
> +
> +Schema update snippets should be called
> +   schema/<updatename>.sql
> +
> +They should contain DDL commands (ALTER TABLE etc.) to make whatever
> +changes are needed.
> +
> +They MUST NOT contain BEGIN or COMMIT.
> +
> +They must contain a special comment near the top:
> +
> +  -- ##OSSTEST## <sequence> <status>
> +
> +<updatename> is a string (/^[a-z][0-9a-z-]+$/) which uniquely identifies
> +the update.  It must not be changed because existing installations
> +rely on updates having stable names.
> +
> +<sequence> is a positive integer, which should be unique.  Updates are
> +applied in order.
> +
> +<status> reflects the compatibility of various schema versions, see
> +below.
> +
> +
> +Update orders
> +-------------
> +
> +There are three reasonable plans for schema changes:

You've listed 4, which one is unreasonable then ;-)

> +
> + * Fully intercompatible: both old code and new code are each
> +   compatible with both old schema and new schema.  The code and
> +   schema updates may be done in any order.
> +
> +   Such a schema change is always in state:
> +      Harmless
> +
> + * Explicit conditional: first update the code to understand both
> +   versions of the schema; then update the schema; then drop the
> +   compatibility code.
> +
> +   Such a schema change is in state:
> +      Unfinished (or absent)   in old code
> +      Ready                    in intermediate code
> +      Needed                   in the final code
> +
> + * Code first: the new code works with either old or new schema,
> +   but the old code cannot cope with the new schema.
> +
> +   Such a schema change is in state:
> +      Unfinished (or absent)   in old code
> +      Ready'                   in new code

Is this really "Ready-prime" or is that a typo?

The "Unfinished", "Ready", "Needed" etc are the literal strings to be used
as <status> in the special comment, correct?

Who or what is responsible for cranking the handle on the state machine? I
_think_ it is going to be the commits which make the corresponding changes
to the code or introduce the new schema snippet?

Am I right that it would be unusual to have a literal "Unfinished" in a
schema/*.sql, but rather they would be implicitly in that state by being
absent?

> +
> + * Schema first: the new schema works with any code; but the old
> +   schema does not work with new code.
> +
> +   Such a schema change is in state
> +      Preparatory              in old code
> +      Needed                   in the new code
> +
> +
> +Update order for Populate-then-rely
> +-----------------------------------
> +
> +This is for when we want to record new information and then later rely
> +on it.  There are typically two schema changes: to add the column(s)
> +(`add') and then to add appropriate constraints (`constraint') to
> +prevent it being left blank.

Which of the 4 plans above does this correspond to?

I have a feeling that this actually encapsulates two schema changes in
lockstep, which may have independent determination of the plan, from the
state names it looks like the `add' schema is a "Schema first" change and
the `constraint' one is either "Code first" or "Explicit conditional".

> +1. Commit: new schema update `add', in state Preparatory.

Does "Commit" mean simple push to pretest or does it have to pass through
push gate too? 

> +1. Commit: new schema update `constraint', in state Unfinished.

The two instances of "1." are because these can happen in parallel?

> +
> +2. Apply: `add'.

This is something which automated machinery does I think?

> +
> +2. Commit: code to populate new column; changing `add' to state
> +   Needed and `constraint' to state Ready.
> +
> +3. Optionally commit: idempotent utility script to populate missing
> +   data.  (This can be done with DML in the `constraint' update.)
> +
> +3. Wait for all executions of old code to finish.

this is automated also?

Also the two "3." happen in parallel? I'm not sure because "Commit" and
"Wait" in parallel seems racy.

> +5. Apply: `constraint'.

Does `constraint' go to `Needed' at this point, or does that depend on the
nature of the change (and therefore whether it is "Code first" or "Explicit
conditional"?

> +
> +
> +States and rules for push and db update
> +---------------------------------------
> +
> +  Harmless
> +  Preparatory
> +     No restrictions
> +
> +  Unfinished
> +  (sql fragment entirely missing is equivalent to Unfinished)
> +     Schema update: prevented
> +     Code push: unrestricted
> +
> +  Ready
> +     Schema update: need all live code to be Preparatory/Ready/Needed
> +     Code push: unrestricted
> +
> +  Needed
> +     Schema update: need all live code to be Preparatory/Ready/Needed
> +     Code push: depends on schema update
> +
> +
> +"Push depends on schema update" is not currently implemented.
> +
> +"Checks for live old code" means

These two phrases don't existing in this document.

I think "Push depends on schema update" was called "Schema first" further
up?

>  to look for the state of this schema
> +update in other running versions of osstest.  This is not currently
> +implemented.
> diff --git a/schema/schema-updates.sql b/schema/schema-updates.sql
> new file mode 100644
> index 0000000..cd8dc0c
> --- /dev/null
> +++ b/schema/schema-updates.sql
> @@ -0,0 +1,6 @@
> +-- ##OSSTEST## 001 Harmless
> +
> +CREATE TABLE schema_updates (
> +    updatename TEXT PRIMARY KEY,
> +    applytime integer NOT NULL
> +);

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user
  2015-12-10 13:51 ` [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user Ian Jackson
@ 2015-12-10 15:45   ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:45 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 7/7] Schema: drop old resource_log table
  2015-12-10 13:51 ` [OSSTEST PATCH 7/7] Schema: drop old resource_log table Ian Jackson
@ 2015-12-10 15:46   ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:46 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 8/7] Schema: When creating, check that no updates are applied
  2015-12-10 15:34       ` [OSSTEST PATCH 8/7] Schema: When creating, check that no updates are applied Ian Jackson
@ 2015-12-10 15:46         ` Ian Campbell
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Campbell @ 2015-12-10 15:46 UTC (permalink / raw)
  To: Ian Jackson, xen-devel

On Thu, 2015-12-10 at 15:34 +0000, Ian Jackson wrote:
> If you try to run mg-schema-create on an existing instance it bombs
> out right at the beginning because it tries to create the `flights'
> table, which already exists.
> 
> But in the future the `flights' table might be removed in an update,
> which would remove this safety catch.  Then running the create might
> partially succeed, leaving debris a production instance.
> 
> Detect this situation by looking for applied schema updates, and
> bombing out if there are any.
> 
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [OSSTEST PATCH 5/7] Schema: Support database schema updates
  2015-12-10 15:43   ` Ian Campbell
@ 2015-12-10 16:46     ` Ian Jackson
  0 siblings, 0 replies; 20+ messages in thread
From: Ian Jackson @ 2015-12-10 16:46 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

Ian Campbell writes ("Re: [OSSTEST PATCH 5/7] Schema: Support database schema updates"):
> On Thu, 2015-12-10 at 13:51 +0000, Ian Jackson wrote:
> > See schema/README.schema, introduced in this patch, for the design.
> 
> That is all I have done here. Some of the questions I had might be answered
> in the code.

Fair enough.  I have actually executed this, although obviously not on
any production instances.

> > +Update orders
> > +-------------
> > +
> > +There are three reasonable plans for schema changes:
> 
> You've listed 4, which one is unreasonable then ;-)

Oops, fixed.

> > +      Unfinished (or absent)   in old code
> > +      Ready'                   in new code
> 
> Is this really "Ready-prime" or is that a typo?

Typo.

> The "Unfinished", "Ready", "Needed" etc are the literal strings to be used
> as <status> in the special comment, correct?

Yes.

I see that I used the terms `state' and `status' interchangeably.  I
have changed `state' to `status' (and adjusted grammar) in the docs.

The code still talks about `state' in a slightly confusing way.  I can
fix that if you like.

> Who or what is responsible for cranking the handle on the state machine? I
> _think_ it is going to be the commits which make the corresponding changes
> to the code or introduce the new schema snippet?

Exactly.

> Am I right that it would be unusual to have a literal "Unfinished" in a
> schema/*.sql, but rather they would be implicitly in that state by being
> absent?

You might want to introduce a schema change early in a series as
documentation or for testing purposes.  `Unfinished' updates can be
applied with -fff so you can test parts of the code before the rest
is ready.

> > +Update order for Populate-then-rely
> > +-----------------------------------
> > +
> > +This is for when we want to record new information and then later rely
> > +on it.  There are typically two schema changes: to add the column(s)
> > +(`add') and then to add appropriate constraints (`constraint') to
> > +prevent it being left blank.
> 
> Which of the 4 plans above does this correspond to?
> 
> I have a feeling that this actually encapsulates two schema changes in
> lockstep, which may have independent determination of the plan, from the
> state names it looks like the `add' schema is a "Schema first" change and
> the `constraint' one is either "Code first" or "Explicit conditional".

Yes.  I will explain this.

> > +1. Commit: new schema update `add', in state Preparatory.
> 
> Does "Commit" mean simple push to pretest or does it have to pass through
> push gate too? 

I will clarify this.

> The two instances of "1." are because these can happen in parallel?

No.  I failed to update the numbering.

> > +2. Apply: `add'.
> 
> This is something which automated machinery does I think?

No.  I will clarify this.

> > +3. Wait for all executions of old code to finish.
> 
> this is automated also?

Not really.


> > +5. Apply: `constraint'.
> 
> Does `constraint' go to `Needed' at this point, or does that depend on the
> nature of the change (and therefore whether it is "Code first" or "Explicit
> conditional"?

No, it doesn't.  I will clarify this.


> > +"Push depends on schema update" is not currently implemented.
> > +
> > +"Checks for live old code" means
> 
> These two phrases don't existing in this document.
> 
> I think "Push depends on schema update" was called "Schema first" further
> up?

This is confusing.  I will rewrite it.

Ian.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2015-12-10 16:46 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-10 13:51 [OSSTEST PATCH 0/7] Support database schema updates Ian Jackson
2015-12-10 13:51 ` [OSSTEST PATCH 1/7] mg-schema-test-database: Fix argument parsing for _SUFFIX Ian Jackson
2015-12-10 15:04   ` Ian Campbell
2015-12-10 13:51 ` [OSSTEST PATCH 2/7] Schema: Rename schema file Ian Jackson
2015-12-10 15:04   ` Ian Campbell
2015-12-10 13:51 ` [OSSTEST PATCH 3/7] Schema: Remove SET OWNER and GRANT/REVOKE from schema/initial.sql Ian Jackson
2015-12-10 15:07   ` Ian Campbell
2015-12-10 15:16     ` Ian Jackson
2015-12-10 13:51 ` [OSSTEST PATCH 4/7] Schema: Introduce mg-schema-create Ian Jackson
2015-12-10 15:08   ` Ian Campbell
2015-12-10 15:19     ` Ian Jackson
2015-12-10 15:34       ` [OSSTEST PATCH 8/7] Schema: When creating, check that no updates are applied Ian Jackson
2015-12-10 15:46         ` Ian Campbell
2015-12-10 13:51 ` [OSSTEST PATCH 5/7] Schema: Support database schema updates Ian Jackson
2015-12-10 15:43   ` Ian Campbell
2015-12-10 16:46     ` Ian Jackson
2015-12-10 13:51 ` [OSSTEST PATCH 6/7] Schema: Check that schema creation and update runs as the right user Ian Jackson
2015-12-10 15:45   ` Ian Campbell
2015-12-10 13:51 ` [OSSTEST PATCH 7/7] Schema: drop old resource_log table Ian Jackson
2015-12-10 15:46   ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.