matschaffer / knife-solo

DEPRECATED: Please consider using https://knife-zero.github.io/, ansible, or visit https://www.chef.io/ for other ideas
MIT License
787 stars 213 forks source link

Leverage knife's bootstrap command #121

Open patcon opened 12 years ago

patcon commented 12 years ago

Could we not use knife bootstrap's methods directly? If they aren't yet flexible enough, perhaps we could work at submitting pull requests.

Part of my rationale for this query is so that knife-solo could more easily leverage parts of knife-rackspace, etc

matschaffer commented 12 years ago

I've tried to do this before and here's what stalled me: You have to profile the OS a bit first to know what to run (distro, version, curl/wget, sudo/not). You'd probably want to do that in ruby, but knife bootstrap's connection code isn't geared toward gathering results (it's parallel, which is cool, but it throws away a lot of the output and return values). But if you answer that by writing your own connection code (like I did), there's not much left to use in knife bootstrap's code.

Granted the prepare code is kind of a mess right now. I'd love to reduce it to a discovery step (something like ohai) followed by a single script similar to what knife bootstrap does.

But you make a point with the pull requests. Perhaps the way it handles output and connections could be enhanced to promote reuse.

If think if you wanted to leverage knife-rackspace it might be better to start with that goal and see where it leads you.

patcon commented 12 years ago

coolio. Thanks man :) very much appreciated. I'll see what I can figure out

tmatilai commented 11 years ago

@patcon In the comments of #120 there is a pointer to a POC for plugging knife-solo into knife-ec2. knife-rackspace for sure can be monkey patched the same way. But going one level lower to knife bootstrap might really be a better idea.

patcon commented 11 years ago

:+1: that's awesome @tmatilai! I only had time to skim, but I'm sure I'll be back in the convo later :)

tmatilai commented 11 years ago

Okay, my knife-bootstrap-solo branch has a seemingly working spike to add knife-solo support to knife bootstrap. You can pass --solo option to it or specify solo true in knife.rb. The latter method works also with knife-ec2, knife-rackspace, etc. without modifications to them.

The branch is built on top of "usage-and-validation" branch (diff). Added helper methods should probably be extracted from polluting Knife::Bootstrap class. And integration test should be written. But at least a promising start. =)

tmatilai commented 11 years ago

And of course then the goal will be to add some better way to "registrate" us to knife bootstrap than monkey patching...

patcon commented 11 years ago

You are a legend, sir. Do you do coderwall? I'd love to endorse you

tmatilai commented 11 years ago

He, here you go. :D

tmatilai commented 11 years ago

This is relevant to our interests: danielsdeleo/bootstrapper.

tmatilai commented 11 years ago

I made a gist explaining the usage of the spike: https://gist.github.com/4559333.

russellcardullo commented 11 years ago

Hi @tmatilai. The gist above works well when running 'knife bootstrap'. However when running in combination with knife-ec2 I'm seeing a similar issue as #166 with the option name for :ssh_indentity.

Knife-ec2 has:

option :identity_file,
  :short => "-i IDENTITY_FILE",
  :long => "--identity-file IDENTITY_FILE",
  :description => "The SSH identity file used for authentication"

Whereas in knife-solo/ssh_command.rb we have

option :ssh_identity,
    :short       => '-i FILE',
    :long        => '--ssh-identity FILE',
    :description => 'The ssh identity file'

The consequence of this is that when running knife ec2 server create the parameter passed in through the -i option will get assigned to identity_file rather than ssh_identity and the ssh command in knife-solo will prompt for a password rather than using the identity file.

I can submit a pull request to rename this option, but wanted to get your thoughts on this. Is there a better approach we could try that would better handle option clashes?

tmatilai commented 11 years ago

@russellcardullo, good catch. In my opinion we want to be compatible with Knife::Bootstrap and there the option is also :identity_file. So a PR would be welcome.

matschaffer commented 11 years ago

:+1: for matching Knife::Bootstrap

On Wednesday, January 23, 2013, Teemu Matilainen wrote:

@russellcardullo https://github.com/russellcardullo, good catch. In my opinion we want to be compatible with Knife::Bootstrap and there the option is also :identity_file. So a PR would be welcome.

— Reply to this email directly or view it on GitHubhttps://github.com/matschaffer/knife-solo/issues/121#issuecomment-12624070.

-Mat

about.me/matschaffer

patcon commented 11 years ago

Something to keep an eye on: https://github.com/opscode/chef/pull/595

Would allow forwarding ssh sessions during bootstrap like we can do with vagrant: http://stackoverflow.com/a/8191279/504018 https://github.com/myplanetdigital/vagrant-ariadne/blob/master/cookbooks-override/ariadne/libraries/helpers.rb

tmatilai commented 11 years ago

@patcon That is cool, but I can not come up with any use case for knife-solo. knife solo prepare installs Chef via http/omnibus or rubygems and maybe installs some system packages, but it shouldn't need to make any SSH connections. Do you have some cookbooks that use ssh client?

patcon commented 11 years ago

@tmatilai sorry, I imagine it would be a flag on knife solo cook. That was, the chef-solo run can clone any private repos that the locally running user can. A linked library file in a cookbook should allow it, as in the previously linked file.

Net::SSH reference: http://net-ssh.github.com/ssh/v2/api/classes/Net/SSH.html#M000002

tmatilai commented 11 years ago

@patcon Yes my last question was targeting knife solo cook. Long night and bad phrasing. =)

Your use case is interesting. I have only used same cookbooks that are used with chef-client/server and failed to think out of the box. This really gives a nice extra feature to knife-solo that you can't do with chef-client. Feel free to open a new issue as it is not strictly related to this one.

patcon commented 11 years ago

done deal! :)

tknerr commented 11 years ago

Will your knife-solo-bootstrap branch be integrated for the 0.3.0 release?

I'll give it a shot and let you know if it worked for me

tmatilai commented 11 years ago

@tknerr there has not been much discussion about that branch. I think we could merge it, but at least we have to decide the configuration namespace before (hello #199). The POC uses knife[:solo], but it should be consistent with other options. If other options will have flat namespace (i.e. knife[:solo_path]) that is of course fine. If :solo is used as namespace (i.e. knife[:solo][:path]) we have to change it.

Also some integration test should be added. I'll try to find some time for that branch soon.

tknerr commented 11 years ago

Ok cool, let me know once there's something ready I can test

tmatilai commented 11 years ago

I merged the current master to the branch, made it a lot less intrusive, and opened PR #207. All testing much appreciated! /cc @tknerr

tknerr commented 11 years ago

Just gave it a try in a clean environment, Chef installs fine and everything works as expected :+1:

knife bootstrap 33.33.77.10 --solo -x vagrant -i W:\home\.vagrant.d\insecure_private_key --bootstrap-version 10.16.2-1

Next I tried overriding the run_list and json params using the knife bootstrap -r and -j params:

knife bootstrap 33.33.77.10 --solo -x vagrant -i W:\home\.vagrant.d\insecure_private_key --bootstrap-version 10.16.2-1 -r "recipe[foobar]" -j "{ 'some': 'json' }"

I would have expected that they take precedence over ./nodes/33.33.77.10.json, but they don't, which might come as a surprise. Once I deleted the ./nodes/33.33.77.10.json file the -r and -j command line params were honored as expected and as a result the ./nodes/33.33.77.10.json file was written with the supplied values.

It's arguable whether the commandline params should override if a node.json already exists. What would you expect in this case?

Another thing I noticed is a conflict with chefspec 0.9.0. This happened when running the above commands without bundle exec but having the chefspec gem installed. Using bundle exec and a Gemfile without chefspec I had success as described above. Not sure whether this should be fixed in chefspec or here:

R:\stuff\test\esx-bootstrap>knife bootstrap --help
X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec/monkey_patches/provider.rb:6:in `alias_method': undefined m
ethod `build_from_file' for class `Class' (NameError)
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec/monkey_patches/provider.rb:6:in `singletonclass'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec/monkey_patches/provider.rb:5:in `<class:Provider>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec/monkey_patches/provider.rb:4:in `<class:Chef>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec/monkey_patches/provider.rb:2:in `<top (required)>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:55:in `require'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:55:in `require'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.9.0/lib/chefspec.rb:21:in `<top (required)>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:59:in `require'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:59:in `rescue in require'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/site_ruby/1.9.1/rubygems/custom_require.rb:35:in `require'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chefspec-0.8.0/lib/chef/knife/cookbook_create_specs.rb:1:in `<top (required)>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife/core/subcommand_loader.rb:37:in `load'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife/core/subcommand_loader.rb:37:in `block in load_commands'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife/core/subcommand_loader.rb:37:in `each'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife/core/subcommand_loader.rb:37:in `load_commands'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife.rb:119:in `load_commands'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/knife.rb:167:in `run'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/lib/chef/application/knife.rb:123:in `run'
        from X:/tools/vagrant/vagrant/vagrant/embedded/lib/ruby/gems/1.9.1/gems/chef-11.4.0/bin/knife:25:in `<top (required)>'
        from X:/tools/vagrant/vagrant/vagrant/embedded/bin/knife:19:in `load'
        from X:/tools/vagrant/vagrant/vagrant/embedded/bin/knife:19:in `<main>'
tknerr commented 11 years ago

Just tried whether it also works with knife-ec2, but the --solo option is not recognized

knife ec2 server create --solo -r 'recipe[foobar]' -I ami-524e4726 -f m1.small --region aws-eu-west -j '{ \"some\": \"json\" }'
Error: invalid option: --solo
USAGE: knife ec2 server create (options)
    -Z, --availability-zone ZONE     The Availability Zone
    -A, --aws-access-key-id KEY      Your AWS Access Key ID
    ...

My .chef/knife.rb:

# knife-solo integration for knife-ec2
knife[:solo] = true
knife[:aws_access_key_id] = "mykey"
knife[:aws_secret_access_key] = "mysecretkey"

Ideas?

matschaffer commented 11 years ago

:+1: for cli attrs overriding nodes/x.json. Better yet maybe we could merge? Though I'm not sure exactly what behavior for stuff like run_list would be best if we try to merge attrs.

On Saturday, February 23, 2013, tknerr wrote:

Just tried whether it also works with knife-ec2, but the --solo option is not recognized

knife ec2 server create --solo -r 'recipe[foobar]' -I ami-524e4726 -f m1.small --region aws-eu-west -j '{ \"some\": \"json\" }' Error: invalid option: --solo USAGE: knife ec2 server create (options) -Z, --availability-zone ZONE The Availability Zone -A, --aws-access-key-id KEY Your AWS Access Key ID ...

My .chef/knife.rb:

knife-solo integration for knife-ec2

knife[:solo] = true knife[:aws_access_key_id] = "mykey" knife[:aws_secret_access_key] = "mysecretkey"

Ideas?

— Reply to this email directly or view it on GitHubhttps://github.com/matschaffer/knife-solo/issues/121#issuecomment-13991846.

-Mat

about.me/matschaffer

tknerr commented 11 years ago

As for the knife-ec2 problem I had with --solo: it's not a bug because the --solo flag is only added to knife bootstrap, not knife-ec2 plugin.

Reading the POC again I now understand that the knife[:solo] = true is enough to make knife-solo kick in, so when I omit the --solo param I get one step further:

knife ec2 server create -r "recipe[sample-app]" -S "W:/home/.ssh/my_key" -I "ami-524e4726" -f "m1.small" -Z "eu-west-1c" --region "aws-eu-west" -j '{ "some": "json" }' -N "sample-app" -G "ssh,http" -VV
ERROR: Excon::Errors::SocketError: getaddrinfo: No such host is known.  (SocketError)

No clue what this means, but I'm stuck here now.

tknerr commented 11 years ago

I got something wrong with the knife-ec2 commandline parameters:

Using this command it works:

knife ec2 server create -VV -r recipe[sample-app] -x ubuntu -S my-keypair -i W:/home/.ssh/mykey -I ami-524e4726 -f m1.small --region eu-west-1 -Z eu-west-1c -j '{ "some": "json" }' -N sample-app -G ssh,http

Chef installs, recipes run, node json is written to ./nodes/sample-app.json. Perfect!

Awesome stuff, now that I call a really transparent integration of knife-solo! :+1: :+1: :+1:

So the only real issue remaining from my testing is with chefspec.

Concerning the CLI attributes overriding I have not really a preference. It just must be documented - and maybe should issue a warning if you would override (and in fact overwrite!) an existing node json?

tmatilai commented 11 years ago

Oh, wow! I'll try to address all the topics from my point of view.

chefspec: the stack trace shows two versions of chefspec: 0.8.0 and 0.9.0. Run gem cleanup chefspec to get rid of the old ones (CHEF-3255). But I have a feeling that chefspec has not yet released a version that is compatible with the latest chef releases. Gem dependecy problems are still affecting many parts of the Chef ecosystem.

--solo option: I don't think that we can get knife bootstrap to officially support --solo option at least in the short term. I will anyway try to get the support to different bootstrap plugins (knife-ec2, etc.). If it is not accepted, we can decide to monkey patch them, too. It should be quite gentle after Knife::Bootstrap has been modified. And knife[:solo] = true configuration is anyway a workaround.

--run-list and --json-attributes: This is a tricky one. knife solo cook has now --override-runlist option to override the run list from node/.json. But knife bootstrap, knife solo bootstrap and knife solo prepare don't. Merging the run list is not a good option as the order of items is important. The bootstrap commands could maybe just override the values read from the file, but should at least warn the user. Prepare could maybe just warn without doing anything. In my opinion none of the commands should overwrite the current config file. One option is also to prompt from the user in these cases.

What did I forget? Thanks a lot @tknerr for the valuable feedback! These are exactly the cases we have to take care of.

tknerr commented 11 years ago

Quick heads-up and comments:

chefspec: I had my gems messed up. After cleaning the chefspec gem the issue remained: as you said it's an incompatibility between chefspec and chef 11, but not related to knife-solo. It works all well with chef 10.x + knife-solo + chefspec

--solo option: actually I like both: knife[:solo] = true because it switches transparently and --solo because it switches explicity to knife-solo. Having both options is nice. If not officially supported, I would vote for monkeypatching so that --solo is consistently available in knife bootstrap and knife-ec2|rackspace|etc...

--run-list and --json-attributes: agree, definitely :-1: for merging the run_list. How about this: if the node json already exists and either -r or -j are specified the process exits and the user is asked to either omit the -r / -j option or to remove the node json. If the node json does not exist it is assumed to be the first-time bootstrap and thus passing -r and -j is fine and the node json is written accordingly afterwards.

Not sure how to handle --override-runlist though...

tmatilai commented 11 years ago

Of course knife[:solo] will stay supported, but would be nice to be able to override it using --[no-]solo also with knife-ec2 and friends.

A problem with erroring out is that unless we inject also validation directly to knife-ec2, the instance is already created before we get control. I have been thinking if we could get a better validation (hook?) support natively to Knife, but it will also take time.

--json-attributes we could quite safely deep merge. I like the idea of adding/overriding node attributes of individual (EC2) instances while most of the configuration is read from an existing node config. But this would require that we write the node config ("dna.json") to the node instead of uploading it (which should be quite easy to do after the great solo.rb renovation).

--override-runlist should behave correctly already. It is passed as an option to chef-solo and thus it overrides run_list from node config.

andruby commented 11 years ago

@tmatilai: deep merging --json-attributes would be the behavior I actually expect.