Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

 -}
 + }
 +  }
 +   }
 +
 +   @Test
 +   public void testCreateInWrongAvailabilityZone() {
 +  String serverId = null;
 +  for (String zoneId : zones) {
 + ServerApi serverApi = api.getServerApiForZone(zoneId);
 + try {
 +serverId = createServer(zoneId, err, 
 Server.Status.ERROR).getId();
 +Server server = serverApi.get(serverId);
 +assertEquals(server.getStatus(), Server.Status.ERROR);
 + } finally {
 +serverApi.delete(serverId);

I actually don't know about this... this is mostly reformatting old code for 
style.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840288

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 -}
 + }
 +  }
 +   }
 +
 +   @Test
 +   public void testCreateInWrongAvailabilityZone() {
 +  String serverId = null;
 +  for (String zoneId : zones) {
 + ServerApi serverApi = api.getServerApiForZone(zoneId);
 + try {
 +serverId = createServer(zoneId, err, 
 Server.Status.ERROR).getId();
 +Server server = serverApi.get(serverId);
 +assertEquals(server.getStatus(), Server.Status.ERROR);
 + } finally {
 +serverApi.delete(serverId);

 this is mostly reformatting old code for style

Perhaps we can improve it (or at least iron out a possible bug) at the same 
time? ;-)

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840353

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

 -  }
 -   }
 -}
 -
 -@Test
 -public void testCreateInAvailabilityZone() {
 -String serverId = null;
 -for (String zoneId : zones) {
 -ServerApi serverApi = api.getServerApiForZone(zoneId);
 -try {
 -serverId = createServer(zoneId, nova, 
 Server.Status.ACTIVE).getId();
 -Server server = serverApi.get(serverId);
 -assertEquals(server.getStatus(), Server.Status.ACTIVE);
 -} finally {
 -serverApi.delete(serverId);
 +   @Test(description = GET /v${apiVersion}/{tenantId}/servers)

I think this is also just identation fixes.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840424

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

 + * See the License for the specific language governing permissions and
 + * limitations under the License.
 + */
 +package org.jclouds.openstack.nova.v2_0.domain;
 +
 +import java.beans.ConstructorProperties;
 +
 +import com.google.common.base.Objects;
 +import com.google.common.base.Objects.ToStringHelper;
 +
 +import static com.google.common.base.Preconditions.checkArgument;
 +
 +/**
 + * Nova (or Neutron) network definition
 + * Used to provide support for network, port, and fixed_ip when booting Nova 
 servers.
 + * OpenStack will support either a Nova Network or Neutron, but not both at 
 the same time.

OpenStack documentation has is capitalized:
http://docs.openstack.org/trunk/openstack-ops/content/nova-network-deprecation.html

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840577

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

 -}
 + }
 +  }
 +   }
 +
 +   @Test
 +   public void testCreateInWrongAvailabilityZone() {
 +  String serverId = null;
 +  for (String zoneId : zones) {
 + ServerApi serverApi = api.getServerApiForZone(zoneId);
 + try {
 +serverId = createServer(zoneId, err, 
 Server.Status.ERROR).getId();
 +Server server = serverApi.get(serverId);
 +assertEquals(server.getStatus(), Server.Status.ERROR);
 + } finally {
 +serverApi.delete(serverId);

I think all of this code will benefit from a separate cleanup PR.
Also: the live tests have been very fragile for me, so they should be ... 
refactored.
I don't want to modify this because it already got merged, and will probably 
have to be fixed in master first and then backported (which is why separate PR).

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10840920

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 @@ -537,7 +558,8 @@ public NovaTemplateOptions nodeNames(IterableString 
 nodeNames) {
 }
  
 /**
 -* {@inheritDoc}
 +* brEnsures NovaTemplateOptions can work with networks specified as 
 Strings.

What's the `br` for here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826214

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 @@ -545,6 +567,15 @@ public NovaTemplateOptions networks(IterableString 
 networks) {
 }
  
 /**
 +* brEnsures NovaTemplateOptions can work with networks specified as 
 Strings.

What's the `br` for here?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826218

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 + */
 +package org.jclouds.openstack.nova.v2_0.domain;
 +
 +import java.beans.ConstructorProperties;
 +
 +import com.google.common.base.Objects;
 +import com.google.common.base.Objects.ToStringHelper;
 +
 +import static com.google.common.base.Preconditions.checkArgument;
 +
 +/**
 + * Nova (or Neutron) network definition
 + * Used to provide support for network, port, and fixed_ip when booting Nova 
 servers.
 + * OpenStack will support either a Nova Network or Neutron, but not both at 
 the same time.
 + * Specifying a port is only possible with Neutron.
 + * @author Zack Shoylev

[minor] Blank line before `@author`?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826266

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 + * See the License for the specific language governing permissions and
 + * limitations under the License.
 + */
 +package org.jclouds.openstack.nova.v2_0.domain;
 +
 +import java.beans.ConstructorProperties;
 +
 +import com.google.common.base.Objects;
 +import com.google.common.base.Objects.ToStringHelper;
 +
 +import static com.google.common.base.Preconditions.checkArgument;
 +
 +/**
 + * Nova (or Neutron) network definition
 + * Used to provide support for network, port, and fixed_ip when booting Nova 
 servers.
 + * OpenStack will support either a Nova Network or Neutron, but not both at 
 the same time.

[minor] Is this network?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826287

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 @@ -364,6 +386,16 @@ public CreateServerOptions availabilityZone(String 
 availabilityZone) {
 public SetString getNetworks() {
return networks;
 }
 +   
 +   /**
 +* Get custom networks specified for the server.

[minor] Gets

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826455

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 -  }
 -   }
 -}
 -
 -@Test
 -public void testCreateInAvailabilityZone() {
 -String serverId = null;
 -for (String zoneId : zones) {
 -ServerApi serverApi = api.getServerApiForZone(zoneId);
 -try {
 -serverId = createServer(zoneId, nova, 
 Server.Status.ACTIVE).getId();
 -Server server = serverApi.get(serverId);
 -assertEquals(server.getStatus(), Server.Status.ACTIVE);
 -} finally {
 -serverApi.delete(serverId);
 +   @Test(description = GET /v${apiVersion}/{tenantId}/servers)

I like this (the description), but is it something we do anywhere else..?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826467

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 +serverId = createServer(zoneId, nova, 
 Server.Status.ACTIVE).getId();
 +Server server = serverApi.get(serverId);
 +assertEquals(server.getStatus(), Server.Status.ACTIVE);
 + } finally {
 +serverApi.delete(serverId);
 + }
 +  }
 +   }
 +
 +   /**
 +* This needs to be supported by the provider, and is usually not 
 supported.
 +* However this can be tested on devstack:
 +* In apis/openstack-nova:
 +* mvn -Plive clean install 
 -Dtest.openstack-nova.endpoint=http://localhost:5000/v2.0; 
 -Dtest.openstack-nova.identity=demo:demo 
 -Dtest.openstack-nova.credential=devstack 
 -Dtest=org.jclouds.openstack.nova.v2_0.features.ServerApiLiveTest#testCreateWithNetworkOptions
 +*/
 +   @Test(enabled = false)

Hm...is there some way we can programmatically test whether to run this test or 
not, rather than adding a disabled test to the code base, that is liable to rot?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826500

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 +  withImage(server.getImage().getId()).
 +  name(newName).
 +  adminPass(password).
 +  ipv4Address(1.1.1.1).
 +  ipv6Address(fe80::100);
 +
 +serverApi.rebuild(serverId, options);
 +
 +Server rebuiltServer = serverApi.get(serverId);
 +
 +assertEquals(newName, rebuiltServer.getName());
 +assertEquals(1.1.1.1, rebuiltServer.getAccessIPv4());
 +assertEquals(fe80::100, rebuiltServer.getAccessIPv6());
 +
 + } finally {
 +serverApi.delete(serverId);

What if serverId is still null at this point?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826578

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 -}
 + }
 +  }
 +   }
 +
 +   @Test
 +   public void testCreateInWrongAvailabilityZone() {
 +  String serverId = null;
 +  for (String zoneId : zones) {
 + ServerApi serverApi = api.getServerApiForZone(zoneId);
 + try {
 +serverId = createServer(zoneId, err, 
 Server.Status.ERROR).getId();
 +Server server = serverApi.get(serverId);
 +assertEquals(server.getStatus(), Server.Status.ERROR);
 + } finally {
 +serverApi.delete(serverId);

What if serverId is still null at this point?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826577

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Andrew Phillips]

 +serverApi.delete(serverId);
 + }
 +  }
 +   }
 +
 +   private Server createServer(String regionId, Server.Status serverStatus) {
 +  ServerApi serverApi = api.getServerApiForZone(regionId);
 +  CreateServerOptions options = new CreateServerOptions();
 +  ServerCreated server = serverApi.create(hostName, 
 imageIdForZone(regionId), flavorRefForZone(regionId), options);
 +
 +  blockUntilServerInState(server.getId(), serverApi, serverStatus);
 +
 +  return serverApi.get(server.getId());
 +   }
 +
 +   private Server createServer(String regionId, String availabilityZoneId, 
 Server.Status serverStatus) {

[minor] Rather than duplicate almost the whole method, add a null switch for 
availabilityZone?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313/files#r10826604

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

Closed #313.

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

[jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

...orts.
You can merge this Pull Request by running:

  git pull https://github.com/rackspace/jclouds 
backport-add-nova-neutron-networks

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

  https://github.com/jclouds/jclouds/pull/313

-- Commit Summary --

  * JCLOUDS-486 This will allow booting up nova servers with fixed IPs and 
ports.

-- File Changes --

M 
apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java
 (7)
M 
apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java
 (48)
A 
apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/Network.java
 (173)
M 
apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java
 (103)
M 
apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapterExpectTest.java
 (49)
M 
apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/features/ServerApiLiveTest.java
 (288)
A apis/openstack-nova/src/test/resources/new_server_nova_networks.json (41)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/313.patch
https://github.com/jclouds/jclouds/pull/313.diff

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

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[Zack Shoylev]

Note: backporting

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313#issuecomment-37348415

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[CloudBees pull request builder plugin]

[jclouds-pull-requests 
#654](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/654/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313#issuecomment-37352983

Re: [jclouds] JCLOUDS-486 This will allow booting up nova servers with fixed IPs and p... (#313)

[CloudBees pull request builder plugin]

[jclouds-java-7-pull-requests 
#1124](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1124/) 
SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/313#issuecomment-37353113