You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Jeremy Daggett <no...@github.com> on 2014/12/05 18:44:07 UTC

[jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

You can merge this Pull Request by running:

  git pull https://github.com/rackerlabs/jclouds-labs-openstack cloudnetworks-uk

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds-labs-openstack/pull/174

-- Commit Summary --

  * Support Rackspace Cloud Networks UK

-- File Changes --

    M pom.xml (1)
    A rackspace-cloudnetworks-uk/README.md (4)
    A rackspace-cloudnetworks-uk/pom.xml (125)
    A rackspace-cloudnetworks-uk/src/main/java/org/jclouds/rackspace/cloudnetworks/uk/CloudNetworksUKProviderMetadata.java (118)
    A rackspace-cloudnetworks-uk/src/test/java/org/jclouds/rackspace/cloudnetworks/uk/CloudNetworksUKNetworkApiLiveTest.java (76)
    A rackspace-cloudnetworks-uk/src/test/java/org/jclouds/rackspace/cloudnetworks/uk/CloudNetworksUKPortApiLiveTest.java (85)
    A rackspace-cloudnetworks-uk/src/test/java/org/jclouds/rackspace/cloudnetworks/uk/CloudNetworksUKProviderTest.java (28)
    A rackspace-cloudnetworks-uk/src/test/java/org/jclouds/rackspace/cloudnetworks/uk/CloudNetworksUKSubnetApiLiveTest.java (89)
    A rackspace-cloudnetworks-uk/src/test/resources/logback.xml (69)

-- Patch Links --

https://github.com/jclouds/jclouds-labs-openstack/pull/174.patch
https://github.com/jclouds/jclouds-labs-openstack/pull/174.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
My only concern about doing that is that we don't know *when* that will happen (perhaps there are other PRs in the queue that will be opened first, etc, and this cleanup can be eventually forgotten). Keeping the PR where things are spotted open until they are fixed usually adds some pressure that helps focusing.

Having said this, review is not harder if the commit is just added to this PR, but I'm am OK in addressing that in another one. Your call.


---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-67069801

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Jeremy Daggett <no...@github.com>.
@nacx @zack-shoylev I completely agree that we should fix things as we find them. However, IMHO, I think we should step back and address any Neutron test cleanup first and then the providers in separate PRs. I just updated the logger files for now. Thoughts?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-66667179

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Zack Shoylev <no...@github.com>.
The suggested cleanup should be applied to the US provider as well. Expand PR?
Some of this (resource cleanup) will be useful in the Neutron live tests also.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-66217496

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +    <appender name="WIREFILE" class="ch.qos.logback.core.FileAppender">
> +        <file>target/test-data/jclouds-wire.log</file>
> +
> +        <encoder>
> +            <Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
> +        </encoder>
> +    </appender>
> +
> +    <appender name="BLOBSTOREFILE" class="ch.qos.logback.core.FileAppender">
> +        <file>target/test-data/jclouds-blobstore.log</file>
> +
> +        <encoder>
> +            <Pattern>%d %-5p [%c] [%thread] %m%n</Pattern>
> +        </encoder>
> +    </appender>

Remove this logger, as it is not needed in this provider.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21495502

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
The cleanup is pretty straightforward. If it makes sense in US and Neutron I'd expand this PR with a second commit addressing that.
Let's make sure things get fixed when we spot them.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-66240821

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Jeremy Daggett <no...@github.com>.
> +
> +         assertEquals(network.getId(), net.getId());
> +         assertEquals(network.getName(), "jclouds-test");
> +         assertTrue(network.getSubnets().isEmpty());
> +         assertNotNull(networkApi.update(net.getId(), Network.updateBuilder().name("jclouds-live-test").build()));
> +
> +         network = networkApi.get(net.getId());
> +
> +         assertEquals(network.getId(), net.getId());
> +         assertEquals(network.getName(), "jclouds-live-test");
> +         assertTrue(network.getSubnets().isEmpty());
> +
> +         Network net2 = networkApi.create(Network.createBuilder("jclouds-test2").build());
> +         assertNotNull(net2);
> +
> +         assertTrue(networkApi.delete(net.getId()));

@nacx We should add this to our best practices. :+1:  

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21694580

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Jeremy Daggett <no...@github.com>.
Closed #174.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#event-219094749

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Zack Shoylev <no...@github.com>.
My suggestion would be to fix the remaining review comments (except cleanup-related) then merge, then work on refactoring the tests according to jclouds-803 and make a separate PR. I should be able to help with that.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-69993397

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +         assertEquals(retrievedSubnet.getId(), subnet.getId());
> +         assertEquals(retrievedSubnet.getCidr(), "192.168.100.0/30");
> +         assertTrue(retrievedSubnet.getDnsNameservers().isEmpty());
> +         assertEquals(retrievedSubnet.getAllocationPools().size(), 1);
> +         assertEquals(retrievedSubnet.getHostRoutes().size(), 1);
> +         assertNotNull(subnetApi.update(retrievedSubnet.getId(), Subnet.updateBuilder().name("jclouds-live-test-update").build()));
> +
> +         retrievedSubnet = subnetApi.get(retrievedSubnet.getId());
> +
> +         assertEquals(retrievedSubnet.getId(), subnet.getId());
> +         assertEquals(retrievedSubnet.getName(), "jclouds-live-test-update");
> +         assertTrue(retrievedSubnet.getDnsNameservers().isEmpty());
> +
> +         assertTrue(subnetApi.delete(subnet.getId()));
> +         assertTrue(networkApi.delete(networkId));

Same cleanup thing.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21495523

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Jeremy Daggett <no...@github.com>.
@nacx @zack-shoylev Refactoring these tests is going to take a lot more work than anticipated. I just submitted [JCLOUDS-803](https://issues.apache.org/jira/browse/JCLOUDS-803) to track the issue.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-69971569

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds-labs-openstack #2053](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs-openstack/2053/) SUCCESS
This pull request looks good
[(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-labs-openstack/pull/174#issuecomment-65828376

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
> +    </logger>
> +
> +    <logger name="jclouds.wire">
> +        <level value="DEBUG" />
> +        <appender-ref ref="WIREFILE" />
> +    </logger>
> +
> +    <logger name="jclouds.headers">
> +        <level value="DEBUG" />
> +        <appender-ref ref="WIREFILE" />
> +    </logger>
> +
> +    <logger name="jclouds.blobstore">
> +        <level value="DEBUG" />
> +        <appender-ref ref="BLOBSTOREFILE" />
> +    </logger>

Remove this logger.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21495508

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Zack Shoylev <no...@github.com>.
Alright, +1 from me too.
Is there a list in jira for all the 1.9.0 items? There was more than one email with TODOs on the mailing list...

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-69998596

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
+1. We've commented the data provider approach on IRC and it is definitely a cleaner one and worth implementing, but out of the scope of this PR. I'm ok of addressing that in another PR now that we have a specific issue for that. Let's make it part of the list of things TBD for 1.9.0.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-69996944

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
> +         /****/
> +
> +         assertNotNull(ipv4PortId);
> +
> +         Port ipv4Port = portApi.get(ipv4PortId);
> +         assertNotNull(ipv4Port);
> +         assertEquals(ipv4Port.getId(), ipv4PortId);
> +         assertEquals(ipv4Port.getName(), "JClouds-Live-IPv4-Port");
> +
> +         assertNotNull(portApi.update(ipv4PortId, Port.updateBuilder().name("Updated").build()));
> +         Port updatedIpv4Port = portApi.get(ipv4PortId);
> +         assertEquals(updatedIpv4Port.getName(), "Updated");
> +
> +         assertTrue(portApi.delete(ipv4PortId));
> +         assertTrue(subnetApi.delete(ipv4SubnetId));
> +         assertTrue(networkApi.delete(networkId));

Same comment about resource cleanup.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21495442

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Jeremy Daggett <no...@github.com>.
@nacx @zack-shoylev Thanks! Pushed to **master** e934350

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-70009224

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Zack Shoylev <no...@github.com>.
I am ok with addressing these fixes in multiple PRs. Makes it easy to review too. @nacx ?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-66800665

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Ignasi Barrera <no...@github.com>.
> +
> +         assertEquals(network.getId(), net.getId());
> +         assertEquals(network.getName(), "jclouds-test");
> +         assertTrue(network.getSubnets().isEmpty());
> +         assertNotNull(networkApi.update(net.getId(), Network.updateBuilder().name("jclouds-live-test").build()));
> +
> +         network = networkApi.get(net.getId());
> +
> +         assertEquals(network.getId(), net.getId());
> +         assertEquals(network.getName(), "jclouds-live-test");
> +         assertTrue(network.getSubnets().isEmpty());
> +
> +         Network net2 = networkApi.create(Network.createBuilder("jclouds-test2").build());
> +         assertNotNull(net2);
> +
> +         assertTrue(networkApi.delete(net.getId()));

Move the cleanup to a separate method that runs `@AfterMethod(alwaytRun = true)` to avoid leaking resources if the test fails.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174/files#r21495405

Re: [jclouds-labs-openstack] Add support for Rackspace Cloud Networks UK (#174)

Posted by Zack Shoylev <no...@github.com>.
I would prefer a separate PR, but I am alright with doing it either way, as long as we can get the fixes in before next release.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs-openstack/pull/174#issuecomment-67071296