Feature/external deployment replication #109
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <% if p('replicator_enabled') == true %> | ||
| check process pxc-replicator | ||
| with pidfile /var/vcap/sys/run/bpm/pxc-replicator/pxc-replicator.pid | ||
| start program "/var/vcap/jobs/bpm/bin/bpm start pxc-replicator" with timeout <%= p('monit_startup_timeout') %> seconds | ||
| stop program "/var/vcap/jobs/bpm/bin/bpm stop pxc-replicator" | ||
| group vcap | ||
| <% end %> | ||
|
Member
There was a problem hiding this comment. Does this need to be a long-lived process? Could setup be a "one shot" task? Maybe okay to keep it in bpm, but maybe use
Author
There was a problem hiding this comment. I thought it's useful to have it running to be aware when it stops working. a one shot would probably do the job just as well, until the remote changes in a way that replication breaks. At this point the replica instance would not indicate at all that the replication broke. Currently, if the source instance would be restored to another state, the job would notice and restart the replication by getting a fresh dump of the changed upstream. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| --- | ||
| name: pxc-replicator | ||
|
|
||
| templates: | ||
| bpm.yml.erb: config/bpm.yml | ||
| bin/setup: bin/setup | ||
| bin/post-start: bin/post-start | ||
| source.mysql.cnf.erb: config/source.mysql.cnf | ||
| source.admin.mysql.cnf.erb: config/source.admin.mysql.cnf | ||
| config/client-cert.pem.erb: certificates/client-cert.pem | ||
| config/client-key.pem.erb: certificates/client-key.pem | ||
| config/server-ca.pem.erb: certificates/server-ca.pem | ||
|
|
||
| packages: | ||
| - percona-xtradb-cluster-8.0 | ||
| - percona-xtradb-cluster-8.4 | ||
| - percona-xtrabackup-2.4 | ||
| - percona-xtrabackup-8.0 | ||
| - percona-xtrabackup-8.4 | ||
| - pxc-utils | ||
|
|
||
| consumes: | ||
| - name: source | ||
| type: source | ||
| optional: true | ||
| properties: | ||
| - port | ||
| - mysql_version | ||
| - mysql_backup_username | ||
| - mysql_backup_password | ||
| - tls | ||
|
|
||
| properties: | ||
| source_admin_username: | ||
| description: | | ||
| this is here for compatibility with older deployments of the pxc-release. | ||
| technically, the `roadmin` user is viable as a replication user, but it misses `REPLICATION SLAVE` privilege. | ||
| providing the source_admin_username & password will ensure that the roadmin will updated with the missing permission | ||
|
|
||
| source_admin_password: | ||
| description: | | ||
| this is here for compatibility with older deployments of the pxc-release. | ||
| technically, the `roadmin` user is viable as a replication user, but it misses `REPLICATION SLAVE` privilege. | ||
| providing the source_admin_username & password will ensure that the roadmin will updated with the missing permission | ||
| monit_startup_timeout: | ||
|
Member
There was a problem hiding this comment. With bpm this is probably not super useful - the pid gets dropped as soon as the job starts and monit will treat the job as healthy. But this made me wonder if we even really need monit for this use case. See my comments on the monit template. |
||
| description: 'How long to wait for monit to show running for the process, will also be used as a timeout for the post-start health check' | ||
| default: 180 | ||
| mysql_version: | ||
| description: 'deployed version' | ||
| default: 8.0 | ||
| logging.format.timestamp: | ||
|
Member
There was a problem hiding this comment. Doesn't look like this is used anywhere. |
||
| default: rfc3339 | ||
| replicator_enabled: | ||
| description: 'Whether to enable the job ( skips writing monit file if set to false )' | ||
| default: false | ||
| mysql_backup_username: | ||
| description: 'user to use for connection to source instance' | ||
| mysql_backup_password: | ||
| description: 'password to use for connection to source instance' | ||
| host: | ||
| description: 'hostname of the source database instance' | ||
| port: | ||
| description: 'Whether to enable the job ( skips writing monit file if set to false )' | ||
|
|
||
| default: 3306 | ||
|
Comment on lines
+60
to
+64
Member
There was a problem hiding this comment. Not to make this complicated, but may make sense to add source prefix here. either nested with |
||
| tls: | ||
| description: 'TLS certificates to use for connections' | ||
| example: { | ||
| "ca": "...", | ||
| "certificate": "...", | ||
| "private_key": "..." | ||
| } | ||
| default: {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/usr/bin/env bash | ||
| <% if p('replicator_enabled') then %> | ||
| . /var/vcap/packages/pxc-utils/logging.sh | ||
|
|
||
| #it's a bash feature. | ||
| #https://www.gnu.org/software/bash/manual/html_node/Bash-Variables.html#index-SECONDS | ||
| SECONDS=0 | ||
| PATH="${PATH}:/var/vcap/packages/percona-xtradb-cluster-<%= p('mysql_version') -%>/bin" | ||
|
|
||
| log "starting post-start, checking replication" | ||
| while [[ $SECONDS -lt 300 ]]; do | ||
| if mysql --defaults-file=/var/vcap/jobs/pxc-mysql/config/mylogin.cnf <<< "SHOW REPLICA STATUS\G" | grep 'Replica_IO_Running: Yes'; then | ||
| log "IO thread is running" | ||
| else | ||
| log "IO thread is running" | ||
| continue | ||
| fi | ||
|
|
||
| if mysql --defaults-file=/var/vcap/jobs/pxc-mysql/config/mylogin.cnf <<< "SHOW REPLICA STATUS\G" | grep 'Replica_SQL_Running: Yes'; then | ||
| log "SQL thread is running" | ||
| exit 0 | ||
| else | ||
| log "SQL thread not running" && continue | ||
| continue | ||
| fi | ||
| sleep 5 | ||
| done | ||
|
|
||
| log "timed out waiting for replication to report healthy" | ||
| exit -1 | ||
| <% end %> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| #!/usr/bin/env bash | ||
| set -eo pipefail | ||
|
|
||
| . /var/vcap/packages/pxc-utils/logging.sh | ||
|
|
||
|
|
||
| <%- if_p('tls.client.ca','tls.client.certificate','tls.client.private_key') do -%> | ||
| SOURCE_SSL_CONFIG="SOURCE_SSL_CA = '/var/vcap/jobs/pxc-replicator/certificates/server-ca.pem', SOURCE_SSL_CERT = '/var/vcap/jobs/pxc-replicator/certificates/client-cert.pem', SOURCE_SSL_KEY = '/var/vcap/jobs/pxc-replicator/certificates/client-key.pem'" | ||
|
Member
There was a problem hiding this comment. I didn't think deeply about this during my initial review pass so up-front apologies for not surfacing this earlier. These certificates are going to be read by the mysqld process running in the pxc-mysql bpm context. That job currently would not have access to the pxc-replicator job certificates without adjusting the bpm.yml. Rather than doing this, I think maybe just configure certificates on the pxc-mysql job. This should be the point of the pxc-mysql
Member
There was a problem hiding this comment.
Not necessarily bad a security perspective but the replication user would need to be specifically configured for it. i.e. CREATE USER $replicationUSER REQUIRE X509 .... ;
-- or
CREATE USER $replicationUSER REQUIRE SUBJECT '/CN=some-client-identity' ...;As I mentioned in an earlier comment, mutual TLS is not a requirement for an MVP but maybe a nice-to-have. If you need it, it would add it as an option to user creation.
Member
There was a problem hiding this comment. Worth noting that best practice would also set SOURCE_SSL_VERIFY_SERVER_CERT (hostname verification). This would require the primary's TLS certificate to contain the bosh dns server name (provided by the link here) Bosh docs. In the context of cf-d this is usually sql-db.service.cf.internal but would require explicit configuration. |
||
| <%- end.else do if_link('source') do |link| -%> | ||
| <%- if link.p('tls') != nil && link.p('tls')['client'] != nil then -%> | ||
| SOURCE_SSL_CONFIG="SOURCE_SSL_CA = '/var/vcap/jobs/pxc-replicator/certificates/server-ca.pem', SOURCE_SSL_CERT = '/var/vcap/jobs/pxc-replicator/certificates/client-cert.pem', SOURCE_SSL_KEY = '/var/vcap/jobs/pxc-replicator/certificates/client-key.pem'" | ||
| <%- else -%> | ||
| SOURCE_SSL_CONFIG="" | ||
| <%- end -%> | ||
| <%- end end -%> | ||
| function is_replication_configured(){ | ||
| ! test "$($self_mysql <<< "SHOW REPLICA STATUS\G")" = "" | ||
| } | ||
| function is_replica_io_running (){ | ||
| $self_mysql <<< "SHOW REPLICA STATUS\G" | grep 'Replica_IO_Running: Yes' | ||
| } | ||
| function is_replica_sql_running (){ | ||
| $self_mysql <<< "SHOW REPLICA STATUS\G" | grep 'Replica_SQL_Running: Yes' | ||
| } | ||
| function enable_replication (){ | ||
| $self_mysql <<< "STOP REPLICA; | ||
| CHANGE REPLICATION SOURCE TO | ||
| SOURCE_HOST='${SOURCE_ADDR}', | ||
| SOURCE_USER='${SOURCE_USER}', | ||
| SOURCE_PASSWORD='${SOURCE_PASS}', | ||
| SOURCE_AUTO_POSITION=1, | ||
| ${SOURCE_SSL_CONFIG}; | ||
| START REPLICA;" | ||
| } | ||
| function ensure_backup_user_has_replication_slave(){ | ||
| if [[ ! -z "${source_admin_mysql}" ]]; then | ||
| log "found source_admin_creds, ensuring source_replica user permissions" | ||
| ${source_admin_mysql} <<< "GRANT REPLICATION SLAVE ON *.* TO '${SOURCE_USER}'@'%';" | ||
| fi | ||
| } | ||
| function sync_databases(){ | ||
| log "resyncing with full backup" | ||
| ensure_backup_user_has_replication_slave | ||
| log $($self_mysql <<< "STOP REPLICA;") | ||
| log $($self_mysql <<< "RESET REPLICA;") | ||
| log $($self_mysql <<< "RESET MASTER;") | ||
|
Member
There was a problem hiding this comment. Just a note: RESET MASTER is deprecated in v8.4 and removed in MySQL v9, replaced with I understand this is likely used here since pxc-release still supports MySQL v8.0 which still requires this syntax.
Author
There was a problem hiding this comment. yeah, what's the preferred way of handling this? I noticed while reading docs that they're still https://dev.mysql.com/doc/refman/8.4/en/replication-howto-repuser.html it seems that the permissions still does not have an alternative?
Member
There was a problem hiding this comment. Yeah, v8.0 (and earlier) only support We've typically dealt with this kind of thing with version checks: if mysql_major_minor() == "8.0": # <- Delete branch when we don't care about old version anymore
do_legacy_sql_stuff()
else:
do_latest_sql_stuff()
endThat could potentially be done at template rendering time by looking at the MySQL v8.0 is EOL (although we'll probably get one more patch release from Percona for v8.0.46), so there is an argument to only support v8.4 and onwards. Maybe guard against using this feature on v8.0? # In some pxc-replicator template
<%- if p('mysql_version') == "8.0" -%>
raise "Unsupported <helpful error message>"
<%- end -%>
Author
There was a problem hiding this comment. hmm. How'd that play with cf-depl? It seems that 8.4 is still experimental cloudfoundry/cf-deployment@3870c60 I'm not sure if there are any reasons why it's still setup to default to 8.0 outside of being cautious. I'm going to ask around what the plans for runtime are in regards to switching the default. For now I guess I'll try to find all relevant places and sprinkle some
Member
There was a problem hiding this comment. My thinking here was that it could be reasonable to not support this replication feature for EOL MySQL versions to simplify things. However this exposes that, as pxc-release maintainers, we have been bad netizens of cf-d and have not updated the default MySQL version to v8.4. I'll try to follow-up on that. I think it's okay to add a version check to support both the old and new syntax for now. |
||
| log $($source_dump --all-databases --triggers --routines --single-transaction | $self_mysql) | ||
|
abg marked this conversation as resolved.
|
||
| } | ||
| function source_upcheck(){ | ||
| $source_mysql <<< "exit" | ||
| } | ||
| function self_upcheck(){ | ||
| $self_mysql <<< "exit" | ||
| } | ||
|
|
||
| while ! source_upcheck; do | ||
| log "waiting for source to be available" | ||
| sleep 5 | ||
| done | ||
|
|
||
| while ! self_upcheck; do | ||
| log "waiting for self to be available" | ||
| sleep 5 | ||
| done | ||
|
|
||
| log "starting replication setup" | ||
|
|
||
| log "checking existing databases in source instance" | ||
|
|
||
| log "checking replication status" | ||
|
|
||
| if ! is_replication_configured; then | ||
| log "replication is not yet enabled" | ||
| sync_databases | ||
|
|
||
| log "replication is not configured. enabling" | ||
| OUT=$(enable_replication); | ||
| if [[ "${OUT}" != "" ]]; then | ||
| log "failed enabling replication: '$OUT'" | sed "s/${SOURCE_PASS}/<REDACTED>/g" | ||
| else | ||
| log "replication enabled" | ||
| fi | ||
| else | ||
| log "replicaton already enabled, skipping" | ||
| fi | ||
|
|
||
| while true; do | ||
| log "checking running status" | ||
| RUNNING_STATUS="$($self_mysql <<< "SHOW REPLICA STATUS\G" | grep 'Replica_.*_Running: ' | xargs )" | ||
| if is_replica_io_running && is_replica_sql_running; then | ||
| log "replication healthy" | ||
| elif ! is_replica_io_running; then | ||
| log "restarting replica" | ||
| ensure_backup_user_has_replication_slave | ||
| $self_mysql <<< "START REPLICA;" | ||
| sleep 30 | ||
| elif ! is_replica_sql_running ; then | ||
| log "replication Replica_SQL_Running marked as no. Attempting to resync" | ||
| sync_databases | ||
| else | ||
| log "$RUNNING_STATUS" | ||
| for line in $($self_mysql <<< "SHOW REPLICA STATUS\G" | grep 'Err' | tr -d '[:blank:]' ); do | ||
| log "${line/$SOURCE_PASS/<redacted>/}"; | ||
| done | ||
| fi | ||
| log "$RUNNING_STATUS" | ||
| sleep 5 | ||
| done | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| <%- | ||
| if !['rfc3339', 'unix-epoch'].include?(p('logging.format.timestamp')) | ||
| raise "'#{p('logging.format.timestamp')}' is not a valid timestamp format for the property 'logging.format.timestamp'." + | ||
| " Valid options are: 'rfc3339' and 'unix-epoch'." | ||
| end | ||
| path = [ | ||
| "/usr/bin", | ||
| "/bin", | ||
| "/var/vcap/packages/percona-xtradb-cluster-#{p('mysql_version')}/bin", | ||
| ] | ||
| flags = "" | ||
| if_p('tls.client.ca','tls.client.certificate','tls.client.private_key') do | ||
| flags = "--ssl-ca=/var/vcap/jobs/pxc-replicator/certificates/server-ca.pem --ssl-cert=/var/vcap/jobs/pxc-replicator/certificates/client-cert.pem --ssl-key=/var/vcap/jobs/pxc-replicator/certificates/client-key.pem" | ||
|
Member
There was a problem hiding this comment. Any reason not to set these flags via the my.cnf? (i.e. |
||
| end | ||
| if_link('source') do |link| | ||
| if link.p('tls') != nil && link.p('tls')['client'] != nil then | ||
| flags = "--ssl-ca=/var/vcap/jobs/pxc-replicator/certificates/server-ca.pem --ssl-cert=/var/vcap/jobs/pxc-replicator/certificates/client-cert.pem --ssl-key=/var/vcap/jobs/pxc-replicator/certificates/client-key.pem" | ||
| end | ||
| end | ||
| -%> | ||
| --- | ||
| <% if p('replicator_enabled') %> | ||
| processes: | ||
| - name: pxc-replicator | ||
| executable: /var/vcap/jobs/pxc-replicator/bin/setup | ||
| args: [] | ||
| env: | ||
|
Member
There was a problem hiding this comment. It would probably be clearer to just set these env vars in the setup template itself rather than here. I am bouncing around between bpm and setup to figure out the magic variables set in the bpm to understand what's being done in setup. Some of it feels a little magical. |
||
| PATH: <%= path.join(":") %> | ||
| <%- if_p('mysql_backup_username', 'mysql_backup_password' ,'host', 'port') do |user,pass,host| -%> | ||
|
Member
There was a problem hiding this comment. Ideally we would apply the principal of least privilege here. Otherwise this will get flagged by security scans. Replication doesn't need full SELECT access on tables from the primary - it just needs the privilege to stream binary logs. Similarly the backup user just needs minimal privileges for a backup, but not necessarily binlog streaming privileges. |
||
| SOURCE_USER: <%= user %> | ||
| SOURCE_PASS: <%= pass %> | ||
| SOURCE_ADDR: <%= host %> | ||
| <%- end.else do if_link("source") do |link| -%> | ||
| SOURCE_USER: <%= link.p('mysql_backup_username') %> | ||
| SOURCE_PASS: <%= link.p('mysql_backup_password') %> | ||
| SOURCE_ADDR: <%= link.address %> | ||
| <%- end end -%> | ||
| self_mysql: 'mysql --defaults-file=/var/vcap/jobs/pxc-mysql/config/mylogin.cnf' | ||
| source_dump: 'mysqldump --defaults-file=/var/vcap/jobs/pxc-replicator/config/source.mysql.cnf <%= flags -%>' | ||
| source_mysql: 'mysql --defaults-file=/var/vcap/jobs/pxc-replicator/config/source.mysql.cnf <%= flags -%>' | ||
| <%- if_p('source_admin_username','source_admin_password','host') do -%> | ||
| source_admin_mysql: 'mysql --defaults-file=/var/vcap/jobs/pxc-replicator/config/source.admin.mysql.cnf <%= flags -%>' | ||
| <%- end -%> | ||
| persistent_disk: true | ||
| ephemeral_disk: true | ||
|
Comment on lines
+44
to
+45
Member
There was a problem hiding this comment. Are these used anywhere? I did not see a |
||
| additional_volumes: | ||
| - path: /var/vcap/sys/run/pxc-mysql | ||
|
abg marked this conversation as resolved.
|
||
| writeable: false | ||
| - path: /var/vcap/jobs/pxc-mysql/config | ||
|
Member
There was a problem hiding this comment. If this is needed, I would mark this I think better would be to use a "replication admin" role. Maybe worth adding a
Author
There was a problem hiding this comment. my current thinking is to just generate a bosh user for the initial root account of the replica target and set that via the set-repilcator-target ops file.. Mostly because I noticed that if I don't change the initial root user name between cf-d and my replica deployments, they both used defaults for naming the root user. So after initial sync, the targets root account used the pw from cf-d.. |
||
| writeable: false | ||
| - path: /var/vcap/jobs/pxc-replicator/certificates | ||
| writeable: false | ||
| <% end %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <%- if_p('tls.client') do |client_tls| -%> | ||
| <%= client_tls['certificate'] %> | ||
| <%- end.else_if_p do if_link('source') do |link| -%> | ||
| <%= link.tls.client.certificate -%> | ||
| <%- end end -%> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <%- if_p('tls.client') do |client_tls| -%> | ||
| <%= client_tls['private_key'] %> | ||
| <%- end.else_if_p do if_link('source') do |link| -%> | ||
| <%= link.tls.client.private_key -%> | ||
| <%- end end -%> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <%- if_p('tls.client') do |client_tls| -%> | ||
| <%= client_tls['ca'] %> | ||
| <%- end.else_if_p do if_link('source') do |link| -%> | ||
| <%= link.tls.client.ca -%> | ||
| <%- end end -%> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <% if_p('source_admin_username','source_admin_password','host','port') do |user,pass,host,port| %> | ||
| [client] | ||
|
Member
There was a problem hiding this comment. Are these actually optional? If this job is deployed, it just fails if these aren't set right? Maybe be worth just failing on the property lookup rather than having a guard here i.e. [client]
user = <% p('source_admin_username') %>
password = <%= p('source_admin_password') %>
...Note you can also put the |
||
| user = <%= user %> | ||
| password = <%= pass %> | ||
| host = <%= host %> | ||
| port = <%= port %> | ||
| <% end %> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <% if_p('mysql_backup_username', 'mysql_backup_password' ,'host', 'port') do |user,pass,host,port| %> | ||
| [client] | ||
| user = <%= user %> | ||
| password = <%= pass %> | ||
| host = <%= host %> | ||
| port = <%= port %> | ||
| <% end.else_if_p do if_link("source") do |link| %> | ||
| [client] | ||
| user = <%= link.p('mysql_backup_username') %> | ||
| password = <%= link.p('mysql_backup_password') %> | ||
| host = <%= link.address %> | ||
| port = <%= link.p('port') %> | ||
| <% end end %> | ||
|
abg marked this conversation as resolved.
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we reuse the
mysqllink and just expose more properties on it? It already has port and mysql_versionIf we are pulling all this from the "primary" deployments link anyway, I was imagining something like:
Plus or minus some bosh syntax cleanup here.
Maybe a second link isn't so bad, but it adds some more maintenance overhead where if we expose another mysql job property we have to maintain it in two places going forward.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reasoning was mostly about being specific what the link type is used for and not wanting to introduce additional properties to existing consumers to avoid confusion. I have no strong feelings about either direction.