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

Problems with package install #389

Closed quulah closed 7 years ago

quulah commented 7 years ago

I've packaged consul as a .deb package, and have it in an internal repository. I've been trying to persuade this cookbook to install it by using the consul_installation with the package provider.

There's little documentation on the subject, and seems like the tests are limited too. The best I've got so far is giving the following options based on the suggestion in #383.

default['consul']['installation']['provider'] = 'package'
default['consul']['options']['program'] = '/usr/bin/consul'
default['consul']['options']['package_source'] = nil
default['consul']['options']['package_checksum'] = nil

But this results in a warning about new_resource.options and NilClass error:

       [2017-01-04T14:55:07+00:00] WARN: /tmp/kitchen/cache/cookbooks/consul/libraries/consul_installation_package.rb:39:in `block (2 levels) in action_create': property options is declared in both apt_package[consul] and #<ConsulCookbook::Provider::ConsulInstallationPackage:0x00000003b7a9a0>. Use new_resource.options instead. At /tmp/kitchen/cache/cookbooks/consul/libraries/consul_installation_package.rb:39:in `block (2 levels) in action_create'

           ================================================================================
           Error executing action `create` on resource 'consul_installation[0.7.0]'
           ================================================================================

           NoMethodError
           -------------
           undefined method `[]' for nil:NilClass

           Cookbook Trace:
           ---------------
           /tmp/kitchen/cache/cookbooks/consul/libraries/consul_installation_package.rb:39:in `block (2 levels) in action_create'
           /tmp/kitchen/cache/cookbooks/consul/libraries/consul_installation_package.rb:38:in `block in action_create'
           /tmp/kitchen/cache/cookbooks/poise/files/halite_gem/poise/helpers/subcontext_block.rb:54:in `instance_eval'
           /tmp/kitchen/cache/cookbooks/poise/files/halite_gem/poise/helpers/subcontext_block.rb:54:in `subcontext_block'
           /tmp/kitchen/cache/cookbooks/poise/files/halite_gem/poise/helpers/notifying_block.rb:67:in `notifying_block'
           /tmp/kitchen/cache/cookbooks/consul/libraries/consul_installation_package.rb:37:in `action_create'

           Resource Declaration:
           ---------------------
           # In /tmp/kitchen/cache/cookbooks/consul/recipes/default.rb

            50: install = consul_installation node['consul']['version'] do |r|
            51:   if node['consul']['installation']
            52:     node['consul']['installation'].each_pair { |k, v| r.send(k, v) }
            53:   end
            54: end

           Compiled Resource:
           ------------------
           # Declared in /tmp/kitchen/cache/cookbooks/consul/recipes/default.rb:50:in `from_file'

           consul_installation("0.7.0") do
             provider ConsulCookbook::Provider::ConsulInstallationPackage
             action [:create]
             retries 0
             retry_delay 2
             default_guard_interpreter :default
             declared_type :consul_installation
             cookbook_name "consul"
             recipe_name "default"
             version "0.7.0"
             program "/usr/bin/consul"
           end

The program parameter seems to go to the resource properly, but I'm unsure how the rest of the options are interpreted. The error points to line 39.

Do the options from apt_package interfere as the warning suggests?

legal90 commented 7 years ago

@coderanger Could you please assist here? Do we actually need to replace options with new_resource.options in poise-driven providers?

quulah commented 7 years ago

As a quick hack I change the cached version of the cookbook to not give stuff from the options directly to the package resource, but used a temporary parameter in between.

The source parameter wouldn't work in this case any way since apt_package doesn't support it. dpkg_package is required in that case.

Removing checksum was also necessary, as the package resource doesn't have it.

quulah commented 7 years ago

It now installs in my particular case from the repository, but this might require a bit more thought. :)

legal90 commented 7 years ago

Confirmed - the provider package for consul_installation resource doesn't work at the moment:

1. Name "options" could not be used in the block of a built-in package resource:

https://github.com/johnbellone/consul-cookbook/blob/3e123c2/libraries/consul_installation_package.rb#L36-L45 Debugging:

# ...
def action_create
  notifying_block do
    pp options                # => {\"version\"=>\"0.7.2\", \"package_name\"=>\"consul\"}"
    pp new_resource.options   # => {}

    package options[:package_name] do
      pp options                # => nil
      pp new_resource.options   # => {}

      source options[:package_source]
      version options[:version]
      action :upgrade
    end
  end
end

It is nil because the name "options" is already in use as an attribute of package : https://github.com/chef/chef/blob/master/lib/chef/resource/package.rb#L39

@coderanger @johnbellone Do you have any ideas how it could be fixed or worked around?

2. Attribute checksum is not supported for "package" resource

It causes the following error on RHEL:

           NoMethodError
           -------------
           undefined method `checksum' for Chef::Resource::YumPackage

It is a custom attribute available only in "windows_package" resource, so it should be removed.

3. Action :uninstall is not supported for "package" resource

It causes the following exception:

Chef::Exceptions::ValidationFailed
           ----------------------------------
           Option action must be equal to one of: nothing, install, upgrade, remove, purge, reconfig, lock, unlock!  You passed :uninstall.

It is supported only in "chocolatey_package", but is already deprecated in favor of :remove. We should use :remove action instead.

quulah commented 7 years ago

Haven't had time to test it yet, sorry. I'll try as soon as I can.

Looking at the changes, it looks better, but I think it will still fail on Debian based installs when you specify the source. I don't know what's the reason behind it, but if you do so, you get this error message from the apt_package resource instead of it just changing to dpkg_package by itself.

legal90 commented 7 years ago

@quulah I've appended the PR with a new commit, adding a new option - package_provider. So now you can force it to use "dpkg" instead of "apt":

node.default['consul']['options']['package_provider'] = Chef::Provider::Package::Dpkg
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.