Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

Chef inadvertently manages shared config instead of default #20

Closed
swalberg opened this issue Jun 29, 2015 · 6 comments
Closed

Chef inadvertently manages shared config instead of default #20

swalberg opened this issue Jun 29, 2015 · 6 comments

Comments

@swalberg
Copy link

Running in file mode with my options specified in the node at normal level:

   "exhibitor": {
      "cli": {
        "fsconfigdir": "/mnt/exhibitor_state/"
       }
    },

Chef keeps managing the shared config file:

Recipe: exhibitor::default
  * directory[/opt/exhibitor] action create (up to date)
  * directory[/tmp/zookeeper] action create (up to date)
  * directory[/tmp/zookeeper_log_indexes] action create (up to date)
  * file[/mnt/exhibitor_state/exhibitor.properties] action create
    - update content in file /mnt/exhibitor_state/exhibitor.properties from 75c634 to f6cfc4
    --- /mnt/exhibitor_state/exhibitor.properties   2015-06-29 12:47:42.339391000 -0700
    +++ /mnt/exhibitor_state/.exhibitor.properties20150629-29319-7qzuj3 2015-06-29 12:48:48.461299000 -0700
    @@ -1,49 +1,15 @@

Comments at https://github.com/SimpleFinance/chef-exhibitor/blob/master/attributes/default.rb#L34 mention the hash should be managing the default config, but the recipe manages the shared config in https://github.com/SimpleFinance/chef-exhibitor/blob/master/recipes/default.rb#L71-L74:

  node.default[:exhibitor][:cli][:fsconfigdir] = '/tmp'
  node.default[:exhibitor][:cli][:fsconfigname] = 'exhibitor.properties'

  file ::File.join(node[:exhibitor][:cli][:fsconfigdir], node[:exhibitor][:cli][:fsconfigname]) do

I'm not sure of the purpose of the first 2 lines because they won't get used since I'm applying my config at the normal level. However I think the file resource should be node[:exhibitor][:cli][:defaultconfig] instead of what's there.

@jeffbyrnes
Copy link
Contributor

@swalberg sorry for the slow reply. The “Running Exhibitor” docs make this very confusing, but I’ll try to sort it out in the context of this cookbook.

To explain a bit:

  • The node[:exhibitor][:cli][:defaultconfig] attribute is actually a CLI argument that should equal the path to the default config file, which, using this cookbook’s defaults, will end up at /opt/exhibitor/exhibitor.properties.
  • The two attributes you refer to are for the file-based config, which, using a shared filesystem, could be a shared config

Perhaps what you’re really pointing out is that the content of that file should not be Chef-managed? And instead, the defaults should be set, then this file left alone for Exhibitor to handle?

I.e., lines 75-80 of default.rb should not exist, and just allow Exhibitor to handle that file?

@swalberg
Copy link
Author

Hi Jeff, yes, I think we're talking about the same thing, just approaching from different sides. If Chef manages that shared state file then it's always going to reset it to having a single node. Since exhibitor watches for changes to that file, the update will break the cluster.

Removing those lines will probably fix the problem but won't create the default file. The problem, as I see it, is that the code is overloading the meaning of the same attribute but forgetting that other things in the Chef attribute priority scheme may come together to override the intent.

Or, put another way, the code switches between managing the default file and shared state file by setting a node.default, which is totally ignored if you set the shared state attribute at the node because it's set at the normal level, which supersedes default.

@jeffbyrnes
Copy link
Contributor

@swalberg yeah, I think we’re pretty close to understanding each other.

The default config file is always created, at the end of the recipe, and Exhibitor uses that as a kind of “bootstrap” for its own shared config if it doesn’t exist.

If that shared config does exist, it uses that to override what’s in the default config, which includes things like the other Exhibitor/ZK nodes in your cluster.

As for the attributes, I think it’s just that the node[:exhibitor][:config] attribute is overloaded: it’s being used for two separate config files, when it should just be used for the default config, and leave the shared config for Exhibitor to create.

Perhaps best would be to just ensure that the shared config exists, and is owned by Exhibitor, and otherwise leave it be? E.g.,

file ::File.join(node[:exhibitor][:cli][:fsconfigdir], node[:exhibitor][:cli][:fsconfigname]) do
  owner node[:exhibitor][:user]
  mode 00600
end

@jeffbyrnes
Copy link
Contributor

Perhaps f3c489a does the trick? If not, I’ll revert it.

@jeffbyrnes jeffbyrnes self-assigned this Sep 16, 2015
@swalberg
Copy link
Author

Yes, that should do it. Thanks!

@jeffbyrnes
Copy link
Contributor

@swalberg sorry this took so long; shipped 0.6.0 to Supermarket just now!

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

No branches or pull requests

2 participants