Merge lp:~jjo/charms/precise/postgresql/support-no-explicit-version_and_trusty-pg93 into lp:charms/postgresql
- Precise Pangolin (12.04)
- support-no-explicit-version_and_trusty-pg93
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 80 | ||||
Proposed branch: | lp:~jjo/charms/precise/postgresql/support-no-explicit-version_and_trusty-pg93 | ||||
Merge into: | lp:charms/postgresql | ||||
Diff against target: |
247 lines (+73/-22) 3 files modified
config.yaml (+1/-1) hooks/hooks.py (+68/-21) templates/postgresql.conf.tmpl (+4/-0) |
||||
To merge this branch: | bzr merge lp:~jjo/charms/precise/postgresql/support-no-explicit-version_and_trusty-pg93 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Stuart Bishop (community) | Approve | ||
James Troup (community) | Approve | ||
Review via email: mp+202076@code.launchpad.net |
Commit message
- support deploying with version unset in config.yaml (new default)
- add pg_version(): if no explicit version set, find it from apt.Cache() install-able candidate, save it in local_state later usage.
- add support for postgresql>=9.3 (trusty): deprecated unix_socket_
- move ssl cert symlinking into create_ssl_cert()
Description of the change
- 81. By JuanJo Ciarlante
-
add some logging
James Troup (elmo) wrote : | # |
- 82. By JuanJo Ciarlante
-
s/canditate/
candidate/ , clarify apt.Cache being the expensive op
JuanJo Ciarlante (jjo) wrote : | # |
> Trivial: 'canditate' should be 'candidate' throughout.
thanks for spotting, fixed.
>
> pg_version() makes several assumptions (e.g. that a postgresql package
> exists and that it's version matches a specific regex (which won't be
> the case if, e.g. the package grows an epoch) that are probably fine,
> but it caught my eye.
I found this the less painful way to find out which "short-version"
($major.$minor) is available.
>
> | + # expensive operation, save it
>
> What operation does this refer to? The function as a whole?
apt.Cache(), I clarified the comment there.
Stuart Bishop (stub) wrote : | # |
I don't think allowing an unspecified PostgreSQL version is a good idea. It means charm deployment is not repeatable, as which version is deployed depends on the state of the archive. I don't want 9.4 (or worse, 9.4 beta) being installed when my apps are tested with 9.3 just because a newer version is available in the archive. This would also break replication, as a unit does not know what version the other units in the service are using until after the install hook is run and the unit attempt's to join the service's peer relation. Archives will not always be under user control, in particular apt.postgresql.org for official Debian and Ubuntu backports.
If we really need this so the default version under trusty is 9.3, and the default version under precise is 9.1, I think we just want to check 'lsb_release -sc' and hard code the default version to the codename. This may still cause surprises, but would be repeatable.
If we keep the 'apt' import, do we need to add python-apt to the default list of packages to install?
JuanJo Ciarlante (jjo) wrote : | # |
> I don't think allowing an unspecified PostgreSQL version is a good idea. It
> means charm deployment is not repeatable, as which version is deployed depends
> on the state of the archive. I don't want 9.4 (or worse, 9.4 beta) being
> installed when my apps are tested with 9.3 just because a newer version is
> available in the archive. This would also break replication, as a unit does
> not know what version the other units in the service are using until after the
> install hook is run and the unit attempt's to join the service's peer
> relation. Archives will not always be under user control, in particular
> apt.postgresql.org for official Debian and Ubuntu backports.
>
> If we really need this so the default version under trusty is 9.3, and the
> default version under precise is 9.1, I think we just want to check
> 'lsb_release -sc' and hard code the default version to the codename. This may
> still cause surprises, but would be repeatable.
I'm fine with hardcoding it, as far as we have a single charm that can be
deployed to either precise,trusty without requiring the user to overload
the config with a version that's already well-known/mapped for this distro
release.
>
> If we keep the 'apt' import, do we need to add python-apt to the default list
> of packages to install?
Good point, fwiw afaicT both precise,trusty it was already installed there.
- 83. By JuanJo Ciarlante
-
hardcode pg_version map by (LTS) distro release
JuanJo Ciarlante (jjo) wrote : | # |
Thanks stub for the review, PTAL (CID testing case against precise,trusty running atm, will let you know once it finishes, ETA: ~+1.5hr).
Stuart Bishop (stub) wrote : | # |
This no longer worries me and looks good.
- 84. By JuanJo Ciarlante
-
need to: distro_
release. rstrip( ) - 85. By JuanJo Ciarlante
-
move rstrip above, let distro_release value "clean" from there
Preview Diff
1 | === modified file 'config.yaml' | |||
2 | --- config.yaml 2014-01-07 19:00:20 +0000 | |||
3 | +++ config.yaml 2014-01-21 12:55:42 +0000 | |||
4 | @@ -43,7 +43,7 @@ | |||
5 | 43 | access details are updated, so disabling either doesn't mean PostgreSQL | 43 | access details are updated, so disabling either doesn't mean PostgreSQL |
6 | 44 | will never be reloaded. | 44 | will never be reloaded. |
7 | 45 | version: | 45 | version: |
9 | 46 | default: "9.1" | 46 | default: "" |
10 | 47 | type: string | 47 | type: string |
11 | 48 | description: Version of PostgreSQL that we want to install | 48 | description: Version of PostgreSQL that we want to install |
12 | 49 | cluster_name: | 49 | cluster_name: |
13 | 50 | 50 | ||
14 | === modified file 'hooks/hooks.py' | |||
15 | --- hooks/hooks.py 2014-01-09 09:08:11 +0000 | |||
16 | +++ hooks/hooks.py 2014-01-21 12:55:42 +0000 | |||
17 | @@ -53,6 +53,39 @@ | |||
18 | 53 | hookenv.log(msg, lvl) | 53 | hookenv.log(msg, lvl) |
19 | 54 | 54 | ||
20 | 55 | 55 | ||
21 | 56 | def pg_version(): | ||
22 | 57 | '''Return pg_version to use. | ||
23 | 58 | |||
24 | 59 | Return "version" config item if set, else use version from "postgresql" | ||
25 | 60 | package candidate, saving it in local_state for later. | ||
26 | 61 | ''' | ||
27 | 62 | config_data = hookenv.config() | ||
28 | 63 | if config_data['version']: | ||
29 | 64 | version = config_data['version'] | ||
30 | 65 | elif 'pg_version' in local_state: | ||
31 | 66 | version = local_state['pg_version'] | ||
32 | 67 | else: | ||
33 | 68 | log("map version from distro release ...") | ||
34 | 69 | distro_release = run("lsb_release -sc") | ||
35 | 70 | distro_release = distro_release.rstrip() | ||
36 | 71 | version_map = {'precise': '9.1', | ||
37 | 72 | 'trusty': '9.3'} | ||
38 | 73 | version = version_map.get(distro_release) | ||
39 | 74 | if not version: | ||
40 | 75 | log("No PG version map for distro_release={}, " | ||
41 | 76 | "you'll need to explicitly set it".format(distro_release), | ||
42 | 77 | CRITICAL) | ||
43 | 78 | sys.exit(1) | ||
44 | 79 | log("version={} from distro_release='{}'".format( | ||
45 | 80 | version, distro_release)) | ||
46 | 81 | # save it for later | ||
47 | 82 | local_state.setdefault('pg_version', version) | ||
48 | 83 | local_state.save() | ||
49 | 84 | |||
50 | 85 | assert version, "pg_version couldn't find a version to use" | ||
51 | 86 | return version | ||
52 | 87 | |||
53 | 88 | |||
54 | 56 | class State(dict): | 89 | class State(dict): |
55 | 57 | """Encapsulate state common to the unit for republishing to relations.""" | 90 | """Encapsulate state common to the unit for republishing to relations.""" |
56 | 58 | def __init__(self, state_file): | 91 | def __init__(self, state_file): |
57 | @@ -230,7 +263,7 @@ | |||
58 | 230 | if status != 0: | 263 | if status != 0: |
59 | 231 | return False | 264 | return False |
60 | 232 | # e.g. output: "Running clusters: 9.1/main" | 265 | # e.g. output: "Running clusters: 9.1/main" |
62 | 233 | vc = "%s/%s" % (hookenv.config("version"), hookenv.config("cluster_name")) | 266 | vc = "%s/%s" % (pg_version(), hookenv.config("cluster_name")) |
63 | 234 | return vc in output.decode('utf8').split() | 267 | return vc in output.decode('utf8').split() |
64 | 235 | 268 | ||
65 | 236 | 269 | ||
66 | @@ -253,8 +286,9 @@ | |||
67 | 253 | # 'service postgresql restart' fails; it only does a reload. | 286 | # 'service postgresql restart' fails; it only does a reload. |
68 | 254 | # success = host.service_restart('postgresql') | 287 | # success = host.service_restart('postgresql') |
69 | 255 | try: | 288 | try: |
72 | 256 | run('pg_ctlcluster -force {version} {cluster_name} ' | 289 | run('pg_ctlcluster -force {} {} ' |
73 | 257 | 'restart'.format(**hookenv.config())) | 290 | 'restart'.format(pg_version(), |
74 | 291 | hookenv.config('cluster_name'))) | ||
75 | 258 | success = True | 292 | success = True |
76 | 259 | except subprocess.CalledProcessError: | 293 | except subprocess.CalledProcessError: |
77 | 260 | success = False | 294 | success = False |
78 | @@ -405,6 +439,8 @@ | |||
79 | 405 | # Return it as pg_config | 439 | # Return it as pg_config |
80 | 406 | charm_dir = hookenv.charm_dir() | 440 | charm_dir = hookenv.charm_dir() |
81 | 407 | template_file = "{}/templates/postgresql.conf.tmpl".format(charm_dir) | 441 | template_file = "{}/templates/postgresql.conf.tmpl".format(charm_dir) |
82 | 442 | if not config_data['version']: | ||
83 | 443 | config_data['version'] = pg_version() | ||
84 | 408 | pg_config = Template( | 444 | pg_config = Template( |
85 | 409 | open(template_file).read()).render(config_data) | 445 | open(template_file).read()).render(config_data) |
86 | 410 | host.write_file( | 446 | host.write_file( |
87 | @@ -580,7 +616,7 @@ | |||
88 | 580 | 616 | ||
89 | 581 | 617 | ||
90 | 582 | def create_recovery_conf(master_host, restart_on_change=False): | 618 | def create_recovery_conf(master_host, restart_on_change=False): |
92 | 583 | version = hookenv.config('version') | 619 | version = pg_version() |
93 | 584 | cluster_name = hookenv.config('cluster_name') | 620 | cluster_name = hookenv.config('cluster_name') |
94 | 585 | postgresql_cluster_dir = os.path.join( | 621 | postgresql_cluster_dir = os.path.join( |
95 | 586 | postgresql_data_dir, version, cluster_name) | 622 | postgresql_data_dir, version, cluster_name) |
96 | @@ -632,6 +668,18 @@ | |||
97 | 632 | hookenv.open_port(new_service_port) | 668 | hookenv.open_port(new_service_port) |
98 | 633 | 669 | ||
99 | 634 | 670 | ||
100 | 671 | def create_ssl_cert(cluster_dir): | ||
101 | 672 | # Debian by default expects SSL certificates in the datadir. | ||
102 | 673 | server_crt = os.path.join(cluster_dir, 'server.crt') | ||
103 | 674 | server_key = os.path.join(cluster_dir, 'server.key') | ||
104 | 675 | if not os.path.exists(server_crt): | ||
105 | 676 | os.symlink('/etc/ssl/certs/ssl-cert-snakeoil.pem', | ||
106 | 677 | server_crt) | ||
107 | 678 | if not os.path.exists(server_key): | ||
108 | 679 | os.symlink('/etc/ssl/private/ssl-cert-snakeoil.key', | ||
109 | 680 | server_key) | ||
110 | 681 | |||
111 | 682 | |||
112 | 635 | def set_password(user, password): | 683 | def set_password(user, password): |
113 | 636 | if not os.path.isdir("passwords"): | 684 | if not os.path.isdir("passwords"): |
114 | 637 | os.makedirs("passwords") | 685 | os.makedirs("passwords") |
115 | @@ -711,7 +759,7 @@ | |||
116 | 711 | # - manipulate /var/lib/postgresql/VERSION/CLUSTER symlink | 759 | # - manipulate /var/lib/postgresql/VERSION/CLUSTER symlink |
117 | 712 | #------------------------------------------------------------------------------ | 760 | #------------------------------------------------------------------------------ |
118 | 713 | def config_changed_volume_apply(): | 761 | def config_changed_volume_apply(): |
120 | 714 | version = hookenv.config('version') | 762 | version = pg_version() |
121 | 715 | cluster_name = hookenv.config('cluster_name') | 763 | cluster_name = hookenv.config('cluster_name') |
122 | 716 | data_directory_path = os.path.join( | 764 | data_directory_path = os.path.join( |
123 | 717 | postgresql_data_dir, version, cluster_name) | 765 | postgresql_data_dir, version, cluster_name) |
124 | @@ -820,7 +868,7 @@ | |||
125 | 820 | @hooks.hook() | 868 | @hooks.hook() |
126 | 821 | def config_changed(force_restart=False): | 869 | def config_changed(force_restart=False): |
127 | 822 | config_data = hookenv.config() | 870 | config_data = hookenv.config() |
129 | 823 | update_repos_and_packages(config_data["version"]) | 871 | update_repos_and_packages() |
130 | 824 | 872 | ||
131 | 825 | # Trigger volume initialization logic for permanent storage | 873 | # Trigger volume initialization logic for permanent storage |
132 | 826 | volid = volume_get_volume_id() | 874 | volid = volume_get_volume_id() |
133 | @@ -862,6 +910,8 @@ | |||
134 | 862 | create_postgresql_config(postgresql_config) | 910 | create_postgresql_config(postgresql_config) |
135 | 863 | generate_postgresql_hba(postgresql_hba) | 911 | generate_postgresql_hba(postgresql_hba) |
136 | 864 | create_postgresql_ident(postgresql_ident) | 912 | create_postgresql_ident(postgresql_ident) |
137 | 913 | create_ssl_cert(os.path.join( | ||
138 | 914 | postgresql_data_dir, pg_version(), config_data['cluster_name'])) | ||
139 | 865 | 915 | ||
140 | 866 | updated_service_port = config_data["listen_port"] | 916 | updated_service_port = config_data["listen_port"] |
141 | 867 | update_service_port(current_service_port, updated_service_port) | 917 | update_service_port(current_service_port, updated_service_port) |
142 | @@ -879,7 +929,7 @@ | |||
143 | 879 | subprocess.check_call(['sh', '-c', f]) | 929 | subprocess.check_call(['sh', '-c', f]) |
144 | 880 | 930 | ||
145 | 881 | config_data = hookenv.config() | 931 | config_data = hookenv.config() |
147 | 882 | update_repos_and_packages(config_data["version"]) | 932 | update_repos_and_packages() |
148 | 883 | if not 'state' in local_state: | 933 | if not 'state' in local_state: |
149 | 884 | # Fresh installation. Because this function is invoked by both | 934 | # Fresh installation. Because this function is invoked by both |
150 | 885 | # the install hook and the upgrade-charm hook, we need to guard | 935 | # the install hook and the upgrade-charm hook, we need to guard |
151 | @@ -890,9 +940,10 @@ | |||
152 | 890 | 940 | ||
153 | 891 | # Drop the cluster created when the postgresql package was | 941 | # Drop the cluster created when the postgresql package was |
154 | 892 | # installed, and rebuild it with the requested locale and encoding. | 942 | # installed, and rebuild it with the requested locale and encoding. |
158 | 893 | run("pg_dropcluster --stop 9.1 main") | 943 | version = pg_version() |
159 | 894 | run("pg_createcluster --locale='{}' --encoding='{}' 9.1 main".format( | 944 | run("pg_dropcluster --stop {} main".format(version)) |
160 | 895 | config_data['locale'], config_data['encoding'])) | 945 | run("pg_createcluster --locale='{}' --encoding='{}' {} main".format( |
161 | 946 | config_data['locale'], config_data['encoding'], version)) | ||
162 | 896 | 947 | ||
163 | 897 | postgresql_backups_dir = ( | 948 | postgresql_backups_dir = ( |
164 | 898 | config_data['backup_dir'].strip() or | 949 | config_data['backup_dir'].strip() or |
165 | @@ -1322,7 +1373,7 @@ | |||
166 | 1322 | snapshot_relations() | 1373 | snapshot_relations() |
167 | 1323 | 1374 | ||
168 | 1324 | 1375 | ||
170 | 1325 | def update_repos_and_packages(version): | 1376 | def update_repos_and_packages(): |
171 | 1326 | extra_repos = hookenv.config('extra_archives') | 1377 | extra_repos = hookenv.config('extra_archives') |
172 | 1327 | extra_repos_added = local_state.setdefault('extra_repos_added', set()) | 1378 | extra_repos_added = local_state.setdefault('extra_repos_added', set()) |
173 | 1328 | if extra_repos: | 1379 | if extra_repos: |
174 | @@ -1336,6 +1387,7 @@ | |||
175 | 1336 | fetch.apt_update(fatal=True) | 1387 | fetch.apt_update(fatal=True) |
176 | 1337 | local_state.save() | 1388 | local_state.save() |
177 | 1338 | 1389 | ||
178 | 1390 | version = pg_version() | ||
179 | 1339 | # It might have been better for debversion and plpython to only get | 1391 | # It might have been better for debversion and plpython to only get |
180 | 1340 | # installed if they were listed in the extra-packages config item, | 1392 | # installed if they were listed in the extra-packages config item, |
181 | 1341 | # but they predate this feature. | 1393 | # but they predate this feature. |
182 | @@ -1409,7 +1461,7 @@ | |||
183 | 1409 | def promote_database(): | 1461 | def promote_database(): |
184 | 1410 | '''Take the database out of recovery mode.''' | 1462 | '''Take the database out of recovery mode.''' |
185 | 1411 | config_data = hookenv.config() | 1463 | config_data = hookenv.config() |
187 | 1412 | version = config_data['version'] | 1464 | version = pg_version() |
188 | 1413 | cluster_name = config_data['cluster_name'] | 1465 | cluster_name = config_data['cluster_name'] |
189 | 1414 | postgresql_cluster_dir = os.path.join( | 1466 | postgresql_cluster_dir = os.path.join( |
190 | 1415 | postgresql_data_dir, version, cluster_name) | 1467 | postgresql_data_dir, version, cluster_name) |
191 | @@ -1814,7 +1866,7 @@ | |||
192 | 1814 | log("Cloning master {}".format(master_unit)) | 1866 | log("Cloning master {}".format(master_unit)) |
193 | 1815 | 1867 | ||
194 | 1816 | config_data = hookenv.config() | 1868 | config_data = hookenv.config() |
196 | 1817 | version = config_data['version'] | 1869 | version = pg_version() |
197 | 1818 | cluster_name = config_data['cluster_name'] | 1870 | cluster_name = config_data['cluster_name'] |
198 | 1819 | postgresql_cluster_dir = os.path.join( | 1871 | postgresql_cluster_dir = os.path.join( |
199 | 1820 | postgresql_data_dir, version, cluster_name) | 1872 | postgresql_data_dir, version, cluster_name) |
200 | @@ -1837,12 +1889,7 @@ | |||
201 | 1837 | output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) | 1889 | output = subprocess.check_output(cmd, stderr=subprocess.STDOUT) |
202 | 1838 | log(output, DEBUG) | 1890 | log(output, DEBUG) |
203 | 1839 | # Debian by default expects SSL certificates in the datadir. | 1891 | # Debian by default expects SSL certificates in the datadir. |
210 | 1840 | os.symlink( | 1892 | create_ssl_cert(postgresql_cluster_dir) |
205 | 1841 | '/etc/ssl/certs/ssl-cert-snakeoil.pem', | ||
206 | 1842 | os.path.join(postgresql_cluster_dir, 'server.crt')) | ||
207 | 1843 | os.symlink( | ||
208 | 1844 | '/etc/ssl/private/ssl-cert-snakeoil.key', | ||
209 | 1845 | os.path.join(postgresql_cluster_dir, 'server.key')) | ||
211 | 1846 | create_recovery_conf(master_host) | 1893 | create_recovery_conf(master_host) |
212 | 1847 | except subprocess.CalledProcessError as x: | 1894 | except subprocess.CalledProcessError as x: |
213 | 1848 | # We failed, and this cluster is broken. Rebuild a | 1895 | # We failed, and this cluster is broken. Rebuild a |
214 | @@ -1872,7 +1919,7 @@ | |||
215 | 1872 | 1919 | ||
216 | 1873 | 1920 | ||
217 | 1874 | def postgresql_is_in_backup_mode(): | 1921 | def postgresql_is_in_backup_mode(): |
219 | 1875 | version = hookenv.config('version') | 1922 | version = pg_version() |
220 | 1876 | cluster_name = hookenv.config('cluster_name') | 1923 | cluster_name = hookenv.config('cluster_name') |
221 | 1877 | postgresql_cluster_dir = os.path.join( | 1924 | postgresql_cluster_dir = os.path.join( |
222 | 1878 | postgresql_data_dir, version, cluster_name) | 1925 | postgresql_data_dir, version, cluster_name) |
223 | @@ -1998,7 +2045,7 @@ | |||
224 | 1998 | """ Return the directory path of the postgresql configuration files. """ | 2045 | """ Return the directory path of the postgresql configuration files. """ |
225 | 1999 | if config_data is None: | 2046 | if config_data is None: |
226 | 2000 | config_data = hookenv.config() | 2047 | config_data = hookenv.config() |
228 | 2001 | version = config_data['version'] | 2048 | version = pg_version() |
229 | 2002 | cluster_name = config_data['cluster_name'] | 2049 | cluster_name = config_data['cluster_name'] |
230 | 2003 | return os.path.join("/etc/postgresql", version, cluster_name) | 2050 | return os.path.join("/etc/postgresql", version, cluster_name) |
231 | 2004 | 2051 | ||
232 | 2005 | 2052 | ||
233 | === modified file 'templates/postgresql.conf.tmpl' | |||
234 | --- templates/postgresql.conf.tmpl 2014-01-02 22:50:28 +0000 | |||
235 | +++ templates/postgresql.conf.tmpl 2014-01-21 12:55:42 +0000 | |||
236 | @@ -17,7 +17,11 @@ | |||
237 | 17 | # CONNECTIONS AND AUTHENTICATION | 17 | # CONNECTIONS AND AUTHENTICATION |
238 | 18 | #------------------------------------------------------------------------------ | 18 | #------------------------------------------------------------------------------ |
239 | 19 | 19 | ||
240 | 20 | {% if version >= "9.3" -%} | ||
241 | 21 | unix_socket_directories = '/var/run/postgresql' | ||
242 | 22 | {% else -%} | ||
243 | 20 | unix_socket_directory = '/var/run/postgresql' | 23 | unix_socket_directory = '/var/run/postgresql' |
244 | 24 | {% endif -%} | ||
245 | 21 | 25 | ||
246 | 22 | {% if listen_ip != "" -%} | 26 | {% if listen_ip != "" -%} |
247 | 23 | listen_addresses = '{{listen_ip}}' | 27 | listen_addresses = '{{listen_ip}}' |
Trivial: 'canditate' should be 'candidate' throughout.
pg_version() makes several assumptions (e.g. that a postgresql package
exists and that it's version matches a specific regex (which won't be
the case if, e.g. the package grows an epoch) that are probably fine,
but it caught my eye.
| + # expensive operation, save it
What operation does this refer to? The function as a whole?