Merge lp:~mariosplivalo/charms/trusty/percona-cluster/enable-binlogs into lp:~openstack-charmers-archive/charms/trusty/percona-cluster/next

Proposed by Mario Splivalo on 2015-03-09
Status: Superseded
Proposed branch: lp:~mariosplivalo/charms/trusty/percona-cluster/enable-binlogs
Merge into: lp:~openstack-charmers-archive/charms/trusty/percona-cluster/next
Prerequisite: lp:~niedbalski/charms/trusty/percona-cluster/fix-1425528
Diff against target: 88 lines (+38/-1)
3 files modified
config.yaml (+11/-0)
hooks/percona_hooks.py (+23/-1)
templates/my.cnf (+4/-0)
To merge this branch: bzr merge lp:~mariosplivalo/charms/trusty/percona-cluster/enable-binlogs
Reviewer Review Type Date Requested Status
James Page 2015-03-09 Needs Fixing on 2015-03-31
Felipe Reyes (community) 2015-03-09 Needs Fixing on 2015-03-11
Edward Hope-Morley 2015-03-09 Pending
Billy Olsen 2015-03-09 Pending
Jorge Niedbalski 2015-03-09 Pending
Review via email: mp+252248@code.launchpad.net

This proposal has been superseded by a proposal from 2015-10-14.

Description of the Change

This adds two charm configuration options - enable_binlogs and binlogs_path. Binlog_path defaults to /var/log/mysql/mysql.bin (it's because my.cnf which is packaged with mysql-common package suggests that location, and also because mysql charm defaults with binlogs at that location), while enable_binlogs defaults to 'False'.

To post a comment you must log in.
Felipe Reyes (freyes) wrote :

Hi Mario,

This feature makes a lot of sense, and it worked for me, but please add unit tests for your patch, I also have minor comments in the diff.

Thanks,

review: Needs Fixing
James Page (james-page) wrote :

bzr merge lp:~mariosplivalo/charms/trusty/percona-cluster/enable-binlogs
...
Text conflict in hooks/charmhelpers/contrib/database/mysql.py
1 conflicts encountered.

review: Needs Fixing
James Page (james-page) :
review: Needs Fixing
Mario Splivalo (mariosplivalo) wrote :

> bzr merge lp:~mariosplivalo/charms/trusty/percona-cluster/enable-binlogs
> ...
> Text conflict in hooks/charmhelpers/contrib/database/mysql.py
> 1 conflicts encountered.

I belive this is because of the prerequisite branch that needs merging first.

Mario Splivalo (mariosplivalo) wrote :

Based on discussion with Ed, this will be changed in a manner so that a new function is introduced, 'configure_binlogs'. That one will be called from install hook, after mysql has been installed, and will restart mysql for the change to take place.
It will also be called from config_changed hook, but it won't restart mysql - it will log a WARNING inside logs to tell administrator/operator to manually restart mysql when appropriate.

59. By Mario Splivalo on 2015-10-16

Isolate creation of binlog directory into a separate function

60. By Mario Splivalo on 2015-10-18

Merged /next

61. By Mario Splivalo on 2015-12-03

Conflicts resolved (work from another workstation)

Unmerged revisions

61. By Mario Splivalo on 2015-12-03

Conflicts resolved (work from another workstation)

60. By Mario Splivalo on 2015-10-18

Merged /next

59. By Mario Splivalo on 2015-10-16

Isolate creation of binlog directory into a separate function

58. By Mario Splivalo on 2015-10-09

Merged from /next

57. By Mario Splivalo on 2015-03-12

Fixed typos, as suggested by freyes

56. By Mario Splivalo on 2015-03-12

Fix lint errors

55. By Mario Splivalo on 2015-03-06

Remove call to lchownr as it won't change owner of the base directory.
Instead, use os.chown.

54. By Mario Splivalo on 2015-03-06

Fix directory creation

53. By Mario Splivalo on 2015-03-05

Make sure mysql can start even when binlog_path is pointing to
nonexistant path

52. By Mario Splivalo on 2015-03-04

Create binlog_directory, if missing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'config.yaml'
2--- config.yaml 2015-07-27 09:44:19 +0000
3+++ config.yaml 2015-10-09 11:16:10 +0000
4@@ -145,3 +145,14 @@
5 description: |
6 A comma-separated list of nagios servicegroups. If left empty, the
7 nagios_context will be used as the servicegroup.
8+ enable-binlogs:
9+ default: False
10+ type: boolean
11+ description: Turns on MySQL binary logs. The placement of the logs is controlled with the binlogs_path config option.
12+ binlogs-path:
13+ default: /var/log/mysql/mysql-bin.log
14+ type: string
15+ description: |
16+ Location on the filesystem where binlogs are going to be placed.
17+ Default mimics what mysql-common would do for mysql (although not the
18+ best place for binlogs)
19
20=== modified file 'hooks/percona_hooks.py'
21--- hooks/percona_hooks.py 2015-09-23 18:29:21 +0000
22+++ hooks/percona_hooks.py 2015-10-09 11:16:10 +0000
23@@ -4,6 +4,8 @@
24 import json
25 import os
26 import socket
27+import pwd
28+import grp
29 import time
30
31 from charmhelpers.core.hookenv import (
32@@ -31,6 +33,7 @@
33 service_start,
34 file_hash,
35 lsb_release,
36+ mkdir
37 )
38 from charmhelpers.core.templating import render
39 from charmhelpers.fetch import (
40@@ -124,7 +127,9 @@
41 'sst_password': config('sst-password'),
42 'innodb_file_per_table': config('innodb-file-per-table'),
43 'table_open_cache': config('table-open-cache'),
44- 'lp1366997_workaround': config('lp1366997-workaround')
45+ 'lp1366997_workaround': config('lp1366997-workaround'),
46+ 'binlogs_path': config('binlogs-path'),
47+ 'enable_binlogs': config('enable-binlogs')
48 }
49
50 if config('prefer-ipv6'):
51@@ -137,6 +142,23 @@
52 else:
53 context['ipv6'] = False
54
55+ # When render_config is called from the install hook, mysql is not
56+ # yet installed. So we need to create binlogs_path directory world-writable
57+ # We need that directory as otherwise mysql won't start
58+ binlogs_directory = os.path.dirname(context['binlogs_path'])
59+ if not os.path.isdir(binlogs_directory):
60+ mkdir(binlogs_directory, 'root', 'root', 0o777)
61+
62+ # Assuming this is also called from config_changed, we now check if
63+ # mysql uid/gid exists on the system. If so we chown binlogs_path to mysql
64+ try:
65+ uid = pwd.getpwnam('mysql').pw_uid
66+ gid = grp.getgrnam('mysql').gr_gid
67+ os.chown(binlogs_directory, uid, gid)
68+ os.chmod(binlogs_directory, 0o750)
69+ except KeyError:
70+ pass
71+
72 context.update(PerconaClusterHelper().parse_config())
73 render(os.path.basename(MY_CNF), MY_CNF, context, perms=0o444)
74
75
76=== modified file 'templates/my.cnf'
77--- templates/my.cnf 2015-06-04 14:40:54 +0000
78+++ templates/my.cnf 2015-10-09 11:16:10 +0000
79@@ -25,6 +25,10 @@
80 # In order for Galera to work correctly binlog format should be ROW
81 binlog_format=ROW
82
83+{% if enable_binlogs -%}
84+log_bin={{ binlogs_path }}
85+{% endif -%}
86+
87 # MyISAM storage engine has only experimental support
88 default_storage_engine=InnoDB
89

Subscribers

People subscribed via source and target branches