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

[Breaking] Support Vagrant 1.1+ and remove vagrant gem dependency. #7

Merged
merged 3 commits into from
Mar 23, 2013

Conversation

fnichol
Copy link
Contributor

@fnichol fnichol commented Mar 22, 2013

A new strategy is used here when dealing with Vagrant: each Kitchen
Instance gets its own dedicated Vagrantfile which is rendered out just
before any Vagrant CLI commands are invoked. In this way we can avoid
writing a vagrant plugin and we return any Vagrantfile in the project
root back to the user to fully control.

An isolated Vagrant sandbox is created for each Instance in a an
internal directory under .kitchen/. For example, given an Instance name
of "default-ubuntu-1204", a Vagrantfile will be created in:

.kitchen/kitchen-vagrant/default-ubuntu-1204/Vagrantfile

A new strategy is used here when dealing with Vagrant: each Kitchen
Instance gets its own dedicated Vagrantfile which is rendered out just
before any Vagrant CLI commands are invoked. In this way we can avoid
writing a vagrant plugin and we return any Vagrantfile in the project
root back to the user to fully control.

An isolated Vagrant sandbox is created for each Instance in a an
internal directory under .kitchen/. For example, given an Instance name
of "default-ubuntu-1204", a Vagrantfile will be created in:

  .kitchen/kitchen-vagrant/default-ubuntu-1204/Vagrantfile
@@ -18,7 +18,6 @@ Gem::Specification.new do |gem|
gem.require_paths = ["lib"]

gem.add_dependency 'test-kitchen', '~> 1.0.0.alpha.0'
gem.add_dependency 'vagrant', '~> 1.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

\o/

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 am always excited to delete gems

@martinisoft
Copy link
Contributor

👍 on this! Diggin' the new Vagrant 1.1 stuff. Very exciting.

@fnichol
Copy link
Contributor Author

fnichol commented Mar 22, 2013

The tricky part now is that I'm currently using FileUtils.cd to set a reasonable ENV['PWD'] for the Vagrant CLI commands. That sure isn't threadsafe and compatible with the Celluloid actors in the concurrent/parallel mode.

My plan of attack is to allow setting of environment variables in the Kitchen::ShellOut.run_command mixin which is wrapping Mixlib::ShellOut. If that's done with an options hash on the end, any consumer of the method shouldn't be impacted.

The second place is the return of Driver#login_command only returns a String. This gets passed into a Kernel.exec call, so I'd like to add environment variable support to this as well. Thinking of returning a very simple value object with the command array and an optional hash.

But, if this sounds crazy, anyone feel free to jump in 😺

@fnichol
Copy link
Contributor Author

fnichol commented Mar 22, 2013

Speaking of yak shaves, this work helps me to pass this branch in Test Kitchen which will perform gem installs in the absence of a Gemfile (under kitchen init). It's trying to install kitchen-vagrant but getting gem dependency errors due to vagrant 😢

@martinisoft
Copy link
Contributor

@fnichol doesn't seem that crazy to make it more threadsafe. Could always be improved later as well.

This is the upstream API update needed to change directory before
issuing the SSH command to connect to a Test Kitchen controlled Vagrant
VM. Coolbeans.
@fnichol
Copy link
Contributor Author

fnichol commented Mar 23, 2013

Whoops that last commit had both upstream API updates. 2 one-liners ain't bad though.

@fnichol
Copy link
Contributor Author

fnichol commented Mar 23, 2013

I'm feeling this…

fnichol added a commit that referenced this pull request Mar 23, 2013
[Breaking] Support Vagrant 1.1+ and remove vagrant gem dependency.
@fnichol fnichol merged commit 2719522 into master Mar 23, 2013
@sethvargo sethvargo deleted the no-vagrant-gem branch November 6, 2013 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants