Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handwritten id updates for several compute resources #2502

Merged
merged 8 commits into from
Oct 23, 2019
Merged

Handwritten id updates for several compute resources #2502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
642d38f
Composer environment and compute attached disk id updates
slevenick Oct 21, 2019
ed51673
Add compute instance
slevenick Oct 21, 2019
73fbeb3
Add instance_from_group and instance_group
slevenick Oct 21, 2019
5122763
IGM id
slevenick Oct 22, 2019
47d5b65
Instance template tests passing
slevenick Oct 22, 2019
e7a26d8
Add new id format to other calls of parseImportId in IGM
slevenick Oct 22, 2019
271f49a
Small refactor to memoize var
slevenick Oct 23, 2019
fdbf488
Refactor, remove parseImportId in IGM
slevenick Oct 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion third_party/terraform/resources/resource_composer_environment.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ func resourceComposerEnvironmentImport(d *schema.ResourceData, meta interface{})
}

// Replace import id for the resource id
id, err := replaceVars(d, config, "{{project}}/{{region}}/{{name}}")
id, err := replaceVars(d, config, "projects/{{project}}/locations/{{region}}/environments/{{name}}")
if err != nil {
return nil, fmt.Errorf("Error constructing id: %s", err)
}
Expand Down
19 changes: 7 additions & 12 deletions third_party/terraform/resources/resource_compute_attached_disk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func resourceAttachedDiskCreate(d *schema.ResourceData, meta interface{}) error
return err
}

d.SetId(fmt.Sprintf("%s:%s", zv.Name, diskName))
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instances/%s/%s", zv.Project, zv.Zone, zv.Name, diskName))

waitErr := computeSharedOperationWaitTime(config.clientCompute, op, zv.Project,
int(d.Timeout(schema.TimeoutCreate).Minutes()), "disk to attach")
Expand Down Expand Up @@ -196,22 +196,17 @@ func resourceAttachedDiskImport(d *schema.ResourceData, meta interface{}) ([]*sc
config := meta.(*Config)

err := parseImportId(
[]string{"projects/(?P<project>[^/]+)/zones/(?P<zone>[^/]+)/instances/[^/]+",
"(?P<project>[^/]+)/(?P<zone>[^/]+)/[^/]+"}, d, config)
[]string{"projects/(?P<project>[^/]+)/zones/(?P<zone>[^/]+)/instances/(?P<instance>[^/]+)/(?P<disk>[^/]+)",
"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<instance>[^/]+)/(?P<disk>[^/]+)"}, d, config)
if err != nil {
return nil, err
}

// In all acceptable id formats the actual id will be the last in the path
id := GetResourceNameFromSelfLink(d.Id())
d.SetId(id)

IDParts := strings.Split(d.Id(), ":")
if len(IDParts) != 2 {
return nil, fmt.Errorf("unable to determine attached disk id - id should be '{google_compute_instance.name}:{google_compute_disk.name}'")
id, err := replaceVars(d, config, "projects/{{project}}/zones/{{zone}}/instances/{{instance}}/{{disk}}")
if err != nil {
return nil, err
}
d.Set("instance", IDParts[0])
d.Set("disk", IDParts[1])
d.SetId(id)

return []*schema.ResourceData{d}, nil
}
Expand Down
47 changes: 25 additions & 22 deletions third_party/terraform/resources/resource_compute_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func getInstance(config *Config, d *schema.ResourceData) (*computeBeta.Instance,
if err != nil {
return nil, err
}
instance, err := config.clientComputeBeta.Instances.Get(project, zone, d.Id()).Do()
instance, err := config.clientComputeBeta.Instances.Get(project, zone, d.Get("name").(string)).Do()
if err != nil {
return nil, handleNotFoundError(err, d, fmt.Sprintf("Instance %s", d.Get("name").(string)))
}
Expand Down Expand Up @@ -690,7 +690,7 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err
}

// Store the ID now
d.SetId(instance.Name)
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instances/%s", project, z, instance.Name))

// Wait for the operation to complete
waitErr := computeSharedOperationWaitTime(config.clientCompute, op, project, createTimeout, "instance to create")
Expand Down Expand Up @@ -866,6 +866,8 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
}
}

zone := GetResourceNameFromSelfLink(instance.Zone)

d.Set("service_account", flattenServiceAccounts(instance.ServiceAccounts))
d.Set("attached_disk", ads)
d.Set("scratch_disk", scratchDisks)
Expand All @@ -878,11 +880,11 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
d.Set("self_link", ConvertSelfLinkToV1(instance.SelfLink))
d.Set("instance_id", fmt.Sprintf("%d", instance.Id))
d.Set("project", project)
d.Set("zone", GetResourceNameFromSelfLink(instance.Zone))
d.Set("zone", zone)
d.Set("name", instance.Name)
d.Set("description", instance.Description)
d.Set("hostname", instance.Hostname)
d.SetId(instance.Name)
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instances/%s", project, zone, instance.Name))

return nil
}
Expand All @@ -902,9 +904,9 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err

// Use beta api directly in order to read network_interface.fingerprint without having to put it in the schema.
// Change back to getInstance(config, d) once updating alias ips is GA.
instance, err := config.clientComputeBeta.Instances.Get(project, zone, d.Id()).Do()
instance, err := config.clientComputeBeta.Instances.Get(project, zone, d.Get("name").(string)).Do()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least in Update, probably worth pulling name out to a variable.

if err != nil {
return handleNotFoundError(err, d, fmt.Sprintf("Instance %s", d.Get("name").(string)))
return handleNotFoundError(err, d, fmt.Sprintf("Instance %s", instance.Name))
}

// Enable partial mode for the resource since it is possible
Expand All @@ -926,14 +928,14 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
func() error {
// retrieve up-to-date metadata from the API in case several updates hit simultaneously. instances
// sometimes but not always share metadata fingerprints.
instance, err := config.clientComputeBeta.Instances.Get(project, zone, d.Id()).Do()
instance, err := config.clientComputeBeta.Instances.Get(project, zone, instance.Name).Do()
if err != nil {
return fmt.Errorf("Error retrieving metadata: %s", err)
}

metadataV1.Fingerprint = instance.Metadata.Fingerprint

op, err := config.clientCompute.Instances.SetMetadata(project, zone, d.Id(), metadataV1).Do()
op, err := config.clientCompute.Instances.SetMetadata(project, zone, instance.Name, metadataV1).Do()
if err != nil {
return fmt.Errorf("Error updating metadata: %s", err)
}
Expand Down Expand Up @@ -961,7 +963,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
return err
}
op, err := config.clientCompute.Instances.SetTags(
project, zone, d.Id(), tagsV1).Do()
project, zone, d.Get("name").(string), tagsV1).Do()
if err != nil {
return fmt.Errorf("Error updating tags: %s", err)
}
Expand All @@ -979,7 +981,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
labelFingerprint := d.Get("label_fingerprint").(string)
req := compute.InstancesSetLabelsRequest{Labels: labels, LabelFingerprint: labelFingerprint}

op, err := config.clientCompute.Instances.SetLabels(project, zone, d.Id(), &req).Do()
op, err := config.clientCompute.Instances.SetLabels(project, zone, instance.Name, &req).Do()
if err != nil {
return fmt.Errorf("Error updating labels: %s", err)
}
Expand All @@ -999,7 +1001,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
}

op, err := config.clientComputeBeta.Instances.SetScheduling(
project, zone, d.Id(), scheduling).Do()
project, zone, instance.Name, scheduling).Do()
if err != nil {
return fmt.Errorf("Error updating scheduling policy: %s", err)
}
Expand Down Expand Up @@ -1040,7 +1042,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
// Delete any accessConfig that currently exists in instNetworkInterface
for _, ac := range instNetworkInterface.AccessConfigs {
op, err := config.clientCompute.Instances.DeleteAccessConfig(
project, zone, d.Id(), ac.Name, networkName).Do()
project, zone, instance.Name, ac.Name, networkName).Do()
if err != nil {
return fmt.Errorf("Error deleting old access_config: %s", err)
}
Expand All @@ -1065,7 +1067,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
}

op, err := config.clientComputeBeta.Instances.AddAccessConfig(
project, zone, d.Id(), networkName, ac).Do()
project, zone, instance.Name, networkName, ac).Do()
if err != nil {
return fmt.Errorf("Error adding new access_config: %s", err)
}
Expand All @@ -1085,7 +1087,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
Fingerprint: instNetworkInterface.Fingerprint,
ForceSendFields: []string{"AliasIpRanges"},
}
op, err := config.clientComputeBeta.Instances.UpdateNetworkInterface(project, zone, d.Id(), networkName, ni).Do()
op, err := config.clientComputeBeta.Instances.UpdateNetworkInterface(project, zone, instance.Name, networkName, ni).Do()
if err != nil {
return errwrap.Wrapf("Error removing alias_ip_range: {{err}}", err)
}
Expand All @@ -1099,7 +1101,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
ranges := d.Get(prefix + ".alias_ip_range").([]interface{})
if len(ranges) > 0 {
if rereadFingerprint {
instance, err = config.clientComputeBeta.Instances.Get(project, zone, d.Id()).Do()
instance, err = config.clientComputeBeta.Instances.Get(project, zone, instance.Name).Do()
if err != nil {
return err
}
Expand All @@ -1109,7 +1111,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
AliasIpRanges: expandAliasIpRanges(ranges),
Fingerprint: instNetworkInterface.Fingerprint,
}
op, err := config.clientComputeBeta.Instances.UpdateNetworkInterface(project, zone, d.Id(), networkName, ni).Do()
op, err := config.clientComputeBeta.Instances.UpdateNetworkInterface(project, zone, instance.Name, networkName, ni).Do()
if err != nil {
return errwrap.Wrapf("Error adding alias_ip_range: {{err}}", err)
}
Expand Down Expand Up @@ -1235,7 +1237,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
if d.HasChange("deletion_protection") {
nDeletionProtection := d.Get("deletion_protection").(bool)

op, err := config.clientCompute.Instances.SetDeletionProtection(project, zone, d.Id()).DeletionProtection(nDeletionProtection).Do()
op, err := config.clientCompute.Instances.SetDeletionProtection(project, zone, d.Get("name").(string)).DeletionProtection(nDeletionProtection).Do()
if err != nil {
return fmt.Errorf("Error updating deletion protection flag: %s", err)
}
Expand Down Expand Up @@ -1338,7 +1340,7 @@ func resourceComputeInstanceUpdate(d *schema.ResourceData, meta interface{}) err
if d.HasChange("shielded_instance_config") {
shieldedVmConfig := expandShieldedVmConfigs(d)

op, err := config.clientComputeBeta.Instances.UpdateShieldedVmConfig(project, zone, d.Id(), shieldedVmConfig).Do()
op, err := config.clientComputeBeta.Instances.UpdateShieldedVmConfig(project, zone, instance.Name, shieldedVmConfig).Do()
if err != nil {
return fmt.Errorf("Error updating shielded vm config: %s", err)
}
Expand Down Expand Up @@ -1492,12 +1494,12 @@ func resourceComputeInstanceDelete(d *schema.ResourceData, meta interface{}) err
if err != nil {
return err
}
log.Printf("[INFO] Requesting instance deletion: %s", d.Id())
log.Printf("[INFO] Requesting instance deletion: %s", d.Get("name").(string))

if d.Get("deletion_protection").(bool) {
return fmt.Errorf("Cannot delete instance %s: instance Deletion Protection is enabled. Set deletion_protection to false for this resource and run \"terraform apply\" before attempting to delete it.", d.Id())
return fmt.Errorf("Cannot delete instance %s: instance Deletion Protection is enabled. Set deletion_protection to false for this resource and run \"terraform apply\" before attempting to delete it.", d.Get("name").(string))
} else {
op, err := config.clientCompute.Instances.Delete(project, zone, d.Id()).Do()
op, err := config.clientCompute.Instances.Delete(project, zone, d.Get("name").(string)).Do()
if err != nil {
return fmt.Errorf("Error deleting instance: %s", err)
}
Expand All @@ -1522,7 +1524,8 @@ func resourceComputeInstanceImportState(d *schema.ResourceData, meta interface{}

d.Set("project", parts[0])
d.Set("zone", parts[1])
d.SetId(parts[2])
d.Set("name", parts[2])
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instances/%s", parts[0], parts[1], parts[2]))

return []*schema.ResourceData{d}, nil
}
Expand Down
2 changes: 1 addition & 1 deletion third_party/terraform/resources/resource_compute_instance_from_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ func resourceComputeInstanceFromTemplateCreate(d *schema.ResourceData, meta inte
}

// Store the ID now
d.SetId(instance.Name)
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instances/%s", project, z, instance.Name))

// Wait for the operation to complete
waitErr := computeSharedOperationWaitTime(config.clientCompute, op, project, int(d.Timeout(schema.TimeoutCreate).Minutes()), "instance to create")
Expand Down
25 changes: 13 additions & 12 deletions third_party/terraform/resources/resource_compute_instance_group.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func resourceComputeInstanceGroupCreate(d *schema.ResourceData, meta interface{}
}

// It probably maybe worked, so store the ID now
d.SetId(fmt.Sprintf("%s/%s", zone, name))
d.SetId(fmt.Sprintf("projects/%s/zones/%s/instanceGroups/%s", project, zone, name))

// Wait for the operation to complete
err = computeOperationWait(config.clientCompute, op, project, "Creating InstanceGroup")
Expand Down Expand Up @@ -378,18 +378,19 @@ func resourceComputeInstanceGroupDelete(d *schema.ResourceData, meta interface{}
}

func resourceComputeInstanceGroupImportState(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
parts := strings.Split(d.Id(), "/")
if len(parts) == 2 {
d.Set("zone", parts[0])
d.Set("name", parts[1])
} else if len(parts) == 3 {
d.Set("project", parts[0])
d.Set("zone", parts[1])
d.Set("name", parts[2])
d.SetId(parts[1] + "/" + parts[2])
} else {
return nil, fmt.Errorf("Invalid compute instance group specifier. Expecting {zone}/{name} or {project}/{zone}/{name}")
config := meta.(*Config)
if err := parseImportId([]string{
"projects/(?P<project>[^/]+)/zones/(?P<zone>[^/]+)/instanceGroups/(?P<name>[^/]+)",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this form to the docs?

"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)",
"(?P<zone>[^/]+)/(?P<name>[^/]+)",
}, d, config); err != nil {
return nil, err
}
id, err := replaceVars(d, config, "projects/{{project}}/zones/{{zone}}/instanceGroups/{{name}}")
if err != nil {
return nil, err
}
d.SetId(id)

return []*schema.ResourceData{d}, nil
}
16 changes: 3 additions & 13 deletions third_party/terraform/resources/resource_compute_instance_group_manager.go.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ func resourceComputeInstanceGroupManagerCreate(d *schema.ResourceData, meta inte
}

// It probably maybe worked, so store the ID now
id, err := replaceVars(d, config, "{{project}}/{{zone}}/{{name}}")
id, err := replaceVars(d, config, "projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{name}}")
if err != nil {
return err
}
Expand Down Expand Up @@ -451,9 +451,6 @@ func flattenFixedOrPercent(fixedOrPercent *computeBeta.FixedOrPercent) []map[str

func getManager(d *schema.ResourceData, meta interface{}) (*computeBeta.InstanceGroupManager, error) {
config := meta.(*Config)
if err := parseImportId([]string{"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<name>[^/]+)", "(?P<name>[^/]+)"}, d, config); err != nil {
return nil, err
}

project, err := getProject(d, config)
if err != nil {
Expand Down Expand Up @@ -595,10 +592,6 @@ func performZoneUpdate(d *schema.ResourceData, config *Config, id string, update
func resourceComputeInstanceGroupManagerUpdate(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

if err := parseImportId([]string{"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<name>[^/]+)", "(?P<name>[^/]+)"}, d, config); err != nil {
return err
}

project, err := getProject(d, config)
if err != nil {
return err
Expand Down Expand Up @@ -818,9 +811,6 @@ func resourceComputeInstanceGroupManagerUpdate(d *schema.ResourceData, meta inte
func resourceComputeInstanceGroupManagerDelete(d *schema.ResourceData, meta interface{}) error {
config := meta.(*Config)

if err := parseImportId([]string{"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<name>[^/]+)", "(?P<name>[^/]+)"}, d, config); err != nil {
return err
}
project, err := getProject(d, config)
if err != nil {
return err
Expand Down Expand Up @@ -1020,12 +1010,12 @@ func flattenUpdatePolicy(updatePolicy *computeBeta.InstanceGroupManagerUpdatePol
func resourceInstanceGroupManagerStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
d.Set("wait_for_instances", false)
config := meta.(*Config)
if err := parseImportId([]string{"(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<name>[^/]+)", "(?P<name>[^/]+)"}, d, config); err != nil {
if err := parseImportId([]string{"projects/(?P<project>[^/]+)/zones/(?P<zone>[^/]+)/instanceGroupManagers/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<zone>[^/]+)/(?P<name>[^/]+)", "(?P<project>[^/]+)/(?P<name>[^/]+)", "(?P<name>[^/]+)"}, d, config); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above- I think we need docs for the full form

return nil, err
}

// Replace import id for the resource id
id, err := replaceVars(d, config, "{{project}}/{{zone}}/{{name}}")
id, err := replaceVars(d, config, "projects/{{project}}/zones/{{zone}}/instanceGroupManagers/{{name}}")
if err != nil {
return nil, fmt.Errorf("Error constructing id: %s", err)
}
Expand Down
Loading