-
Notifications
You must be signed in to change notification settings - Fork 163
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
machine_file resource does not work properly using with_machine_options #390
Comments
Hey @poliva83 - I am trying to confirm this. What version of chef-provisioning are you using? I don't have your I agree with you that The last call is where it loads the username from the spec or the machine_options. I tried updating ...
merged_config = Cheffish::MergedConfig.new({ :machine_options => current_machine_options }, config)
Chef::Provisioning.connect_to_machine(machine_spec, merged_config) This ends up not mattering in the chef-provisioning-aws driver because the It took me wayyyyyyy to long to figure this stuff out, which shows how confusing it is. But I think the idea of the Can you try updating your driver to have the Something we probably want to do is add this logic into chef-provisioning core (so driver authors are less confused). There is a lot of logic in chef-provisioning-aws that should move into chef-provisioning core, but until we have time to do that we may have to duplicate logic across drivers. |
Yeah, I talked to @jkeiser. Some background - we don't support resource cloning in chef-provisioning but we still want the following to work: machine "my_machine" do
machine_options :ssh_username => "centos"
action :setup
end
# This will allocate the machine and add the correct files so that it is ready for convergence
machine "my_machine" do
run_list "recipe[foo::default]"
end The purpose of the The same would happen if we had resource cloning, but chef-provisioning doesn't have resource cloning. |
Additionally, we want it to work even if you don't run the recipe with the |
@tyler-ball I was using chefdk 0.6.0 which has chef-provisioning-1.1.1 gem installed. Seems your right about how we implemented transport_for method in our driver. We expect that certain values like :ssh_user be set in machine_options. I will make changes to our driver based on your above suggestion to opennebula_driver/driver.rb
@jkeiser My current workaround of putting these machine options in a profile allows machine_file to pick up the values. Is that because all machine options you put in profile becomes globally available to all the resources? |
@poliva83 It is because the To be consistent with the AWS driver (most developed driver) you should be using Did you copy those from an existing driver, or is the driver author 'how to' information incorrect? |
@tyler-ball We started development of our driver sometime in January and I think the documentation changed in February about using see commit: 95254ff Not sure about |
As for the figuring out the whole call stack I don't think that is required. I already looked at that before in detail and needed to pop a couple aspirin afterwards because my brain hurt :) So having values in machine_spec (ex. ssh_username) being first priority would mean once their set initially they can never be overridden by machine options later? |
Correct. But that was the behavior we wanted in AWS. Once the SSH key is tied to an instance you cannot change that. Is Opennebula different? |
This is not different in Opennebula. Once I specified ssh_username variable in template and instantiate Opennebula vm I cannot change it. |
@poliva83 @tyler-ball So, what needs to happen here for this issue to be fixed, and/or for the OpenNebula folks to be able to move forward? |
Basically that (quoted from #390 (comment)) |
@tyler-ball @pburkholder I've logged issue internally and will update our driver soon to follow the newer conventions. We are planning on opensourcing driver but there are still some features/issues we need to address (this being one of them) before we feel comfortable to release. |
@poliva83 I'm going to close this since we have a plan of attack. Thanks! |
@tyler-ball @pburkholder Just a small update for final closer of this issue. We updated our driver based on above recommendations and I am no longer experiencing original issue. Thanks again. |
1. Fixes driver consistency concerns Tyler Ball brought up in [chef-provisioning issue #390](chef-boneyard/chef-provisioning#390). 2. Certain machine options are automatically set using these default values unless over written.
The recipe below will provision the machine resource properly and converge recipe but fail when trying to download files afterwards using machine_file resource. However when you move remove the with_machine_options block and put driver_options & machine_options into a profile it actually works.
Maybe has something to do with how machine resource picks up with_machine_options through calls to new_machine_options or current_machine_options.
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provider/machine.rb#L30
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provider/machine.rb#L38
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provider/machine.rb#L130
machine_file resource doesn't appear to have such methods and ends up with Chef::Config object without the correct machine options instead of Cheffish::MergedConfig object.
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provider/machine_file.rb#L24
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provider/machine_file.rb#L19
https://github.com/chef/chef-provisioning/blob/master/lib/chef/provisioning/chef_run_data.rb#L93
Except in case you put machine options inside your profile and then machine_file will end up getting a Cheffish::MergedConfig object with the options defined.
chef-client -c ~/.chef/knife.rb ./provision.rb
CHEF_PROFILE=default chef-client -c ~/.chef/knife.rb ./provision.rb
The text was updated successfully, but these errors were encountered: