You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Alex Heneveld <no...@github.com> on 2015/04/22 13:59:08 UTC
[jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
New generation hardware instance types prefer HVM, with some not supporting Paravirtual. The code used to assume all AWS Hardware instance types were PV, causing the failure described at https://issues.apache.org/jira/browse/JCLOUDS-889 .
This also adds a `boolean deprecated` to `Hardware`, defaulting to `false`, with `TemplateBuilderImpl` preferring non-deprecated hardware types. For AWS types, we deprecate those "previous generation" hardware instance types which AWS says should only be used where an image has been specially optimized for the type. This means new-generation instance types will be preferred.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds/pull/732
-- Commit Summary --
* indicate virtualization type for all ec2 instance types
* add deprecated flag to Hardware and prefer non-deprecated hardware types
-- File Changes --
M apis/ec2/src/main/java/org/jclouds/ec2/compute/domain/EC2HardwareBuilder.java (234)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/ImagesToRegionAndIdMap.java (9)
M apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2TemplateBuilderTest.java (46)
M apis/ec2/src/test/java/org/jclouds/ec2/compute/internal/EC2TemplateBuilderImplTest.java (52)
M compute/src/main/java/org/jclouds/compute/domain/Hardware.java (5)
M compute/src/main/java/org/jclouds/compute/domain/HardwareBuilder.java (12)
M compute/src/main/java/org/jclouds/compute/domain/internal/HardwareImpl.java (12)
M compute/src/main/java/org/jclouds/compute/domain/internal/TemplateBuilderImpl.java (7)
M compute/src/test/java/org/jclouds/compute/domain/internal/TemplateBuilderImplTest.java (58)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/732.patch
https://github.com/jclouds/jclouds/pull/732.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @ahgittin. [These](http://markmail.org/message/rlof5tzan23yoz2z) are the live tests results for AWS in jclouds 1.9.0. I do not have the list of tests that failed, but I'll run them with the latest master so we can compare.
We already know that there are AWS live tests failing and there is a corresponding JIRA for that, so let me run the tests too and compare the results. The ones that should worry us are the ComputeServiceAdapter related ones, but they seem to be passing here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-96835550
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
Thanks @nacx - pretty sure there was another `test.aws.<something>` it complained about - `vpc` or `subnet` or `keyPair` i forget - but i was running individual classes from the IDE. i'll do the maven run and post this overnight.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95614410
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -139,6 +135,20 @@ public EC2HardwareBuilder virtualizationType(VirtualizationType virtualizationTy
> return this;
> }
>
> + public EC2HardwareBuilder virtualizationTypes(VirtualizationType ...virtualizationTypes) {
> + Preconditions.checkArgument(virtualizationTypes.length > 0, "At least one virtualization type is required.");
> + if (virtualizationTypes.length == 1) {
> + this.virtualizationType = new RequiresVirtualizationType(virtualizationTypes[0]);
> + } else {
> + List<RequiresVirtualizationType> supportedVirtualizationTypes = Lists.newArrayList();
[minor] Use `Iterables.transform` to lazily create the entities and only create the ones being considered.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28865863
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
rebased on master
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-102163178
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
Note https://github.com/ahgittin/jclouds/commit/384f94557017b13327bce3b170186094f30aefee#diff-9daaac923918a5d9ff5df58b753c6dc1R221 -- I've marked `t2` deprecated because it requires a VPC.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95184170
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master and 1.9.x. Many thanks and apologies for the delay. I was busy running and working on fixing the aws live tests, and kept this in the backlog. I've merged this and also added a commit to apply the change to the new hardware profiles.
Thanks @ahgittin!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-123051223
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
113 methods - 6 failed, 5 skipped, 102 passed
two failures are clearly related, i'll push a fix for them. the other 5 look aren't obviously related. do you know if they were previously healthy? i've put details on some of the failures at the bottom. i have the full run (including 2g of logs) if you want any more to debug.
```
113 methods, 6 failed, 5 skipped, 102 passed
Failed methods (hide)
testCreateAndListEBSBackedImage
testDeleteVolumeInRegion
testDescribe
testDescribeSpotPriceHistoryInRegion
testListNodesByIds
testStartCCInstance
Skipped methods (hide)
testAddIpPermissionWithCidrExclusionGroup
testDeleteSnapshotInRegion
testDestroyNodes
testGetLaunchPermissionForImage
testRemoveIpPermissionWithCidrExclusionGroup
```
selected error details:
```
org.jclouds.aws.ec2.features.AWSSecurityGroupApiLiveTest
testDescribe
org.jclouds.aws.AWSResponseException: request POST https://ec2.eu-west-1.amazonaws.com/ HTTP/1.1 failed with code 400, error: AWSError{requestId='88d91955-87ae-42e6-865b-08bd6e52f631', requestToken='null', code='InvalidParameterValue', message='Invalid value 'launch-wizard-37' for groupName. You may not reference Amazon VPC security groups by name. Please use the corresponding id for this operation.', context='{Response=, Errors=}'}
```
```
org.jclouds.aws.ec2.features.AWSElasticBlockStoreApiLiveTest
testDeleteVolumeInRegion
java.lang.AssertionError: Sets differ: expected [] but got [Volume [attachments=[], availabilityZone=us-east-1a, createTime=Sat Apr 25 00:25:38 BST 2015, id=vol-cceb1282, region=us-east-1, size=1, snapshotId=null, status=deleting, volumeType=standard, iops=null, encrypted=false]]
at org.jclouds.ec2.features.ElasticBlockStoreApiLiveTest.testDeleteVolumeInRegion(ElasticBlockStoreApiLiveTest.java:286)
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-96185487
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
It seems those are real [violations](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1718/org.apache.jclouds.provider$aws-ec2/violations/). Can you fix them?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95530401
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @ahgittin! I'll have a look at it later today. Meanwhile, do you have live test results for this PR?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95600267
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
I believe the last commit fixes the outstanding issues.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95175094
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Andrea Turli <no...@github.com>.
@nacx, have you had the chance to look at the failing live tests?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-115026376
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
> @@ -43,7 +43,14 @@
> public RegionAndName apply(Image from) {
> checkState(from.getLocation() != null,
> "in ec2, image locations cannot be null; typically, they are Region-scoped");
> - return new RegionAndName(from.getLocation().getId(), from.getProviderId());
> + String[] segments = from.getId().split("/");
> + if (segments.length == 2) {
> + return new RegionAndName(segments[0], segments[1]);
> + } else {
> + // for testing -- EC2TemplateBuilderImplTest.testMatchesHardware* (and others?) set mock IDs
> + // which don't include location
agree
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28867113
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
@aledsage i think that's an old checkin. closing and reopening.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95238874
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Aled Sage <no...@github.com>.
@ahgittin see check style failures at https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1717/org.apache.jclouds.provider$aws-ec2/violations/file/src/test/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeServiceApiMockTest.java/
These are caused by lack of white space around "+".
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95216676
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -43,7 +43,14 @@
> public RegionAndName apply(Image from) {
> checkState(from.getLocation() != null,
> "in ec2, image locations cannot be null; typically, they are Region-scoped");
> - return new RegionAndName(from.getLocation().getId(), from.getProviderId());
> + String[] segments = from.getId().split("/");
> + if (segments.length == 2) {
> + return new RegionAndName(segments[0], segments[1]);
> + } else {
> + // for testing -- EC2TemplateBuilderImplTest.testMatchesHardware* (and others?) set mock IDs
> + // which don't include location
Instead of changing the code to accommodate the tests we should change the tests to use realistic IDs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28865923
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -139,6 +135,20 @@ public EC2HardwareBuilder virtualizationType(VirtualizationType virtualizationTy
> return this;
> }
>
> + public EC2HardwareBuilder virtualizationTypes(VirtualizationType ...virtualizationTypes) {
> + Preconditions.checkArgument(virtualizationTypes.length > 0, "At least one virtualization type is required.");
[minor] also check for nulls to avoid a NPE.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28865820
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
> @@ -139,6 +135,20 @@ public EC2HardwareBuilder virtualizationType(VirtualizationType virtualizationTy
> return this;
> }
>
> + public EC2HardwareBuilder virtualizationTypes(VirtualizationType ...virtualizationTypes) {
> + Preconditions.checkArgument(virtualizationTypes.length > 0, "At least one virtualization type is required.");
> + if (virtualizationTypes.length == 1) {
> + this.virtualizationType = new RequiresVirtualizationType(virtualizationTypes[0]);
> + } else {
> + List<RequiresVirtualizationType> supportedVirtualizationTypes = Lists.newArrayList();
as discussed on IRC, no real benefit here, as we have at most 2 virt types
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28866664
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
> @@ -139,6 +135,20 @@ public EC2HardwareBuilder virtualizationType(VirtualizationType virtualizationTy
> return this;
> }
>
> + public EC2HardwareBuilder virtualizationTypes(VirtualizationType ...virtualizationTypes) {
> + Preconditions.checkArgument(virtualizationTypes.length > 0, "At least one virtualization type is required.");
will do
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732/files#r28866653
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Alex Heneveld <no...@github.com>.
I've run some of the live tests (template builder and compute in aws-ec2). I stopped when I hit some required settings I didn't understand.
"Can this be automated?" :)
If not point me at a crib sheet for AWS and i'll run those live tests overnight tonight.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95601429
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
Closed #732.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#event-360507191
Re: [jclouds] Fixes AWS EC2 virtualization types of hardware profiles
(#732)
Posted by Ignasi Barrera <no...@github.com>.
Running them as follows (from the jclouds repo root) should be enough:
```bash
mvn integration-test -pl :aws-ec2 -Plive -Dtest.aws.identity=<your aws api key> -Dtest.aws.credential=<your aws api secret>
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/732#issuecomment-95602324