Add scratch_disk property to google_compute_instance and deprecate disk
#123
Conversation
google/resource_compute_instance.go
Outdated
| @@ -480,6 +497,15 @@ func resourceComputeInstanceCreate(d *schema.ResourceData, meta interface{}) err | |||
| disks = append(disks, bootDisk) | |||
| } | |||
|
|
|||
| scratchDisks := []*compute.AttachedDisk{} | |||
There was a problem hiding this comment.
Same as #122 (comment), we can keep this element in block scope instead of function scope.
That would mean changing the one other use of scratchDisks from
if len(scratchDisks) > 0 {to
if d.Get("scratch_disk.#").(int) > 0which looks a bit grosser, but holds better semantics to me - we are giving a schema error by comparing schema elements.
There was a problem hiding this comment.
I just went ahead and used a bool again.
There was a problem hiding this comment.
That works even better! 👍
| @@ -1331,6 +1380,34 @@ func testAccComputeInstance_local_ssd(instance string) string { | |||
| }`, instance) | |||
| } | |||
|
|
|||
| func testAccComputeInstance_scratchDisk(instance string) string { | |||
| return fmt.Sprintf(` | |||
| resource "google_compute_instance" "scratch" { | |||
There was a problem hiding this comment.
Same nit as #122 (comment) and #122 (comment);
|
(will address comments later) Note that this also ends up fixing #24. |
|
Thanks for the changes! LGTM. |
| @@ -749,7 +813,7 @@ func testAccCheckComputeInstanceDisk(instance *compute.Instance, source string, | |||
| } | |||
|
|
|||
| for _, disk := range instance.Disks { | |||
| if strings.LastIndex(disk.Source, "/"+source) == len(disk.Source)-len(source)-1 && disk.AutoDelete == delete && disk.Boot == boot { | |||
| if strings.HasSuffix(disk.Source, source) && disk.AutoDelete == delete && disk.Boot == boot { | |||
There was a problem hiding this comment.
Won't stripping the slash prefixing source mean that a disk with name bar would be matched against a disk named foobar? Seems a stretch, but...
|
Rebase and LGTM. 👍 |
…e `disk` (hashicorp#123) * Add scratch_disk property to google_compute_instance * docs for scratch_disk * limit scope of scratchDisks array by using bool, test formatting * add slash back to disk check
…e `disk` (hashicorp#123) * Add scratch_disk property to google_compute_instance * docs for scratch_disk * limit scope of scratchDisks array by using bool, test formatting * add slash back to disk check
<!-- This change is generated by MagicModules. --> /cc @rileykarson
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
This is change 1 in a series of changes to provide a better experience for attaching/detaching disks to/from instances. The end goal is to have a separate resource for attached disks, in order to have some attached disks that are managed by terraform side-by-side with disks that are managed some other way (such as by k8s), and to allow users to attach/detach disks (see #33)
The order of the changes will look roughly like:
boot_diskpropertyscratch_diskproperty and deprecatediskdiskproperty, makeboot_diskRequiredgoogle_compute_attached_diskresource and deprecate theattached_diskpropertyNote that this change branches off of #122. It can still be reviewed by only looking at the final two commits.