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

Convert to basic 12.5 resources #82

Merged
merged 2 commits into from
Oct 29, 2015
Merged

Convert to basic 12.5 resources #82

merged 2 commits into from
Oct 29, 2015

Conversation

jkeiser
Copy link
Contributor

@jkeiser jkeiser commented Oct 2, 2015

This just descends everything from CompatResource and makes necessary adjustments. More changes to follow.


private

action_class.class_eval do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems non-intuitive for authors that we have just exchanged providers for action_class. What would you think about delegating create_acl from the action_class to the resource? Then we wouldn't need this class_eval.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, what happens if there are multiple 'providers' for a single resource? Are there multiple action_classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are a lot of changes we can and should make after moving it over, but was just trying to get it moved in bulk before that sort of surgery. All changes here are purely cosmetic.

There is only ever one provider for a single resource. If you fork a resource to different OS's, you just create new subclass resources.

@tyler-ball
Copy link
Contributor

Just tried this locally - this won't work with chef 12.4.x because that is pinned to chef-zero < 4.3.0 and this wants ~> 4.3 - is this on purpose or are we trying to make this version of Cheffish back-compat with Chef 12.4?

@jkeiser
Copy link
Contributor Author

jkeiser commented Oct 6, 2015

@tyler-ball I'm not sure why chef-zero dep is set to 4.3, but if current cheffish doesn't work with 12.4.3, doesn't that mean current chef-provisioning can't pull in current cheffish? Is that an issue?

The intent of using compat_resource is to preserve compatibility with older versions that already exists. If we're truly incompatible with older versions of chef-zero (I bet it has more to do with test suites than the actual software), then that seems like a bigger issue.

Happy to remove the compat_resource line if we're 12.5 only, but I doubt that's our intent :)

@tyler-ball
Copy link
Contributor

Error that I just found using this Cheffish, master of Chef and master of chef-provisioning

[test-provisioning]$ cat .chef/local-mode-cache/cache/chef-stacktrace.out                                                                                 *[master][ruby-2.1.6@cookbook-test-provisioning]
Generated at 2015-10-06 10:16:13 -0600
LoadError: cannot load such file -- chef/provider/chef_node
/Users/tball/github/chef-provisioning/lib/chef/provider/machine.rb:2:in `require'
/Users/tball/github/chef-provisioning/lib/chef/provider/machine.rb:2:in `<top (required)>'
/Users/tball/github/chef-provisioning/lib/chef/provisioning/recipe_dsl.rb:6:in `require'
/Users/tball/github/chef-provisioning/lib/chef/provisioning/recipe_dsl.rb:6:in `<top (required)>'
/Users/tball/github/chef-provisioning/lib/chef/provisioning.rb:2:in `require'
/Users/tball/github/chef-provisioning/lib/chef/provisioning.rb:2:in `<top (required)>'
/Users/tball/github/chef-provisioning-aws/lib/chef/provisioning/aws_driver.rb:1:in `require'
/Users/tball/github/chef-provisioning-aws/lib/chef/provisioning/aws_driver.rb:1:in `<top (required)>'
/Users/tball/chef_repo/cookbooks/test-provisioning/recipes/test.rb:1:in `require'
/Users/tball/chef_repo/cookbooks/test-provisioning/recipes/test.rb:1:in `from_file'
/Users/tball/github/chef/lib/chef/mixin/from_file.rb:30:in `instance_eval'
/Users/tball/github/chef/lib/chef/mixin/from_file.rb:30:in `from_file'
/Users/tball/github/chef/lib/chef/run_context.rb:313:in `load_recipe_file'
/Users/tball/github/chef/lib/chef/policy_builder/expand_node_object.rb:94:in `block in setup_run_context'
/Users/tball/github/chef/lib/chef/policy_builder/expand_node_object.rb:93:in `each'
/Users/tball/github/chef/lib/chef/policy_builder/expand_node_object.rb:93:in `setup_run_context'
/Users/tball/github/chef/lib/chef/client.rb:485:in `setup_run_context'
/Users/tball/github/chef/lib/chef/client.rb:266:in `run'
/Users/tball/github/chef/lib/chef/application.rb:270:in `block in fork_chef_client'
/Users/tball/github/chef/lib/chef/application.rb:258:in `fork'
/Users/tball/github/chef/lib/chef/application.rb:258:in `fork_chef_client'
/Users/tball/github/chef/lib/chef/application.rb:224:in `block in run_chef_client'
/Users/tball/github/chef/lib/chef/local_mode.rb:44:in `with_server_connectivity'
/Users/tball/github/chef/lib/chef/application.rb:212:in `run_chef_client'
/Users/tball/github/chef/lib/chef/application/client.rb:408:in `block in interval_run_chef_client'
/Users/tball/github/chef/lib/chef/application/client.rb:398:in `loop'
/Users/tball/github/chef/lib/chef/application/client.rb:398:in `interval_run_chef_client'
/Users/tball/github/chef/lib/chef/application/client.rb:388:in `run_application'
/Users/tball/github/chef/lib/chef/application.rb:60:in `run'
/Users/tball/github/chef/bin/chef-client:26:in `<top (required)>'
/Users/tball/.rvm/gems/ruby-2.1.6@cookbook-test-provisioning/bin/chef-client:23:in `load'
/Users/tball/.rvm/gems/ruby-2.1.6@cookbook-test-provisioning/bin/chef-client:23:in `<main>'
/Users/tball/.rvm/gems/ruby-2.1.6@cookbook-test-provisioning/bin/ruby_executable_hooks:15:in `eval'
/Users/tball/.rvm/gems/ruby-2.1.6@cookbook-test-provisioning/bin/ruby_executable_hooks:15:in `<main>'%

@tyler-ball
Copy link
Contributor

7dcb1b1 fixed my previous error - I'm not seeing any errors now! Successfully converged a chef-provisioning resource.

Still cannot use this with chef 12.4.3 because of #82 (comment) - we should see if we can relax the chef-zero constraint here. Maybe even remove it entirely or move it to a development dependency.

attribute :encrypt, :kind_of => [TrueClass, FalseClass]
#attribute :secret, :kind_of => String
property :encrypt, :kind_of => [TrueClass, FalseClass]
#property :secret, :kind_of => String
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason not to just delete this?

@randomcamel
Copy link
Contributor

👍 It seems like it'd be nice to delete commented-out properties rather than changing them as though they matter, but it's not that important.

@tyler-ball
Copy link
Contributor

👍

@jkeiser jkeiser merged commit a5ac1ef into master Oct 29, 2015
@tyler-ball tyler-ball deleted the jk/compat_resource branch November 9, 2015 22:19
@thommay thommay added Improvement Type: Enhancement Adds new functionality. and removed Improvement labels Jan 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants