-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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.
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
Fix perma-diff on instance templates. #1995
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ func resourceComputeInstanceTemplate() *schema.Resource { | |
State: schema.ImportStatePassthrough, | ||
}, | ||
SchemaVersion: 1, | ||
CustomizeDiff: resourceComputeInstanceTemplateCustomizeDiff, | ||
MigrateState: resourceComputeInstanceTemplateMigrateState, | ||
|
||
// A compute instance template is more or less a subset of a compute | ||
|
@@ -99,10 +100,9 @@ func resourceComputeInstanceTemplate() *schema.Resource { | |
}, | ||
|
||
"source_image": &schema.Schema{ | ||
Type: schema.TypeString, | ||
Optional: true, | ||
DiffSuppressFunc: compareSelfLinkRelativePaths, | ||
ForceNew: true, | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Computed: true, | ||
}, | ||
|
||
"interface": &schema.Schema{ | ||
|
@@ -420,6 +420,48 @@ func resourceComputeInstanceTemplate() *schema.Resource { | |
} | ||
} | ||
|
||
func resourceComputeInstanceTemplateCustomizeDiff(diff *schema.ResourceDiff, meta interface{}) error { | ||
config := meta.(*Config) | ||
project, err := getProjectFromDiff(diff, config) | ||
if err != nil { | ||
return err | ||
} | ||
numDisks := diff.Get("disk.#").(int) | ||
for i := 0; i < numDisks; i++ { | ||
key := fmt.Sprintf("disk.%d.source_image", i) | ||
if diff.HasChange(key) { | ||
old, new := diff.GetChange(key) | ||
if old == "" || new == "" { | ||
// no sense in resolving empty strings | ||
err = diff.ForceNew(key) | ||
if err != nil { | ||
return err | ||
} | ||
continue | ||
} | ||
oldResolved, err := resolveImage(config, project, old.(string)) | ||
if err != nil { | ||
return err | ||
} | ||
newResolved, err := resolveImage(config, project, new.(string)) | ||
if err != nil { | ||
return err | ||
} | ||
if oldResolved != newResolved { | ||
err = diff.ForceNew(key) | ||
if err != nil { | ||
return err | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't there be a continue here? otherwise the diff will get cleared on line 456 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch. |
||
} | ||
err = diff.Clear(key) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
return nil | ||
} | ||
|
||
func buildDisks(d *schema.ResourceData, config *Config) ([]*computeBeta.AttachedDisk, error) { | ||
project, err := getProject(d, config) | ||
if err != nil { | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -665,10 +665,10 @@ | |
"versionExact": "v0.11.2" | ||
}, | ||
{ | ||
"checksumSHA1": "JHxGzmxcIS8NyLX9pGhK5beIra4=", | ||
"checksumSHA1": "OOwTGBTHcUmQTPBdyscTMkjApbI=", | ||
"path": "github.com/hashicorp/terraform/helper/schema", | ||
"revision": "41e50bd32a8825a84535e353c3674af8ce799161", | ||
"revisionTime": "2018-04-10T16:50:42Z", | ||
"revision": "35d82b055591e9d47a254e68754216d8849ba67a", | ||
"revisionTime": "2018-09-26T21:21:28Z", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's probably not actually a big deal at all, but this has a versionExact and a revision time that is definitely a few versions ahead of the versionExact. Any idea how that happened? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I had to guess, it's a result of govendor updating the revision and revisionTime, but not the version or versionExact. I've gone ahead and removed the version and versionExact. |
||
"version": "v0.11.2", | ||
"versionExact": "v0.11.2" | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I'd call this
resourceComputeInstanceTemplateSourceImageCustomizeDiff
, since presumably if we added more fields to customize we would want to do a customdiff.All to group them.(also lol, every so often i think that maybe our resources should be like, objects, so we don't have to have these ridiculously long globally-unique function names)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair! I've been doing massive monolithic CustomizeDiff functions, but I think customdiff.All--which I just learned about!--is a better approach.
And yeah, long names are amazing. -_-