Allow ImageRef to be empty when block device is present (#97)
This commit removes the requirement for ImageRef to be set when creating
a server. This is to enable booting from a volume to work properly.
A unit test was added to verify this is possible.
Acceptance tests were also modified to handle this.
diff --git a/acceptance/openstack/compute/v2/bootfromvolume_test.go b/acceptance/openstack/compute/v2/bootfromvolume_test.go
index 05c0a99..844a7cb 100644
--- a/acceptance/openstack/compute/v2/bootfromvolume_test.go
+++ b/acceptance/openstack/compute/v2/bootfromvolume_test.go
@@ -26,22 +26,24 @@
blockDevices := []bootfromvolume.BlockDevice{
bootfromvolume.BlockDevice{
- UUID: choices.ImageID,
- SourceType: bootfromvolume.Image,
- VolumeSize: 10,
+ UUID: choices.ImageID,
+ SourceType: bootfromvolume.Image,
+ DeleteOnTermination: true,
+ DestinationType: "volume",
+ VolumeSize: 2,
},
}
server, err := CreateBootableVolumeServer(t, client, blockDevices, choices)
if err != nil {
- t.Fatal("Unable to create server: %v", err)
+ t.Fatalf("Unable to create server: %v", err)
}
defer DeleteServer(t, client, server)
PrintServer(t, server)
}
-func TestBootFromVolumeMultiEphemeral(t *testing.T) {
+func TestBootFromMultiEphemeralServer(t *testing.T) {
if testing.Short() {
t.Skip("Skipping test that requires server creation in short mode.")
}
@@ -83,7 +85,7 @@
},
}
- server, err := CreateBootableVolumeServer(t, client, blockDevices, choices)
+ server, err := CreateMultiEphemeralServer(t, client, blockDevices, choices)
if err != nil {
t.Fatalf("Unable to create server: %v", err)
}
diff --git a/acceptance/openstack/compute/v2/compute.go b/acceptance/openstack/compute/v2/compute.go
index 42a2c10..9b284c3 100644
--- a/acceptance/openstack/compute/v2/compute.go
+++ b/acceptance/openstack/compute/v2/compute.go
@@ -85,7 +85,6 @@
serverCreateOpts := servers.CreateOpts{
Name: name,
FlavorRef: choices.FlavorID,
- ImageRef: choices.ImageID,
Networks: []servers.Network{
servers.Network{UUID: networkID},
},
@@ -100,7 +99,13 @@
return server, err
}
- return server, nil
+ if err := WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
+ return server, err
+ }
+
+ newServer, err := servers.Get(client, server.ID).Extract()
+
+ return newServer, nil
}
// CreateDefaultRule will create a default security group rule with a
@@ -175,6 +180,53 @@
return keyPair, nil
}
+// CreateMultiEphemeralServer works like CreateServer but is configured with
+// one or more block devices defined by passing in []bootfromvolume.BlockDevice.
+// These block devices act like block devices when booting from a volume but
+// are actually local ephemeral disks.
+// An error will be returned if a server was unable to be created.
+func CreateMultiEphemeralServer(t *testing.T, client *gophercloud.ServiceClient, blockDevices []bootfromvolume.BlockDevice, choices *clients.AcceptanceTestChoices) (*servers.Server, error) {
+ if testing.Short() {
+ t.Skip("Skipping test that requires server creation in short mode.")
+ }
+
+ var server *servers.Server
+
+ networkID, err := GetNetworkIDFromTenantNetworks(t, client, choices.NetworkName)
+ if err != nil {
+ return server, err
+ }
+
+ name := tools.RandomString("ACPTTEST", 16)
+ t.Logf("Attempting to create bootable volume server: %s", name)
+
+ serverCreateOpts := servers.CreateOpts{
+ Name: name,
+ FlavorRef: choices.FlavorID,
+ ImageRef: choices.ImageID,
+ Networks: []servers.Network{
+ servers.Network{UUID: networkID},
+ },
+ }
+
+ server, err = bootfromvolume.Create(client, bootfromvolume.CreateOptsExt{
+ serverCreateOpts,
+ blockDevices,
+ }).Extract()
+
+ if err != nil {
+ return server, err
+ }
+
+ if err := WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
+ return server, err
+ }
+
+ newServer, err := servers.Get(client, server.ID).Extract()
+
+ return newServer, nil
+}
+
// CreateSecurityGroup will create a security group with a random name.
// An error will be returned if one was failed to be created.
func CreateSecurityGroup(t *testing.T, client *gophercloud.ServiceClient) (secgroups.SecurityGroup, error) {
@@ -257,7 +309,54 @@
return server, err
}
- if err = WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
+ if err := WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
+ return server, err
+ }
+
+ return server, nil
+}
+
+// CreateServerWithoutImageRef creates a basic instance with a randomly generated name.
+// The flavor of the instance will be the value of the OS_FLAVOR_ID environment variable.
+// The image is intentionally missing to trigger an error.
+// The instance will be launched on the network specified in OS_NETWORK_NAME.
+// An error will be returned if the instance was unable to be created.
+func CreateServerWithoutImageRef(t *testing.T, client *gophercloud.ServiceClient, choices *clients.AcceptanceTestChoices) (*servers.Server, error) {
+ if testing.Short() {
+ t.Skip("Skipping test that requires server creation in short mode.")
+ }
+
+ var server *servers.Server
+
+ networkID, err := GetNetworkIDFromTenantNetworks(t, client, choices.NetworkName)
+ if err != nil {
+ return server, err
+ }
+
+ name := tools.RandomString("ACPTTEST", 16)
+ t.Logf("Attempting to create server: %s", name)
+
+ pwd := tools.MakeNewPassword("")
+
+ server, err = servers.Create(client, servers.CreateOpts{
+ Name: name,
+ FlavorRef: choices.FlavorID,
+ AdminPass: pwd,
+ Networks: []servers.Network{
+ servers.Network{UUID: networkID},
+ },
+ Personality: servers.Personality{
+ &servers.File{
+ Path: "/etc/test",
+ Contents: []byte("hello world"),
+ },
+ },
+ }).Extract()
+ if err != nil {
+ return server, err
+ }
+
+ if err := WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
return server, err
}
@@ -356,7 +455,7 @@
return server, err
}
- if err = WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
+ if err := WaitForComputeStatus(client, server, "ACTIVE"); err != nil {
return server, err
}
@@ -376,7 +475,7 @@
return volumeAttachment, err
}
- if err = volumes.WaitForStatus(blockClient, volume.ID, "in-use", 60); err != nil {
+ if err := volumes.WaitForStatus(blockClient, volume.ID, "in-use", 60); err != nil {
return volumeAttachment, err
}
@@ -476,7 +575,7 @@
t.Fatalf("Unable to detach volume: %v", err)
}
- if err = volumes.WaitForStatus(blockClient, volumeAttachment.ID, "available", 60); err != nil {
+ if err := volumes.WaitForStatus(blockClient, volumeAttachment.ID, "available", 60); err != nil {
t.Fatalf("Unable to wait for volume: %v", err)
}
t.Logf("Deleted volume: %s", volumeAttachment.VolumeID)
diff --git a/acceptance/openstack/compute/v2/servers_test.go b/acceptance/openstack/compute/v2/servers_test.go
index f5b60c5..fa6603d 100644
--- a/acceptance/openstack/compute/v2/servers_test.go
+++ b/acceptance/openstack/compute/v2/servers_test.go
@@ -3,8 +3,10 @@
package v2
import (
+ "strings"
"testing"
+ "github.com/gophercloud/gophercloud"
"github.com/gophercloud/gophercloud/acceptance/clients"
"github.com/gophercloud/gophercloud/acceptance/tools"
"github.com/gophercloud/gophercloud/openstack/compute/v2/servers"
@@ -86,6 +88,27 @@
}
}
+func TestServersWithoutImageRef(t *testing.T) {
+ client, err := clients.NewComputeV2Client()
+ if err != nil {
+ t.Fatalf("Unable to create a compute client: %v", err)
+ }
+
+ choices, err := clients.AcceptanceTestChoicesFromEnv()
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ server, err := CreateServerWithoutImageRef(t, client, choices)
+ if err != nil {
+ if err400, ok := err.(*gophercloud.ErrUnexpectedResponseCode); ok {
+ if !strings.Contains("Missing imageRef attribute", string(err400.Body)) {
+ defer DeleteServer(t, client, server)
+ }
+ }
+ }
+}
+
func TestServersUpdate(t *testing.T) {
client, err := clients.NewComputeV2Client()
if err != nil {
diff --git a/openstack/compute/v2/extensions/bootfromvolume/testing/requests_test.go b/openstack/compute/v2/extensions/bootfromvolume/testing/requests_test.go
index dca1105..6724dab 100644
--- a/openstack/compute/v2/extensions/bootfromvolume/testing/requests_test.go
+++ b/openstack/compute/v2/extensions/bootfromvolume/testing/requests_test.go
@@ -52,6 +52,49 @@
th.CheckJSONEquals(t, expected, actual)
}
+func TestCreateOptsWithoutImageRef(t *testing.T) {
+ base := servers.CreateOpts{
+ Name: "createdserver",
+ FlavorRef: "performance1-1",
+ }
+
+ ext := bootfromvolume.CreateOptsExt{
+ CreateOptsBuilder: base,
+ BlockDevice: []bootfromvolume.BlockDevice{
+ {
+ UUID: "123456",
+ SourceType: bootfromvolume.Image,
+ DestinationType: "volume",
+ VolumeSize: 10,
+ DeleteOnTermination: false,
+ },
+ },
+ }
+
+ expected := `
+ {
+ "server": {
+ "name": "createdserver",
+ "imageRef": "",
+ "flavorRef": "performance1-1",
+ "block_device_mapping_v2":[
+ {
+ "uuid":"123456",
+ "source_type":"image",
+ "destination_type":"volume",
+ "boot_index": 0,
+ "delete_on_termination": false,
+ "volume_size": 10
+ }
+ ]
+ }
+ }
+ `
+ actual, err := ext.ToServerCreateMap()
+ th.AssertNoErr(t, err)
+ th.CheckJSONEquals(t, expected, actual)
+}
+
func TestCreateMultiEphemeralOpts(t *testing.T) {
base := servers.CreateOpts{
Name: "createdserver",
diff --git a/openstack/compute/v2/servers/requests.go b/openstack/compute/v2/servers/requests.go
index fcc8b2e..4ec2cf0 100644
--- a/openstack/compute/v2/servers/requests.go
+++ b/openstack/compute/v2/servers/requests.go
@@ -219,23 +219,21 @@
b["networks"] = networks
}
- // If ImageRef isn't provided, use ImageName to ascertain the image ID.
+ // If ImageRef isn't provided, check if ImageName was provided to ascertain
+ // the image ID.
if opts.ImageRef == "" {
- if opts.ImageName == "" {
- err := ErrNeitherImageIDNorImageNameProvided{}
- err.Argument = "ImageRef/ImageName"
- return nil, err
+ if opts.ImageName != "" {
+ if sc == nil {
+ err := ErrNoClientProvidedForIDByName{}
+ err.Argument = "ServiceClient"
+ return nil, err
+ }
+ imageID, err := images.IDFromName(sc, opts.ImageName)
+ if err != nil {
+ return nil, err
+ }
+ b["imageRef"] = imageID
}
- if sc == nil {
- err := ErrNoClientProvidedForIDByName{}
- err.Argument = "ServiceClient"
- return nil, err
- }
- imageID, err := images.IDFromName(sc, opts.ImageName)
- if err != nil {
- return nil, err
- }
- b["imageRef"] = imageID
}
// If FlavorRef isn't provided, use FlavorName to ascertain the flavor ID.
@@ -424,23 +422,21 @@
return nil, err
}
- // If ImageRef isn't provided, use ImageName to ascertain the image ID.
+ // If ImageRef isn't provided, check if ImageName was provided to ascertain
+ // the image ID.
if opts.ImageID == "" {
- if opts.ImageName == "" {
- err := ErrNeitherImageIDNorImageNameProvided{}
- err.Argument = "ImageRef/ImageName"
- return nil, err
+ if opts.ImageName != "" {
+ if opts.ServiceClient == nil {
+ err := ErrNoClientProvidedForIDByName{}
+ err.Argument = "ServiceClient"
+ return nil, err
+ }
+ imageID, err := images.IDFromName(opts.ServiceClient, opts.ImageName)
+ if err != nil {
+ return nil, err
+ }
+ b["imageRef"] = imageID
}
- if opts.ServiceClient == nil {
- err := ErrNoClientProvidedForIDByName{}
- err.Argument = "ServiceClient"
- return nil, err
- }
- imageID, err := images.IDFromName(opts.ServiceClient, opts.ImageName)
- if err != nil {
- return nil, err
- }
- b["imageRef"] = imageID
}
return map[string]interface{}{"rebuild": b}, nil