add ability to split config into 2 files #60

MaxFedotov · 2018-08-23T20:20:49Z

Added ability to create two different config files, if variable $split_configis set to true - one for admin\mysql variables (controlled by $config_file and $manage_config_file variables), another for servers\users\hostgroups\scheduler\rules params of ProxySQL (controlled by $proxy_config_file and manage_proxy_config_file variables) . This was done to allow management of main config file by puppet and manage all proxy configuration settings by some external provider (for example, consul-template, bash script), as this file needs to be updated in more frequent intervals that actual puppet agent interval (e.g. for each failover)
Also added creation of sys_owner user and sys_group group in install.pp to fix possible errors if this groups doesn't exists.
Added package_name and package_ensure, because got problems with mariadb conflicting package trying to install during puppet run.
Added reload_config manifest to allow dynamic config reload

mcrauwel · 2018-08-24T11:19:01Z

manifests/install.pp

@@ -27,6 +36,8 @@
  }

  class { '::mysql::client':
+    package_name    => 'mysql-client',


hardcoding like this will break for RedHat/CentOS.

this is also duplicated in #58

Answered in #58

I would remove it from this PR and keep the fix in #58. Agreed?

Removed, as will be changed to variable in #58

manifests/reload_config.pp

bastelfreak · 2018-08-31T22:19:24Z

manifests/init.pp

@@ -132,8 +143,11 @@
  String $monitor_username = $::proxysql::params::monitor_username,
  Sensitive[String] $monitor_password = $::proxysql::params::monitor_password,

+  Boolean $split_config = $::proxysql::params::split_config,


please remove the leading ::

bastelfreak · 2018-08-31T22:20:36Z

templates/proxysql.cnf.erb

 <%
    end
  else
    default_handler.call(k, v)
  end
 end
 -%>
+
+<% if scope.lookupvar('::proxysql::split_config') == true -%>


This should work as well:

if @proxysql::split_config == true

bastelfreak · 2018-08-31T22:22:49Z

templates/proxysql_proxy.cnf.erb

@@ -0,0 +1,56 @@
+<%
+default_handler = Proc.new do |k, v|


could you please migrate this to an epp() template? We require that for new templates. Quote from our docs:

Is a new template added? The preferred language is epp, not erb

MaxFedotov · 2018-09-01T14:08:48Z

Review notes applied - changed all $:: to $. Changed to facts[] instead of $::. Changed scope.lookupvar() in proxysql.cnf.erb. Moved proxysql_proxy.cnf.erb to proxysql_proxy.cnf.epp

mcrauwel · 2018-09-02T17:45:35Z

@MaxFedotov can you rebase on the current master?

MaxFedotov · 2018-09-03T12:22:10Z

Hi,
I've rebased on current master.
But can you please take a look on tests? Everything is working when i test module installation locally.

mcrauwel · 2018-09-03T13:19:27Z

now let's rebase this. I will try to find the issue in the tests...

MaxFedotov · 2018-09-03T13:38:53Z

done, but errors are the same

bastelfreak · 2018-09-03T15:57:07Z

mhm, travis is trolling me at the moment.

bastelfreak · 2018-09-03T20:29:59Z

manifests/install.pp

+    ensure => 'present',
+  }
+
+  user { $::proxysql::sys_owner:


please do not add leading ::

bastelfreak · 2018-09-03T20:30:50Z

manifests/init.pp

@@ -158,6 +168,11 @@
  String $monitor_username = $proxysql::params::monitor_username,
  Sensitive[String] $monitor_password = $proxysql::params::monitor_password,

+  Boolean $split_config = $proxysql::params::split_config,
+
+  String $proxy_config_file = $proxysql::params::proxy_config_file,


Can you please use String[1]? Otherwise empty string is allowed as well.

Oh this is a path. Please use Stdlib::Absolutepath here.

bastelfreak · 2018-09-03T20:32:48Z

manifests/reload_config.pp

+          SAVE MYSQL VARIABLES TO DISK; \"
+        ",
+        subscribe   => $subscribe,
+        require     => [ Service[$::proxysql::service_name] , File['root-mycnf-file'] ],


Please do not add leading ::

MaxFedotov · 2018-09-04T06:35:22Z

Still got this :(
Failure/Error: it { is_expected.to compile.with_all_deps }
dependency cycles found: (File[/etc/apt/sources.list.d/proxysql_repo.list] => Apt::Setting[list-proxysql_repo] => Apt::Source[proxysql_repo] => Class[Proxysql::Repo] => Class[Proxysql::Install] => Group[root] => File[/etc/apt/sources.list.d/proxysql_repo.list])

and + when using stdlib got:
An error occurred while loading ./spec/acceptance/class_spec.rb.
Failure/Error: install_module_dependencies_on(hosts)
Beaker::Host::CommandFailure:
Host 'ubuntu1404-64-1' exited with 1 running:
puppet module install puppetlabs-stdlib -v 5.0.0
Last 10 lines of output were:
Notice: Preparing to install into /etc/puppetlabs/code/environments/production/modules ...
Error: Could not install module 'puppetlabs-stdlib' (v5.0.0)
Module 'puppetlabs-stdlib' (v4.25.1) is already installed
Use puppet module upgrade to install a different version
Use puppet module install --force to re-install only this module

MaxFedotov · 2018-09-05T22:33:51Z

@bastelfreak were you able to fix problems with Travis?

bastelfreak · 2018-09-06T10:42:37Z

metadata.json

@@ -12,6 +12,10 @@
      "name": "puppetlabs-mysql",
      "version_requirement": ">= 2.5.0 < 7.0.0"
    },
+    {
+      "name": "puppetlabs-stdlib",
+      "version_requirement": ">= 1.0.0"


We need to remove this now or wait for releases from puppetlabs: #65

Let's remove this for now and i will create another PR, which will add this and can be merged later :)

… to epp

MaxFedotov · 2018-09-06T11:23:57Z

made rebase on master. Still got errors

bastelfreak · 2018-09-06T13:36:15Z

@mcrauwel can you look into this? Is master broken?

mcrauwel · 2018-09-08T17:13:41Z

master is ok, so it must be something in the PR, it's only happening on deb based systems hence some tests (RHEL/CentOS) are successful

MaxFedotov · 2018-09-08T17:16:04Z

I have no errors when installing it on centos. Will check PR one more time again tomorrow

mcrauwel · 2018-09-08T17:19:59Z

@MaxFedotov you can run the acceptance test locally for the deb env's I used this:

export PUPPET_INSTALL_TYPE=agent BEAKER_IS_PE=no BEAKER_PUPPET_COLLECTION=puppet5 BEAKER_debug=true BEAKER_setfile=ubuntu1604-64{hypervisor=docker} CHECK=beaker
bundle exec rake beaker

these env variables are coming from the .travis.yml file

mcrauwel · 2018-09-08T18:08:27Z

current issue looks like a bug in the debian8 package for proxysql...

mcrauwel · 2018-09-08T18:20:36Z

created upstream issue sysown/proxysql#1679

bastelfreak · 2018-09-08T18:23:27Z

should we, for now, remove debian 8 from .travis.yml/metadata.json? So we could merge this now and later add it back. This would allow us to do a new release finally.

mcrauwel · 2018-09-08T18:28:09Z

Let’s wait for a day or 2 to see if they can quickly fix it?

…

Sent from my iPhone

On 8 Sep 2018, at 20:23, Tim Meusel ***@***.***> wrote: should we, for now, remove debian 8 from .travis.yml/metadata.json? So we could merge this now and later add it back. This would allow us to do a new release finally. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mcrauwel · 2018-09-08T20:41:29Z

update: i found the error and opened an upstream PR sysown/proxysql#1680

mcrauwel · 2018-09-10T06:57:50Z

Hooray! Thanks for your contributions @MaxFedotov and thanks for the help @bastelfreak!

mcrauwel reviewed Aug 24, 2018

View reviewed changes

bastelfreak reviewed Aug 31, 2018

View reviewed changes

bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels Aug 31, 2018

MaxFedotov force-pushed the configs_separation branch 6 times, most recently from b25c7ba to e1da828 Compare September 1, 2018 14:04

MaxFedotov force-pushed the configs_separation branch from e1da828 to 79ea7ea Compare September 3, 2018 12:01

MaxFedotov force-pushed the configs_separation branch from 79ea7ea to 5305e3c Compare September 3, 2018 13:32

bastelfreak reviewed Sep 3, 2018

View reviewed changes

manifests/install.pp Outdated

ensure => 'present',

}

user { $::proxysql::sys_owner:

Copy link

Member

bastelfreak Sep 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please do not add leading ::

bastelfreak reviewed Sep 3, 2018

View reviewed changes

bastelfreak reviewed Sep 6, 2018

View reviewed changes

MaxFedotov added 4 commits September 6, 2018 14:18

add ability to split config into 2 files

b31cf96

add config reload

51ef5f7

add config reload

3d7d6a0

apply review notes. Change $:: to $. Use facts[]. Change erb template…

fe84efd

… to epp

MaxFedotov and others added 3 commits September 6, 2018 14:18

change :: to :. Use Stdlib::Absolutepath for params

0c9e8ab

add puppetlabs stdlib to dependencies

0367744

Remove references to stdlib

ed2a8e5

MaxFedotov force-pushed the configs_separation branch from 454c69a to ed2a8e5 Compare September 6, 2018 11:23

move user/group management to prerequisites to prevent dependency cycles

d2594b3

checksums were changed while fixing sysown/proxysql#1679

0b75147

mcrauwel merged commit 5bdf86f into voxpupuli:master Sep 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add ability to split config into 2 files #60

add ability to split config into 2 files #60

MaxFedotov commented Aug 23, 2018 •

edited

Loading

mcrauwel Aug 24, 2018

mcrauwel Aug 24, 2018

MaxFedotov Aug 24, 2018

mcrauwel Aug 24, 2018

MaxFedotov Aug 25, 2018 •

edited

Loading

bastelfreak Aug 31, 2018

bastelfreak Aug 31, 2018

bastelfreak Aug 31, 2018

MaxFedotov commented Sep 1, 2018

mcrauwel commented Sep 2, 2018

MaxFedotov commented Sep 3, 2018

mcrauwel commented Sep 3, 2018

MaxFedotov commented Sep 3, 2018

bastelfreak commented Sep 3, 2018

bastelfreak Sep 3, 2018

bastelfreak Sep 3, 2018

bastelfreak Sep 3, 2018

bastelfreak Sep 3, 2018

MaxFedotov commented Sep 4, 2018 •

edited

Loading

MaxFedotov commented Sep 5, 2018

bastelfreak Sep 6, 2018

MaxFedotov Sep 6, 2018

MaxFedotov commented Sep 6, 2018 •

edited

Loading

bastelfreak commented Sep 6, 2018

mcrauwel commented Sep 8, 2018

MaxFedotov commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

bastelfreak commented Sep 8, 2018

mcrauwel commented Sep 8, 2018 via email

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 10, 2018

add ability to split config into 2 files #60

add ability to split config into 2 files #60

Conversation

MaxFedotov commented Aug 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxFedotov Aug 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxFedotov commented Sep 1, 2018

mcrauwel commented Sep 2, 2018

MaxFedotov commented Sep 3, 2018

mcrauwel commented Sep 3, 2018

MaxFedotov commented Sep 3, 2018

bastelfreak commented Sep 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxFedotov commented Sep 4, 2018 • edited Loading

MaxFedotov commented Sep 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MaxFedotov commented Sep 6, 2018 • edited Loading

bastelfreak commented Sep 6, 2018

mcrauwel commented Sep 8, 2018

MaxFedotov commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 8, 2018

bastelfreak commented Sep 8, 2018

mcrauwel commented Sep 8, 2018 via email

mcrauwel commented Sep 8, 2018

mcrauwel commented Sep 10, 2018

MaxFedotov commented Aug 23, 2018 •

edited

Loading

MaxFedotov Aug 25, 2018 •

edited

Loading

MaxFedotov commented Sep 4, 2018 •

edited

Loading

MaxFedotov commented Sep 6, 2018 •

edited

Loading