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

Add support for configuring disk resource policies for google_compute_instance_template. #8237

Merged

Conversation

rayward
Copy link
Contributor

@rayward rayward commented Jan 17, 2021

Fixes #6719

Per documentation (and checked), https://pkg.go.dev/google.golang.org/api/compute/v0.beta#AttachedDiskInitializeParams only the resource policy name is supported for instance templates, a uri can't be used. A link can be used in user code, but it will be converted to the resource name.

@ghost ghost added size/m labels Jan 17, 2021
@ghost ghost requested a review from ScottSuarez January 17, 2021 23:58
@ScottSuarez
Copy link
Collaborator

/gcbrun

@ScottSuarez
Copy link
Collaborator

can you add this field to the documentation? Possibly an example there-in too?

@rayward
Copy link
Contributor Author

rayward commented Jan 21, 2021

@ScottSuarez I'm second guessing the decision to support self links on this field when the underlying API doesn't. Do you think I should keep this behaviour (of converting a link) or make it short names only, it's easy enough for a user to reference the name:

resource_policies = [google_compute_resource_policy.my_snapshot_schedule.name]

The source field for disks also doesn't support self-links for example - https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/compute_instance_template#source

@rayward
Copy link
Contributor Author

rayward commented Jan 21, 2021

I've (tentatively) documented the field, but will adjust it pending the above question.

@ScottSuarez
Copy link
Collaborator

Hi, sorry for the delay on this. I was out of town. I spoke with my team on this and we prefer resources to reference each other by the .id property if possible. Its something we've consistently maintained within the provider.

@rayward
Copy link
Contributor Author

rayward commented Jan 27, 2021

I spoke with my team on this and we prefer resources to reference each other by the .id property if possible.

I think .id is effectively interchangeable for .self_link from what I can see. In any case the resource itself only supports the short name so I can keep the conversion code in and update the documentation and example to use .id.

@ScottSuarez
Copy link
Collaborator

/gcbrun

@ScottSuarez
Copy link
Collaborator

I'll focus on trying to merge this for you today! Sorry for the delay my friend :>

@ghost
Copy link

ghost commented Mar 4, 2021

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 ghost locked as resolved and limited conversation to collaborators Mar 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for configuring disk resource policies for google_compute_instance_template
2 participants