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

to_yaml changes everytime #20

Closed
liuyu opened this issue Apr 5, 2016 · 38 comments
Closed

to_yaml changes everytime #20

liuyu opened this issue Apr 5, 2016 · 38 comments
Labels

Comments

@liuyu
Copy link

liuyu commented Apr 5, 2016

Hello, I install filebeat.

Although generating facts.yaml dynamically is definitely something i want ,but Puppet will change it almost everytime .

I search one-blog to fix it . I try to change <%= @filebeat_config.to_yaml() %> to <%= @filebeat_config.to_yaml.sort() %> and any parameters is did't work .

some many people encounter this problem. say: "I have the same problem and this is not a bug in the module itself. In the routes.yaml.erb template there's a call to the method 'to_yaml', which in Ruby 1.8.7 is not stable which means the order of the dumped lines is not always the same. In Ruby >=1.9 this is no longer the case.
Unless the module developer's do not want the replace the call to_yaml with something dirty and very hacky for us Ruby 1.8.7 users, we will have to live with this, I'm afraid."

My env :

# ruby --version
ruby 1.8.7 (2012-02-08 patchlevel 358) [x86_64-linux]

# puppet --version
3.7.3
@pcfens pcfens added the bug label Apr 5, 2016
@pcfens
Copy link
Owner

pcfens commented Apr 5, 2016

Does setting the line to <%= YAML::dump(@filebeat_config) %> work? I'm not really a ruby programmer so I'm not sure what else to try short of setting the entire template up field by field (which is a nice thought, but makes future upkeep of this module a lot toughter).

@liuyu
Copy link
Author

liuyu commented Apr 6, 2016

Thank you for the response.

I setting the line to <%= YAML::dump(@filebeat_config) %> is did't work. puppet also change it everytime.

I'm not really ruby too ....

I have two choice:

  1. update ruby to 1.9, I don't want to do .
  2. search ruby 1.8 and puppet syntax , how the method 'to_yaml' work ?

@pcfens
Copy link
Owner

pcfens commented Apr 6, 2016

Puppet 4 ships ruby with it in the packages, so upgrading to puppet 4 is another option to add to the list, though possibly harder than the other 2.

I'm looking at options to see how I might be able to make this work on puppet 3.x with Ruby 1.8.

@liuyu
Copy link
Author

liuyu commented Apr 7, 2016

thx,
"Hash preserves order. It enumerates its elements in theorder in which the keys are inserted. " on Ruby 1.9.x news

If you want to compatible puppet 3.x with Ruby 1.8, give up 'to_yaml' ?

like :

filebeat:

  # General filebeat configuration options
  #
  # Event count spool threshold - forces network flush if exceeded
  spool_size: <%= @filebeat_spool_size %>

@pcfens
Copy link
Owner

pcfens commented Apr 7, 2016

That's what I'm thinking - it's just a matter of writing out a rather long template with a lot of options. The fix won't be super quick, but I'm working on it.

I'll probably use the existing template when the ruby version is newer than 1.8.7 so that the module is always ready for changes.

@pcfens
Copy link
Owner

pcfens commented Apr 7, 2016

There are enough options that it may not be practical to cover everything until someone needs it.

What output options do you consider important to support in the module? How about other sections like run_options or logging?

@liuyu
Copy link
Author

liuyu commented Apr 7, 2016

Partition to four options ?
First filebeat:
spool_size
idle_timeout
registry_file
config_dir

Third prospector:
loop paths and document_type

Fourth output:
logstash
hosts -> array
loadbalance
worker
index

Second logging:
level
to_syslog
to_files
files loop

Some as default , Some sa variables ?

@pcfens
Copy link
Owner

pcfens commented Apr 7, 2016

I'm going to try and keep behavior the same, so prospectors end up in a configuration directory as individual files instead of in the main filebeat.yml file. I'll make sure that logging, logstash outputs, and top level options are all there when I push something up.

@pcfens
Copy link
Owner

pcfens commented Apr 7, 2016

Can you try out the ruby_18_template branch to see if that fixes your problems? If it does I'll merge it and release a new version to the forge tomorrow.

Thanks for your help!

@liuyu
Copy link
Author

liuyu commented Apr 7, 2016

ok , I'm going to test.

@liuyu
Copy link
Author

liuyu commented Apr 7, 2016

Is work ,thx very much !

but there are a few small problems.

  1. filebeat.yml format is not beautiful
# more /etc/filebeat/filebeat.yml
### Filebeat configuration managed by Puppet (Ruby 1.8 version) ###
---
  filebeat:
    spool_size: 1024
    publish_async: false
    idle_timeout: 5s
    registry_file: .filebeat
    config_dir: /etc/filebeat/conf.d

  output:
    logstash:
      hosts:
        - 192.168.134.222:6782
        - 192.168.134.223:6782
      worker: 3
      loadbalance: true



  logging:
    to_syslog: false
    to_files: false

    files:
      path: /var/log/filebeat
      name: filebeat.log
      keepfiles: 7


    level: info
  1. move logging options can't work , need change options value.
    2.1 move 'to_syslog' to first line , puppet can't change
    2.2 change 'to_syslog' true to false , just changed
  2. 'filebeat::prospector' paths *must be array * . If there is only one line, it will not work.
    this is did't work:
        filebeat::prospector { 'syslog':
        paths => [
          '/var/log/messages',
        ],
          doc_type => 'syslog',
        }

@pcfens
Copy link
Owner

pcfens commented Apr 7, 2016

I'll work on the appearance of the file after we've got everything working.

Can you give me a little more information about the logging options and prospector issues? I'm not able to replicate either of the issues as I understand them.

@liuyu
Copy link
Author

liuyu commented Apr 8, 2016

OK

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

Did this fix your problem in a way that everything still works? If it does I'll merge this and publish a new release.

@blysik
Copy link

blysik commented Apr 8, 2016

I'm hitting this problem now also, with RHEL6 hosts with puppet 3.8.6. Each puppet run re-orders the filebeat.yml file, calling a filebeat service refresh.

@blysik
Copy link

blysik commented Apr 8, 2016

Looked at your ruby_18_template branch. One thing to note is that all output 'port' values need to be integers. So for example:

output:
logstash:
port: 5044
host: logstash.example.com

I had to make this change in your master branch also, and convert [output][logstash][port] to integer. (Same with redis output, and I assume same with elasticsearch.)

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

I've always defined the port as part of the hostname (localhost:5044) instead which is probably why I haven't run into this particular issue. I'll get that updated shortly.

Did everything else work in the ruby_18_template branch?

@blysik
Copy link

blysik commented Apr 8, 2016

I haven't actually tested the branch yet, just looked at it. I'm still early in the investigation of moving from 'beaver' to 'filebeat'.

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

Oh ok - let me know if I can help at all. I've tested the ruby 1.8 template in vagrant without any issues, but I want to make sure it actually solves problems (and doesn't create new ones) before I push it up.

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

@blysik You might want to check out 6066de3 to see if it solves your issues.

@blysik
Copy link

blysik commented Apr 8, 2016

Yes! That's essentially what I introduced on my side, though I forgot to check for port being null.

@blysik
Copy link

blysik commented Apr 8, 2016

Actually, isn't this equivalent to checking if it's not nil?

<%- if @filebeat_config['output']['redis']['port'] -%>
<%- @filebeat_config['output']['redis']['port'] = Integer(@filebeat_config['output']['redis']['port']) -%>

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

It works the same - I was on autopilot after creating the other template. I wanted to make sure that values that are false were written out to YAML as well, so I was explicitly checking for nil.

I've changed it to what you were doing for consistency sake.

@blysik
Copy link

blysik commented Apr 8, 2016

First bug I've run into, after trying the branch:

Error: Could not retrieve catalog from remote server: Error 400 on SERVER: Failed to parse template filebeat/filebeat.yml.ruby18.erb:
  Filepath: /etc/puppet/environments/production/modules/filebeat/templates/filebeat.yml.ruby18.erb
  Line: 203
  Detail: undefined method `[]' for nil:NilClass
 at /etc/puppet/environments/production/modules/filebeat/manifests/config.pp:21 on node xxxx.com
Warning: Not using cache on failed catalog
Error: Could not retrieve catalog; skipping run

That is a line in the template about geoips, which I don't set?

<%- if @filebeat_config['shipper']['geoip']['paths'] != nil -%>
    geoip:
      paths:
    <%- @filebeat_config['shipper']['geoip']['paths'].each do |path| -%>
      - <%= path %>
    <%- end -%>
    <%- end -%>

@blysik
Copy link

blysik commented Apr 8, 2016

I was able to make this work by wrapping it in another if checking for geoip:

<%- if @filebeat_config['shipper']['geoip'] -%>
    <%- if @filebeat_config['shipper']['geoip']['paths'] != nil -%>
    geoip:
      paths:
    <%- @filebeat_config['shipper']['geoip']['paths'].each do |path| -%>
      - <%= path %>
    <%- end -%>
    <%- end -%>
    <%- end -%>

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

Does this work?

    <%- if @filebeat_config['shipper']['geoip'] != nil and @filebeat_config['shipper']['geoip']['paths'] != nil -%>
    geoip:
      paths:
    <%- @filebeat_config['shipper']['geoip']['paths'].each do |path| -%>
      - <%= path %>
    <%- end -%>
    <%- end -%>

@blysik
Copy link

blysik commented Apr 8, 2016

Looks like it would. Wrapping it in the larger if block may make more sense if there end up being other options under geoip, however?

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

I should have read the docs sooner. It looks like filebeat ignores the geoip settings anyway, so I'm going to remove that chunk.

@pcfens
Copy link
Owner

pcfens commented Apr 8, 2016

Are there any other issues or missing pieces? If everyone is happy with this I'll go ahead and merge it.

@blysik
Copy link

blysik commented Apr 8, 2016

It's working good for me, other than that geoip part. Great!

@pcfens pcfens closed this as completed in ebc6a36 Apr 9, 2016
@pcfens
Copy link
Owner

pcfens commented Apr 9, 2016

I'll push a new release up to the forge on Monday morning Let me know if you find something that needs fixing.

@liuyu
Copy link
Author

liuyu commented Apr 9, 2016

Thx very much , I take the time to test it again. It's working good for me too. Great!

@blysik
Copy link

blysik commented Apr 9, 2016

So interestingly, I'm trying this on a Mac OS X machine with 10.9.5 and ruby 2.0.0p481, and I'm seeing this problem again.

@pcfens
Copy link
Owner

pcfens commented Apr 10, 2016

What are your rubyversion and kernel facts?

@blysik
Copy link

blysik commented Apr 11, 2016

$ facter kernel
Darwin

$ facter rubyversion
2.0.0

I did some hacking to get Darwin to work properly. I am confused that it's giving this behavior, with ruby 2.0.0.

@pcfens
Copy link
Owner

pcfens commented Apr 11, 2016

I don't know enough about Ruby to really know why behavior would change. I was more interested in what you kernel fact showed since Darwin should have failed with an error.

Is it idempotent if you change the last line in the template to <%= @filebeat_config.to_yaml.sort() %>?

@pcfens
Copy link
Owner

pcfens commented Apr 11, 2016

I'm not able to replicate this one on Ubuntu with ruby 2.0p484. Did you have any luck? I'm holding back pushing the fix to the forge until we're pretty sure it's not going to create more issues.

@blysik
Copy link

blysik commented Apr 11, 2016

(I've added support for Darwin myself. I'm hopeful I will be able to contribute back up.)

For now, I'm treating any Darwin like a ruby1.8.7 host. I'm hopeful when I can move to puppet4, this error will go away.

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

No branches or pull requests

3 participants