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

Proposed by Mario Splivalo
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 Needs Fixing
Felipe Reyes Needs Fixing
Edward Hope-Morley Pending
Billy Olsen Pending
Jorge Niedbalski 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.
Revision history for this message
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
Revision history for this message
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
Revision history for this message
James Page (james-page) :
review: Needs Fixing
Revision history for this message
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.

Revision history for this message
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

Isolate creation of binlog directory into a separate function

60. By Mario Splivalo

Merged /next

61. By Mario Splivalo

Conflicts resolved (work from another workstation)

Unmerged revisions

61. By Mario Splivalo

Conflicts resolved (work from another workstation)

60. By Mario Splivalo

Merged /next

59. By Mario Splivalo

Isolate creation of binlog directory into a separate function

58. By Mario Splivalo

Merged from /next

57. By Mario Splivalo

Fixed typos, as suggested by freyes

56. By Mario Splivalo

Fix lint errors

55. By Mario Splivalo

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

54. By Mario Splivalo

Fix directory creation

53. By Mario Splivalo

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

52. By Mario Splivalo

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