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

Introduce user provider and resource #268

Merged
merged 1 commit into from Jan 6, 2015

Conversation

@martinb3
Copy link
Collaborator

martinb3 commented Dec 31, 2014

  • Fixes #269, adds an elasticsearch_user resource with provider that creates a user, group, and removes the automatic homedir
  • Pins chef to 11.16.4 since chef 12 and many libraries like poise aren't compatible yet
  • Adds an elasticsearch_test kitchenCI suite that tests edge cases, so the default test suite can test the expected way people use the cookbook
  • Tweak rubocop rules to avoid style complaints about methods that are too complex (controversial rule) and compact class styles (also controversial at the moment)
  • Updated README.md with an example of how to use the elasticsearch_user resource
@martinb3 martinb3 force-pushed the martinb3-cookbooks:next_martinb3 branch from 0695427 to 6c57315 Jan 2, 2015
@martinb3

This comment has been minimized.

Copy link
Collaborator Author

martinb3 commented Jan 2, 2015

Force pushed a new commit message into my branch, to add the fixes #XXX to the commit message.

@karmi karmi added the enhancement label Jan 4, 2015
class Resource::ElasticsearchUser < Resource
include Poise

actions(:create, :remove)

This comment has been minimized.

Copy link
@karmi

karmi Jan 4, 2015

Member

What does everybody think about getting rid of the () parentheses for these DSL-like notations? I guess it would look & feel cleaner?

This comment has been minimized.

Copy link
@martinb3

martinb3 Jan 5, 2015

Author Collaborator

I would recommend leaving them, since we're emulating a style that other users and chef community people will recognize. It seems like they are most often omitted in LWRPs and not omitted in HWRPs. I don't feel that strongly about it though, if everyone else feels differently. I just want to be sure what we write is approachable enough that we can get good contributions from the community.

not_if { ::File.symlink?("#{computed_homedir}") }
only_if { ::File.directory?("#{computed_homedir}") }
end
end # end notifying block

This comment has been minimized.

Copy link
@karmi

karmi Jan 4, 2015

Member

I think I'd prefer skipping the # end ... comments, since nested do...end blocks are a reality in Ruby anyway, everybody should be able to read the code even without the helping comments? If the nesting would make the code really incomprehensible, it's probably a sign that some code should be extracted to a method, lambda, etc?

@@ -0,0 +1,15 @@
# create user with crazy override options

This comment has been minimized.

Copy link
@karmi

karmi Jan 4, 2015

Member

Can we define "crazy" or be more specific here? :)

@karmi

This comment has been minimized.

Copy link
Member

karmi commented Jan 4, 2015

When I run integration tests, I'm getting the following exception:

$ time bundle exec rake kitchen:default-centos-65
...
Chef Client failed. 4 resources updated in 24.21197519 seconds
       [2015-01-04T18:17:17+00:00] ERROR: elasticsearch_user[elasticsearch] (elasticsearch::default line 10) had an error: NoMethodError: undefined method `new_resource' for Chef::Resource::ElasticsearchUser
       [2015-01-04T18:17:17+00:00] FATAL: Chef::Exceptions::ChildConvergeError: Chef run process exited unsuccessfully (exit code 1)

Full trace:

       [2015-01-04T18:16:53+00:00] INFO: *** Chef 11.16.4 ***
       # ...
       Recipe: elasticsearch::default
         * elasticsearch_user[elasticsearch] action create[2015-01-04T18:17:17+00:00] INFO: Processing elasticsearch_user[elasticsearch] action create (elasticsearch::default line 10)


           ================================================================================

       Error executing action `create` on resource 'elasticsearch_user[elasticsearch]'
           ================================================================================

           NoMethodError
           -------------
           undefined method `new_resource' for Chef::Resource::ElasticsearchUser

           Cookbook Trace:
           ---------------
           /tmp/kitchen/cache/cookbooks/elasticsearch/libraries/user.rb:15:in `block in <class:ElasticsearchUser>'
           /tmp/kitchen/cache/cookbooks/poise/libraries/lazy_default.rb:38:in `instance_eval'
           /tmp/kitchen/cache/cookbooks/poise/libraries/lazy_default.rb:38:in `set_or_return'
           /tmp/kitchen/cache/cookbooks/poise/libraries/lwrp_polyfill.rb:42:in `block in attribute'
           /tmp/kitchen/cache/cookbooks/elasticsearch/libraries/user.rb:36:in `block (3 levels) in action_create'
           /tmp/kitchen/cache/cookbooks/elasticsearch/libraries/user.rb:34:in `block (2 levels) in action_create'
           /tmp/kitchen/cache/cookbooks/poise/libraries/subcontext_block.rb:79:in `instance_eval'
           /tmp/kitchen/cache/cookbooks/poise/libraries/subcontext_block.rb:79:in `subcontext_block'
           /tmp/kitchen/cache/cookbooks/poise/libraries/notifying_block.rb:33:in `notifying_block'
           /tmp/kitchen/cache/cookbooks/elasticsearch/libraries/user.rb:27:in `block in action_create'
           /tmp/kitchen/cache/cookbooks/elasticsearch/libraries/user.rb:26:in `action_create'

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

            10: elasticsearch_user 'elasticsearch'

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

           elasticsearch_user("elasticsearch") do
             action :create
             retries 0
             retry_delay 2
             guard_interpreter :default
             cookbook_name "elasticsearch"
             recipe_name "default"
             username "elasticsearch"
             groupname "elasticsearch"
             comment "Elasticsearch User"
           end
@karmi

This comment has been minimized.

Copy link
Member

karmi commented Jan 5, 2015

It seems like some Rspec-based gems are not depending properly on the rspec gem, because this happens:

$ bundle exec rake -T
rake aborted!
LoadError: cannot load such file -- rspec/core/rake_task
/Users/karmi/Playground/ElasticSearch/Varia/cookbook-elasticsearch-new/Rakefile:49:in `<top (required)>'

As soon as I add rspec to the group :unit in the Gemfile, it works.

@karmi

This comment has been minimized.

Copy link
Member

karmi commented Jan 5, 2015

When I try to run the RSpec unit tests, I'm getting the following error:

$ bundle exec rake spec
/Users/karmi/.rbenv/versions/2.2.0/bin/ruby -I/Users/karmi/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/rspec-core-3.1.7/lib:/Users/karmi/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/rspec-support-3.1.2/lib /Users/karmi/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/rspec-core-3.1.7/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb test/unit
/Users/karmi/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/chefspec-0.0.1/lib/chefspec.rb:1:in `require': cannot load such file -- chef (LoadError)
    from /Users/karmi/.rbenv/versions/2.2.0/lib/ruby/gems/2.2.0/gems/chefspec-0.0.1/lib/chefspec.rb:1:in `<top (required)>'
    from /Users/karmi/Playground/ElasticSearch/Varia/cookbook-elasticsearch-new/test/unit/spec/spec_helper.rb:3:in `require'
@martinb3 martinb3 force-pushed the martinb3-cookbooks:next_martinb3 branch 2 times, most recently from 3b28ecb to 04d010a Jan 5, 2015
@martinb3

This comment has been minimized.

Copy link
Collaborator Author

martinb3 commented Jan 5, 2015

I fixed the unit tests and rspec dependency (I guess chefspec doesn't depend on it correctly); I'd recommend clearing out any Gemfile.lock and running those again. Regarding the kitchen tests, I am running them with bundle exec kitchen test default-centos-65. I found one mistake there, with a wrong variable name, and fixed it (the error is misleading; the way the DSL is evaluated means that new_resource.bad_variable will complain about new_resource not existing on the right object when bad_variable doesn't exist).

In short, try it again? :) Thanks!

@karmi karmi force-pushed the elastic:next branch from 2e2958f to dbd4238 Jan 5, 2015
@martinb3 martinb3 force-pushed the martinb3-cookbooks:next_martinb3 branch from 04d010a to 1d17218 Jan 6, 2015
martinb3 added a commit that referenced this pull request Jan 6, 2015
- Fixes #269, adds an `elasticsearch_user` resource with provider that creates a user, group, and removes the automatic homedir

- Pins chef to 11.16.4 since chef 12 and many libraries like poise aren't compatible yet, explicit depends on rspec

- Adds a unit test with chefspec matcher under `libraries/matchers.rb` for testing chef resources were created with correct data and actions

- Adds an `elasticsearch_test` kitchenCI suite that tests edge cases, so the default test suite can test the expected way people use the cookbook

- Tweak rubocop rules to avoid style complaints about methods that are too complex (controversial rule) and compact class styles (also controversial at the moment)

- Updated `README.md` with an example of how to use the `elasticsearch_user` resource

Closes #268
@martinb3 martinb3 force-pushed the martinb3-cookbooks:next_martinb3 branch from 1d17218 to d67badf Jan 6, 2015
@martinb3

This comment has been minimized.

Copy link
Collaborator Author

martinb3 commented Jan 6, 2015

Pushed new version of this branch with Gemfile fixes.

martinb3 added a commit that referenced this pull request Jan 6, 2015
- Fixes #269, adds an `elasticsearch_user` resource with provider that creates a user, group, and removes the automatic homedir

- Pins chef to 11.16.4 since chef 12 and many libraries like poise aren't compatible yet, explicit depends on rspec

- Adds a unit test with chefspec matcher under `libraries/matchers.rb` for testing chef resources were created with correct data and actions

- Adds an `elasticsearch_test` kitchenCI suite that tests edge cases, so the default test suite can test the expected way people use the cookbook

- Tweak rubocop rules to avoid style complaints about methods that are too complex (controversial rule) and compact class styles (also controversial at the moment)

- Updated `README.md` with an example of how to use the `elasticsearch_user` resource
@martinb3 martinb3 force-pushed the martinb3-cookbooks:next_martinb3 branch from d67badf to 1440a9f Jan 6, 2015
@martinb3 martinb3 merged commit 1440a9f into elastic:next Jan 6, 2015
1 check was pending
1 check was pending
continuous-integration/travis-ci The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.