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

Fixing UI Installation #306

Closed
wants to merge 35 commits into from
Closed

Fixing UI Installation #306

wants to merge 35 commits into from

Conversation

miroswan
Copy link

@miroswan miroswan commented Apr 4, 2016

I believe /var/lib/consul needs to exist in order to symlink /opt/consul-webui/$VERSION to /var/lib/consul/ui. This was failing for me prior to adding the directory resource to the LWRP for web-ui installation.

@miroswan
Copy link
Author

miroswan commented Apr 5, 2016

Looks like the tests need to be fixed themselves.

@Ginja
Copy link
Contributor

Ginja commented Apr 5, 2016

Thanks for the PR! A few requests:

  • Could you change the directory resource so it's not hardcoded, but rather reflects where it's being symlinked (see below)?
  • Could you add a resource to remove the directory in action_remove?
  • If you wouldn't mind taking a shot at fixing the tests that'd be awesome, but don't sweat it.
base = ::File.dirname options[:symlink_to]
# => /var/lib/consul

directory base

@miroswan
Copy link
Author

miroswan commented Apr 7, 2016

I can take a shot at this later in the week.

@miroswan
Copy link
Author

I fixed my tests locally and updated travis.yml in attempt to fix the faraday situation. Don't think travis is configured to run subsequent builds against this PR so it's hard to tell if my changes will fix the faraday issue as well.

-- Edit --
Travis is updating, but bundle update did not help.

@miroswan
Copy link
Author

It appears that faraday 0.9.0 is having problems. When I bump it down to 0.8.0, the problem goes away, but then poise-boiler reverts to version 0. Seems like a strange dependency issue.

legal90 and others added 7 commits April 15, 2016 12:12
Since the ConsulInstallationWebui provider relies on non-core chef resources, we can't use an inline recipe.

```
1) ConsulCookbook::Provider::ConsulInstallationWebui webui installation
 Failure/Error:
   it do is_expected.to unzip_zipfile('consul_0.6.4_web_ui.zip')
     .with(source: 'https://releases.hashicorp.com/consul/0.6.4/consul_0.6.4_web_ui.zip',
           path: '/opt/consul-webui/0.6.4')
   end

 NoMethodError:
   consul_installation[0.6.4] (/Users/user/github/chef-consul/test/spec/libraries/consul_installation_webui_spec.rb line 12) had an error: NoMethodError: No resource or method named `zipfile' for `ConsulCookbook::Provider::ConsulInstallationWebui ""'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/dsl/recipe.rb:102:in `rescue in method_missing'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/dsl/recipe.rb:99:in `method_missing'
 # ./libraries/consul_installation_webui.rb:50:in `block in action_create'
 # ./libraries/consul_installation_webui.rb:45:in `action_create'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/provider.rb:144:in `run_action'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource.rb:596:in `run_action'
 # ./.bundle/ruby/2.2.0/gems/chefspec-4.6.0/lib/chefspec/extensions/chef/resource.rb:22:in `run_action'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/runner.rb:73:in `run_action'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/runner.rb:105:in `block (2 levels) in converge'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/runner.rb:105:in `each'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/runner.rb:105:in `block in converge'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/resource_list.rb:84:in `block in execute_each_resource'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/stepable_iterator.rb:116:in `call'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/stepable_iterator.rb:116:in `call_iterator_block'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/stepable_iterator.rb:85:in `step'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/stepable_iterator.rb:104:in `iterate'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/stepable_iterator.rb:55:in `each_with_index'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/resource_collection/resource_list.rb:82:in `execute_each_resource'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/runner.rb:104:in `converge'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/client.rb:668:in `block in converge'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/client.rb:663:in `catch'
 # ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/client.rb:663:in `converge'
 # ./.bundle/ruby/2.2.0/gems/chefspec-4.6.0/lib/chefspec/solo_runner.rb:126:in `converge'
 # ./test/spec/libraries/consul_installation_webui_spec.rb:17:in `block (3 levels) in <top (required)>'
 # ------------------
 # --- Caused by: ---
 # NoMethodError:
 #   undefined method `zipfile' for #<ConsulCookbook::Provider::ConsulInstallationWebui:0x007fc246a8ee80>
 #   ./.bundle/ruby/2.2.0/gems/chef-12.8.1/lib/chef/dsl/recipe.rb:100:in `method_missing'
```
Fix consul logging for sysvinit provider
The poise inversion system needs to be added to consul_service otherwise windows tests will not pass
# Referencing https://github.com/nomad/shenzhen/issues/96 to
# fix build issue regarding the faraday gem
group :faraday do
gem 'faraday', '~> 0.8.0'

This comment was marked as outdated.

@shortdudey123
Copy link
Contributor

Tests are correctly fixed in #305

johnbellone and others added 4 commits April 18, 2016 21:41
Base name should be appended by "386" for all x86 platforms, including "i686".
consul_installation: Fix binary base url for Linux x86
Prevent "consul" service to be restarted on update
@shortdudey123
Copy link
Contributor

@miroswan can you rebase on master? several tests have been fixed causing your merge conflicts

@miroswan
Copy link
Author

miroswan commented May 6, 2016

Hadn't checked in on this in a while. I'll take a look at this soon.

@miroswan
Copy link
Author

miroswan commented May 6, 2016

I rebased as requested, but it appears that some previous commits from upstream (for which I am not the author) have snuck into my PR. Outside of that, the tests are passing.

@miroswan
Copy link
Author

miroswan commented May 9, 2016

I think I'm going to do a fresh PR since it's a small change.

@miroswan miroswan closed this May 9, 2016
@miroswan
Copy link
Author

miroswan commented May 9, 2016

It appears that someone added my fix from a different branch. Just waiting for the next release to the supermarket now.

@gdavison
Copy link
Contributor

gdavison commented May 9, 2016

Yeah, I hadn't noticed you'd submitted a PR when I created mine. If you had anything in your PR that you think should be added, go ahead.

@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.

6 participants