Keepalived config not merged since you are specifying the json in the puppet hash variable

[victorgp]

I think the config json shouldn't be merged, but just applied as it is because you are defining the actual hash in the puppet variable. Why?
Because i've been having this issue:

This was my client config file

{
  "client": {
    "name": "myhost",
    "address": "123.123.123.123",
    "bind": "127.0.0.1",
    "keepalive": {
      "extra_useless_parameter": 100,
      "thresholds": {
        "warning": 300,
        "critical": 600
      },
      "handlers": [
        "default",
        "mailer"
      ],
      "occurrences": 5,
      "refresh": 2702
    },
  }
}

I wanted to remove that "extra_useless_parameter", so i removed it in puppet. This sensu-puppet module is aware that the config file values changed, so it notifies the sensu service and provokes a restart.
So far, so good, the problem is that the "sensu_client_config" resource that you created, is not removing that "extra_useless_value" because is doing a json merge between the old config file and the new values, therefore the state of the config file is exactly the same as the previous one. This is causing that everytime puppet runs, it notifies the sensu service and restart it forever and ever.

[ghost]

Could someone take a look at this?

[jamtur01]

I'll try to take a look at this on the weekend.

[jamtur01]

So I think this will create other problems - you're ignoring the fact this is a hash.

[ghost]

Thank you for looking into this.

@victorgp what do you think?

[victorgp]

@jamtur01 what do you mean with "the fact this is a hash"? in puppet you set the hash, and then the ruby code is merging the old one with the new one, if the old one is wrong (having a wrong hash key) and you want to remove that wrong key, if you do the merge, you can't remove it.

[jamtur01]

@victorgp Why not handle it like the existing custom method?

[victorgp]

Because if there is already a keepalived parameter, i think i should use that parameter if i want to set the keepalived options, i don't see why i should use the custom method for that.

[jamtur01]

@victorgp You misunderstand me - look at the logic in the custom method - https://github.com/sensu/sensu-puppet/blob/master/lib/puppet/provider/sensu_client_config/json.rb#L92

[ghost]

I don't see why this would cause a problem and why anything would have to merged; shouldn't all of the needed data be passed into the keepalive function?

Do you agree that there is a bug the way it is? What sort of problems would happen with this change exactly as it is?

We've been running this on a fairly large scale for at least a month with no ill-effects that we can tell.

[jamtur01]

My suggestion is that it's the same issue the logic the custom method is designed to address - why not use a consistent approach?

[ghost]

Wouldn't direct assignment be much faster than a merge and a delete? Maybe both methods could be changed to just assignment.

[jamtur01]

They are set this way because of sort order (this from memory - @rodjek @jlambert121 or @johnf might remember more). If you change the order of your JSON but not the content and directly assign then Sensu sees that as a change and restarts.

[jamtur01]

Okay - I can't replicate the prior issue. I'm going to merge this. Folks are welcome to ping me if this breaks stuff.

[victorgp]

Great, thank you @jamtur01