Skip to content

Commit

Permalink
Read compute instance boot disk initialization params from API (#948)
Browse files Browse the repository at this point in the history
* read boot disk initialization param from API

* make fmt

* Mark the initialize_params list as computed to support boot source

* Ensure private family test follow naming pattern

* Improve docs
  • Loading branch information
rosbo authored Jan 17, 2018
1 parent 823542a commit 681cc72
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 72 deletions.
56 changes: 45 additions & 11 deletions google/resource_compute_instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,28 +79,33 @@ func resourceComputeInstance() *schema.Resource {
"initialize_params": &schema.Schema{
Type: schema.TypeList,
Optional: true,
Computed: true,
ForceNew: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"size": &schema.Schema{
Type: schema.TypeInt,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.IntAtLeast(1),
},

"type": &schema.Schema{
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ValidateFunc: validation.StringInSlice([]string{"pd-standard", "pd-ssd"}, false),
},

"image": &schema.Schema{
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
DiffSuppressFunc: diskImageDiffSuppress,
},
},
},
Expand Down Expand Up @@ -582,6 +587,20 @@ func getInstance(config *Config, d *schema.ResourceData) (*computeBeta.Instance,
return instance, nil
}

func getDisk(diskUri string, d *schema.ResourceData, config *Config) (*compute.Disk, error) {
source, err := ParseDiskFieldValue(diskUri, d, config)
if err != nil {
return nil, err
}

disk, err := config.clientCompute.Disks.Get(source.Project, source.Zone, source.Name).Do()
if err != nil {
return nil, err
}

return disk, err
}

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

Expand Down Expand Up @@ -820,7 +839,7 @@ func resourceComputeInstanceRead(d *schema.ResourceData, meta interface{}) error
extraAttachedDisks := []map[string]interface{}{}
for _, disk := range instance.Disks {
if disk.Boot {
d.Set("boot_disk", flattenBootDisk(d, disk))
d.Set("boot_disk", flattenBootDisk(d, disk, config))
} else if disk.Type == "SCRATCH" {
scratchDisks = append(scratchDisks, flattenScratchDisk(disk))
sIndex++
Expand Down Expand Up @@ -1309,7 +1328,7 @@ func expandBootDisk(d *schema.ResourceData, config *Config, zone *compute.Zone,
return disk, nil
}

func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk) []map[string]interface{} {
func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk, config *Config) []map[string]interface{} {
result := map[string]interface{}{
"auto_delete": disk.AutoDelete,
"device_name": disk.DeviceName,
Expand All @@ -1318,15 +1337,30 @@ func flattenBootDisk(d *schema.ResourceData, disk *computeBeta.AttachedDisk) []m
// originally specified to avoid diffs.
"disk_encryption_key_raw": d.Get("boot_disk.0.disk_encryption_key_raw"),
}

diskDetails, err := getDisk(disk.Source, d, config)
if err != nil {
log.Printf("[WARN] Cannot retrieve boot disk details: %s", err)

if _, ok := d.GetOk("boot_disk.0.initialize_params.#"); ok {
// If we can't read the disk details due to permission for instance,
// copy the initialize_params from what the user originally specified to avoid diffs.
m := d.Get("boot_disk.0.initialize_params")
result["initialize_params"] = m
}
} else {
result["initialize_params"] = []map[string]interface{}{{
"type": GetResourceNameFromSelfLink(diskDetails.Type),
// If the config specifies a family name that doesn't match the image name, then
// the diff won't be properly suppressed. See DiffSuppressFunc for this field.
"image": diskDetails.SourceImage,
"size": diskDetails.SizeGb,
}}
}

if disk.DiskEncryptionKey != nil {
result["disk_encryption_key_sha256"] = disk.DiskEncryptionKey.Sha256
}
if _, ok := d.GetOk("boot_disk.0.initialize_params.#"); ok {
// initialize_params is not returned from the API, so copy it from what the user
// originally specified to avoid diffs.
m := d.Get("boot_disk.0.initialize_params")
result["initialize_params"] = m
}

return []map[string]interface{}{result}
}
Expand Down
104 changes: 45 additions & 59 deletions google/resource_compute_instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestAccComputeInstance_basic1(t *testing.T) {
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params", "create_timeout"},
ImportStateVerifyIgnore: []string{"create_timeout"},
},
},
})
Expand Down Expand Up @@ -253,10 +253,9 @@ func TestAccComputeInstance_attachedDisk(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand All @@ -283,10 +282,9 @@ func TestAccComputeInstance_attachedDisk_sourceUrl(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand Down Expand Up @@ -395,10 +393,9 @@ func TestAccComputeInstance_bootDisk_sourceUrl(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand Down Expand Up @@ -448,10 +445,9 @@ func TestAccComputeInstance_scratchDisk(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand Down Expand Up @@ -547,10 +543,9 @@ func TestAccComputeInstance_service_account(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand All @@ -575,10 +570,9 @@ func TestAccComputeInstance_scheduling(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand All @@ -604,10 +598,9 @@ func TestAccComputeInstance_subnet_auto(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand All @@ -633,10 +626,9 @@ func TestAccComputeInstance_subnet_custom(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand Down Expand Up @@ -721,7 +713,6 @@ func TestAccComputeInstance_private_image_family(t *testing.T) {
var instance compute.Instance
var instanceName = fmt.Sprintf("instance-test-%s", acctest.RandString(10))
var diskName = fmt.Sprintf("instance-testd-%s", acctest.RandString(10))
var imageName = fmt.Sprintf("instance-testi-%s", acctest.RandString(10))
var familyName = fmt.Sprintf("instance-testf-%s", acctest.RandString(10))

resource.Test(t, resource.TestCase{
Expand All @@ -730,7 +721,7 @@ func TestAccComputeInstance_private_image_family(t *testing.T) {
CheckDestroy: testAccCheckComputeInstanceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccComputeInstance_private_image_family(diskName, imageName, familyName, instanceName),
Config: testAccComputeInstance_private_image_family(diskName, familyName, instanceName),
Check: resource.ComposeTestCheckFunc(
testAccCheckComputeInstanceExists(
"google_compute_instance.foobar", &instance),
Expand Down Expand Up @@ -763,7 +754,7 @@ func TestAccComputeInstance_forceChangeMachineTypeManually(t *testing.T) {
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params", "create_timeout"},
ImportStateVerifyIgnore: []string{"create_timeout"},
},
},
})
Expand All @@ -790,10 +781,9 @@ func TestAccComputeInstance_multiNic(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-central1-a", instanceName),
},
},
})
Expand All @@ -818,10 +808,9 @@ func TestAccComputeInstance_guestAccelerator(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
},
},
})
Expand All @@ -847,10 +836,9 @@ func TestAccComputeInstance_minCpuPlatform(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
},
},
})
Expand All @@ -875,10 +863,9 @@ func TestAccComputeInstance_primaryAliasIpRange(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
},
},
})
Expand All @@ -903,10 +890,9 @@ func TestAccComputeInstance_secondaryAliasIpRange(t *testing.T) {
),
},
resource.TestStep{
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
ImportStateVerifyIgnore: []string{"boot_disk.0.initialize_params"},
ResourceName: "google_compute_instance.foobar",
ImportState: true,
ImportStateId: fmt.Sprintf("%s/%s/%s", getTestProjectFromEnv(), "us-east1-d", instanceName),
},
},
})
Expand Down Expand Up @@ -2225,7 +2211,7 @@ resource "google_compute_instance" "foobar" {
`, acctest.RandString(10), acctest.RandString(10), instance, address)
}

func testAccComputeInstance_private_image_family(disk, image, family, instance string) string {
func testAccComputeInstance_private_image_family(disk, family, instance string) string {
return fmt.Sprintf(`
resource "google_compute_disk" "foobar" {
name = "%s"
Expand All @@ -2234,7 +2220,7 @@ resource "google_compute_disk" "foobar" {
}
resource "google_compute_image" "foobar" {
name = "%s"
name = "%s-1"
source_disk = "${google_compute_disk.foobar.self_link}"
family = "%s"
}
Expand All @@ -2258,7 +2244,7 @@ resource "google_compute_instance" "foobar" {
foo = "bar"
}
}
`, disk, image, family, instance)
`, disk, family, family, instance)
}

func testAccComputeInstance_multiNic(instance, network, subnetwork string) string {
Expand Down
7 changes: 5 additions & 2 deletions website/docs/r/compute_instance.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ The `initialize_params` block supports:
one of: the image's `self_link`, `projects/{project}/global/images/{image}`,
`projects/{project}/global/images/family/{family}`, `global/images/{image}`,
`global/images/family/{family}`, `family/{family}`, `{project}/{family}`,
`{project}/{image}`, `{family}`, or `{image}`.
`{project}/{image}`, `{family}`, or `{image}`. If referred by family, the
images names must include the family name. For instance, the image
`centos-6-v20180104` includes its family name `centos-6`. These images can
be referred by family name here.

The `scratch_disk` block supports:

Expand Down Expand Up @@ -276,7 +279,7 @@ exported:

## Import

~> **Note:** The fields `boot_disk.0.initialize_params`, `boot_disk.0.disk_entryption_raw` and `attached_disk.*.disk_encryption_key_raw` cannot be imported automatically. The API doesn't return this information. If you are setting one of these fields in your config, you will need to update your state manually after importing the resource.
~> **Note:** The fields `boot_disk.0.disk_entryption_raw` and `attached_disk.*.disk_encryption_key_raw` cannot be imported automatically. The API doesn't return this information. If you are setting one of these fields in your config, you will need to update your state manually after importing the resource.

Instances can be imported using the `project`, `zone` and `name`, e.g.

Expand Down

0 comments on commit 681cc72

Please sign in to comment.