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

web UI install missing since 1.0 #215

Closed
tomzo opened this issue Sep 11, 2015 · 18 comments
Closed

web UI install missing since 1.0 #215

tomzo opened this issue Sep 11, 2015 · 18 comments

Comments

@tomzo
Copy link
Contributor

tomzo commented Sep 11, 2015

I've been porting my wrapper cookbook to 1.0 of consul. I get an impression that installing UI package is now not supported.

@johnbellone please confirm if this is true. I could contribute it. How would you see it implemented? Do you think it should be another resource or rather an option in consul_service?

@johnbellone
Copy link
Contributor

@tomzo I think it should be a separate resource. A contribution would be greatly appreciated!

@scalp42
Copy link
Contributor

scalp42 commented Sep 11, 2015

@tomzo @johnbellone have a look at #216 might help

@tomzo
Copy link
Contributor Author

tomzo commented Sep 12, 2015

@scalp42 I see you added dependency on ark. I don't think it is consistent with how consul_service is installed.
I have something like in this in my wrapper cookbook:

libartifact_file "consul-ui-#{node['consul']['version']}" do
  artifact_name 'consul-ui'
  artifact_version node['consul']['version']
  owner node['consul']['service_user']
  group node['consul']['service_group']
  install_path node['ai_consul']['install_path']
  remote_checksum node['consul']['checksums']["#{node['consul']['version']}_web_ui"]
  remote_url "https://dl.bintray.com/mitchellh/consul/#{node['consul']['version']}_web_ui.zip"
end

I plan to move it into consul cookbook as new resource consul_ui. I'll submit a PR today or tomorrow.

@johnbellone
Copy link
Contributor

I hadn't thought of an attribute in the consul service resource. But I don't want to add a dependency on ark. I personally would probably create a consul ui service resource.

tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 12, 2015
@tomzo
Copy link
Contributor Author

tomzo commented Sep 12, 2015

@johnbellone I have created a consul_ui resource very similar to your consul_service, using libartifact_file. I tested it in vagrant deployment, seems fine.

However I tried writing a chefspec test in style that you have.

require 'spec_helper'
require_relative '../../../libraries/consul_ui'

describe ConsulCookbook::Resource::ConsulUI do
  step_into(:consul_ui)

  context 'consul ui install' do
    recipe do
      consul_ui 'myconsul-ui' do
        owner 'myconsul'
        group 'myconsul'
        version '0.5.1'
        install_path '/opt'
      end
    end

    it do is_expected.to create_libartifact_file('myconsul-ui')
      .with(owner: 'myconsul',group: 'myconsul',
            remote_url: "https://dl.bintray.com/mitchellh/consul/0.5.1_web_ui.zip",
            install_path: '/opt')
    end
  end
end

But keep getting this error:

consul-cookbook$ chef exec bundle exec rspec
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 38454

================================================================================
Error executing action `install` on resource 'consul_ui[myconsul-ui]'
================================================================================

NoMethodError
-------------
No resource or method named `libartifact_file' for `ConsulCookbook::Provider::ConsulUI ""'

Resource Declaration:
---------------------
# In /home/tomzo/code/open/consul-cookbook/test/spec/libraries/consul_ui_spec.rb

  9:       consul_ui 'myconsul-ui' do
 10:         owner 'myconsul'
 11:         group 'myconsul'
 12:         version '0.5.1'
 13:         install_path '/opt'
 14:       end
 15:     end

Compiled Resource:
------------------
# Declared in /home/tomzo/code/open/consul-cookbook/test/spec/libraries/consul_ui_spec.rb:9:in `block (3 levels) in <top (required)>'

consul_ui("myconsul-ui") do
  action [:install]
  retries 0
  retry_delay 2
  default_guard_interpreter :default
  declared_type :consul_ui
  owner "myconsul"
  group "myconsul"
  version "0.5.1"
  install_path "/opt"
end

Have you had a problem like this here?
I noticed you have no chefspec tests for consul_service, was that the reason to skip them?

@johnbellone
Copy link
Contributor

Let me take a look real quick.

@johnbellone
Copy link
Contributor

I am going to loop in @coderanger on this one.

It looks like it may be possibly due to how the Poise Service library is working in the background. I was able to add a stub method on the Chef::Provider class and get past the error. But this obviously means some of the resource's methods and values would also need to be stubbed. Since I also own the libartifact cookbook I tried locally to use a Provider (instead of a Fused provider) and had no luck either.

I went down the thought path that this was possibly because actions isn't being called (and PoiseService::ServiceMixin is called after include Poise) but that didn't seem to change anything either. I am stumped, but I'll take a look into this later this afternoon.

@coderanger
Copy link
Contributor

In general you can't use stuff like ServiceMixin with the fused mode because you need to get the mixin included in the provider and fused mode doesn't expose that you.

@johnbellone
Copy link
Contributor

@coderanger this isn't the mixin using fused mode.

The library cookbook libartifact has a fused resource which isn't adding a matcher. When we call that resource in this cookbook service provider it fails only it RSpec with the above errors.

I tried ditching libartifact fused resource and no luck. It looks like the marchers aren't being added. They are added automatically after 2.2?

@coderanger
Copy link
Contributor

So the specific spec files aren't in git that I can see, but I think this is because you are using normal style cookbooks with the Halite spec helper. The Halite helper doesn't know to load dependent cookbooks because with full-bore Halite stuffs you are using normal requires for dependent stuff. If you set up ChefSpec's Berkshelf integration it should handle loading dependent cookbooks, or you depending on the setup you could just require '../[etc]/../libartifact_file.rb' directly.

@johnbellone
Copy link
Contributor

I can push up the changes, but require 'chefspec/berkshelf' still results in a failure. Hm.

@johnbellone
Copy link
Contributor

Take a look at spec_helper.rb - I assume that's what you meant @coderanger ?

@coderanger
Copy link
Contributor

So that will get the cookbooks loaded, the next problem is that you need to tell the fake Chef run to actually load that cookbook. If you use the recipe 'name' form instead of recipe do .. end that triggers the normal loading behavior. It's on my list to improve this, currently Halite-based cookbooks are super special cased (https://github.com/poise/halite/blob/master/lib/halite/spec_helper/runner.rb#L69-L83), and even then it needs a bunch of stuff from poise-boiler to work right.

@johnbellone
Copy link
Contributor

That seemed to do it.

@tomzo
Copy link
Contributor Author

tomzo commented Sep 14, 2015

If you use the recipe 'name' form instead of recipe do .. end that triggers the normal loading behavior.

I could do this with recipe 'name' style but it means I need to add a test cookbook with recipe content?
@johnbellone I recall there were test cookbooks here. I guess you removed them in 1.0, would you be fine if I added a consul_test cookbook back?

@johnbellone
Copy link
Contributor

@tomzo Yup, feel free, you'll need to add it in a group :test, :integration block in the Berksfile, too.

tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 15, 2015
tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 15, 2015
tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 15, 2015
tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 15, 2015
tomzo added a commit to ai-traders/consul-cookbook that referenced this issue Sep 15, 2015
@tomzo
Copy link
Contributor Author

tomzo commented Sep 15, 2015

John,
Thanks for help.
I created a PR #218

@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

No branches or pull requests

4 participants