Bugfixes and tidying for vcloud-director and automated test tool

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/dmtf/RasdItem.java

@@ -20,7 +20,6 @@ package org.jclouds.vcloud.director.v1_5.domain.dmtf;
 
 import static com.google.common.base.Objects.equal;
 import static com.google.common.base.Preconditions.checkNotNull;
-import static org.jclouds.dmtf.DMTFConstants.OVF_NS;
 import static org.jclouds.vcloud.director.v1_5.VCloudDirectorConstants.VCLOUD_1_5_NS;
 
 import java.net.URI;
@@ -44,7 +43,7 @@ import com.google.common.collect.Sets;
  * 
  * @author grkvlt@apache.org
  */
-@XmlRootElement(name = "Item", namespace = OVF_NS)
+@XmlRootElement(name = "Item", namespace = VCLOUD_1_5_NS)
 public class RasdItem extends ResourceAllocationSettingData {
 
    public static Builder<?> builder() {

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/network/NetworkConnection.java

@@ -76,7 +76,7 @@ public class NetworkConnection {
       @XmlEnumValue("pool") POOL("pool"),
       @XmlEnumValue("dhcp") DHCP("dhcp"),
       @XmlEnumValue("manual") MANUAL("manual"),
-      @XmlEnumValue("none") NONE("none"),
+      @XmlEnumValue("NONE") NONE("none"),
       @XmlEnumValue("") UNRECOGNIZED("unrecognized");
       
       public static final List<IpAddressAllocationMode> ALL = ImmutableList.of(POOL, DHCP, MANUAL, NONE);

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/params/ControlAccessParams.java

@@ -133,10 +133,11 @@ public class ControlAccessParams {
       this.sharedToEveryone = sharedToEveryone;
       if (sharedToEveryone) {
 	      this.everyoneAccessLevel = checkNotNull(everyoneAccessLevel, "everyoneAccessLevel");
+	      this.accessSettings = null;
       } else {
-         checkNotNull(accessSettings, "accessSettings");
+	      this.everyoneAccessLevel = null;
+         this.accessSettings = Iterables.isEmpty(checkNotNull(accessSettings, "accessSettings")) ? null : ImmutableList.copyOf(accessSettings);
       }
-      this.accessSettings = Iterables.isEmpty(accessSettings) ? null : ImmutableList.copyOf(accessSettings);
    }
 
    @XmlElement(name = "IsSharedToEveryone", required = true)

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/OrgClientLiveTest.java

@@ -185,7 +185,7 @@ public class OrgClientLiveTest extends BaseVCloudDirectorClientLiveTest {
       checkControlAccessParams(params);
    }
 
-   @Test(description = "GET /org/{id}/catalog/{catalogId}/action/controlAccess", dependsOnMethods = { "testGetControlAccess" })
+   @Test(description = "POST /org/{id}/catalog/{catalogId}/action/controlAccess", dependsOnMethods = { "testGetControlAccess" })
    public void testModifyControlAccess() {
       // Setup params
       ControlAccessParams params = orgClient.getControlAccess(orgURI, testCatalogId);

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VAppClientLiveTest.java

@@ -395,8 +395,9 @@ public class VAppClientLiveTest extends AbstractVAppClientLiveTest {
 
    @Test(description = "POST /vApp/{id}/action/discardSuspendedState", dependsOnMethods = { "testDeployVApp" })
    public void testDiscardSuspendedState() {
-      // Suspend the VApp
+      // Power on, then suspend the VApp
-      vApp = suspendVApp(vApp.getHref());
+      vApp = powerOnVApp(vAppURI);
+      vApp = suspendVApp(vAppURI);
       
       // The method under test
       Task discardSuspendedState = vAppClient.discardSuspendedState(vApp.getHref());

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VAppTemplateClientLiveTest.java

@@ -112,7 +112,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       if (waitForTask) {
          Task cloneTask = Iterables.getFirst(clonedVappTemplate.getTasks(), null);
          assertNotNull(cloneTask, "vdcClient.cloneVAppTemplate returned VAppTemplate that did not contain any tasks");
-         retryTaskSuccess.apply(cloneTask);
+         assertTaskSucceeds(cloneTask);
       }
 
       return clonedVappTemplate;
@@ -174,7 +174,6 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
    
    @Test(description = "GET /vAppTemplate/{id}/leaseSettingsSection")
    public void testGetLeaseSettingsSection() {
-      // FIXME Wrong case for Vapp
       LeaseSettingsSection leaseSettingsSection = vAppTemplateClient.getLeaseSettingsSection(vAppTemplateURI);
       
       checkLeaseSettingsSection(leaseSettingsSection);
@@ -240,7 +239,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
                .build();
       
       final Task task = vAppTemplateClient.modifyVAppTemplate(vAppTemplateURI, template);
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
 
       VAppTemplate newTemplate = vAppTemplateClient.getVAppTemplate(vAppTemplateURI);
       assertEquals(newTemplate.getName(), name);
@@ -258,7 +257,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       Metadata metadata = Metadata.builder().fromMetadata(oldMetadata).entry(metadataEntry).build();
       
       final Task task = vAppTemplateClient.getMetadataClient().mergeMetadata(vAppTemplateURI, metadata);
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
 
       Metadata newMetadata = vAppTemplateClient.getMetadataClient().getMetadata(vAppTemplateURI);
       Map<String,String> expectedMetadataMap = ImmutableMap.<String,String>builder()
@@ -283,7 +282,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
    @Test(description = "DELETE /vAppTemplate/{id}/metadata/{key}", dependsOnMethods = { "testGetMetadataValue" })
    public void testDeleteVAppTemplateMetadataValue() {
       final Task deletionTask = vAppTemplateClient.getMetadataClient().deleteMetadataEntry(vAppTemplateURI, key);
-      retryTaskSuccess.apply(deletionTask);
+      assertTaskSucceeds(deletionTask);
 
       Metadata newMetadata = vAppTemplateClient.getMetadataClient().getMetadata(vAppTemplateURI);
       checkMetadataKeyAbsentFor("vAppTemplate", newMetadata, key);
@@ -318,7 +317,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
                .build();
       
       final Task task = vAppTemplateClient.modifyCustomizationSection(vAppTemplateURI, customizationSection);
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
 
       CustomizationSection newCustomizationSection = vAppTemplateClient.getCustomizationSection(vAppTemplateURI);
       assertEquals(newCustomizationSection.isCustomizeOnInstantiate(), newVal);
@@ -338,7 +337,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
                .build();
       
       final Task task = vAppTemplateClient.modifyLeaseSettingsSection(vAppTemplateURI, leaseSettingSection);
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
       
       LeaseSettingsSection newLeaseSettingsSection = vAppTemplateClient.getLeaseSettingsSection(vAppTemplateURI);
       assertEquals(newLeaseSettingsSection.getStorageLeaseInSeconds(), (Integer) storageLeaseInSeconds);
@@ -411,7 +410,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       
       // Delete the template
       final Task task = vAppTemplateClient.deleteVappTemplate(clonedVappTemplate.getHref());
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
 
       // Confirm that can't access post-delete, i.e. template has been deleted
       VAppTemplate deleted = vAppTemplateClient.getVAppTemplate(clonedVappTemplate.getHref());
@@ -436,7 +435,7 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       // First disable so that enable really has some work to do...
       vAppTemplateClient.disableDownload(vAppTemplateURI);
       final Task task = vAppTemplateClient.enableDownload(vAppTemplateURI);
-      retryTaskSuccess.apply(task);
+      assertTaskSucceeds(task);
       
       // TODO Check that it really is enabled. The only thing I can see for determining this 
       // is the undocumented "download" link in the VAppTemplate. But that is brittle and we
@@ -462,9 +461,9 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       // TODO Need assertion that command had effect
    }
    
-   @Test(description = "POST /vAppTemplate/{id}/action/relocate") // FIXME Need a datastore reference
+   // TODO How to obtain a datastore reference?
+   @Test(description = "POST /vAppTemplate/{id}/action/relocate")
    public void testRelocateVAppTemplate() throws Exception {
-      // TODO Need assertion that command had effect
       Reference dataStore = null; // FIXME
       RelocateParams relocateParams = RelocateParams.builder()
                .datastore(dataStore)
@@ -472,9 +471,12 @@ public class VAppTemplateClientLiveTest extends AbstractVAppClientLiveTest {
       
       final Task task = vAppTemplateClient.relocateVm(vAppTemplateURI, relocateParams);
       assertTaskSucceedsLong(task);
+
+      // TODO Need assertion that command had effect
    }
    
-   @Test(description = "GET /vAppTemplate/{id}/shadowVms")
+   // NOTE This will fail unless we can relocate a template to another datastore
+   @Test(description = "GET /vAppTemplate/{id}/shadowVms", dependsOnMethods = { "testRelocateVAppTemplate" })
    public void testGetShadowVms() {
       References references = vAppTemplateClient.getShadowVms(vAppTemplateURI);
       

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VmClientLiveTest.java

@@ -198,7 +198,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       checkVm(vm);
 
       // Check the required fields are set
-      assertEquals(vm.isDeployed(), Boolean.FALSE, String.format(OBJ_FIELD_EQ, VAPP, "deployed", "FALSE", vm.isDeployed().toString()));
+      assertEquals(vm.isDeployed(), Boolean.FALSE, String.format(OBJ_FIELD_EQ, VM, "deployed", "FALSE", vm.isDeployed().toString()));
 
       // Check status
       assertVmStatus(vm.getHref(), Status.POWERED_OFF);
@@ -349,7 +349,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       vm = vmClient.getVm(vm.getHref());
 
       // Check status
-      assertFalse(vm.isDeployed(), String.format(OBJ_FIELD_EQ, VAPP, "deployed", "FALSE", vm.isDeployed().toString()));
+      assertFalse(vm.isDeployed(), String.format(OBJ_FIELD_EQ, VM, "deployed", "FALSE", vm.isDeployed().toString()));
       assertVmStatus(vmURI, Status.POWERED_OFF);
    }
 
@@ -663,7 +663,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       }
    }
 
-   @Test(description = "GET /vApp/{id}/screen/action/acquireTicket", dependsOnMethods = { "testDeployVm" })
+   @Test(description = "POST /vApp/{id}/screen/action/acquireTicket", dependsOnMethods = { "testDeployVm" })
    public void testGetScreenTicket() {
       // Power on Vm
       vm = powerOnVm(vm.getHref());
@@ -911,7 +911,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       // See the description in testModifyVirtualHardwareSectionDisks
    }
 
-   @Test(description = "PUT /vApp/{id}/metadata", dependsOnMethods = { "testGetVm" })
+   @Test(description = "PUT /vApp/{id}/metadata/{key}", dependsOnMethods = { "testGetVm" })
    public void testSetMetadataValue() {
       key = name("key-");
       String value = name("value-");
@@ -922,7 +922,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       MetadataValue newMetadataValue = vmClient.getMetadataClient().getMetadataValue(vm.getHref(), key);
 
       // Check the retrieved object is well formed
-      checkMetadataValueFor(VAPP, newMetadataValue, value);
+      checkMetadataValueFor(VM, newMetadataValue, value);
    }
    
    @Test(description = "GET /vApp/{id}/metadata", dependsOnMethods = { "testSetMetadataValue" })
@@ -957,7 +957,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       Metadata newMetadata = vmClient.getMetadataClient().getMetadata(vm.getHref());
 
       // Check the retrieved object is well formed
-      checkMetadataKeyAbsentFor(VAPP, newMetadata, key);
+      checkMetadataKeyAbsentFor(VM, newMetadata, key);
    }
 
    @Test(description = "POST /vApp/{id}/metadata", dependsOnMethods = { "testGetMetadata" })
@@ -982,7 +982,7 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
             .build();
 
       // Check the retrieved object is well formed
-      checkMetadataFor(VAPP, newMetadata, expectedMetadataMap);
+      checkMetadataFor(VM, newMetadata, expectedMetadataMap);
    }
 
    /**
@@ -1004,6 +1004,9 @@ public class VmClientLiveTest extends AbstractVAppClientLiveTest {
       delete = vAppClient.getVApp(delete.getHref());
       Vm temp = Iterables.getOnlyElement(delete.getChildren().getVms());
 
+      // Power off the Vm
+      temp = powerOffVm(temp.getHref());
+
       // The method under test
       Task deleteVm = vmClient.deleteVm(temp.getHref());
       assertTrue(retryTaskSuccess.apply(deleteVm), String.format(TASK_COMPLETE_TIMELY, "deleteVm"));

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/internal/VCloudDirectorTestSession.java

@@ -107,8 +107,8 @@ public class VCloudDirectorTestSession implements Closeable {
 
    @Override
    public void close() {
+      // NOTE we only need to close the user context to log out
       Closeables.closeQuietly(userContext);
-      Closeables.closeQuietly(adminContext);
    }
 
    public VCloudDirectorContext getUserContext() {

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/login/SessionClientLiveTest.java

@@ -64,7 +64,7 @@ public class SessionClientLiveTest extends BaseContextLiveTest<RestContext<Sessi
    private SessionClient client;
    private SessionWithToken sessionWithToken;
    
-   @Test(testName = "POST /sessions")
+   @Test(description = "POST /sessions")
    public void testLogin() {
       String user = identity.substring(0, identity.lastIndexOf('@'));
       String org = identity.substring(identity.lastIndexOf('@') + 1);