You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Michael Wilson <no...@github.com> on 2015/09/16 02:09:29 UTC
[jclouds] JCLOUDS-1001. Add preemptible support to the Google Compute
provider. (#857)
The Google Compute provider should support preemptible instances.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/857
-- Commit Summary --
* JCLOUDS-1001. Add preemptible support to the Google Compute provider.
-- File Changes --
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceAdapter.java (2)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/compute/options/GoogleComputeEngineTemplateOptions.java (17)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/domain/Instance.java (9)
M providers/google-compute-engine/src/main/java/org/jclouds/googlecomputeengine/features/InstanceApi.java (4)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/compute/GoogleComputeEngineServiceLiveTest.java (5)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiLiveTest.java (9)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/features/InstanceApiMockTest.java (6)
M providers/google-compute-engine/src/test/java/org/jclouds/googlecomputeengine/parse/ParseInstanceTest.java (2)
M providers/google-compute-engine/src/test/resources/instance_get.json (3)
M providers/google-compute-engine/src/test/resources/instance_insert_full.json (3)
M providers/google-compute-engine/src/test/resources/instance_list.json (3)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/857.patch
https://github.com/jclouds/jclouds/pull/857.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
So, I've got two failures with my run, but it looks to be unrelated to my change:
Results :
Failed tests:
UrlMapApiLiveTest.testInsertUrlMap:63->BaseGoogleComputeEngineApiLiveTest.assertOperationDoneSuccessfully:86 » IllegalState
UrlMapApiLiveTest.testDeleteUrlMap:193->BaseGoogleComputeEngineApiLiveTest.assertOperationDoneSuccessfully:85 » NullPointer
Tests run: 164, Failures: 2, Errors: 0, Skipped: 11
I suspect these are probably a setup issue on my side.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-142056230
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
Looks like I missed a line in there. I've pushed up the change, but I haven't done a full live test run -- working on that now. However:
[pool-1-thread-1] Test testCreatePreemptibleNodeWithSsd(org.jclouds.googlecomputeengine.compute.GoogleComputeEngineServiceLiveTest) succeeded: 169288ms
Test suite progress: tests succeeded: 1, failed: 0, skipped: 0.
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 171.35 sec - in org.jclouds.googlecomputeengine.compute.GoogleComputeEngineServiceLiveTest
Results :
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
Which is heartening!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-141533308
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Ignasi Barrera <no...@github.com>.
As you say, the failures are likely caused by some "collision" in the url maps, or something environment related. I've run all live tests and all passed, so everything is :+1:!
I've also amended the commit to use a 3 space indent, and pushed it to master as [962980cd](http://git-wip-us.apache.org/repos/asf/jclouds/commit/962980cd). Thanks @meowfaceman!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-142111100
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
Just realized an issue with this and GoogleComputeEngineServiceAdapter --
Line 180 in this PR has the following line:
Scheduling.create(OnHostMaintenance.MIGRATE, true, options.preemptible()) // scheduling
The scheduling can't be set to OnHostMaintenance.MIGRATE and preemptible simultaneously. Is there any desire/need to make the maintenance configurable via GoogleComputeEngineTemplateOptions as well?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-140587490
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
I've changed the aforementioned line such that scheduling is set to TERMINATE and automaticRestart to FALSE if instances are preemptible. This seemed a lot less invasive to me. Let me know your thoughts.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-140632089
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
Thanks for the catch. Will do!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-141147263
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Ignasi Barrera <no...@github.com>.
Closed #857.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#event-415243057
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Ignasi Barrera <no...@github.com>.
You'll have to cherry-pick the commit in https://github.com/jclouds/jclouds/pull/854 to have all live tests passing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-141152351
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Michael Wilson <no...@github.com>.
This has been heavily based on @ajatix's work here: https://github.com/jclouds/jclouds-labs-google/pull/146.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-140585026
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Ignasi Barrera <no...@github.com>.
Running the live tests I get this consistent failure:
```text
testCreatePreemptibleNodeWithSsd(org.jclouds.googlecomputeengine.compute.GoogleComputeEngineServiceLiveTest) Time elapsed: 126.254 sec <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:494)
at org.testng.Assert.assertTrue(Assert.java:42)
at org.testng.Assert.assertTrue(Assert.java:52)
at org.jclouds.googlecomputeengine.compute.GoogleComputeEngineServiceLiveTest.testCreatePreemptibleNodeWithSsd(GoogleComputeEngineServiceLiveTest.java:95)
```
Could you have a look at it? You can run the single test with:
```bash
mvn -pl :google-compute-engine test \
-Dtest.google-cloud.json-key=path/to/json/key/file.json \
-Dtest=GoogleComputeEngineServiceLiveTest#testCreatePreemptibleNodeWithSsd
```
Or all of them with:
```bash
mvn -pl :google-compute-engine -Plive clean install \
-Dtest.google-cloud.json-key=path/to/json/key/file.json
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-141123212
Re: [jclouds] JCLOUDS-1001. Add preemptible support to the Google
Compute provider. (#857)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @meowfaceman! lgtm. I'll give it a try and merge it!
> Is there any desire/need to make the maintenance configurable via GoogleComputeEngineTemplateOptions as well?
That would introduce some complexity to make sure the provided options are consistent. As long as the current ones are sane defaults, I'd don't think we need to add that in this pull request. Feel free to open a jira or a new PR to fix it, though.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/857#issuecomment-140910248