Re: [jclouds/jclouds-labs] Vagrant provider (#160)

 thanks @nacx @andreaturli. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-275071440

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

YAY! congrats @neykov! Thanks @nacx for the great review! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-275069296

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Thanks for all the effort @neykov! Pushed to [master](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/3caff210) and [2.0.x](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/fadcd5d9). -- You are receiving this because you are subscribed to this thread. Reply to this email

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Closed #160. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#event-935875896

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

nacx approved this pull request. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#pullrequestreview-18361716

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Happy to hear that @nacx. Squashed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-272506866

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + +Building + + +1. Vagrant bindings + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2.

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 1 commit. 76e37e7 Update README -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160/files/7129162ed18a01634c4b9244beefca889c65c4d0..76e37e704801218356f754b484fc697764782def

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 1 commit. 7129162 Fix tests for imageId=centos/7 -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160/files/6796d960c854f15bf1d6a642eadaa7c93fe1853d..7129162ed18a01634c4b9244beefca889c65c4d0

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Thanks! Mind squashing the commits into a single one to make the merge cleaner? I'll have a final look then and merge the PR. Thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 1 commit. 4cb8ba5 Run live tests against a single, configurable imageId. -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 1 commit. 1c397bd Fix terminated state, fix NPE -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160/files/3ea1e3f264166e80aeb10dbd0ead8b8394186f86..1c397bd970704cde5b43d1061759ee9b5735c54f

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

https://paste.apache.org/vdES ``` Tests run: 28, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1,651.118 sec - in org.jclouds.vagrant.compute.UbuntuXenialLiveTest ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

>Here's an example output from a test run Can you share the results of running the entire `UbuntuXenialLiveTest` class, not just one method? (without the wire logs, I'm just interested in seeing which tests pass and which ones fail). That class is *the contract* of our ComputeService

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

nacx commented on this pull request. > +public abstract class VagrantComputeServiceAdapterLiveTest extends > BaseComputeServiceLiveTest { + + public VagrantComputeServiceAdapterLiveTest() { + provider = "vagrant"; + } + + @Override + protected Module getSshModule() { + return

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; +import com.google.common.io.Files; + +import vagrant.Vagrant; +import vagrant.api.CommandIOListener; +import vagrant.api.VagrantApi; +import vagrant.api.domain.Box;

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Here's an example output from a test run: ``` $ mvn clean install -Plive -Dtest=UbuntuXenialLiveTest#testGet ... --- T E S T S --- Running org.jclouds.vagrant.compute.UbuntuXenialLiveTest

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 1 commit. 3ea1e3f Guice assisted Factory for VagrantApiFacade -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

nacx requested changes on this pull request. I like the new approach; it is much clean. I've just added a small comment and I think once done we can start thinking about merging it. Do you have an example live test output, so we can see the state of the actual implementation? > +import

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

I've abstracted away the access to vagrant - it's all in `VagrantCliProvider` now. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-271253432

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 2 commits. 3c5807d Get vagrant home from environment cd6dbdd Introduce an abstractaion layer between jclouds and the Vagrant CLI API -- You are receiving this because you are subscribed to this thread. View it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Sounds like a plan! Thanks! -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-270665792

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

thanks @nacx and @andreaturli for your opinions - here's what I'll end up doing then: * keep the `vagrant-java-binding` dependency - this will allow others to use it without pulling in jclouds dependencies * abstract the access to the library in the provider so it can be switched at any

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

>If you prefer to avoid the additional dependency I can bring in just the bits >that are needed for the jclouds provider? Your call :) I don't have a special issue with this dependency, because it does not add any transitive dependencies that conflict in how jclouds works. We usually recommend

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

andreaturli commented on this pull request. > + @Override + protected void configure() { + super.configure(); + bind(new TypeLiteral>() { + }).to(VagrantComputeServiceAdapter.class); + bind(new

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

andreaturli commented on this pull request. > + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2. Vagrant provider + * `git clone

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2. Vagrant provider + * `git clone

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Fully agree @nacx, that was my point as well. If you prefer to avoid the additional dependency I can bring in just the bits that are needed for the jclouds provider. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

>What I'd like though is to keep it self-contained without any additional >dependencies like guice, guava or jclouds logging. That's to allow anyone >using it without pulling in jclouds dependencies. Don't think that would fit with the dev/support model of jclouds, wdyt? There are no issues for

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

andreaturli commented on this pull request. > + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2. Vagrant provider + * `git clone

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@andreaturli Don't mind moving the project under the jclouds umbrella. What I'd like though is to keep it self-contained without any additional dependencies like guice, guava or jclouds logging. That's to allow anyone using it without pulling in jclouds dependencies. Don't think that would fit

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Addressed all comments I haven't replied directly to. Also a lot of improvements all around the board. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-269615901

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + +UserName +$USER +GroupName +daemon +StandardOutPath +/Users/$USER/.polipo/polipo.log +StandardErrorPath +/Users/$USER/.polipo/polipo.log + + +``` + +* `sudo chown root:wheel

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2. Vagrant provider + * `git clone

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + +Building + + +1. Vagrant bindings + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath +2.

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@neykov pushed 33 commits. b911159 First pass on review comments 7083b15 VagrantNodeRegistry with eviction for terminated machines 79142b0 Centralize machine file config access c2b1c70 Skip non-jclouds vagrant machines 31b8bbf Streamline machine name usage 1200a2a Simplify machine cleanup

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

andreaturli commented on this pull request. > + +Building + + +1. Vagrant bindings + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd vagrant-java-bindings` + * `mvn clean install` + * Copy `target/vagrant-java-bindings-0.0.1-SNAPSHOT.jar` to jcloud's classpath

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

thx @neykov, sorry for being late in this discussion. Just thinking aloud, would it be acceptable for you to donate the https://github.com/neykov/vagrant-java-bindings to jclouds as well? I'm seeing it as a maven submodule of jclouds-vagrant which is completely in sync with the jclouds-vagrant

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + } +}, null); + } + + @Override + public Iterable listLocations() { + return ImmutableList.of( + new LocationBuilder().id("localhost").description("localhost").scope(LocationScope.HOST).build()); + }

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + } + + @Override + protected Set getIso3166Codes() { + return ImmutableSet.of(); + } + + @Test + @Override + public void testDefaultTemplateBuilder() throws IOException { + Template defaultTemplate =

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + + LoginCredentials loginCredentials = LoginCredentials.builder() +.user(sshConfig.getUser()) +.privateKey(privateKey) +.build(); + return new NodeAndInitialCredentials(node, newMachine.getId(),

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

neykov commented on this pull request. > + String privateKey; + try { + privateKey = Files.toString(new File(sshConfig.getIdentityFile()), Charset.defaultCharset()); + } catch (IOException e) { + throw new IllegalStateException("Invalid private key " +

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Thanks for taking the time to go through the code. Will implement the changes and get back to you. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-266657373

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

nacx requested changes on this pull request. Thanks @neykov! I've reviewed the PR and added my comments. > +Vagrant provider for jclouds + + +Building + + +1. Vagrant bindings + * `git clone https://github.com/neykov/vagrant-java-bindings` + * `cd

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

My apologies @neykov. This pull request got lost in my review queue. I'll review it later today and make sure we move forward with this. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Hi @nacx. Bumped version to `2.1.0-SNAPSHOT`. What work would be needed to get this merged into labs? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-266230148

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

@nacx There's no vagrant API, so it's using the cli under the hood. It's possible to install a plugin which exposes an API, but since the provider makes most sense when used locally better not introduce another abstraction layer (and add extra setup steps for users). I've updated the

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

Thanks @neykov! Looking at it, the bindings just run the required vagrant commands, so there is no actual API exposed by vagrant, right? In that case I'd say ti is OK for us to add the bindings dependency (given it has no transitive dependencies), but we can't rely on a snapshot version that is

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

A couple of unit tests missing, but will be easy to add if everything else is good to go. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-203009569

Re: [jclouds/jclouds-labs] Vagrant provider (#160)

bump @nacx, @andreaturli Let me know if anything else needed for a review. --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/jclouds/jclouds-labs/pull/160#issuecomment-203009249