Skip to content

Commit

Permalink
Don't try to dedupe guest accelerator expansion code
Browse files Browse the repository at this point in the history
The API calls to Google to create guest accelerators take different values
for instances and instance templates. Instance templates don't have a zone
and can thus *only* be passed a guest accelerator name.
  • Loading branch information
Nic Cope committed Nov 15, 2017
1 parent c16efb8 commit 5ff4880
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 30 deletions.
4 changes: 4 additions & 0 deletions google/field_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ func ParseOrganizationCustomRoleName(role string) (*OrganizationFieldValue, erro
return parseOrganizationFieldValue("roles", role, false)
}

func ParseAcceleratorFieldValue(accelerator string, d TerraformResourceData, config *Config) (*ZonalFieldValue, error) {
return parseZonalFieldValue("acceleratorTypes", accelerator, "project", "zone", d, config, false)
}

// ------------------------------------------------------------
// Base helpers used to create helpers for specific fields.
// ------------------------------------------------------------
Expand Down
31 changes: 30 additions & 1 deletion google/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,11 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
return fmt.Errorf("Error creating network interfaces: %s", err)
}

accels, err := expandInstanceGuestAccelerators(d, config)
if err != nil {
return fmt.Errorf("Error creating guest accelerators: %s", err)
}

// Create the instance information
instance := &compute.Instance{
CanIpForward: d.Get("can_ip_forward").(bool),
Expand All @@ -650,7 +655,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
Tags: resourceInstanceTags(d),
Labels: expandLabels(d),
ServiceAccounts: expandServiceAccounts(d.Get("service_account").([]interface{})),
GuestAccelerators: expandGuestAccelerators(zone.Name, d.Get("guest_accelerator").([]interface{})),
GuestAccelerators: accels,
MinCpuPlatform: d.Get("min_cpu_platform").(string),
Scheduling: scheduling,
}
Expand Down Expand Up @@ -1123,6 +1128,30 @@ func expandAttachedDisk(diskConfig map[string]interface{}, d *schema.ResourceDat
return disk, nil
}

// See comment on expandInstanceTemplateGuestAccelerators regarding why this
// code is duplicated.
func expandInstanceGuestAccelerators(d TerraformResourceData, config *Config) ([]*compute.AcceleratorConfig, error) {
configs, ok := d.GetOk("guest_accelerator")
if !ok {
return nil, nil
}
accels := configs.([]interface{})
guestAccelerators := make([]*compute.AcceleratorConfig, len(accels))
for i, raw := range accels {
data := raw.(map[string]interface{})
at, err := ParseAcceleratorFieldValue(data["type"].(string), d, config)
if err != nil {
return nil, fmt.Errorf("cannot parse accelerator type: %v", err)
}
guestAccelerators[i] = &compute.AcceleratorConfig{
AcceleratorCount: int64(data["count"].(int)),
AcceleratorType: at.RelativeLink(),
}
}

return guestAccelerators, nil
}

func resourceComputeInstanceDelete(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

Expand Down
31 changes: 30 additions & 1 deletion google/resource_compute_instance_template.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,35 @@ func buildDisks(d *schema.ResourceData, config *Config) ([]*compute.AttachedDisk
return disks, nil
}

// We don't share this code with compute instances because instances want a
// partial URL, but instance templates want the bare accelerator name (despite
// the docs saying otherwise).
//
// Using a partial URL on an instance template results in:
// Invalid value for field 'resource.properties.guestAccelerators[0].acceleratorType':
// 'zones/us-east1-b/acceleratorTypes/nvidia-tesla-k80'.
// Accelerator type 'zones/us-east1-b/acceleratorTypes/nvidia-tesla-k80'
// must be a valid resource name (not an url).
func expandInstanceTemplateGuestAccelerators(d TerraformResourceData, config *Config) []*compute.AcceleratorConfig {
configs, ok := d.GetOk("guest_accelerator")
if !ok {
return nil
}
accels := configs.([]interface{})
guestAccelerators := make([]*compute.AcceleratorConfig, len(accels))
for i, raw := range accels {
data := raw.(map[string]interface{})
guestAccelerators[i] = &compute.AcceleratorConfig{
AcceleratorCount: int64(data["count"].(int)),
// We can't use ParseAcceleratorFieldValue here because an instance
// template does not have a zone we can use.
AcceleratorType: data["type"].(string),
}
}

return guestAccelerators
}

func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

Expand Down Expand Up @@ -545,7 +574,7 @@ func resourceComputeInstanceTemplateCreate(d *schema.ResourceData, meta interfac

instanceProperties.ServiceAccounts = expandServiceAccounts(d.Get("service_account").([]interface{}))

instanceProperties.GuestAccelerators = expandGuestAccelerators("", d.Get("guest_accelerator").([]interface{}))
instanceProperties.GuestAccelerators = expandInstanceTemplateGuestAccelerators(d, config)

instanceProperties.Tags = resourceInstanceTags(d)
if _, ok := d.GetOk("labels"); ok {
Expand Down
28 changes: 0 additions & 28 deletions google/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -596,19 +596,6 @@ func expandServiceAccounts(configs []interface{}) []*compute.ServiceAccount {
return accounts
}

func expandGuestAccelerators(zone string, configs []interface{}) []*compute.AcceleratorConfig {
guestAccelerators := make([]*compute.AcceleratorConfig, len(configs))
for i, raw := range configs {
data := raw.(map[string]interface{})
guestAccelerators[i] = &compute.AcceleratorConfig{
AcceleratorCount: int64(data["count"].(int)),
AcceleratorType: acceleratorRef(zone, data["type"].(string)),
}
}

return guestAccelerators
}

func flattenGuestAccelerators(accelerators []*compute.AcceleratorConfig) []map[string]interface{} {
acceleratorsSchema := make([]map[string]interface{}, len(accelerators))
for i, accelerator := range accelerators {
Expand All @@ -619,18 +606,3 @@ func flattenGuestAccelerators(accelerators []*compute.AcceleratorConfig) []map[s
}
return acceleratorsSchema
}

// Instances want a partial URL, but instance templates want the bare
// accelerator name without zone (despite the docs saying otherwise).
//
// Using a partial URL on an instance template results in:
// Invalid value for field 'resource.properties.guestAccelerators[0].acceleratorType':
// 'zones/us-east1-b/acceleratorTypes/nvidia-tesla-k80'.
// Accelerator type 'zones/us-east1-b/acceleratorTypes/nvidia-tesla-k80'
// must be a valid resource name (not an url).
func acceleratorRef(zone, accelerator string) string {
if strings.HasPrefix(accelerator, "zones/") || zone == "" {
return accelerator
}
return fmt.Sprintf("zones/%s/acceleratorTypes/%s", zone, accelerator)
}

0 comments on commit 5ff4880

Please sign in to comment.