Skip to content

Commit 5cf6a5d

Browse files
committed
Fix perma-diff on instance templates.
When using instance templates, if you use one of our image shorthands at the moment, you'll get a perma-diff. This is because the config gets resolved to another format before it is set in state, and so we need to set that other format in state. Unfortunately, resolving images requires network access, so we have to do this with CustomizeDiff. CustomizeDiff was having trouble (I think? More on this below) on setting a field as not ForceNew once the field was already set, so I moved the decision for whether a field was ForceNew or not into CustomizeDiff. I also resolved the old and new images, and if they were the same, cleared the diff. Unfortunately, you can't actually clear a field on a sub-block right now. You have to clear top-level fields only. So this will currently throw an error. I opened hashicorp/terraform#18795 to fix that. Once that's merged, and we vendor it here, this patch fixes the problem. If hashicorp/terraform#18795 doesn't get merged, the next best workaround is to keep track of _all_ the fields under `disk` with a diff in our CustomizeDiff, check whether they've all changed or not, and if they've all changed, clear the changes on `disk`, which I _think_ will resolve the issue. That's just a massive pain, unfortunately.
1 parent 7911cab commit 5cf6a5d

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

google/resource_compute_instance_template.go

+46-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func resourceComputeInstanceTemplate() *schema.Resource {
2020
State: schema.ImportStatePassthrough,
2121
},
2222
SchemaVersion: 1,
23+
CustomizeDiff: resourceComputeInstanceTemplateCustomizeDiff,
2324
MigrateState: resourceComputeInstanceTemplateMigrateState,
2425

2526
// A compute instance template is more or less a subset of a compute
@@ -99,10 +100,9 @@ func resourceComputeInstanceTemplate() *schema.Resource {
99100
},
100101

101102
"source_image": &schema.Schema{
102-
Type: schema.TypeString,
103-
Optional: true,
104-
DiffSuppressFunc: compareSelfLinkRelativePaths,
105-
ForceNew: true,
103+
Type: schema.TypeString,
104+
Optional: true,
105+
Computed: true,
106106
},
107107

108108
"interface": &schema.Schema{
@@ -420,6 +420,48 @@ func resourceComputeInstanceTemplate() *schema.Resource {
420420
}
421421
}
422422

423+
func resourceComputeInstanceTemplateCustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error {
424+
config := meta.(*Config)
425+
project, err := getProjectFromDiff(diff, config)
426+
if err != nil {
427+
return err
428+
}
429+
numDisks := diff.Get("disk.#").(int)
430+
for i := 0; i < numDisks; i++ {
431+
key := fmt.Sprintf("disk.%d.source_image", i)
432+
if diff.HasChange(key) {
433+
old, new := diff.GetChange(key)
434+
if old == "" || new == "" {
435+
// no sense in resolving empty strings
436+
err = diff.ForceNew(key)
437+
if err != nil {
438+
return err
439+
}
440+
continue
441+
}
442+
oldResolved, err := resolveImage(config, project, old.(string))
443+
if err != nil {
444+
return err
445+
}
446+
newResolved, err := resolveImage(config, project, new.(string))
447+
if err != nil {
448+
return err
449+
}
450+
if oldResolved != newResolved {
451+
err = diff.ForceNew(key)
452+
if err != nil {
453+
return err
454+
}
455+
}
456+
err = diff.Clear(key)
457+
if err != nil {
458+
return err
459+
}
460+
}
461+
}
462+
return nil
463+
}
464+
423465
func buildDisks(d *schema.ResourceData, config *Config) ([]*computeBeta.AttachedDisk, error) {
424466
project, err := getProject(d, config)
425467
if err != nil {

google/utils.go

+14
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,20 @@ func getProject(d TerraformResourceData, config *Config) (string, error) {
6464
return getProjectFromSchema("project", d, config)
6565
}
6666

67+
// getProjectFromDiff reads the "project" field from the given diff and falls
68+
// back to the provider's value if not given. If the provider's value is not
69+
// given, an error is returned.
70+
func getProjectFromDiff(d *schema.ResourceDiff, config *Config) (string, error) {
71+
res, ok := d.GetOk("project")
72+
if ok {
73+
return res.(string), nil
74+
}
75+
if config.Project != "" {
76+
return config.Project, nil
77+
}
78+
return "", fmt.Errorf("%s: required field is not set", "project")
79+
}
80+
6781
func getProjectFromInstanceState(is *terraform.InstanceState, config *Config) (string, error) {
6882
res, ok := is.Attributes["project"]
6983

0 commit comments

Comments
 (0)