You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Andrew Bayer <no...@github.com> on 2013/09/26 03:32:00 UTC
[jclouds] Fix all but one EC2 live test. (#154)
The only one remaining is
org.jclouds.ec2.compute.functions.PasswordCredentialsFromWindowsInstanceLiveTest#testWindowsAdminWorks
- the custom AMI it was using doesn't seem to exist any more and I
don't know what would be needed to get it working with another AMI.
You can merge this Pull Request by running:
git pull https://github.com/abayer/jclouds-1 cleanup-ec2-live-tests
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/154
-- Commit Summary --
* Fix all but one EC2 live test.
-- File Changes --
M apis/ec2/pom.xml (6)
M apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java (1)
M apis/ec2/src/test/java/org/jclouds/ec2/compute/EC2ComputeServiceLiveTest.java (29)
M apis/ec2/src/test/java/org/jclouds/ec2/compute/extensions/EC2ImageExtensionLiveTest.java (21)
M apis/ec2/src/test/java/org/jclouds/ec2/features/ElasticBlockStoreApiLiveTest.java (15)
M apis/ec2/src/test/java/org/jclouds/ec2/features/KeyPairApiLiveTest.java (15)
M compute/src/main/java/org/jclouds/compute/domain/internal/ComputeMetadataImpl.java (20)
M compute/src/main/java/org/jclouds/compute/domain/internal/NodeMetadataImpl.java (19)
M compute/src/test/java/org/jclouds/compute/extensions/internal/BaseImageExtensionLiveTest.java (33)
M compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java (77)
M compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java (6)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/154.patch
https://github.com/jclouds/jclouds/pull/154.diff
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #718](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/718/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25330580
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
Ok, this is the latest and greatest, on top of the JCLOUDS-303 fix. Down to 8 failing tests, 1 skipped in aws-ec2, out of 170 total, and no failures/skipped in ec2. Comment on ecc918b lists the failing tests.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25330294
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -94,4 +94,17 @@ void testCreateKeyPair() {
> assertEquals(listPair.getSha1OfPrivateKey(), result.getSha1OfPrivateKey());
> }
>
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + String keyName = PREFIX + "1";
> + try {
> + client.deleteKeyPairInRegion(null, keyName);
> + } catch (Exception e) {
> +
Same as above. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607650
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
'k, removed it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25197367
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -307,11 +303,10 @@ public boolean apply(SecurityGroup input) {
>
>
> }
> -
> +*/
> // testDeleteSecurityGroup currently disabled until I can find a way to get it to delete the security group while a terminated
> // instance is still floating around in EC2. - abayer, 6/14/13
Comment still valid? Test seems to be **en**abled now?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112621
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -220,4 +220,17 @@ void testDeleteSnapshotInRegion() {
> assert client.describeSnapshotsInRegion(snapshot.getRegion(), snapshotIds(snapshot.getId())).size() == 0;
> }
>
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + try {
> + client.deleteSnapshotInRegion(snapshot.getRegion(), snapshot.getId());
> + client.deleteVolumeInRegion(defaultRegion, volumeId);
> + } catch (Exception e) {
> + // we don't really care about any exception here, so just delete away.
Actually, the exceptions this is most likely to hit are "hey, there aren't any such snapshots/volumes!" in which case, well, that's fine, so toss the exception.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607645
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -94,4 +94,17 @@ void testCreateKeyPair() {
> assertEquals(listPair.getSha1OfPrivateKey(), result.getSha1OfPrivateKey());
> }
>
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + String keyName = PREFIX + "1";
> + try {
> + client.deleteKeyPairInRegion(null, keyName);
> + } catch (Exception e) {
> +
Again, add a log statement?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591375
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #442](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/442/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25266126
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -165,6 +168,22 @@ public String getHostname() {
> return hostname;
> }
>
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public int compareTo(ResourceMetadata<ComputeType> that) {
> + if (that instanceof NodeMetadata) {
> + NodeMetadata thatMetadata = NodeMetadata.class.cast(that);
> + return start()
> + .compare(this.getId(), thatMetadata.getId())
> + .compare(this.getType(), thatMetadata.getType())
> + .compare(this.getName(), that.getName(), natural().nullsLast())
> + .result();
See above
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591396
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Richard Downer <no...@github.com>.
Regarding the Windows case, IIRC the Windows AMI numbers change regularly - probably when Microsoft release security patches, new AMIs are created. In the short term, you could probably just pick the current AMI for "Windows Server 2008 R2 Base". But really we'd want to remove any hard-coded AMI references in favour of a name, or as Adrian suggests, drop the case completely.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25153739
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -74,6 +77,23 @@ public String getId() {
> return tags;
> }
>
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public int compareTo(ResourceMetadata<ComputeType> that) {
> + if (that instanceof ComputeMetadata) {
> + ComputeMetadata thatMetadata = ComputeMetadata.class.cast(that);
> + return start()
> + .compare(this.getId(), thatMetadata.getId())
> + .compare(this.getType(), thatMetadata.getType())
> + .compare(this.getName(), that.getName(), natural().nullsLast())
> + .result();
Pull this out into a `public int compareTo(ComputeMetadata that)` method, or would that break stuff?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591392
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -345,4 +367,13 @@ public boolean apply(SecurityGroup input) {
> }
> });
> }
> +
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + view.getComputeService().destroyNodesMatching(inGroup(nodeGroup));
Do we need a `try...catch` here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591424
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -63,7 +62,8 @@
> */
> @Test(groups = "live", singleThreaded = true, testName = "PlacementGroupApiLiveTest")
> public class PlacementGroupApiLiveTest extends BaseComputeServiceContextLiveTest {
> - ArrayList<String> supportedRegions = newArrayList(Region.US_EAST_1, Region.US_WEST_2, Region.EU_WEST_1);
> + ArrayList<String> supportedRegions = newArrayList(Region.US_EAST_1, Region.US_WEST_2, Region.EU_WEST_1,
[minor] `List` instead of concrete implementation type?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112769
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -307,11 +303,10 @@ public boolean apply(SecurityGroup input) {
>
>
> }
> -
> +*/
So this is now commented out, I take it? Any reason for leaving it in, in that case, rather than simply deleting the code? Or at least using "enabled = false" or so, so it shows up in the test statistics as being disabled?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112605
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -269,6 +283,9 @@ public boolean apply(Location arg0) {
>
> // create volume only to make a snapshot
> Volume volume = ebsClient.createVolumeInAvailabilityZone(zone.getId(), 4);
> + // Sleep for 5 seconds to make sure the volume creation finishes.
> + Thread.sleep(5000);
Yak... ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591345
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -220,4 +220,17 @@ void testDeleteSnapshotInRegion() {
> assert client.describeSnapshotsInRegion(snapshot.getRegion(), snapshotIds(snapshot.getId())).size() == 0;
> }
>
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + try {
> + client.deleteSnapshotInRegion(snapshot.getRegion(), snapshot.getId());
> + client.deleteVolumeInRegion(defaultRegion, volumeId);
> + } catch (Exception e) {
> + // we don't really care about any exception here, so just delete away.
Perhaps log something, just so we can see that something's not quite right here..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591371
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
Whoopsie. Debug code. I'll clean up when I get back next week.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-26812625
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #241](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/241/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25200531
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -74,6 +77,23 @@ public String getId() {
> return tags;
> }
>
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public int compareTo(ResourceMetadata<ComputeType> that) {
> + if (that instanceof ComputeMetadata) {
> + ComputeMetadata thatMetadata = ComputeMetadata.class.cast(that);
> + return start()
> + .compare(this.getId(), thatMetadata.getId())
> + .compare(this.getType(), thatMetadata.getType())
> + .compare(this.getName(), that.getName(), natural().nullsLast())
> + .result();
I was looking at how HardwareImpl handled its compareTo and aping it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607667
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
@richardcloudsoft - yeah, I tried with a newer 2012 AMI and it didn't work, so I'm just gonna disable the test.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25187425
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -343,7 +344,8 @@ public void testCreateTwoNodesWithRunScript() throws Exception {
> nodes = newTreeSet(concat(e.getSuccessfulNodes(), e.getNodeErrors().keySet()));
> throw e;
> }
> - assertEquals(nodes.size(), 2);
> +
> + assertEquals(nodes.size(), 2, "nodes is " + nodes);
`"expected 2 nodes but was: " + nodes`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591436
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
See https://github.com/jclouds/jclouds/pull/190
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-26862415
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Adrian Cole <no...@github.com>.
Nits aside, I think we should just kill the failing windows test and corresponding property.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25139326
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -92,6 +92,16 @@ protected void checkUserMetadataContains(NodeMetadata node, ImmutableMap<String,
> }
> }
>
> + @Override
> + protected void checkTagsInNodeEquals(NodeMetadata node, ImmutableSet<String> tags) {
> + if (view.unwrapApi(EC2Api.class).getTagApi().isPresent()) {
> + super.checkTagsInNodeEquals(node, tags);
> + } else {
> + assertTrue(node.getTags().isEmpty(), "not expecting tags when tag extension isn't present" + node);
Need a space or other separator after `present`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591308
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #240](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/240/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25198576
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> + *
> + * @see <a href=
> + * "http://docs.amazonwebservices.com/AWSEC2/latest/APIReference/index.html?ApiReference-query-DescribesecurityGroupInfo.html"
> + * />
> + * @author Adrian Cole
> + */
> +public class AWSEC2DescribeSecurityGroupsResponseHandler extends
> + HandlerForGeneratedRequestWithResult<Set<SecurityGroup>> {
> +
> + private final AWSEC2SecurityGroupHandler securityGroupHandler;
> +
> + private StringBuilder currentText = new StringBuilder();
> + private Builder<SecurityGroup> securityGroups = ImmutableSet.<SecurityGroup> builder();
> + private boolean inSecurityGroupInfo;
> +
> + protected int itemDepth;
Declare protected vars first?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112733
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -150,12 +160,11 @@ public void testExtendedOptionsAndLogin() throws Exception {
> assertEquals(instance.getKeyName(), group);
>
> // make sure we made our dummy group and also let in the user's group
> - assertEquals(Sets.newTreeSet(instance.getGroupNames()), ImmutableSortedSet.<String> of("jclouds#" + group + "#"
> - + instance.getRegion(), group));
> + assertEquals(Sets.newTreeSet(instance.getGroupNames()), ImmutableSortedSet.<String> of("jclouds#" + group, group));
Okiedokie. I was just using what was there. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607595
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #246](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/246/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25263612
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -183,4 +186,22 @@ public boolean apply(SshClient input) {
> }
> }, getSpawnNodeMaxWait(), 1l, SECONDS).apply(client));
> }
> +
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + view.getComputeService().destroyNodesMatching(inGroup(imageGroup));
> +
> + Optional<? extends Image> image = getImage();
> +
> + if (image.isPresent() && image.get().getStatus() != Image.Status.DELETED) {
> + Optional<ImageExtension> imageExtension = view.getComputeService().getImageExtension();
> + if (imageExtension.isPresent()) {
> + imageExtension.get().deleteImage(image.get().getId());
> + }
> + }
Do we need a `try...catch` around this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591403
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Adrian Cole <no...@github.com>.
personally, disabling a test with no plan of fixing it is pretty much the same as commenting out a huge swath of code. I'd prefer removing it, and if/when someone ponies up to re-support this pattern (if it even makes sense to in context of windows being only partially supported as yet), they can always look at source history.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25191941
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
Failures were a whoopsie on my part - should be fixed in the next runs.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25200245
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -150,12 +160,11 @@ public void testExtendedOptionsAndLogin() throws Exception {
> assertEquals(instance.getKeyName(), group);
>
> // make sure we made our dummy group and also let in the user's group
> - assertEquals(Sets.newTreeSet(instance.getGroupNames()), ImmutableSortedSet.<String> of("jclouds#" + group + "#"
> - + instance.getRegion(), group));
> + assertEquals(Sets.newTreeSet(instance.getGroupNames()), ImmutableSortedSet.<String> of("jclouds#" + group, group));
`ImmutableSortedSet.copyOf(...)` instead of `Sets.newTreeSet`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591342
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #702](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/702/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25200530
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -269,6 +283,9 @@ public boolean apply(Location arg0) {
>
> // create volume only to make a snapshot
> Volume volume = ebsClient.createVolumeInAvailabilityZone(zone.getId(), 4);
> + // Sleep for 5 seconds to make sure the volume creation finishes.
> + Thread.sleep(5000);
Yup. Yak. I'll come up with somethin gbetter later. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607627
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> protected String groupId;
>
> + @BeforeGroups(groups = { "integration", "live" })
> + @Override
> + public void setupContext() {
> + super.setupContext();
> + cleanup();
Yeah, that was a dumb moment on my part. Moving it to the end - my main concern is cleaning up after itself, rather than avoiding collision.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607750
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #706](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/706/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25262086
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -165,6 +168,22 @@ public String getHostname() {
> return hostname;
> }
>
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public int compareTo(ResourceMetadata<ComputeType> that) {
> + if (that instanceof NodeMetadata) {
> + NodeMetadata thatMetadata = NodeMetadata.class.cast(that);
> + return start()
> + .compare(this.getId(), thatMetadata.getId())
> + .compare(this.getType(), thatMetadata.getType())
> + .compare(this.getName(), that.getName(), natural().nullsLast())
> + .result();
Ok, yeah, here we don't need it. I'll remove it - I thought I might have wanted to compare more fields, but yeah, I don't. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607687
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #700](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/700/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25189205
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #707](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/707/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25263791
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
Only one non-minor comment, really: are the `System.err` calls (rather than e.g. using a logger) in AWSEC2ListNodesStrategy.java intentional?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-26766790
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -856,7 +858,7 @@ protected void doCheckJavaIsInstalledViaSsh(NodeMetadata node, String taskName)
> @Override
> protected void tearDownContext() {
> if (nodes != null) {
> - testDestroyNodes();
> + client.destroyNodesMatching(inGroup(group));
Added, though I think that when we're just doing destroyNodesMatching, we're in less danger of exceptions due to there being no instances to destroy etc. But can't hurt. =)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607889
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -92,6 +92,16 @@ protected void checkUserMetadataContains(NodeMetadata node, ImmutableMap<String,
> }
> }
>
> + @Override
> + protected void checkTagsInNodeEquals(NodeMetadata node, ImmutableSet<String> tags) {
> + if (view.unwrapApi(EC2Api.class).getTagApi().isPresent()) {
> + super.checkTagsInNodeEquals(node, tags);
> + } else {
> + assertTrue(node.getTags().isEmpty(), "not expecting tags when tag extension isn't present" + node);
This seemed like the easiest approach to me, and it's what we were doing for the checkUserMetadataContains - I'm open to something else, but this seems fine to me.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607585
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #441](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/441/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25263092
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -343,7 +344,8 @@ public void testCreateTwoNodesWithRunScript() throws Exception {
> nodes = newTreeSet(concat(e.getSuccessfulNodes(), e.getNodeErrors().keySet()));
> throw e;
> }
> - assertEquals(nodes.size(), 2);
> +
> + assertEquals(nodes.size(), 2, "nodes is " + nodes);
Added.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607860
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #698](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/698/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25139668
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Gaul <no...@github.com>.
Did we backport this to 1.6.x? I ran EC2 tests in preparation for 1.6.3 and found:
```
Failed tests: testTemplateBuilderWithLessRegions(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): []
testTemplateBuilderCanUseImageIdWithoutFetchingAllImages(org.jclouds.aws.ec2.compute.AWSEC2TemplateBuilderLiveTest): []
testCreateAndListEBSBackedImage(org.jclouds.aws.ec2.features.AWSAMIApiLiveTest): expected [22368] but found [22369]
testNoSsh(org.jclouds.aws.ec2.features.AWSKeyPairApiLiveTest): error runScript on filtered nodes options({loginUser=ec2-user, loginPasswordPresent=true, loginPrivateKeyPresent=true, runAsRoot=false, wrapInInitScript=false})(..)
testDescribeSpotPriceHistoryInRegion(...)
testStartCCInstance(org.jclouds.aws.ec2.features.PlacementGroupApiLiveTest): org.jclouds.compute.RunNodesException: error running 1 node group(gaulec2cccluster) location(us-east-1) image(ami-0da96764) size(cc2.8xlarge) options({scriptPresent=true, userDataCksum=2f4a740b})(..)
testCreateAndRunAService(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): Optional.get() cannot be called on an absent value
testAScriptExecutionAfterBootWithBasicTemplate(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): Optional.get() cannot be called on an absent value
testCreateTwoNodesWithRunScript(org.jclouds.aws.ec2.compute.AWSEC2ComputeServiceLiveTest): Optional.get() cannot be called on an absent value
Tests run: 169, Failures: 9, Errors: 0, Skipped: 10
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-26766555
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> + private IpPermission.Builder builder = IpPermission.builder();
> +
> + /**
> + * {@inheritDoc}
> + */
> + @Override
> + public IpPermission getResult() {
> + try {
> + return builder.build();
> + } finally {
> + builder = IpPermission.builder();
> + }
> + }
> +
> + private String userId;
> + private String groupId;
Declare above, with other vars?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112743
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -856,7 +858,7 @@ protected void doCheckJavaIsInstalledViaSsh(NodeMetadata node, String taskName)
> @Override
> protected void tearDownContext() {
> if (nodes != null) {
> - testDestroyNodes();
> + client.destroyNodesMatching(inGroup(group));
`try...catch` needed here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591438
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -91,6 +92,12 @@ private void createSecurityGroupInRegion(String region, String name, int... port
> logger.debug("<< created securityGroup(%s)", name);
>
> ImmutableSet.Builder<IpPermission> permissions = ImmutableSet.builder();
> + String id;
> + if (name.startsWith("sg-")) {
Out of curiosity...what is "magic" about the `sg-` prefix?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112693
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #237](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/237/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25139647
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> +import org.jclouds.domain.LocationBuilder;
> +import org.jclouds.domain.LocationScope;
> +import org.jclouds.ec2.util.IpPermissions;
> +import org.jclouds.net.domain.IpPermission;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.base.Supplier;
> +import com.google.common.collect.ImmutableSet;
> +
> +/**
> + * @author Andrew Bayer
> + */
> +@Test(groups = "unit", testName = "AWSEC2SecurityGroupToSecurityGroupTest")
> +public class AWSEC2SecurityGroupToSecurityGroupTest {
> +
> + static Location provider = new LocationBuilder().scope(LocationScope.REGION).id("us-east-1")
Not `protected`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112755
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #435](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/435/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25200125
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> + if (securityGroupExtension.isPresent()) {
> + Optional<SecurityGroup> group = getGroup(securityGroupExtension.get());
> +
> + if (group.isPresent()) {
> + securityGroupExtension.get().removeSecurityGroup(group.get().getId());
> + }
> + }
> + }
> +
> +
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + try {
> + cleanup();
> + } catch (Exception e) {
[minor] `} catch (Exception ignored) {`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112632
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -345,4 +367,13 @@ public boolean apply(SecurityGroup input) {
> }
> });
> }
> +
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + view.getComputeService().destroyNodesMatching(inGroup(nodeGroup));
Added.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607810
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> protected String groupId;
>
> + @BeforeGroups(groups = { "integration", "live" })
> + @Override
> + public void setupContext() {
> + super.setupContext();
> + cleanup();
Odd name of a method to call during _setup_? Is this meant to remove any mess left by (failed) cleanups of previous runs? In that case, is there a way we can uniquify the test data for this test so the chance of a collision with prior runs is removed?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6591418
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #245](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/245/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25262085
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Phillips <no...@github.com>.
> @@ -83,8 +79,10 @@ protected AWSEC2ListNodesStrategy(AWSEC2Api client, @Region Supplier<Set<String>
> spotInstancesByIdInRegion(idsByRegions))),
>
> spotConverter), notNull());
> -
> - return concat(super.pollRunningInstancesByRegionsAndIds(idsByRegions), spots);
> + System.err.println("spots: " + spots);
> + Iterable<? extends RunningInstance> superInsts = super.pollRunningInstancesByRegionsAndIds(idsByRegions);
> + System.err.println("superInsts: " + superInsts);
Use a logger rather than `System.err`? Or is this left-over debug code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r7112708
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by Andrew Bayer <no...@github.com>.
> @@ -183,4 +186,22 @@ public boolean apply(SshClient input) {
> }
> }, getSpawnNodeMaxWait(), 1l, SECONDS).apply(client));
> }
> +
> + @AfterClass(groups = { "integration", "live" })
> + @Override
> + protected void tearDownContext() {
> + view.getComputeService().destroyNodesMatching(inGroup(imageGroup));
> +
> + Optional<? extends Image> image = getImage();
> +
> + if (image.isPresent() && image.get().getStatus() != Image.Status.DELETED) {
> + Optional<ImageExtension> imageExtension = view.getComputeService().getImageExtension();
> + if (imageExtension.isPresent()) {
> + imageExtension.get().deleteImage(image.get().getId());
> + }
> + }
Yeah, actually, good point. I'll add it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154/files#r6607703
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #239](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/239/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25188996
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #701](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/701/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25198581
Re: [jclouds] Fix all but one EC2 live test. (#154)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #257](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/257/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/154#issuecomment-25330555