Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

global_values in evironments do not get applied #26

Closed
EugenMayer opened this issue Jun 24, 2016 · 8 comments
Closed

global_values in evironments do not get applied #26

EugenMayer opened this issue Jun 24, 2016 · 8 comments
Assignees

Comments

@EugenMayer
Copy link

EugenMayer commented Jun 24, 2016

Having this common.yaml: https://goo.gl/ew5po4
I get a warning, that slow_query_log and log_queries_not_using_index being overriden by the "global" defaults, so they end up being 0, not 1, in development.

I expected the environment specific defaults to superseed the global defaults, not the other way arround?

tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
Warning, merging duplicate data values.
slow_query_log => '0' being replaced by : '1' from FileDataSource
Warning, merging duplicate data values.
log_queries_not_using_index => '0' being replaced by : '1' from FileDataSource
Template generation completed
Executing ["supervisorctl", "restart", "mysql"]...
mysql: stopped
mysql: started
@EugenMayer
Copy link
Author

interestingly, when i add consul to the template sources and add the template to consul ( i had to, due to #27 ) i do get the proper behavior, so development->mysql.erb->config->slow_query_log superseeds defaults->global->slow_query_log

this seem to be a bug for sure, as far as i understand

@EugenMayer
Copy link
Author

EugenMayer commented Jun 28, 2016

I could reproduce it once again:

This run is without having anything in conul - default is picked up

(root:nginx_base)➜  include  tiller -e production
tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
Consul : No templates could be fetched from /configuration/templates/sources
Template generation completed
Executing ["supervisorctl", "restart", "nginx"]...
nginx: stopped
nginx: started

this one is with having set configuration/globals/httpd/all/open_file_cache_valid to 12s

(root:nginx_base)➜  include  tiller -e production
tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
Consul : No templates could be fetched from /configuration/templates/sources
Warning, merging duplicate data values.
open_file_cache_valid => '12s' being replaced by : '45s' from DefaultsDataSource
Template generation completed
Executing ["supervisorctl", "restart", "nginx"]...
nginx: stopped
nginx: started

As you see, 12s are replaced by 45s

now i added the value to the production environment configuration/globals/httpd/production/open_file_cache_valid to 11s

(root:nginx_base)➜  include  tiller -e production
tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
Consul : No templates could be fetched from /configuration/templates/sources
Warning, merging duplicate data values.
open_file_cache_valid => '11s' being replaced by : '45s' from DefaultsDataSource
Template generation completed
Executing ["supervisorctl", "restart", "nginx"]...
nginx: stopped
nginx: started

still, overriden by defaults. My common.yaml looks like this:

consul:
  url: "http://localhost:8500"
  dc: "stable"
  register_services: false
  register_nodes: false
  templates: '/configuration/templates/sources'
  values:
    global: '/configuration/globals/httpd/all'
    per_env: '/configuration/globals/httpd/%e'
    template: '/configuration/services/%e/%t'
    target: '/configuration/templates/targets/%t/%e'
exec: [ "supervisorctl" , "restart" , "nginx"]
data_sources: [  "defaults", "file", "consul"]
template_sources: [ "consul","file" ]
defaults:
  assetcache.erb:
    target: /etc/nginx/dw/include/assetcache
    config:
      open_file_cache_valid: '45s'
      expires: '30d'
environments:
  production:
    config:
      open_file_cache_valid: '45s'
  staging:
    config:
      open_file_cache_valid: '45s'
  development:
    config:
      open_file_cache_valid: '0s'
      expires: '0s'

@EugenMayer
Copy link
Author

i think this has nothing to do with consul, its a broken defaults plugin.

having this configuration

exec: [ "supervisorctl" , "restart" , "nginx"]
# care with the overrides, see issue https://github.com/markround/tiller/issues/26
data_sources: [  "file","defaults"]
template_sources: [ "file" ]
defaults:
  assetcache.erb:
    target: /etc/nginx/drupalwiki/include/assetcache
    config:
      open_file_cache_valid: '45s'
      expires: '30d'
environments:
  production:
    config:
      open_file_cache_valid: '45s'
  staging:
    config:
      open_file_cache_valid: '45s'
  development:
    global_values:
      open_file_cache_valid: '0s'
      expires: '0s'

i see

(root:nginx_base)➜  include  tiller -d -v -e development
tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
Executable: /var/lib/gems/2.1.0/gems/tiller-0.7.9/bin/tiller
Using common.yaml v2 format configuration file
Using configuration from /etc/tiller
Using plugins from /usr/local/lib/tiller
Using environment development
Template sources loaded [FileTemplateSource]
Data sources loaded [FileDataSource, DefaultsDataSource]
#<FileDataSource:0x000000019096c8> : Using values from v2 format common.yaml
#<FileDataSource:0x00000001909218> : Using values from v2 format common.yaml
#<DefaultsDataSource:0x00000001908d90> : Using values from v2 format common.yaml
#<DefaultsDataSource:0x00000001908750> : Using values from v2 format common.yaml
Available templates : ["assetcache.erb"]
#<FileDataSource:0x0000000190f6e0> : Using values from v2 format common.yaml
#<DefaultsDataSource:0x0000000190f230> : Using values from v2 format common.yaml
Warning, merging duplicate data values.
open_file_cache_valid => '0s' being replaced by : '45s' from DefaultsDataSource
Warning, merging duplicate data values.
expires => '0s' being replaced by : '30d' from DefaultsDataSource
Building template assetcache.erb
Setting ownership/permissions on /etc/nginx/drupalwiki/include/assetcache
Template generation completed
Executing ["supervisorctl", "restart", "nginx"]...
Child process forked with PID 204.
nginx: stopped
nginx: started
Child process exited with status 0
Child process finished, Tiller is stopping.

as you see open_file_cache_valid => '0s' being replaced by : '45s' from DefaultsDataSource

so first the env development is picked up with 0, then overriden by global?

@EugenMayer
Copy link
Author

EugenMayer commented Jun 29, 2016

@markround as promised, i created a docker image for easy testing:

To reproduce

git clone https://github.com/eugen.mayer/tiller-test
cd tillertest/datasourcemerge
docker-compose up

The you see

tiller    | tiller v0.7.9 (https://github.com/markround/tiller) <[email protected]>
tiller    | Executable: /var/lib/gems/2.1.0/gems/tiller-0.7.9/bin/tiller
tiller    | Using common.yaml v2 format configuration file
tiller    | Using configuration from /etc/tiller
tiller    | Using plugins from /usr/local/lib/tiller
tiller    | Using environment production
tiller    | Template sources loaded [FileTemplateSource]
tiller    | Data sources loaded [DefaultsDataSource, FileDataSource]
tiller    | #<DefaultsDataSource:0x0000000146a720> : Using values from v2 format common.yaml
tiller    | #<DefaultsDataSource:0x00000001469d70> : Using values from v2 format common.yaml
tiller    | #<FileDataSource:0x000000014695a0> : Using values from v2 format common.yaml
tiller    | #<FileDataSource:0x00000001469118> : Using values from v2 format common.yaml
tiller    | Available templates : ["test.erb"]
tiller    | #<DefaultsDataSource:0x00000001468448> : Using values from v2 format common.yaml
tiller    | #<FileDataSource:0x0000000146fb58> : Using values from v2 format common.yaml
tiller    | Building template test.erb
tiller    | Setting ownership/permissions on /tmp/test
tiller    | Template generation completed
tiller    | Executing ["cat", "/tmp/test"]...
tiller    | ------------ TEMPLATE -----------------
tiller    | current-envirnment: production
tiller    | global_val: 1
tiller    | should_be_overriden: global
tiller    | ------------ TEMPLATE -----------------
tiller    | Child process forked with PID 7.
tiller    | Child process exited with status 0
tiller    | Child process finished, Tiller is stopping.

As you see, the environment is production, while the value of should_be_overriden is still overriden.
https://github.com/EugenMayer/tiller-test/blob/master/datasourcemerge/build/tiller/common.yaml#L13

You can build the image yourself or run it with a custom cmd to change the environment like

docker run --rm EugenMayer/tiller-test:merge -e development

You can of course build the image yourself, if you have no hub.docker.io account

cd datasourcemerge
docker-compose build && docker-compose up

@markround this is a huge blocker for me, it makes tiller more or less completely useless for what i have chosen it. It would be great if we could sort this out as soon as possible. Let me know how to assist you

@EugenMayer
Copy link
Author

EugenMayer commented Jun 29, 2016

@markround i could drill down the issue a bit

a) i fixed the wrong defintion in production causing the first issue EugenMayer/tiller-test@00288c3#diff-a186fce8e470a58e6e6848844574b2cbL11

so the general overriding works, but not entirely. When you run the container with

docker run --rm eugenmayer/tiller-test:merge -e development

you see, that the global_values definition of development is not applied as documented here: https://github.com/markround/tiller#defaults-per-environment.

So the only issue left here is, that global_values in environments are not applied

There is an additional issue, i will create a new issue for that (#29)

@EugenMayer EugenMayer changed the title global superseeds environment based defaults? global_values in evironments do not get applied Jun 29, 2016
@EugenMayer
Copy link
Author

Also waiting here, anyhow i could help?

@markround markround self-assigned this Jul 15, 2016
@markround
Copy link
Owner

markround commented Jul 15, 2016

Hi,

Firstly - many thanks for the incredibly detailed issues and test cases around them. Much appreciated!

Anyway, looking at this, I think it's again a similar issue to #32. I can see your common.yaml looks like:

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
  test.erb:
    target: /tmp/test
    config:
      global_val: '1'
      should_be_overriden: 'global'
environments:
  production:
    test.erb:
      config:
        should_be_overriden: 'production'
  development:
    global_values:
      should_be_overriden: 'development'

Which will not work in development, as the global value will always be over-written by a template value, which is what the defaults module is providing. Just to re-iterate my comment on #32, "a template var always over-rides a global var, and can only be over-ridden by another template var from a higher priority plugin."

This would work, as it uses template vars to over-ride the defaults global var:

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
  should_be_overriden: 'global'
  global_val: '1'
environments:
  production:
    test.erb:
      target: /tmp/test
      config:
        should_be_overriden: 'production'
  development:
    test.erb:
      target: /tmp/test
      config:
        should_be_overriden: 'development'

Or, if you want to avoid duplication, define the template for all environments and stick to using template vars:

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
    test.erb:
      target: /tmp/test
      config:
        should_be_overriden: 'global'
        global_val: '1'
environments:
  production:
    test.erb:
      config:
        should_be_overriden: 'production'
  development:
    test.erb:
      config:
        should_be_overriden: 'development'

Or something like this, using global vars that then get over-ridden by template vars in the environments, but specifying the template target as a default template var, so you don't have to repeat that bit of the definitions:

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
  should_be_overriden: 'global'
  global_val: '1'
  test.erb:
    target: /tmp/test
environments:
  production:
    test.erb:
      config:
        should_be_overriden: 'production'
  development:
    test.erb:
      config:
        should_be_overriden: 'development'

Or even making it more efficient by using global vars to over-ride things (the template still gets generated in the both environments as it's defined in the defaults section):

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
  should_be_overriden: 'global'
  global_val: '1'
  test.erb:
    target: /tmp/test
environments:
  production:
    global_values:
      should_be_overriden: 'production'
  development:
    global_values:
      should_be_overriden: 'development'

Or even a mix of the two approaches:

exec: [ "cat" , "/tmp/test"]
data_sources: [  "defaults", "file"]
template_sources: [ "file" ]
defaults:
  should_be_overriden: 'global'
  global_val: '1'
  test.erb:
    target: /tmp/test
environments:
  production:
    test.erb:
      config:
        should_be_overriden: 'production'
  development:
    global_values:
      should_be_overriden: 'development'

@markround
Copy link
Owner

Closing this issue as I think this is down to a mis-understanding of Tiller's behaviour. I've updated the docs to hopefully make this clearer. Please re-open if you believe this to be a genuine bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants