sous-chefs / consul

Development repository for the consul cookbook
https://supermarket.chef.io/cookbooks/consul
Apache License 2.0
192 stars 244 forks source link

Have the vault and consul cookbooks been abandoned? #493

Closed empath closed 5 years ago

empath commented 6 years ago

Seems like there's very little activity and they're both several versions behind.

Are there any active forks that have done any work to update to the latest versions?

I'm probably going to fork and start the work myself, but I'd like not to duplicate effort if someone has already done it or is already working on it.

gdavison commented 6 years ago

Hi @empath,

I've started (very slowly, admittedly) on adding the new v1.x parameters in a backwards-compatible way

johnbellone commented 6 years ago

I have been accepting collaborators and maintainers for the past several months. That includes the management of the releases for the two cookbooks. @gdavison has offered to help out. And we can perform a release if that's what is needed. But I was under the impression that there was some more work that needed to go on before doing it.

shayangz commented 6 years ago

A release would help a lot if possible specially for supporting missing config parameters.

scalp42 commented 6 years ago

The issue is that old parameters were renamed as well (statsd_addr vs statsd_address). I think we should just rescue missing/invalid params and trust the user, instead of updating cookbook all the time IMO.

I'd not mind putting a PR together.

In the meantime, I'd port over the default recipe from consul cookbook and skip the consul_config recipe or write whatever needed onfig to ['consul']['service']['config_dir'] and keep the defaults as minimal as possible. The new config in conf.d will override whatever defaults.

@shayangz (tweak if using Vault etc):

# if you want extra node metadata, feel free to skip otherwise
if node['ec2']
  node[cookbook_name]['config'] = node[cookbook_name]['config'].merge(
    node_meta: {
      instance_type: node['ec2']['instance_type'],
      local_hostname: node['ec2']['local_hostname'],
      local_ipv4: node['ec2']['local_ipv4'],
      instance_id: node['ec2']['instance_id'],
      region: node['ec2']['region'],
      availability_zone: node['ec2']['availability_zone'],
      public_ipv4: node.to_h.dig('ec2', 'public_ipv4'),
      public_hostname: node.to_h.dig('ec2', 'public_hostname'),
      chef_environment: node.chef_environment,
      datacenter: node[cookbook_name]['config']['datacenter'],
    }
  )
else
  node[cookbook_name]['config'] = node[cookbook_name]['config'].merge(
    node_meta: {
      chef_environment: node.chef_environment,
      datacenter: node[cookbook_name]['config']['datacenter'],
    }
  )
end

blacklist = ['acl_master_token']
chef_consul_config = Hash[node[cookbook_name]['config'].reject do |k|
  blacklist.include?(k)
end.sort_by { |key, val| key.to_s }]

directory '/etc/consul' do
  owner node['consul']['service_user'] if "grep #{node['consul']['service_user']} /etc/passwd"
  group node['consul']['service_group'] if "grep #{node['consul']['service_group']} /etc/group"
  not_if 'test -d /etc/consul'
end

# if including consul cookbook, put it in node['consul']['service']['config_dir']
# file ::File.join(node['consul']['service']['config_dir'], 'chef.json') do

file ::File.join('/etc/consul', 'chef.json') do
  owner node['consul']['service_user'] if "grep #{node['consul']['service_user']} /etc/passwd"
  group node['consul']['service_group'] if "grep #{node['consul']['service_group']} /etc/group"
  mode '0640'
  content lazy { JSON.pretty_generate(chef_consul_config , quirks_mode: true) }
  action node[cookbook_name]['nuke'] ? :delete : :create
  notifies :reload, "consul_service[#{node['consul']['service_name']}]", :delayed
end

Hopefully it helps!

Remind me of the good old days of ⚙️ 0️⃣ 💪

gdavison commented 6 years ago

496 is work-in-progress

scalp42 commented 6 years ago

@gdavison thanks for the PR.

I'd be more leaning toward letting the user handle the actual JSON rather than creating new classes per major (I'm guessing) Consul versions.

Would greatly reduce the churn as well if new attributes are added in minor versions.

Or best of both worlds I guess, simply adding a new consul_custom_config resource. What do you think?

pierresouchay commented 6 years ago

@scalp42 Completely agree here.

@gdavison @johnbellone Can we help to release new versions of this Cookbook ?

We need those PR in a new release:

While the first one has been merged, no new version has been released

scalp42 commented 6 years ago

I believe there's still the issue of not handling multiple services in the consul_definition, see https://www.consul.io/docs/agent/services.html#multiple-service-definitions

The Connect support is just a few keys added to a JSON file at the end of the day. It's one of the very few cookbooks that I decided to fork as I don't see the progress going in the right direction to be honest and it's unfortunate.

Something something mongodb cookbook from back in the days.

gdavison commented 6 years ago

My personal preference is usually to have formal parameter definitions for config. However, Consul has a ton for settings, so I think what might make sense is to have some core configs handled by the cookbook, and allow other configs to be set in the wrapper cookbooks that use this one. Not sure what @johnbellone's perspective is, and I'll defer architectural decisions to him.

The easiest way to add arbitrary custom configs would be with a file or template resource, located in node['consul']['service']['config_dir'], though I haven't tried this myself. Consul will read any JSON or HCL file in the config directory.

damacus commented 5 years ago

Hi @scalp42 would you like to help maintain the consul cookbook on a more formal basis, your PRs have been most helpful so far!

damacus commented 5 years ago

Closing as I think the question has been answered.

pierresouchay commented 5 years ago

@damacus we are not using the community cookbook for the same reasons as @scalp42 but we would be glad to help it improving and handle more easily new features of Consul

damacus commented 5 years ago

That would be awesome!

@pierresouchay please feel free to drop by the sous-chefs room in the Chef community Slack.

In the mean time I think we have a small contingent of people willing to moderise this cookbook.

scalp42 commented 5 years ago

I'm strongly considering forking the cookbook ala mongodb from back in the days. The new ACL model in Consul feels like a good time to tackle this, removing poise, properly using test-kitchen, etc etc.

I'm not even sure who's leading the development on this cookbook to be transparent as @pierresouchay and I have emitted the same concerns but bad patterns (the whole v0/v1 provider approach) were merged so it's disappointing.

damacus commented 5 years ago

@scalp42 feel free to do that right here. We're looking for new community contributors right now, and you seem like you know what your doing, and you're active.

lock[bot] commented 4 years ago

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.