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

do not delete consul data dir #213

Merged

Conversation

scalp42
Copy link
Contributor

@scalp42 scalp42 commented Aug 27, 2015

Hi @johnbellone,

Disabling Consul service deletes the data dir (which could contain a lot of other things).

I propose that we dont remove the data dir by default which is very opinionated.

Plus, it does not appear to be actually working (assuming a data dir on /consul):

==> zs2:     ================================================================================
==> zs2:     Error executing action `disable` on resource 'consul_service[consul]'
==> zs2:     ================================================================================
==> zs2:
==> zs2:     Errno::ENOTEMPTY
==> zs2:     ----------------
==> zs2:     directory[/consul] (/var/chef/cache/cookbooks/consul/libraries/consul_service.rb line 171) had an error: Errno::ENOTEMPTY: Directory not empty @ dir_s_rmdir - /consul
==> zs2:
==> zs2:     Cookbook Trace:
==> zs2:     ---------------
==> zs2:     /var/chef/cache/cookbooks/poise/files/halite_gem/poise/helpers/notifying_block.rb:69:in `notifying_block'
==> zs2:     /var/chef/cache/cookbooks/consul/libraries/consul_service.rb:162:in `action_disable'
==> zs2:
==> zs2:     Resource Declaration:
==> zs2:     ---------------------
==> zs2:     # In /var/chef/cache/cookbooks/gz-consul/recipes/default.rb
==> zs2:
==> zs2:      50: consul_service node['consul']['service_name'] do |r|
==> zs2:      51:   data_dir node['gz-consul']['config']['data_dir']
==> zs2:      52:   user node['consul']['service_user']
==> zs2:      53:   group node['consul']['service_group']
==> zs2:      54:   version node['consul']['version']
==> zs2:      55:   config_file node['gz-consul']['config_path']
==> zs2:      56:   node['consul']['service'].each_pair { |k, v| r.send(k, v) }
==> zs2:      57:   if node['gz-consul']['nuke']
==> zs2:      58:     action [:disable, :stop]
==> zs2:      59:   else
==> zs2:      60:     subscribes :restart, %|file[#{node['gz-consul']['config_path']}|, :delayed
==> zs2:      61:     action service_status
==> zs2:      62:   end
==> zs2:      63:   ignore_failure true if node['gz-consul']['nuke']

Thanks for considering it, deleting the data dir should be left to the user.

@scalp42
Copy link
Contributor Author

scalp42 commented Aug 27, 2015

We have the same issue if we define a check within consul config dir:

==> zs1:     ================================================================================
==> zs1:     Error executing action `disable` on resource 'consul_service[consul]'
==> zs1:     ================================================================================
==> zs1:
==> zs1:     Errno::ENOTEMPTY
==> zs1:     ----------------
==> zs1:     directory[/etc/consul] (/var/chef/cache/cookbooks/consul/libraries/consul_service.rb line 167) had an error: Errno::ENOTEMPTY: Directory not empty @ dir_s_rmdir - /etc/consul
==> zs1:
==> zs1:     Cookbook Trace:
==> zs1:     ---------------
==> zs1:     /var/chef/cache/cookbooks/poise/files/halite_gem/poise/helpers/notifying_block.rb:69:in `notifying_block'
==> zs1:     /var/chef/cache/cookbooks/consul/libraries/consul_service.rb:162:in `action_disable'
==> zs1:
==> zs1:     Resource Declaration:
==> zs1:     ---------------------
==> zs1:     # In /var/chef/cache/cookbooks/gz-consul/recipes/default.rb
==> zs1:
==> zs1:      50: consul_service node['consul']['service_name'] do |r|
==> zs1:      51:   data_dir node['gz-consul']['config']['data_dir']
==> zs1:      52:   user node['consul']['service_user']
==> zs1:      53:   group node['consul']['service_group']
==> zs1:      54:   version node['consul']['version']
==> zs1:      55:   config_file node['gz-consul']['config_path']
==> zs1:      56:   node['consul']['service'].each_pair { |k, v| r.send(k, v) }
==> zs1:      57:   if node['gz-consul']['nuke']
==> zs1:      58:     action [:disable, :stop]
==> zs1:      59:   else
==> zs1:      60:     subscribes :restart, %|file[#{node['gz-consul']['config_path']}|, :delayed
==> zs1:      61:     action service_status
==> zs1:      62:   end
==> zs1:      63:   ignore_failure true if node['gz-consul']['nuke']
==> zs1:
==> zs1:     Compiled Resource:
==> zs1:     ------------------
==> zs1:     # Declared in /var/chef/cache/cookbooks/gz-consul/recipes/default.rb:50:in `from_file'
==> zs1:
==> zs1:     consul_service("consul") do
==> zs1:       action [:disable, :stop]
==> zs1:       ignore_failure true
==> zs1:       retries 0
==> zs1:       retry_delay 2
==> zs1:       default_guard_interpreter :default
==> zs1:       declared_type :consul_service
==> zs1:       cookbook_name "gz-consul"
==> zs1:       recipe_name "default"
==> zs1:       data_dir "/consul"
==> zs1:       user "consul"
==> zs1:       group "consul"
==> zs1:       version "0.5.2"
==> zs1:       config_file "/etc/consul.json"
==> zs1:       install_method "binary"
==> zs1:       config_dir "/etc/consul"
==> zs1:       binary_url "https://dl.bintray.com/mitchellh/consul/%{filename}.zip"
==> zs1:       source_url "https://github.com/hashicorp/consul"
==> zs1:       install_path "/opt"
==> zs1:     end
==> zs1:
==> zs1:
==> zs1:
==> zs1: * consul_service[consul] action stop
==> zs1:
==> zs1: * poise_service[consul] action stop

@johnbellone
Copy link
Contributor

You're probably not going to want to delete the configuration directory, too?

@scalp42
Copy link
Contributor Author

scalp42 commented Aug 28, 2015

Correct, let me cherry pick our commit @johnbellone

@scalp42
Copy link
Contributor Author

scalp42 commented Aug 28, 2015

Here you go @johnbellone, thanks for looking at it!

johnbellone added a commit that referenced this pull request Aug 28, 2015
@johnbellone johnbellone merged commit ff4445d into sous-chefs:master Aug 28, 2015
@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants