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.

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

Diff suppress unconventional public image family naming pattern #1024

Conversation

rosbo
Copy link
Contributor

@rosbo rosbo commented Jan 30, 2018

Help for #988.

There is an acceptance test that will catch new image patterns and fail if we don't support them.

I would highly encourage people to use the google_compute_image data source or an image self_link.

I wish we could make a network call in DiffSuppressFunc...

I am starting to wonder if we should just stop reading the image field for the api for disk and instance (import won't work properly and we won't detect image changes outside Terraform). Reading from the image from the API was added in #948.

@rosbo rosbo force-pushed the diff-suppress-unconventionel-public-image-family-name branch from b8f5498 to e84188d Compare January 30, 2018 21:24
@@ -495,3 +496,91 @@ func diskImageProjectNameEquals(project1, project2 string) bool {

return project1 == project2
}

func diskImageEquals(oldImageName, newImageName string) bool {
return strings.Contains(oldImageName, newImageName)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the case where it's the image name that's a substring of the other one (as opposed to the family name that's a substring of the image name)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Like, could this be oldImageName == newImageName?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see :) Yes! Now that I added a specific one for for testing equality for family.

Done. I kept the diskImageEquals method even if it is trivial. I find that it reads better in diskImageDiffSuppress.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, thanks :)

@rosbo rosbo merged commit 3a65e36 into hashicorp:master Jan 31, 2018
@rosbo rosbo deleted the diff-suppress-unconventionel-public-image-family-name branch January 31, 2018 17:33
chrisst pushed a commit to chrisst/terraform-provider-google that referenced this pull request Nov 9, 2018
…icorp#1024)

* DiffSuppress for all public images family names

* Update documentation

* TF_ACC guard

* diskImageEquals should use ==
@ghost
Copy link

ghost commented Mar 29, 2020

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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants